Improving webpki verifier CRL support ergonomics

rustls/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rustls"
-version = "0.22.0-alpha.3"
+version = "0.22.0-alpha.4"
 edition = "2021"
 rust-version = "1.61"
 license = "Apache-2.0 OR ISC OR MIT"
@@ -19,7 +19,7 @@ rustversion = { version = "1.0.6", optional = true }
 log = { version = "0.4.4", optional = true }
 ring = { version = "0.17", optional = true }
 subtle = "2.5.0"
-webpki = { package = "rustls-webpki", version = "=0.102.0-alpha.4", features = ["alloc", "std"], default-features = false }
+webpki = { package = "rustls-webpki", version = "=0.102.0-alpha.6", features = ["alloc", "std"], default-features = false }
 pki-types = { package = "rustls-pki-types", version = "0.2.1", features = ["std"] }
 zeroize = "1.6.0"
 

rustls/src/webpki/client_verifier.rs

@@ -2,20 +2,16 @@ use alloc::sync::Arc;
 use alloc::vec::Vec;
 
 use pki_types::{CertificateDer, CertificateRevocationListDer, UnixTime};
-use webpki::{
-    BorrowedCertRevocationList, OwnedCertRevocationList, RevocationCheckDepth, UnknownStatusPolicy,
-};
+use webpki::{CertRevocationList, RevocationCheckDepth, UnknownStatusPolicy};
 
-use super::{borrow_crls, crl_error, pki_error, VerifierBuilderError};
+use super::{pki_error, VerifierBuilderError};
 use crate::verify::{
     ClientCertVerified, ClientCertVerifier, DigitallySignedStruct, HandshakeSignatureValid,
     NoClientAuth,
 };
+use crate::webpki::parse_crls;
 use crate::webpki::verify::{verify_signed_struct, verify_tls13, ParsedCertificate};
-use crate::{
-    CertRevocationListError, DistinguishedName, Error, RootCertStore, SignatureScheme,
-    WebPkiSupportedAlgorithms,
-};
+use crate::{DistinguishedName, Error, RootCertStore, SignatureScheme, WebPkiSupportedAlgorithms};
 
 /// A builder for configuring a `webpki` client certificate verifier.
 ///
