Add new with extra roots on macos/ios

rustls-platform-verifier/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rustls-platform-verifier"
-version = "0.3.4"
+version = "0.4.0"
 authors = ["ComplexSpaces <[email protected]>", "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"]

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,16 +57,30 @@ 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>]
+                }
+
             ),+
+
         ];
     };
 
     {@ $( $name:ident [ $target:meta ] => $test_case:expr ),+ , } => {
         $(
             #[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<E: std::error::Error + PartialEq + 'static>(test_case: &TestCase<E>) {
+fn test_with_mock_root<E: std::error::Error + PartialEq + 'static>(
+    test_case: &TestCase<E>,
+    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<E: std::error::Error + PartialEq + 'static>(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,
+}

rustls-platform-verifier/src/tests/verification_real_world/mod.rs

@@ -126,9 +126,10 @@ fn real_world_test<E: std::error::Error>(test_case: &TestCase<E>) {
     );
 
     // 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();

rustls-platform-verifier/src/verification/apple.rs

@@ -43,21 +43,38 @@ fn system_time_to_cfdate(time: pki_types::UnixTime) -> Result<CFDate, TlsError>
 /// 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<pki_types::CertificateDer<'static>>,
     /// Testing only: The root CA certificate to trust.
     #[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
     test_only_root_ca_override: Option<Vec<u8>>,
     pub(super) crypto_provider: OnceCell<Arc<CryptoProvider>>,
 }
 
 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
     /// [`set_provider`][Verifier::set_provider]/[`with_provider`][Verifier::with_provider] or
     /// [`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<pki_types::CertificateDer<'static>>) -> 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::<Result<Vec<_>, _>>()?;
+
+        // 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))

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<Item = pki_types::TrustAnchor<'static>>,
-    ) -> Self {
+    pub fn new_with_extra_roots(roots: Vec<pki_types::CertificateDer<'static>>) -> Self {
         Self {
             inner: OnceCell::new(),
-            extra_roots: roots.into_iter().collect::<Vec<_>>().into(),
+            extra_roots: roots
+                .into_iter()
+                .flat_map(|root| {
+                    webpki::anchor_from_trusted_cert(&root).map(|anchor| anchor.to_owned())
+                })
+                .collect::<Vec<_>>()
+                .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(