From c35587a9dfe63e3d901a4144a320412c7598f2c8 Mon Sep 17 00:00:00 2001 From: stormshield-gt <143998166+stormshield-gt@users.noreply.github.com.> Date: Tue, 27 Aug 2024 15:23:36 +0200 Subject: [PATCH 1/6] Fix typos --- rustls-platform-verifier/src/verification/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rustls-platform-verifier/src/verification/windows.rs b/rustls-platform-verifier/src/verification/windows.rs index 3ff76dd..8abe087 100644 --- a/rustls-platform-verifier/src/verification/windows.rs +++ b/rustls-platform-verifier/src/verification/windows.rs @@ -136,7 +136,7 @@ impl CertChain { extra_params.pwszServerName = server_null_terminated.as_mut_ptr(); let mut params = CERT_CHAIN_POLICY_PARA::zeroed_with_size(); - // Ignore any errors when trying to obtain OCSP recovcation information. + // Ignore any errors when trying to obtain OCSP revocation information. // This is also done in OpenSSL, Secure Transport from Apple, etc. params.dwFlags = CERT_CHAIN_POLICY_IGNORE_ALL_REV_UNKNOWN_FLAGS; // `extra_params` outlives `params`. From 157c5c2d95005505645922a407f2cbdad790c093 Mon Sep 17 00:00:00 2001 From: stormshield-gt <143998166+stormshield-gt@users.noreply.github.com.> Date: Fri, 13 Sep 2024 16:18:04 +0200 Subject: [PATCH 2/6] Add extra roots on Windows --- .../src/verification/windows.rs | 87 ++++++++++++++++--- 1 file changed, 75 insertions(+), 12 deletions(-) diff --git a/rustls-platform-verifier/src/verification/windows.rs b/rustls-platform-verifier/src/verification/windows.rs index 8abe087..e743784 100644 --- a/rustls-platform-verifier/src/verification/windows.rs +++ b/rustls-platform-verifier/src/verification/windows.rs @@ -40,17 +40,18 @@ use windows_sys::Win32::{ CERT_E_WRONG_USAGE, CRYPT_E_REVOKED, FILETIME, TRUE, }, Security::Cryptography::{ - CertAddEncodedCertificateToStore, CertCloseStore, CertFreeCertificateChain, - CertFreeCertificateChainEngine, CertFreeCertificateContext, CertGetCertificateChain, - CertOpenStore, CertSetCertificateContextProperty, CertVerifyCertificateChainPolicy, - HTTPSPolicyCallbackData, AUTHTYPE_SERVER, CERT_CHAIN_CACHE_END_CERT, CERT_CHAIN_CONTEXT, + CertAddEncodedCertificateToStore, CertCloseStore, CertCreateCertificateChainEngine, + CertFreeCertificateChain, CertFreeCertificateChainEngine, CertFreeCertificateContext, + CertGetCertificateChain, CertOpenStore, CertSetCertificateContextProperty, + CertVerifyCertificateChainPolicy, HTTPSPolicyCallbackData, AUTHTYPE_SERVER, + CERT_CHAIN_CACHE_END_CERT, CERT_CHAIN_CONTEXT, CERT_CHAIN_ENGINE_CONFIG, CERT_CHAIN_POLICY_IGNORE_ALL_REV_UNKNOWN_FLAGS, CERT_CHAIN_POLICY_PARA, CERT_CHAIN_POLICY_SSL, CERT_CHAIN_POLICY_STATUS, CERT_CHAIN_REVOCATION_ACCUMULATIVE_TIMEOUT, CERT_CHAIN_REVOCATION_CHECK_END_CERT, CERT_CONTEXT, CERT_OCSP_RESPONSE_PROP_ID, CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG, CERT_STORE_ADD_ALWAYS, CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, CERT_STORE_PROV_MEMORY, - CERT_STRONG_SIGN_PARA, CERT_USAGE_MATCH, CRYPT_INTEGER_BLOB, CTL_USAGE, - USAGE_MATCH_TYPE_AND, X509_ASN_ENCODING, + CERT_STRONG_SIGN_PARA, CERT_TRUST_IS_PARTIAL_CHAIN, CERT_USAGE_MATCH, CRYPT_INTEGER_BLOB, + CTL_USAGE, USAGE_MATCH_TYPE_AND, X509_ASN_ENCODING, }, }; @@ -76,8 +77,6 @@ struct CERT_CHAIN_PARA { } use crate::verification::invalid_certificate; -#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] -use windows_sys::Win32::Security::Cryptography::CERT_CHAIN_ENGINE_CONFIG; // SAFETY: see method implementation unsafe impl ZeroedWithSize for CERT_CHAIN_PARA { @@ -110,7 +109,6 @@ unsafe impl ZeroedWithSize for CERT_CHAIN_POLICY_PARA { } } -#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] // SAFETY: see method implementation unsafe impl ZeroedWithSize for CERT_CHAIN_ENGINE_CONFIG { fn zeroed_with_size() -> Self { @@ -273,11 +271,36 @@ impl CertificateStore { self.engine.map(|e| e.as_ptr() as isize).unwrap_or(0) } + fn new_with_extra_roots( + roots: &[pki_types::CertificateDer<'static>], + ) -> Result { + let mut inner = Self::new()?; + let mut exclusive_store = CertificateStore::new()?; + for root in roots { + exclusive_store.add_cert(root)?; + } + + let mut config = CERT_CHAIN_ENGINE_CONFIG::zeroed_with_size(); + config.hExclusiveRoot = exclusive_store.inner.as_ptr(); + + let mut engine = 0; + // SAFETY: `engine` is valid to be written to and the config is valid to be read. + let res = unsafe { CertCreateCertificateChainEngine(&config, &mut engine) }; + + #[allow(clippy::as_conversions)] + let engine = call_with_last_error(|| match NonNull::new(engine as *mut c_void) { + Some(c) if res == TRUE => Some(c), + _ => None, + })?; + inner.engine = Some(engine); + + Ok(inner) + } + #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] fn new_with_fake_root(root: &[u8]) -> Result { use windows_sys::Win32::Security::Cryptography::{ - CertCreateCertificateChainEngine, CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL, - CERT_CHAIN_ENABLE_CACHE_AUTO_UPDATE, + CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL, CERT_CHAIN_ENABLE_CACHE_AUTO_UPDATE, }; let mut inner = Self::new()?; @@ -442,6 +465,9 @@ pub struct Verifier { #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: Option>, pub(super) crypto_provider: OnceCell>, + /// Extra trust anchors to add to the verifier above and beyond those provided by + /// the system-provided trust stores. + extra_roots: Vec>, } impl Verifier { @@ -456,6 +482,22 @@ impl Verifier { #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: None, crypto_provider: OnceCell::new(), + extra_roots: Vec::new(), + } + } + + /// Creates a new instance of a TLS certificate verifier that utilizes the + /// Windows certificate facilities and augmented by the provided extra root certificates. + /// + /// A [`CryptoProvider`] must be set with + /// [`set_provider`][Verifier::set_provider]/[`with_provider`][Verifier::with_provider] or + /// [`CryptoProvider::install_default`] before the verifier can be used. + pub fn new_with_extra_roots(roots: Vec>) -> Self { + Self { + #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] + test_only_root_ca_override: None, + crypto_provider: OnceCell::new(), + extra_roots: roots, } } @@ -465,6 +507,7 @@ impl Verifier { Self { test_only_root_ca_override: Some(root.into()), crypto_provider: OnceCell::new(), + extra_roots: Vec::new(), } } @@ -521,7 +564,27 @@ impl Verifier { .chain(Some(0)) .collect(); - let cert_chain = store.new_chain_in(&primary_cert, now)?; + let mut cert_chain = store.new_chain_in(&primary_cert, now)?; + + // We only use `TrustStatus` here because it hasn't had verification performed on it. + // SAFETY: The pointer is guaranteed to be non-null. + let is_partial_chain = unsafe { *cert_chain.inner.as_ptr() } + .TrustStatus + .dwErrorStatus + & CERT_TRUST_IS_PARTIAL_CHAIN + != 0; + + // If we have extra roots and building the chain gave us an error, we try to build a + // new one with the extra roots. + if !self.extra_roots.is_empty() && is_partial_chain { + let mut store = CertificateStore::new_with_extra_roots(&self.extra_roots)?; + + for cert in intermediate_certs.iter().copied() { + store.add_cert(cert)?; + } + + cert_chain = store.new_chain_in(&primary_cert, now)?; + } let status = cert_chain.verify_chain_policy(server)?; From 7caa740aa702776984e58c66883e7e8a79859bf9 Mon Sep 17 00:00:00 2001 From: stormshield-gt <143998166+stormshield-gt@users.noreply.github.com.> Date: Tue, 27 Aug 2024 17:24:29 +0200 Subject: [PATCH 3/6] Enable test on Windows --- .../src/tests/verification_mock/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rustls-platform-verifier/src/tests/verification_mock/mod.rs b/rustls-platform-verifier/src/tests/verification_mock/mod.rs index 59e322a..93f9366 100644 --- a/rustls-platform-verifier/src/tests/verification_mock/mod.rs +++ b/rustls-platform-verifier/src/tests/verification_mock/mod.rs @@ -43,7 +43,7 @@ macro_rules! mock_root_test_cases { } paste::paste!{ - #[cfg(all($target, not(windows), not(target_os = "android")))] + #[cfg(all($target, not(target_os = "android")))] #[test] pub fn [<$name _extra>](){ super::[<$name _extra>]() @@ -60,7 +60,7 @@ macro_rules! mock_root_test_cases { $name, paste::paste!{ - #[cfg(all($target, not(windows), not(target_os = "android")))] + #[cfg(all($target, not(target_os = "android")))] [<$name _extra>] } @@ -77,7 +77,7 @@ macro_rules! mock_root_test_cases { } paste::paste!{ - #[cfg(all($target, not(windows), not(target_os = "android")))] + #[cfg(all($target, not(target_os = "android")))] pub(super) fn [<$name _extra>]() { test_with_mock_root(&$test_case, Roots::ExtraAndPlatform); } @@ -336,7 +336,7 @@ fn test_with_mock_root( let verifier = match root_src { Roots::OnlyExtra => Verifier::new_with_fake_root(ROOT1), // TODO: time - #[cfg(all(unix, not(target_os = "android")))] + #[cfg(not(target_os = "android"))] Roots::ExtraAndPlatform => Verifier::new_with_extra_roots(vec![ROOT1.into()]), }; let mut chain = test_case @@ -379,6 +379,6 @@ enum Roots { /// Test with loading the extra roots and the platform trust store. /// /// Right now, not all platforms are supported. - #[cfg(all(unix, not(target_os = "android")))] + #[cfg(not(target_os = "android"))] ExtraAndPlatform, } From 09e9276705a31e91eb945b20b030a3a4ec698a84 Mon Sep 17 00:00:00 2001 From: stormshield-gt <143998166+stormshield-gt@users.noreply.github.com.> Date: Tue, 17 Sep 2024 09:22:26 +0200 Subject: [PATCH 4/6] Parse the extra roots only once on Windows --- .../src/verification/windows.rs | 176 ++++++++++-------- 1 file changed, 101 insertions(+), 75 deletions(-) diff --git a/rustls-platform-verifier/src/verification/windows.rs b/rustls-platform-verifier/src/verification/windows.rs index e743784..0803082 100644 --- a/rustls-platform-verifier/src/verification/windows.rs +++ b/rustls-platform-verifier/src/verification/windows.rs @@ -212,69 +212,15 @@ impl Drop for Certificate { } } -/// An in-memory Windows certificate store. -/// -/// # Safety -/// -/// `CertificateStore` creates `Certificate` objects that may outlive the -/// `CertificateStore`. This is only safe to do if the certificate store is -/// constructed with `CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG`. -struct CertificateStore { - inner: NonNull, // HCERTSTORE - // In production code, this is always `None`. - // - // During tests, we set this to `Some` as the tests use a - // custom verification engine that only uses specific roots. - engine: Option>, // HCERTENGINECONTEXT -} - -impl Drop for CertificateStore { - fn drop(&mut self) { - let engine_ptr = self.engine_ptr(); - if self.engine.take().is_some() { - // SAFETY: The engine pointer is guaranteed to be non-null. - unsafe { CertFreeCertificateChainEngine(engine_ptr) }; - } - - // SAFETY: See the `CertificateStore` documentation. - unsafe { CertCloseStore(self.inner.as_ptr(), 0) }; - } +#[derive(Debug)] +struct CertEngine { + inner: NonNull, // HCERTENGINECONTEXT } -impl CertificateStore { - /// Creates a new, in-memory certificate store. - fn new() -> Result { - let store = call_with_last_error(|| { - // SAFETY: Called with valid constants and result is checked to be non-null. - // The `CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG` flag is critical; - // see the `CertificateStore` documentation for more info. - NonNull::new(unsafe { - CertOpenStore( - CERT_STORE_PROV_MEMORY, - 0, // Set to zero since this uses `PROV_MEMORY`. - 0, // This field shouldn't be used. - CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, - ptr::null(), - ) - }) - })?; - - // Use the system's default root store and rules. - Ok(Self { - inner: store, - engine: None, - }) - } - - fn engine_ptr(&self) -> isize { - #[allow(clippy::as_conversions)] - self.engine.map(|e| e.as_ptr() as isize).unwrap_or(0) - } - +impl CertEngine { fn new_with_extra_roots( roots: &[pki_types::CertificateDer<'static>], ) -> Result { - let mut inner = Self::new()?; let mut exclusive_store = CertificateStore::new()?; for root in roots { exclusive_store.add_cert(root)?; @@ -292,9 +238,7 @@ impl CertificateStore { Some(c) if res == TRUE => Some(c), _ => None, })?; - inner.engine = Some(engine); - - Ok(inner) + Ok(Self { inner: engine }) } #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] @@ -303,8 +247,6 @@ impl CertificateStore { CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL, CERT_CHAIN_ENABLE_CACHE_AUTO_UPDATE, }; - let mut inner = Self::new()?; - let mut root_store = CertificateStore::new()?; root_store.add_cert(root)?; @@ -330,6 +272,86 @@ impl CertificateStore { Some(c) if res == TRUE => Some(c), _ => None, })?; + + Ok(Self { inner: engine }) + } + + #[allow(clippy::as_conversions)] + fn as_ptr(&self) -> isize { + self.inner.as_ptr() as isize + } +} + +impl Drop for CertEngine { + fn drop(&mut self) { + // SAFETY: The engine pointer is guaranteed to be non-null. + unsafe { CertFreeCertificateChainEngine(self.as_ptr()) }; + } +} + +// SAFETY: We know no other threads is mutating the `CertEngine`, because it would require `unsafe`. +// Across the FFI, `CertGetCertificateChain` don't mutate it either. +unsafe impl Sync for CertEngine {} +// SAFETY: All methods of `CertEngine`, including `Drop`, are safe to be called from other +// threads, because all contained resources are owned by Windows and we only maintain reference counted handles to them. +unsafe impl Send for CertEngine {} + +/// An in-memory Windows certificate store. +/// +/// # Safety +/// +/// `CertificateStore` creates `Certificate` objects that may outlive the +/// `CertificateStore`. This is only safe to do if the certificate store is +/// constructed with `CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG`. +struct CertificateStore { + inner: NonNull, // HCERTSTORE + // In production code, this is always `None`. + // + // During tests, we set this to `Some` as the tests use a + // custom verification engine that only uses specific roots. + engine: Option, // HCERTENGINECONTEXT +} + +impl Drop for CertificateStore { + fn drop(&mut self) { + // SAFETY: See the `CertificateStore` documentation. + unsafe { CertCloseStore(self.inner.as_ptr(), 0) }; + } +} + +impl CertificateStore { + /// Creates a new, in-memory certificate store. + fn new() -> Result { + let store = call_with_last_error(|| { + // SAFETY: Called with valid constants and result is checked to be non-null. + // The `CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG` flag is critical; + // see the `CertificateStore` documentation for more info. + NonNull::new(unsafe { + CertOpenStore( + CERT_STORE_PROV_MEMORY, + 0, // Set to zero since this uses `PROV_MEMORY`. + 0, // This field shouldn't be used. + CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, + ptr::null(), + ) + }) + })?; + + // Use the system's default root store and rules. + Ok(Self { + inner: store, + engine: None, + }) + } + + #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] + fn new_with_fake_root(root: &[u8]) -> Result { + let mut inner = Self::new()?; + + let mut root_store = CertificateStore::new()?; + root_store.add_cert(root)?; + + let engine = CertEngine::new_with_fake_root(root)?; inner.engine = Some(engine); Ok(inner) @@ -370,6 +392,7 @@ impl CertificateStore { &self, certificate: &Certificate, now: pki_types::UnixTime, + engine: Option<&CertEngine>, ) -> Result { let mut cert_chain = ptr::null_mut(); @@ -428,7 +451,7 @@ impl CertificateStore { let parameters = NonNull::from(¶meters).cast().as_ptr(); CertGetCertificateChain( - self.engine_ptr(), + engine.map(CertEngine::as_ptr).unwrap_or(0), certificate.inner.as_ptr(), &time, self.inner.as_ptr(), @@ -467,7 +490,7 @@ pub struct Verifier { pub(super) crypto_provider: OnceCell>, /// Extra trust anchors to add to the verifier above and beyond those provided by /// the system-provided trust stores. - extra_roots: Vec>, + extra_roots: Option, } impl Verifier { @@ -482,7 +505,7 @@ impl Verifier { #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: None, crypto_provider: OnceCell::new(), - extra_roots: Vec::new(), + extra_roots: None, } } @@ -492,13 +515,16 @@ impl Verifier { /// A [`CryptoProvider`] must be set with /// [`set_provider`][Verifier::set_provider]/[`with_provider`][Verifier::with_provider] or /// [`CryptoProvider::install_default`] before the verifier can be used. - pub fn new_with_extra_roots(roots: Vec>) -> Self { - Self { + pub fn new_with_extra_roots( + roots: Vec>, + ) -> Result { + let cert_engine = CertEngine::new_with_extra_roots(&roots)?; + Ok(Self { #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: None, crypto_provider: OnceCell::new(), - extra_roots: roots, - } + extra_roots: Some(cert_engine), + }) } /// Creates a test-only TLS certificate verifier which trusts our fake root CA cert. @@ -507,7 +533,7 @@ impl Verifier { Self { test_only_root_ca_override: Some(root.into()), crypto_provider: OnceCell::new(), - extra_roots: Vec::new(), + extra_roots: None, } } @@ -564,7 +590,7 @@ impl Verifier { .chain(Some(0)) .collect(); - let mut cert_chain = store.new_chain_in(&primary_cert, now)?; + let mut cert_chain = store.new_chain_in(&primary_cert, now, store.engine.as_ref())?; // We only use `TrustStatus` here because it hasn't had verification performed on it. // SAFETY: The pointer is guaranteed to be non-null. @@ -576,14 +602,14 @@ impl Verifier { // If we have extra roots and building the chain gave us an error, we try to build a // new one with the extra roots. - if !self.extra_roots.is_empty() && is_partial_chain { - let mut store = CertificateStore::new_with_extra_roots(&self.extra_roots)?; + if is_partial_chain && self.extra_roots.is_some() { + let mut store = CertificateStore::new()?; for cert in intermediate_certs.iter().copied() { store.add_cert(cert)?; } - cert_chain = store.new_chain_in(&primary_cert, now)?; + cert_chain = store.new_chain_in(&primary_cert, now, self.extra_roots.as_ref())?; } let status = cert_chain.verify_chain_policy(server)?; From 05ddb1c8cce9273e7e47510e256e7d12db0d1537 Mon Sep 17 00:00:00 2001 From: stormshield-gt <143998166+stormshield-gt@users.noreply.github.com.> Date: Tue, 17 Sep 2024 14:22:16 +0200 Subject: [PATCH 5/6] Parse the extra roots only once on Apple --- .../src/verification/apple.rs | 48 ++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/rustls-platform-verifier/src/verification/apple.rs b/rustls-platform-verifier/src/verification/apple.rs index 5dc3a22..07bd944 100644 --- a/rustls-platform-verifier/src/verification/apple.rs +++ b/rustls-platform-verifier/src/verification/apple.rs @@ -45,10 +45,10 @@ fn system_time_to_cfdate(time: pki_types::UnixTime) -> Result pub struct Verifier { /// Extra trust anchors to add to the verifier above and beyond those provided by /// the system-provided trust stores. - extra_roots: Vec>, + extra_roots: Vec, /// Testing only: The root CA certificate to trust. #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] - test_only_root_ca_override: Option>, + test_only_root_ca_override: Option, pub(super) crypto_provider: OnceCell>, } @@ -72,13 +72,22 @@ impl Verifier { /// facilities with the addition of extra root certificates to trust. /// /// See [Verifier::new] for the external requirements the verifier needs. - pub fn new_with_extra_roots(roots: Vec>) -> Self { - Self { - extra_roots: roots, + pub fn new_with_extra_roots( + roots: Vec>, + ) -> Result { + let extra_roots = roots + .into_iter() + .map(|root| { + SecCertificate::from_der(&root) + .map_err(|_| TlsError::InvalidCertificate(CertificateError::BadEncoding)) + }) + .collect::, _>>()?; + Ok(Self { + extra_roots, #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: None, crypto_provider: OnceCell::new(), - } + }) } /// Creates a test-only TLS certificate verifier which trusts our fake root CA cert. @@ -86,7 +95,7 @@ impl Verifier { pub(crate) fn new_with_fake_root(root: &[u8]) -> Self { Self { extra_roots: Vec::new(), - test_only_root_ca_override: Some(root.into()), + test_only_root_ca_override: Some(SecCertificate::from_der(root).unwrap()), crypto_provider: OnceCell::new(), } } @@ -141,29 +150,24 @@ impl Verifier { .map_err(|e| invalid_certificate(e.to_string()))?; } - let raw_extra_roots = self.extra_roots.iter(); + #[cfg(not(any(test, feature = "ffi-testing", feature = "dbg")))] + let extra_roots = self.extra_roots.as_slice(); #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] - let extra_root = self - .test_only_root_ca_override - .as_ref() - .map(|root| pki_types::CertificateDer::from_slice(root)); - + let extra_roots: Vec<_> = self + .extra_roots + .iter() + .chain(self.test_only_root_ca_override.as_ref()) + .cloned() + .collect(); #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] - let raw_extra_roots = raw_extra_roots.chain(&extra_root).to_owned(); - - let extra_roots = raw_extra_roots - .map(|root| { - SecCertificate::from_der(root) - .map_err(|_| TlsError::InvalidCertificate(CertificateError::BadEncoding)) - }) - .collect::, _>>()?; + let extra_roots = extra_roots.as_slice(); // If any extra roots were provided by the user (or tests), provide them to the trust // evaluation regardless of their system trust settings or status. if !extra_roots.is_empty() { trust_evaluation - .set_anchor_certificates(&extra_roots) + .set_anchor_certificates(extra_roots) .map_err(|e| TlsError::Other(OtherError(Arc::new(e))))?; // We want to trust both the system-installed and the extra roots. This must be set From ddcaa3e343445126ab456a31e9aa9027a14ce5e2 Mon Sep 17 00:00:00 2001 From: stormshield-gt <143998166+stormshield-gt@users.noreply.github.com.> Date: Tue, 17 Sep 2024 11:35:20 +0200 Subject: [PATCH 6/6] Uniformize to faillible new_with_extra_roots everywhere --- .../src/tests/verification_mock/mod.rs | 5 +++-- .../src/tests/verification_real_world/mod.rs | 3 ++- rustls-platform-verifier/src/verification/others.rs | 8 +++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/rustls-platform-verifier/src/tests/verification_mock/mod.rs b/rustls-platform-verifier/src/tests/verification_mock/mod.rs index 93f9366..9fbc8ac 100644 --- a/rustls-platform-verifier/src/tests/verification_mock/mod.rs +++ b/rustls-platform-verifier/src/tests/verification_mock/mod.rs @@ -116,7 +116,8 @@ pub(super) fn verification_without_mock_root() { .iter() .cloned() .collect(), - ); + ) + .unwrap(); #[cfg(not(target_os = "freebsd"))] let verifier = Verifier::new(); @@ -337,7 +338,7 @@ fn test_with_mock_root( let verifier = match root_src { Roots::OnlyExtra => Verifier::new_with_fake_root(ROOT1), // TODO: time #[cfg(not(target_os = "android"))] - Roots::ExtraAndPlatform => Verifier::new_with_extra_roots(vec![ROOT1.into()]), + Roots::ExtraAndPlatform => Verifier::new_with_extra_roots(vec![ROOT1.into()]).unwrap(), }; let mut chain = test_case .chain diff --git a/rustls-platform-verifier/src/tests/verification_real_world/mod.rs b/rustls-platform-verifier/src/tests/verification_real_world/mod.rs index 532be9e..430255d 100644 --- a/rustls-platform-verifier/src/tests/verification_real_world/mod.rs +++ b/rustls-platform-verifier/src/tests/verification_real_world/mod.rs @@ -133,7 +133,8 @@ fn real_world_test(test_case: &TestCase) { .iter() .cloned() .collect(), - ); + ) + .unwrap(); #[cfg(not(target_os = "freebsd"))] let verifier = Verifier::new(); diff --git a/rustls-platform-verifier/src/verification/others.rs b/rustls-platform-verifier/src/verification/others.rs index a145031..300e8f3 100644 --- a/rustls-platform-verifier/src/verification/others.rs +++ b/rustls-platform-verifier/src/verification/others.rs @@ -53,8 +53,10 @@ impl Verifier { /// Creates a new verifier whose certificate validation is provided by /// WebPKI, using root certificates provided by the platform and augmented by /// the provided extra root certificates. - pub fn new_with_extra_roots(roots: Vec>) -> Self { - Self { + pub fn new_with_extra_roots( + roots: Vec>, + ) -> Result { + Ok(Self { inner: OnceCell::new(), extra_roots: roots .into_iter() @@ -66,7 +68,7 @@ impl Verifier { #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: None, crypto_provider: OnceCell::new(), - } + }) } /// Creates a test-only TLS certificate verifier which trusts our fake root CA cert.