From 32ed4109727339b4953199043059ea4cebae2a22 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Mon, 26 May 2025 09:19:24 +0300 Subject: [PATCH 1/2] Fix the case of SSL_CERT_DIR containing number of paths According to OpenSSL docs SSL_CERT_DIR is an env seperated list of directories (for Unix it's colon on Windows it's semi-colon). Reference: https://docs.openssl.org/3.5/man1/openssl-rehash/#options Signed-off-by: Jon Doron --- src/lib.rs | 50 +++++++++++++++++++++++++++++++++++--------------- src/unix.rs | 5 ++++- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 629e2c3..a64e7d9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -118,9 +118,10 @@ use macos as platform; /// [c_rehash]: https://www.openssl.org/docs/manmaster/man1/c_rehash.html pub fn load_native_certs() -> CertificateResult { let paths = CertPaths::from_env(); - match (&paths.dir, &paths.file) { - (Some(_), _) | (_, Some(_)) => paths.load(), - (None, None) => platform::load_native_certs(), + match (&paths.dirs, &paths.file) { + (v, _) if !v.is_empty() => paths.load(), + (_, Some(_)) => paths.load(), + _ => platform::load_native_certs(), } } @@ -189,14 +190,21 @@ impl CertificateResult { /// Certificate paths from `SSL_CERT_FILE` and/or `SSL_CERT_DIR`. struct CertPaths { file: Option, - dir: Option, + dirs: Vec, } impl CertPaths { fn from_env() -> Self { Self { file: env::var_os(ENV_CERT_FILE).map(PathBuf::from), - dir: env::var_os(ENV_CERT_DIR).map(PathBuf::from), + // Read `SSL_CERT_DIR`, split it on the platform delimiter (`:` on Unix, `;` on Windows), + // and return the entries as `PathBuf`s. + // + // See + dirs: match env::var_os(ENV_CERT_DIR) { + Some(dirs) => env::split_paths(&dirs).collect(), + None => Vec::new(), + }, } } @@ -204,7 +212,7 @@ impl CertPaths { /// /// See [`load_certs_from_paths()`]. fn load(&self) -> CertificateResult { - load_certs_from_paths(self.file.as_deref(), self.dir.as_deref()) + load_certs_from_paths_internal(self.file.as_deref(), &self.dirs) } } @@ -223,8 +231,20 @@ impl CertPaths { /// subject to the rules outlined above for `file`. The directory is not /// scanned recursively and may be empty. pub fn load_certs_from_paths(file: Option<&Path>, dir: Option<&Path>) -> CertificateResult { + let dir = match dir { + Some(d) => vec![d], + None => Vec::new(), + }; + + load_certs_from_paths_internal(file, dir.as_ref()) +} + +fn load_certs_from_paths_internal( + file: Option<&Path>, + dir: &[impl AsRef], +) -> CertificateResult { let mut out = CertificateResult::default(); - if file.is_none() && dir.is_none() { + if file.is_none() && dir.is_empty() { return out; } @@ -232,8 +252,8 @@ pub fn load_certs_from_paths(file: Option<&Path>, dir: Option<&Path>) -> Certifi load_pem_certs(cert_file, &mut out); } - if let Some(cert_dir) = dir { - load_pem_certs_from_dir(cert_dir, &mut out); + for cert_dir in dir.iter() { + load_pem_certs_from_dir(cert_dir.as_ref(), &mut out); } out.certs @@ -451,21 +471,21 @@ mod tests { let result = CertPaths { file: Some(file_path.clone()), - dir: None, + dirs: vec![], } .load(); assert_eq!(result.certs.len(), 2); let result = CertPaths { file: None, - dir: Some(dir_path.clone()), + dirs: vec![dir_path.clone()], } .load(); assert_eq!(result.certs.len(), 2); let result = CertPaths { file: Some(file_path), - dir: Some(dir_path), + dirs: vec![dir_path], } .load(); assert_eq!(result.certs.len(), 2); @@ -519,7 +539,7 @@ mod tests { test_cert_paths_bad_perms(CertPaths { file: None, - dir: Some(temp_dir.path().into()), + dirs: vec![temp_dir.path().into()], }) } @@ -536,7 +556,7 @@ mod tests { test_cert_paths_bad_perms(CertPaths { file: Some(file_path.clone()), - dir: None, + dirs: vec![], }); } @@ -544,7 +564,7 @@ mod tests { fn test_cert_paths_bad_perms(cert_paths: CertPaths) { let result = cert_paths.load(); - if let (None, None) = (cert_paths.file, cert_paths.dir) { + if let (None, true) = (cert_paths.file, cert_paths.dirs.is_empty()) { panic!("only one of file or dir should be set"); }; diff --git a/src/unix.rs b/src/unix.rs index 7bb869f..cb8c224 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -4,7 +4,10 @@ pub fn load_native_certs() -> CertificateResult { let likely_locations = openssl_probe::probe(); CertPaths { file: likely_locations.cert_file, - dir: likely_locations.cert_dir, + dirs: likely_locations + .cert_dir + .into_iter() + .collect(), } .load() } From 1438c63107b79309c958241ed08fa42f3cb790bc Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Sat, 31 May 2025 16:24:04 +0300 Subject: [PATCH 2/2] Tests: Verify support for multiple directories in SSL_CERT_DIR Adds a test to ensure that SSL_CERT_DIR correctly handles multiple paths separated by the platform-specific separator (: on Unix, ; on Windows). The test sets up two temporary directories: * The first is left empty. * The second contains a copy of the test certificate. It then sets SSL_CERT_DIR to include both directories and calls check_site() to verify that the certificate is successfully loaded from the second path. Signed-off-by: Jon Doron --- tests/smoketests.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/smoketests.rs b/tests/smoketests.rs index 2e7bb1e..2c98e78 100644 --- a/tests/smoketests.rs +++ b/tests/smoketests.rs @@ -197,6 +197,39 @@ fn badssl_with_dir_from_env() { check_site("self-signed.badssl.com").unwrap(); } +#[test] +#[serial] +#[ignore] +fn ssl_cert_dir_multiple_paths_are_respected() { + unsafe { + // SAFETY: safe because of #[serial] + common::clear_env(); + } + + // Create 2 temporary directories + let temp_dir1 = tempfile::TempDir::new().unwrap(); + let temp_dir2 = tempfile::TempDir::new().unwrap(); + + // Copy the certificate to the 2nd dir, leaving the 1st one + // empty. + let original = Path::new("tests/badssl-com-chain.pem") + .canonicalize() + .unwrap(); + let cert = temp_dir2.path().join("5d30f3c5.3"); + std::fs::copy(original, cert).unwrap(); + + let list_sep = if cfg!(windows) { ';' } else { ':' }; + let value = format!( + "{}{}{}", + temp_dir1.path().display(), + list_sep, + temp_dir2.path().display() + ); + + env::set_var("SSL_CERT_DIR", value); + check_site("self-signed.badssl.com").unwrap(); +} + #[test] #[serial] #[ignore]