Skip to content

Commit

Permalink
Change TLS trust store
Browse files Browse the repository at this point in the history
Motivation
----------
We are currently using the native store to loads certs for use
with kv. The native package has a dependency on OpenSSL which we
don't want. We are allowing reqwest to load its default trust
store that it uses for rusttls - webpki-roots. We should change
both of these so that we load webpki-roots ourselves and set the
config to use on reqwest - so that we control it and have better
tracibility into versions being used as well as consistency
across both HTTP and KV.

Changes
-------
Refactor TLS throughout so that TLS configs are only setup once
per cluster entry in the config.
Refactor TLS so that HTTP and KV use the same TLS object type.
Always uses webspki-roots as our trust store.
  • Loading branch information
chvck committed Aug 31, 2023
1 parent 2283464 commit 03ff487
Show file tree
Hide file tree
Showing 16 changed files with 290 additions and 273 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER: 'aarch64-linux-gnu-gcc'
with:
command: build
args: --verbose --target ${{ matrix.target }} --features static-link-openssl
args: --verbose --target ${{ matrix.target }}
if: matrix.os == 'ubuntu-22.04'

- name: Build
Expand All @@ -72,7 +72,7 @@ jobs:
with:
command: build
# We build windows as a release build as debug builds are stack overflowing on startup.
args: --verbose --release --target ${{ matrix.target }} --features static-link-openssl
args: --verbose --release --target ${{ matrix.target }}
env:
VCPKGRS_DYNAMIC: 1
RUSTFLAGS: -Ctarget-feature=+crt-static
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ jobs:
CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER: 'aarch64-linux-gnu-gcc'
with:
command: build
args: --verbose --release --target ${{ matrix.target }} --features static-link-openssl
args: --verbose --release --target ${{ matrix.target }}
if: matrix.os == 'ubuntu-22.04'

- name: Build
Expand All @@ -78,7 +78,7 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: build
args: --verbose --release --features static-link-openssl
args: --verbose --release
env:
VCPKGRS_DYNAMIC: 1
RUSTFLAGS: -Ctarget-feature=+crt-static
Expand Down
64 changes: 24 additions & 40 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 1 addition & 10 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ reqwest = { version = "0.11", features = ["json", "rustls-tls", "blocking"] }
rpassword = "7.0.0"
rust-embed = "6.3"
rustls-pemfile = "1.0.0"
rustls-native-certs = "0.6.2"
serde = "1.0"
serde_json = "1.0"
serde_derive = "1.0"
Expand All @@ -65,10 +64,7 @@ toml = "0.7.3"
trust-dns-resolver = { version = "0.22.0", features = ["dns-over-rustls"] }
url = "2.1"
uuid = { version = "1.1.2", features = ["v4"] }

[target.'cfg(not(target_os = "macos"))'.dependencies]
# Our dependencies don't use OpenSSL on Macos
openssl = { version = "0.10.48", features = ["vendored"], optional=true }
webpki-roots = "0.25.1"

[dev-dependencies]
dunce = "1.0.1"
Expand All @@ -79,11 +75,6 @@ nu-test-support = { version = "0.83.1"}
strum = "0.24.1"
strum_macros = "0.24.3"

[features]

# Enable to statically link OpenSSL; otherwise the system version will be used. Not enabled by default because it takes a while to build
static-link-openssl = ["dep:openssl"]

[[bin]]
name = "cbsh"
path = "src/main.rs"
Expand Down
2 changes: 1 addition & 1 deletion src/cli/cbenv_managed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn clusters(
.map(|(k, v)| {
let mut collected = NuValueMap::default();
collected.add_bool("active", k == &active, span);
collected.add_bool("tls", v.tls_config().enabled(), span);
collected.add_bool("tls", v.tls_config().is_some(), span);
collected.add_string("identifier", k.clone(), span);
collected.add_string("username", String::from(v.username()), span);
collected.add_string(
Expand Down
25 changes: 20 additions & 5 deletions src/cli/cbenv_register.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::config::{
CapellaOrganizationConfig, ClusterConfig, ClusterTlsConfig, ShellConfig, DEFAULT_KV_BATCH_SIZE,
};
use crate::config::{CapellaOrganizationConfig, ClusterConfig, ShellConfig, DEFAULT_KV_BATCH_SIZE};
use crate::state::State;
use std::fs;
use std::sync::{Arc, Mutex, MutexGuard};

use crate::cli::error::generic_error;
use crate::{ClusterTimeouts, RemoteCluster, RemoteClusterResources, RemoteClusterType};
use crate::{
ClusterTimeouts, RemoteCluster, RemoteClusterResources, RemoteClusterType, RustTlsConfig,
};
use nu_engine::CallExt;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
Expand Down Expand Up @@ -151,6 +151,21 @@ fn clusters_register(
.split(",")
.map(|s| s.to_string())
.collect::<Vec<String>>();
let tls_config = if tls_enabled {
Some(
RustTlsConfig::new(tls_accept_all_certs, cert_path).map_err(|e| {
ShellError::GenericError(
e.message(),
"".to_string(),
call.head.into(),
Some(e.expanded_message()),
Vec::new(),
)
})?,
)
} else {
None
};
let cluster = RemoteCluster::new(
RemoteClusterResources {
hostnames: hostnames.clone(),
Expand All @@ -161,7 +176,7 @@ fn clusters_register(
active_collection: collection,
display_name,
},
ClusterTlsConfig::new(tls_enabled, cert_path, tls_accept_all_certs),
tls_config,
ClusterTimeouts::default(),
capella,
DEFAULT_KV_BATCH_SIZE,
Expand Down
3 changes: 3 additions & 0 deletions src/cli_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub struct CliOptions {
pub no_motd: bool,
pub disable_tls: bool,
pub tls_cert_path: Option<String>,
pub tls_accept_all_certs: bool,
pub config_path: Option<String>,
pub logger_prefix: Option<String>,
pub display_name: Option<String>,
Expand Down Expand Up @@ -222,6 +223,7 @@ pub fn parse_commandline_args(
let display_name: Option<String> =
call.get_flag(context, &mut stack, "display-name")?;
let no_config_prompt = call.has_flag("disable-config-prompt");
let tls_accept_all_certs = call.has_flag("tls-accept-all-certs");

fn extract_contents(
expression: Option<Expression>,
Expand Down Expand Up @@ -295,6 +297,7 @@ pub fn parse_commandline_args(
logger_prefix,
display_name,
no_config_prompt,
tls_accept_all_certs,
});
}
}
Expand Down
Loading

0 comments on commit 03ff487

Please sign in to comment.