@@ -178,14 +174,7 @@ impl ClientCertVerifierBuilder {
         Ok(Arc::new(WebPkiClientVerifier::new(
             self.roots,
             self.root_hint_subjects,
-            self.crls
-                .into_iter()
-                .map(|der_crl| {
-                    BorrowedCertRevocationList::from_der(der_crl.as_ref())
-                        .and_then(|crl| crl.to_owned())
-                        .map_err(crl_error)
-                })
-                .collect::<Result<Vec<_>, CertRevocationListError>>()?,
+            parse_crls(self.crls)?,
             self.revocation_check_depth,
             self.unknown_revocation_policy,
             self.anon_policy,
@@ -249,7 +238,7 @@ impl ClientCertVerifierBuilder {
 pub struct WebPkiClientVerifier {
     roots: Arc<RootCertStore>,
     root_hint_subjects: Vec<DistinguishedName>,
-    crls: Vec<OwnedCertRevocationList>,
+    crls: Vec<CertRevocationList<'static>>,
     revocation_check_depth: RevocationCheckDepth,
     unknown_revocation_policy: UnknownStatusPolicy,
     anonymous_policy: AnonymousClientPolicy,
@@ -293,7 +282,7 @@ impl WebPkiClientVerifier {
     pub(crate) fn new(
         roots: Arc<RootCertStore>,
         root_hint_subjects: Vec<DistinguishedName>,
-        crls: Vec<OwnedCertRevocationList>,
+        crls: Vec<CertRevocationList<'static>>,
         revocation_check_depth: RevocationCheckDepth,
         unknown_revocation_policy: UnknownStatusPolicy,
         anonymous_policy: AnonymousClientPolicy,
@@ -334,21 +323,21 @@ impl ClientCertVerifier for WebPkiClientVerifier {
         now: UnixTime,
     ) -> Result<ClientCertVerified, Error> {
         let cert = ParsedCertificate::try_from(end_entity)?;
-        let crls = borrow_crls(&self.crls);
 
-        let revocation = if crls.is_empty() {
+        let crl_refs = self.crls.iter().collect::<Vec<_>>();
+
+        let revocation = if self.crls.is_empty() {
             None
         } else {
-            let mut builder = webpki::RevocationOptionsBuilder::new(&crls)
-                .expect("invalid crls")
-                .with_depth(self.revocation_check_depth);
-            if matches!(
-                self.unknown_revocation_policy,
-                webpki::UnknownStatusPolicy::Allow
-            ) {
-                builder = builder.allow_unknown_status();
-            }
-            Some(builder.build())
+            Some(
+                webpki::RevocationOptionsBuilder::new(&crl_refs)
+                    // Note: safe to unwrap here - new is only fallible if no CRLs are provided
+                    //       and we verify this above.
+                    .unwrap()
+                    .with_depth(self.revocation_check_depth)
+                    .with_status_policy(self.unknown_revocation_policy)
+                    .build(),
+            )
         };
 
         cert.0
@@ -359,6 +348,7 @@ impl ClientCertVerifier for WebPkiClientVerifier {
                 now,
                 webpki::KeyUsage::client_auth(),
                 revocation,
+                None,
             )
             .map_err(pki_error)
             .map(|_| ClientCertVerified::assertion())

rustls/src/webpki/mod.rs

@@ -1,7 +1,10 @@
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use core::fmt;
+
+use pki_types::CertificateRevocationListDer;
 use std::error::Error as StdError;
+use webpki::{CertRevocationList, OwnedCertRevocationList};
 
 use crate::error::{CertRevocationListError, CertificateError, Error};
 
@@ -104,13 +107,13 @@ fn crl_error(e: webpki::Error) -> CertRevocationListError {
     }
 }
 
-fn borrow_crls(
-    crls: &Vec<webpki::OwnedCertRevocationList>,
-) -> Vec<&dyn webpki::CertRevocationList> {
-    #[allow(trivial_casts)] // Cast to &dyn trait is required.
+fn parse_crls(
+    crls: Vec<CertificateRevocationListDer<'_>>,
+) -> Result<Vec<CertRevocationList<'_>>, CertRevocationListError> {
     crls.iter()
-        .map(|crl| crl as &dyn webpki::CertRevocationList)
-        .collect::<Vec<_>>()
+        .map(|der| OwnedCertRevocationList::from_der(der.as_ref()).map(Into::into))
+        .collect::<Result<Vec<_>, _>>()
+        .map_err(crl_error)
 }
 
 mod tests {

rustls/src/webpki/server_verifier.rs

@@ -4,9 +4,7 @@ use alloc::sync::Arc;
 use alloc::vec::Vec;
 
 use pki_types::{CertificateDer, CertificateRevocationListDer, UnixTime};
-use webpki::{
-    BorrowedCertRevocationList, OwnedCertRevocationList, RevocationCheckDepth, UnknownStatusPolicy,
-};
+use webpki::{CertRevocationList, RevocationCheckDepth, UnknownStatusPolicy};
 
 use crate::verify::{
     DigitallySignedStruct, HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier,
@@ -17,11 +15,8 @@ use crate::webpki::verify::{
     verify_server_cert_signed_by_trust_anchor_impl, verify_signed_struct, verify_tls13,
     ParsedCertificate,
 };
-use crate::webpki::{borrow_crls, crl_error, verify_server_name, VerifierBuilderError};
-use crate::{
-    CertRevocationListError, Error, RootCertStore, ServerName, SignatureScheme,
-    WebPkiSupportedAlgorithms,
-};
+use crate::webpki::{parse_crls, verify_server_name, VerifierBuilderError};
+use crate::{Error, RootCertStore, ServerName, SignatureScheme, WebPkiSupportedAlgorithms};
 
 /// A builder for configuring a `webpki` server certificate verifier.
 ///
@@ -127,14 +122,7 @@ impl ServerCertVerifierBuilder {
 
         Ok(Arc::new(WebPkiServerVerifier::new(
             self.roots,
-            self.crls
-                .into_iter()
-                .map(|der_crl| {
-                    BorrowedCertRevocationList::from_der(der_crl.as_ref())
-                        .and_then(|crl| crl.to_owned())
-                        .map_err(crl_error)
-                })
-                .collect::<Result<Vec<_>, CertRevocationListError>>()?,
+            parse_crls(self.crls)?,
             self.revocation_check_depth,
             self.unknown_revocation_policy,
             supported_algs,
@@ -146,7 +134,7 @@ impl ServerCertVerifierBuilder {
 #[allow(unreachable_pub)]
 pub struct WebPkiServerVerifier {
     roots: Arc<RootCertStore>,
-    crls: Vec<OwnedCertRevocationList>,
+    crls: Vec<CertRevocationList<'static>>,
     revocation_check_depth: RevocationCheckDepth,
     unknown_revocation_policy: UnknownStatusPolicy,
     supported: WebPkiSupportedAlgorithms,
@@ -188,7 +176,7 @@ impl WebPkiServerVerifier {
     ///   certificate verification and TLS handshake signature verification.
     pub(crate) fn new(
         roots: impl Into<Arc<RootCertStore>>,
-        crls: Vec<OwnedCertRevocationList>,
+        crls: Vec<CertRevocationList<'static>>,
         revocation_check_depth: RevocationCheckDepth,
         unknown_revocation_policy: UnknownStatusPolicy,
         supported: WebPkiSupportedAlgorithms,
@@ -253,22 +241,22 @@ impl ServerCertVerifier for WebPkiServerVerifier {
     ) -> Result<ServerCertVerified, Error> {
         let cert = ParsedCertificate::try_from(end_entity)?;
 
-        let crls = borrow_crls(&self.crls);
-        let revocation = if crls.is_empty() {
+        let crl_refs = self.crls.iter().collect::<Vec<_>>();
+
+        let revocation = if self.crls.is_empty() {
             None
         } else {
             // Note: unwrap here is safe because RevocationOptionsBuilder only errors when given
             //       empty CRLs.
-            let mut builder = webpki::RevocationOptionsBuilder::new(&crls)
-                .unwrap()
-                .with_depth(self.revocation_check_depth);
-            if matches!(
-                self.unknown_revocation_policy,
-                webpki::UnknownStatusPolicy::Allow
-            ) {
-                builder = builder.allow_unknown_status();
-            }
-            Some(builder.build())
+            Some(
+                webpki::RevocationOptionsBuilder::new(crl_refs.as_slice())
+                    // Note: safe to unwrap here - new is only fallible if no CRLs are provided
+                    //       and we verify this above.
+                    .unwrap()
+                    .with_depth(self.revocation_check_depth)
+                    .with_status_policy(self.unknown_revocation_policy)
+                    .build(),
+            )
         };
 
         // Note: we use the crate-internal `_impl` fn here in order to provide revocation

rustls/src/webpki/verify.rs

@@ -285,6 +285,7 @@ pub(crate) fn verify_server_cert_signed_by_trust_anchor_impl(
         now,
         webpki::KeyUsage::server_auth(),
         revocation,
+        None,
     );
     match result {
         Ok(_) => Ok(()),