From 880f5c075f233ec2aa6362b66939d4471dfd97f7 Mon Sep 17 00:00:00 2001 From: Thomas Sibley Date: Sun, 13 Oct 2024 22:26:36 -0700 Subject: [PATCH 1/5] feat: Support a NEXTCLADE_EXTRA_CA_CERTS environment variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allows adding additional CA certificates to the trust store specifically for Nextclade, for when modifying the system's trust store isn't desirable/possible. Works across all platforms. Currently requires using landed but unreleased changes to rustls-platform-verifier so that macOS and Windows have the needed Verifier::new_with_extra_certs() API.¹ That API also has signature changes proposed² which, while unmerged, seem likely to land, so use those in anticipation. They actually bring the signature back closer to the latest release (0.3.4). ¹ ² Related-to: Related-to: --- Cargo.lock | 26 ++++++++++---- Cargo.toml | 6 +++- packages/nextclade-cli/Cargo.toml | 4 +++ packages/nextclade-cli/src/io/http_client.rs | 36 ++++++++++++++++++-- 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7497ae280..f209e2ac9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1848,7 +1848,11 @@ dependencies = [ "regex", "reqwest", "rstest", + "rustls", + "rustls-pemfile", + "rustls-pki-types", "rustls-platform-verifier", + "rustls-webpki", "schemars", "semver 1.0.17", "serde", @@ -2502,6 +2506,7 @@ version = "0.23.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "415d9944693cb90382053259f89fbb077ea730ad7273047ec63b19bc9b160ba8" dependencies = [ + "log", "once_cell", "ring", "rustls-pki-types", @@ -2540,9 +2545,8 @@ checksum = "0e696e35370c65c9c541198af4543ccd580cf17fc25d8e05c5a242b202488c55" [[package]] name = "rustls-platform-verifier" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "afbb878bdfdf63a336a5e63561b1835e7a8c91524f51621db870169eac84b490" +version = "0.4.0" +source = "git+https://github.com/rustls/rustls-platform-verifier.git?rev=ca87571fe47730665917b8c29f14a3989f3cbe57#ca87571fe47730665917b8c29f14a3989f3cbe57" dependencies = [ "core-foundation", "core-foundation-sys", @@ -2555,15 +2559,14 @@ dependencies = [ "rustls-webpki", "security-framework", "security-framework-sys", - "webpki-roots", - "winapi", + "webpki-root-certs", + "windows-sys 0.52.0", ] [[package]] name = "rustls-platform-verifier-android" version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f87165f0995f63a9fbeea62b64d10b4d9d8e78ec6d7d51fb2125fda7bb36788f" +source = "git+https://github.com/rustls/rustls-platform-verifier.git?rev=ca87571fe47730665917b8c29f14a3989f3cbe57#ca87571fe47730665917b8c29f14a3989f3cbe57" [[package]] name = "rustls-webpki" @@ -3476,6 +3479,15 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "webpki-root-certs" +version = "0.26.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8c6dfa3ac045bc517de14c7b1384298de1dbd229d38e08e169d9ae8c170937c" +dependencies = [ + "rustls-pki-types", +] + [[package]] name = "webpki-roots" version = "0.26.6" diff --git a/Cargo.toml b/Cargo.toml index 5d35ecb97..44a8640c2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,7 +73,11 @@ regex = "=1.8.4" reqwest = { version = "=0.12.8", default-features = false, features = ["blocking", "deflate", "gzip", "brotli", "socks", "rustls-tls"] } rstest = "=0.17.0" rstest_reuse = "=0.5.0" -rustls-platform-verifier = "=0.3.4" +rustls = { version = "=0.23.14", default-features = false, features = ["ring", "logging", "std", "tls12"] } +rustls-platform-verifier = { git = "https://github.com/rustls/rustls-platform-verifier.git", rev = "ca87571fe47730665917b8c29f14a3989f3cbe57" } +rustls-pemfile = "=2.2.0" +rustls-pki-types = "=1.9.0" +rustls-webpki = { version = "=0.102.8", features = ["ring"] } schemars = { version = "=0.8.12", features = ["chrono", "either", "enumset", "indexmap"] } semver = { version = "=1.0.17", features = ["serde"] } serde = { version = "=1.0.164", features = ["derive"] } diff --git a/packages/nextclade-cli/Cargo.toml b/packages/nextclade-cli/Cargo.toml index a2770c26d..d76923374 100644 --- a/packages/nextclade-cli/Cargo.toml +++ b/packages/nextclade-cli/Cargo.toml @@ -41,7 +41,11 @@ pretty_assertions = { workspace = true } rayon = { workspace = true } regex = { workspace = true } reqwest = { workspace = true } +rustls = { workspace = true } rustls-platform-verifier = { workspace = true } +rustls-pemfile = { workspace = true } +rustls-pki-types = { workspace = true } +rustls-webpki = { workspace = true } schemars = { workspace = true } semver = { workspace = true } serde = { workspace = true } diff --git a/packages/nextclade-cli/src/io/http_client.rs b/packages/nextclade-cli/src/io/http_client.rs index b8cd5c0c4..a280fe195 100644 --- a/packages/nextclade-cli/src/io/http_client.rs +++ b/packages/nextclade-cli/src/io/http_client.rs @@ -1,12 +1,19 @@ use clap::{Parser, ValueHint}; -use eyre::Report; +use eyre::{Report, WrapErr}; use log::info; use nextclade::make_internal_error; use nextclade::utils::info::{this_package_name, this_package_version_str}; use reqwest::blocking::Client; use reqwest::{Method, Proxy}; -use rustls_platform_verifier; +use rustls::ClientConfig; +use rustls_pemfile; +use rustls_pki_types::CertificateDer; +use rustls_platform_verifier::Verifier; +use std::env; +use std::io::BufReader; +use std::fs::File; use std::str::FromStr; +use std::sync::Arc; use std::time::Duration; use url::Url; @@ -62,8 +69,13 @@ impl HttpClient { let user_agent = format!("{} {}", this_package_name(), this_package_version_str()); + let tls_config = ClientConfig::builder() + .dangerous() // …but the rustls_platform_verifier::Verifier is safe + .with_custom_certificate_verifier(Arc::new(Verifier::new_with_extra_roots(extra_ca_certs()?)?)) + .with_no_client_auth(); + let client = client_builder - .use_preconfigured_tls(rustls_platform_verifier::tls_config()) + .use_preconfigured_tls(tls_config) .connection_verbose(verbose) .connect_timeout(Some(Duration::from_secs(60))) .user_agent(user_agent) @@ -112,3 +124,21 @@ impl HttpClient { Ok(content) } } + +fn extra_ca_certs() -> Result>, Report> { + match env::var_os("NEXTCLADE_EXTRA_CA_CERTS") { + Some(filename) => { + let file = File::open(filename.clone()) + .wrap_err_with(|| format!("When opening NEXTCLADE_EXTRA_CA_CERTS file {filename:?}"))?; + + let mut reader = BufReader::new(file); + + let certs = rustls_pemfile::certs(&mut reader) + .map(|c| c.wrap_err_with(|| "When parsing an extra CA certificate".to_owned())) + .collect::, Report>>()?; + + Ok(certs) + } + None => Ok(vec![]) + } +} From 310eb65f6f0c10a32b7b2c013a225847cf4cf380 Mon Sep 17 00:00:00 2001 From: ivan-aksamentov Date: Mon, 14 Oct 2024 18:59:32 +0200 Subject: [PATCH 2/5] refactor: avoid static lifetime --- packages/nextclade-cli/src/io/http_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextclade-cli/src/io/http_client.rs b/packages/nextclade-cli/src/io/http_client.rs index a280fe195..07496a0e7 100644 --- a/packages/nextclade-cli/src/io/http_client.rs +++ b/packages/nextclade-cli/src/io/http_client.rs @@ -125,7 +125,7 @@ impl HttpClient { } } -fn extra_ca_certs() -> Result>, Report> { +fn extra_ca_certs<'a>() -> Result>, Report> { match env::var_os("NEXTCLADE_EXTRA_CA_CERTS") { Some(filename) => { let file = File::open(filename.clone()) From 58569a8101023aeac7e850822ad953087ea0e00d Mon Sep 17 00:00:00 2001 From: ivan-aksamentov Date: Mon, 14 Oct 2024 19:06:20 +0200 Subject: [PATCH 3/5] refactor: simplify error handling --- packages/nextclade-cli/src/io/http_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextclade-cli/src/io/http_client.rs b/packages/nextclade-cli/src/io/http_client.rs index 07496a0e7..27fc73103 100644 --- a/packages/nextclade-cli/src/io/http_client.rs +++ b/packages/nextclade-cli/src/io/http_client.rs @@ -134,7 +134,7 @@ fn extra_ca_certs<'a>() -> Result>, let mut reader = BufReader::new(file); let certs = rustls_pemfile::certs(&mut reader) - .map(|c| c.wrap_err_with(|| "When parsing an extra CA certificate".to_owned())) + .map(|c| c.wrap_err("When parsing an extra CA certificate")) .collect::, Report>>()?; Ok(certs) From 123d183290b1e531deb9a7a714faeb936c4c0339 Mon Sep 17 00:00:00 2001 From: ivan-aksamentov Date: Mon, 14 Oct 2024 19:22:31 +0200 Subject: [PATCH 4/5] refactor: reuse utility fn to open buffered file --- packages/nextclade-cli/src/io/http_client.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/nextclade-cli/src/io/http_client.rs b/packages/nextclade-cli/src/io/http_client.rs index 27fc73103..bcf6747b4 100644 --- a/packages/nextclade-cli/src/io/http_client.rs +++ b/packages/nextclade-cli/src/io/http_client.rs @@ -1,6 +1,7 @@ use clap::{Parser, ValueHint}; use eyre::{Report, WrapErr}; use log::info; +use nextclade::io::file::open_file_or_stdin; use nextclade::make_internal_error; use nextclade::utils::info::{this_package_name, this_package_version_str}; use reqwest::blocking::Client; @@ -10,8 +11,6 @@ use rustls_pemfile; use rustls_pki_types::CertificateDer; use rustls_platform_verifier::Verifier; use std::env; -use std::io::BufReader; -use std::fs::File; use std::str::FromStr; use std::sync::Arc; use std::time::Duration; @@ -128,12 +127,9 @@ impl HttpClient { fn extra_ca_certs<'a>() -> Result>, Report> { match env::var_os("NEXTCLADE_EXTRA_CA_CERTS") { Some(filename) => { - let file = File::open(filename.clone()) - .wrap_err_with(|| format!("When opening NEXTCLADE_EXTRA_CA_CERTS file {filename:?}"))?; + let mut file = open_file_or_stdin(&Some(filename))?; - let mut reader = BufReader::new(file); - - let certs = rustls_pemfile::certs(&mut reader) + let certs = rustls_pemfile::certs(&mut file) .map(|c| c.wrap_err("When parsing an extra CA certificate")) .collect::, Report>>()?; From 2e4557ce0c0f4d9d0bded2989a1d6e2b8cdc2782 Mon Sep 17 00:00:00 2001 From: ivan-aksamentov Date: Mon, 14 Oct 2024 19:39:58 +0200 Subject: [PATCH 5/5] feat(cli): add --extra-ca-certs Adds a CLI arg with the same meaning as `NEXTCLADE_EXTRA_CA_CERTS` env var - to provide a path to extra CA certs. Perhaps some users might prefer a CLI arg. It was easy, so I thought why not? --- packages/nextclade-cli/src/io/http_client.rs | 27 +++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/nextclade-cli/src/io/http_client.rs b/packages/nextclade-cli/src/io/http_client.rs index bcf6747b4..805e48217 100644 --- a/packages/nextclade-cli/src/io/http_client.rs +++ b/packages/nextclade-cli/src/io/http_client.rs @@ -11,6 +11,7 @@ use rustls_pemfile; use rustls_pki_types::CertificateDer; use rustls_platform_verifier::Verifier; use std::env; +use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; use std::time::Duration; @@ -33,6 +34,13 @@ pub struct ProxyConfig { #[clap(long)] #[clap(value_hint = ValueHint::Other)] pub proxy_pass: Option, + + /// Path to extra CA certificates + /// + /// You can also provide path to CA certificates in environment variable `NEXTCLADE_EXTRA_CA_CERTS`. The argument takes precedence over environment variable if both are provided. + #[clap(long)] + #[clap(value_hint = ValueHint::Other)] + pub extra_ca_certs: Option, } pub struct HttpClient { @@ -68,9 +76,12 @@ impl HttpClient { let user_agent = format!("{} {}", this_package_name(), this_package_version_str()); + let extra_ca_certs_filepath = env::var_os("NEXTCLADE_EXTRA_CA_CERTS").map(PathBuf::from); + let extra_ca_certs_filepath = proxy_conf.extra_ca_certs.as_ref().or(extra_ca_certs_filepath.as_ref()); + let tls_config = ClientConfig::builder() .dangerous() // …but the rustls_platform_verifier::Verifier is safe - .with_custom_certificate_verifier(Arc::new(Verifier::new_with_extra_roots(extra_ca_certs()?)?)) + .with_custom_certificate_verifier(Arc::new(Verifier::new_with_extra_roots(extra_ca_certs(extra_ca_certs_filepath)?)?)) .with_no_client_auth(); let client = client_builder @@ -124,9 +135,12 @@ impl HttpClient { } } -fn extra_ca_certs<'a>() -> Result>, Report> { - match env::var_os("NEXTCLADE_EXTRA_CA_CERTS") { - Some(filename) => { +fn extra_ca_certs<'a>( + extra_ca_certs_filepath: Option>, +) -> Result>, Report> { + extra_ca_certs_filepath.map_or_else( + || Ok(vec![]), + |filename| { let mut file = open_file_or_stdin(&Some(filename))?; let certs = rustls_pemfile::certs(&mut file) @@ -134,7 +148,6 @@ fn extra_ca_certs<'a>() -> Result>, .collect::, Report>>()?; Ok(certs) - } - None => Ok(vec![]) - } + }, + ) }