diff --git a/Cargo.lock b/Cargo.lock index 8abfc98..78eea86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -196,6 +196,12 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf" +[[package]] +name = "paste" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" + [[package]] name = "proc-macro2" version = "1.0.86" @@ -297,13 +303,13 @@ dependencies = [ [[package]] name = "rustls-pki-types" -version = "1.7.0" +version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "976295e77ce332211c0d24d92c0e83e50f5c5f046d11082cea19f3df13a3562d" +checksum = "fc0a2ce646f8655401bb81e7927b812614bd5d91dbc968696be50603510fcaf0" [[package]] name = "rustls-platform-verifier" -version = "0.3.4" +version = "0.4.0" dependencies = [ "android_logger", "base64", @@ -312,13 +318,14 @@ dependencies = [ "jni", "log", "once_cell", + "paste", "rustls", "rustls-native-certs", "rustls-platform-verifier-android", "rustls-webpki", "security-framework", "security-framework-sys", - "webpki-roots", + "webpki-root-certs", "windows-sys", ] @@ -451,10 +458,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" [[package]] -name = "webpki-roots" -version = "0.26.3" +name = "webpki-root-certs" +version = "0.26.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd7c23921eeb1713a4e851530e9b9756e4fb0e89978582942612524cf09f01cd" +checksum = "d7a7077dce97d094d1a558b1dad6a3baf1c5c3ba5b65ecb18c493ae00b955f9e" dependencies = [ "rustls-pki-types", ] diff --git a/rustls-platform-verifier/Cargo.toml b/rustls-platform-verifier/Cargo.toml index e91e5b7..002a52d 100644 --- a/rustls-platform-verifier/Cargo.toml +++ b/rustls-platform-verifier/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rustls-platform-verifier" -version = "0.3.4" +version = "0.4.0" authors = ["ComplexSpaces ", "1Password"] description = "rustls-platform-verifier supports verifying TLS certificates in rustls with the operating system verifier" keywords = ["tls", "certificate", "verification", "os", "native"] @@ -19,7 +19,7 @@ crate-type = ["cdylib", "rlib"] # Enables a C interface to use for testing where `cargo` can't be used. # This feature is not stable, nor is the interface exported when it is enabled. # Do not rely on this or use it in production. -ffi-testing = ["android_logger", "rustls/ring"] +ffi-testing = ["android_logger", "rustls/ring", "paste"] # Enables APIs that expose lower-level verifier types for debugging purposes. dbg = [] # Enables `log::debug` base64-encoded logging of all end-entity certificates processed @@ -34,6 +34,7 @@ log = { version = "0.4" } base64 = { version = "0.22", optional = true } # Only used when the `cert-logging` feature is enabled. jni = { version = "0.19", default-features = false, optional = true } # Only used during doc generation once_cell = "1.9" +paste = { version = "1.0", default-features = false, optional = true } # Only used when `ffi-testing` feature is enabled [target.'cfg(all(unix, not(target_os = "android"), not(target_os = "macos"), not(target_os = "ios"), not(target_os = "tvos"), not(target_arch = "wasm32")))'.dependencies] rustls-native-certs = "0.7" @@ -46,12 +47,12 @@ webpki = { package = "rustls-webpki", version = "0.102", default-features = fals android_logger = { version = "0.13", optional = true } # Only used during testing. [target.'cfg(target_arch = "wasm32")'.dependencies] -webpki-roots = "0.26" webpki = { package = "rustls-webpki", version = "0.102", default-features = false } +webpki-root-certs = "0.26" -# BSD targets require webpki-roots for the real-world verification tests. +# BSD targets require webpki-roots-certs for the real-world verification tests. [target.'cfg(target_os = "freebsd")'.dev-dependencies] -webpki-roots = "0.26" +webpki-root-certs = "0.26" [target.'cfg(any(target_os = "macos", target_os = "ios", target_os = "tvos"))'.dependencies] core-foundation = "0.9" @@ -64,6 +65,7 @@ windows-sys = { version = "0.52", default-features = false, features = ["Win32_F [dev-dependencies] rustls = { version = "0.23", default-features = false, features = ["ring"] } +paste = { version = "1.0", default-features = false } # Only used when `ffi-testing` feature is enabled [package.metadata.docs.rs] rustdoc-args = ["--cfg", "docsrs"] diff --git a/rustls-platform-verifier/src/tests/verification_mock/mod.rs b/rustls-platform-verifier/src/tests/verification_mock/mod.rs index 9509bcf..25d5039 100644 --- a/rustls-platform-verifier/src/tests/verification_mock/mod.rs +++ b/rustls-platform-verifier/src/tests/verification_mock/mod.rs @@ -41,6 +41,14 @@ macro_rules! mock_root_test_cases { pub fn $name() { super::$name() } + + paste::paste!{ + #[cfg(all($target, not(windows), not(target_os = "android")))] + #[test] + pub fn [<$name _extra>](){ + super::[<$name _extra>]() + } + } )+ } @@ -49,8 +57,15 @@ macro_rules! mock_root_test_cases { pub static ALL_TEST_CASES: &'static [fn()] = &[ $( #[cfg($target)] - $name + $name, + + paste::paste!{ + #[cfg(all($target, not(windows), not(target_os = "android")))] + [<$name _extra>] + } + ),+ + ]; }; @@ -58,7 +73,14 @@ macro_rules! mock_root_test_cases { $( #[cfg($target)] pub(super) fn $name() { - test_with_mock_root(&$test_case); + test_with_mock_root(&$test_case, Roots::OnlyExtra); + } + + paste::paste!{ + #[cfg(all($target, not(windows), not(target_os = "android")))] + pub(super) fn [<$name _extra>]() { + test_with_mock_root(&$test_case, Roots::ExtraAndPlatform); + } } )+ }; @@ -86,10 +108,11 @@ pub(super) fn verification_without_mock_root() { ensure_global_state(); // Since Rustls 0.22 constructing a webpki verifier (like the one backing Verifier on unix // systems) without any roots produces `OtherError(NoRootAnchors)` - since our FreeBSD CI - // runner fails to find any roots with openssl-probe we need to provide webpki-roots here + // runner fails to find any roots with openssl-probe we need to provide webpki-root-certs here // or the test will fail with the `OtherError` instead of the expected `CertificateError`. #[cfg(target_os = "freebsd")] - let verifier = Verifier::new_with_extra_roots(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + let verifier = + Verifier::new_with_extra_roots(webpki_root_certs::TLS_SERVER_ROOT_CERTS.iter().cloned()); #[cfg(not(target_os = "freebsd"))] let verifier = Verifier::new(); @@ -221,7 +244,7 @@ mock_root_test_cases! { // with AIA because there's no AIA issuer field in the certificate). // (AIA is an extension that allows downloading of missing data, // like missing certificates, during validation; see - // https://datatracker.ietf.org/doc/html/rfc5280#section-5.2.7). + // https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.2.1). ee_only_dns [ any(windows, unix) ] => TestCase { reference_id: EXAMPLE_COM, chain: &[ROOT1_INT1_EXAMPLE_COM_GOOD], @@ -300,11 +323,18 @@ mock_root_test_cases! { }, } -fn test_with_mock_root(test_case: &TestCase) { +fn test_with_mock_root( + test_case: &TestCase, + root_src: Roots, +) { ensure_global_state(); log::info!("verifying {:?}", test_case.expected_result); - let verifier = Verifier::new_with_fake_root(ROOT1); // TODO: time + let verifier = match root_src { + Roots::OnlyExtra => Verifier::new_with_fake_root(ROOT1), // TODO: time + #[cfg(all(unix, not(target_os = "android")))] + Roots::ExtraAndPlatform => Verifier::new_with_extra_roots(vec![ROOT1.into()]), + }; let mut chain = test_case .chain .iter() @@ -336,3 +366,15 @@ fn test_with_mock_root(test_case: &T ); // TODO: get into specifics of errors returned when it fails. } + +enum Roots { + /// Test with only extra roots, without loading the platform trust store. + /// + /// We want to keep things reproducible given the background-managed nature of trust roots on platforms. + OnlyExtra, + /// 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")))] + ExtraAndPlatform, +} 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 7d94120..cc111b8 100644 --- a/rustls-platform-verifier/src/tests/verification_real_world/mod.rs +++ b/rustls-platform-verifier/src/tests/verification_real_world/mod.rs @@ -126,9 +126,10 @@ fn real_world_test(test_case: &TestCase) { ); // On BSD systems openssl-probe fails to find the system CA bundle, - // so we must provide extra roots from webpki-roots. + // so we must provide extra roots from webpki-root-cert. #[cfg(target_os = "freebsd")] - let verifier = Verifier::new_with_extra_roots(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + let verifier = + Verifier::new_with_extra_roots(webpki_root_certs::TLS_SERVER_ROOT_CERTS.iter().cloned()); #[cfg(not(target_os = "freebsd"))] let verifier = Verifier::new(); diff --git a/rustls-platform-verifier/src/verification/apple.rs b/rustls-platform-verifier/src/verification/apple.rs index 40de6eb..5dc3a22 100644 --- a/rustls-platform-verifier/src/verification/apple.rs +++ b/rustls-platform-verifier/src/verification/apple.rs @@ -43,6 +43,9 @@ fn system_time_to_cfdate(time: pki_types::UnixTime) -> Result /// A TLS certificate verifier that utilizes the Apple platform certificate facilities. #[derive(Debug)] 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>, /// Testing only: The root CA certificate to trust. #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: Option>, @@ -50,7 +53,7 @@ pub struct Verifier { } impl Verifier { - /// Creates a new instance of a TLS certificate verifier that utilizes the macOS certificate + /// Creates a new instance of a TLS certificate verifier that utilizes the Apple certificate /// facilities. /// /// A [`CryptoProvider`] must be set with @@ -58,6 +61,20 @@ impl Verifier { /// [`CryptoProvider::install_default`] before the verifier can be used. pub fn new() -> Self { Self { + extra_roots: Vec::new(), + #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] + test_only_root_ca_override: None, + crypto_provider: OnceCell::new(), + } + } + + /// Creates a new instance of a TLS certificate verifier that utilizes the Apple certificate + /// 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, #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: None, crypto_provider: OnceCell::new(), @@ -68,6 +85,7 @@ impl Verifier { #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] pub(crate) fn new_with_fake_root(root: &[u8]) -> Self { Self { + extra_roots: Vec::new(), test_only_root_ca_override: Some(root.into()), crypto_provider: OnceCell::new(), } @@ -123,21 +141,49 @@ impl Verifier { .map_err(|e| invalid_certificate(e.to_string()))?; } - // When testing, support using fake roots and ignoring values present on the system. + let raw_extra_roots = self.extra_roots.iter(); + + #[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)); + + #[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::, _>>()?; + + // 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) + .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 + // since calling `SecTrustSetAnchorCertificates` "disables the trusting of any + // anchors other than the ones specified by this function call" by default. + trust_evaluation + .set_trust_anchor_certificates_only(false) + .map_err(|e| TlsError::Other(OtherError(Arc::new(e))))?; + } + + // When testing, support using fake roots and ignoring default roots present on the system for + // consistency/reproducibility reasons. // // XXX: This does not currently limit revocation from fetching information online, or prevent // the downloading of root CAs. #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] { - // If these panicked, it would be a programmer bug in the tests. - if let Some(test_root) = &self.test_only_root_ca_override { - let test_root = - SecCertificate::from_der(test_root).expect("failed to parse test root"); - - // Supply the custom root, which will be the only one trusted during evaluation. - trust_evaluation - .set_anchor_certificates(&[test_root]) - .expect("failed to set anchors"); + if self.test_only_root_ca_override.is_some() { + // XXX: The test root was already provided to the trust evaluation as an extra root. + // We only need to stop use of the default system-installed roots. // As per [Apple's docs], building and verifying a certificate chain will // search through the system and keychain to find certificates that it @@ -174,7 +220,7 @@ impl Verifier { CertificateError::UnknownIssuer, )), errors::errSecInvalidExtendedKeyUsage => Ok(TlsError::InvalidCertificate( - CertificateError::Other(OtherError(std::sync::Arc::new(super::EkuError))), + CertificateError::Other(OtherError(Arc::new(super::EkuError))), )), errors::errSecCertificateRevoked => { Ok(TlsError::InvalidCertificate(CertificateError::Revoked)) diff --git a/rustls-platform-verifier/src/verification/others.rs b/rustls-platform-verifier/src/verification/others.rs index 670be4e..a145031 100644 --- a/rustls-platform-verifier/src/verification/others.rs +++ b/rustls-platform-verifier/src/verification/others.rs @@ -53,12 +53,16 @@ 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: impl IntoIterator>, - ) -> Self { + pub fn new_with_extra_roots(roots: Vec>) -> Self { Self { inner: OnceCell::new(), - extra_roots: roots.into_iter().collect::>().into(), + extra_roots: roots + .into_iter() + .flat_map(|root| { + webpki::anchor_from_trusted_cert(&root).map(|anchor| anchor.to_owned()) + }) + .collect::>() + .into(), #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))] test_only_root_ca_override: None, crypto_provider: OnceCell::new(), @@ -154,9 +158,9 @@ impl Verifier { #[cfg(target_arch = "wasm32")] { - root_store - .roots - .extend_from_slice(webpki_roots::TLS_SERVER_ROOTS); + root_store.add_parsable_certificates( + webpki_root_certs::TLS_SERVER_ROOT_CERTS.iter().cloned(), + ); }; WebPkiServerVerifier::builder_with_provider(