Add support for enforcing CRL expiration

rustls · May 17, 2024 · e6b7afa · e6b7afa
1 parent 06dc1d5
commit e6b7afa
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 12 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock
diff --git a/rustls/Cargo.toml b/rustls/Cargo.toml
@@ -23,7 +23,7 @@ log = { version = "0.4.4", optional = true }
 once_cell = { version = "1.16", default-features = false, features = ["alloc", "race"] }
 ring = { version = "0.17", optional = true }
 subtle = { version = "2.5.0", default-features = false }
-webpki = { package = "rustls-webpki", version = "0.102.2", features = ["alloc"], default-features = false }
+webpki = { package = "rustls-webpki", version = "0.102.4", features = ["alloc"], default-features = false }
 pki-types = { package = "rustls-pki-types", version = "1.2", features = ["alloc"] }
 zeroize = "1.7"
 

diff --git a/rustls/src/error.rs b/rustls/src/error.rs
@@ -315,6 +315,9 @@ pub enum CertificateError {
     /// The certificate's revocation status could not be determined.
     UnknownRevocationStatus,
 
+    /// The certificate's revocation status could not be determined, because the CRL is expired.
+    ExpiredRevocationList,
+
     /// A certificate is not correctly signed by the key of its alleged
     /// issuer.
     BadSignature,
@@ -358,6 +361,7 @@ impl PartialEq<Self> for CertificateError {
             (NotValidForName, NotValidForName) => true,
             (InvalidPurpose, InvalidPurpose) => true,
             (ApplicationVerificationFailure, ApplicationVerificationFailure) => true,
+            (ExpiredRevocationList, ExpiredRevocationList) => true,
             _ => false,
         }
     }
@@ -378,7 +382,7 @@ impl From<CertificateError> for AlertDescription {
             Revoked => Self::CertificateRevoked,
             // OpenSSL, BoringSSL and AWS-LC all generate an Unknown CA alert for
             // the case where revocation status can not be determined, so we do the same here.
-            UnknownIssuer | UnknownRevocationStatus => Self::UnknownCA,
+            UnknownIssuer | UnknownRevocationStatus | ExpiredRevocationList => Self::UnknownCA,
             BadSignature => Self::DecryptError,
             InvalidPurpose => Self::UnsupportedCertificate,
             ApplicationVerificationFailure => Self::AccessDenied,

diff --git a/rustls/src/webpki/client_verifier.rs b/rustls/src/webpki/client_verifier.rs
@@ -2,7 +2,7 @@ use alloc::sync::Arc;
 use alloc::vec::Vec;
 
 use pki_types::{CertificateDer, CertificateRevocationListDer, UnixTime};
-use webpki::{CertRevocationList, RevocationCheckDepth, UnknownStatusPolicy};
+use webpki::{CertRevocationList, ExpirationPolicy, RevocationCheckDepth, UnknownStatusPolicy};
 
 use super::{pki_error, VerifierBuilderError};
 #[cfg(doc)]
@@ -30,6 +30,7 @@ pub struct ClientCertVerifierBuilder {
     crls: Vec<CertificateRevocationListDer<'static>>,
     revocation_check_depth: RevocationCheckDepth,
     unknown_revocation_policy: UnknownStatusPolicy,
+    revocation_expiration_policy: ExpirationPolicy,
     anon_policy: AnonymousClientPolicy,
     supported_algs: WebPkiSupportedAlgorithms,
 }
@@ -46,6 +47,7 @@ impl ClientCertVerifierBuilder {
             anon_policy: AnonymousClientPolicy::Deny,
             revocation_check_depth: RevocationCheckDepth::Chain,
             unknown_revocation_policy: UnknownStatusPolicy::Deny,
+            revocation_expiration_policy: ExpirationPolicy::Ignore,
             supported_algs,
         }
     }
@@ -138,6 +140,19 @@ impl ClientCertVerifierBuilder {
         self
     }
 
+    /// Enforce the CRL nextUpdate field (i.e. expiration)
+    ///
+    /// If CRLs are provided with [`with_crls`][Self::with_crls] and the verification time is
+    /// beyond the time in the CRL nextUpdate field, it is expired and treated as an error condition.
+    /// Overrides the default behavior where expired CRLs are not treated as an error condition.
+    ///
+    /// If no CRLs are provided then this setting has no effect as revocation status checks
+    /// are not performed.
+    pub fn enforce_revocation_expiration(mut self) -> Self {
+        self.revocation_expiration_policy = ExpirationPolicy::Enforce;
+        self
+    }
+
     /// Build a client certificate verifier. The built verifier will be used for the server to offer
     /// client certificate authentication, to control how offered client certificates are validated,
     /// and to determine what to do with anonymous clients that do not respond to the client
@@ -165,6 +180,7 @@ impl ClientCertVerifierBuilder {
             parse_crls(self.crls)?,
             self.revocation_check_depth,
             self.unknown_revocation_policy,
+            self.revocation_expiration_policy,
             self.anon_policy,
             self.supported_algs,
         )))
@@ -237,6 +253,7 @@ pub struct WebPkiClientVerifier {
     crls: Vec<CertRevocationList<'static>>,
     revocation_check_depth: RevocationCheckDepth,
     unknown_revocation_policy: UnknownStatusPolicy,
+    revocation_expiration_policy: ExpirationPolicy,
     anonymous_policy: AnonymousClientPolicy,
     supported_algs: WebPkiSupportedAlgorithms,
 }
@@ -305,6 +322,7 @@ impl WebPkiClientVerifier {
         crls: Vec<CertRevocationList<'static>>,
         revocation_check_depth: RevocationCheckDepth,
         unknown_revocation_policy: UnknownStatusPolicy,
+        revocation_expiration_policy: ExpirationPolicy,
         anonymous_policy: AnonymousClientPolicy,
         supported_algs: WebPkiSupportedAlgorithms,
     ) -> Self {
@@ -314,6 +332,7 @@ impl WebPkiClientVerifier {
             crls,
             revocation_check_depth,
             unknown_revocation_policy,
+            revocation_expiration_policy,
             anonymous_policy,
             supported_algs,
         }
@@ -356,6 +375,7 @@ impl ClientCertVerifier for WebPkiClientVerifier {
                     .unwrap()
                     .with_depth(self.revocation_check_depth)
                     .with_status_policy(self.unknown_revocation_policy)
+                    .with_expiration_policy(self.revocation_expiration_policy)
                     .build(),
             )
         };
@@ -605,6 +625,20 @@ test_for_each_provider! {
         builder.build().unwrap();
     }
 
+    #[test]
+    fn test_client_verifier_enforce_expiration() {
+        // We should be able to build a client verifier that allows unknown revocation status
+        let builder = WebPkiClientVerifier::builder_with_provider(
+            test_roots(),
+            provider::default_provider().into(),
+        )
+        .with_crls(test_crls())
+        .enforce_revocation_expiration();
+        // The builder should be Debug.
+        println!("{:?}", builder);
+        builder.build().unwrap();
+    }
+
     #[test]
     fn test_builder_no_roots() {
         // Trying to create a client verifier builder with no trust anchors should fail at build time

diff --git a/rustls/src/webpki/mod.rs b/rustls/src/webpki/mod.rs
@@ -61,6 +61,7 @@ fn pki_error(error: webpki::Error) -> Error {
         CertNotValidForName => CertificateError::NotValidForName.into(),
         CertRevoked => CertificateError::Revoked.into(),
         UnknownRevocationStatus => CertificateError::UnknownRevocationStatus.into(),
+        CrlExpired => CertificateError::ExpiredRevocationList.into(),
         IssuerNotCrlSigner => CertRevocationListError::IssuerInvalidForCrl.into(),
 
         InvalidSignatureForPublicKey

diff --git a/rustls/src/webpki/server_verifier.rs b/rustls/src/webpki/server_verifier.rs
@@ -2,7 +2,7 @@ use alloc::sync::Arc;
 use alloc::vec::Vec;
 
 use pki_types::{CertificateDer, CertificateRevocationListDer, ServerName, UnixTime};
-use webpki::{CertRevocationList, RevocationCheckDepth, UnknownStatusPolicy};
+use webpki::{CertRevocationList, ExpirationPolicy, RevocationCheckDepth, UnknownStatusPolicy};
 
 use crate::crypto::{CryptoProvider, WebPkiSupportedAlgorithms};
 #[cfg(feature = "logging")]
@@ -28,6 +28,7 @@ pub struct ServerCertVerifierBuilder {
     crls: Vec<CertificateRevocationListDer<'static>>,
     revocation_check_depth: RevocationCheckDepth,
     unknown_revocation_policy: UnknownStatusPolicy,
+    revocation_expiration_policy: ExpirationPolicy,
     supported_algs: WebPkiSupportedAlgorithms,
 }
 
@@ -41,6 +42,7 @@ impl ServerCertVerifierBuilder {
             crls: Vec::new(),
             revocation_check_depth: RevocationCheckDepth::Chain,
             unknown_revocation_policy: UnknownStatusPolicy::Deny,
+            revocation_expiration_policy: ExpirationPolicy::Ignore,
             supported_algs,
         }
     }
@@ -83,6 +85,19 @@ impl ServerCertVerifierBuilder {
         self
     }
 
+    /// Enforce the CRL nextUpdate field (i.e. expiration)
+    ///
+    /// If CRLs are provided with [`with_crls`][Self::with_crls] and the verification time is
+    /// beyond the time in the CRL nextUpdate field, it is expired and treated as an error condition.
+    /// Overrides the default behavior where expired CRLs are not treated as an error condition.
+    ///
+    /// If no CRLs are provided then this setting has no effect as revocation status checks
+    /// are not performed.
+    pub fn enforce_revocation_expiration(mut self) -> Self {
+        self.revocation_expiration_policy = ExpirationPolicy::Enforce;
+        self
+    }
+
     /// Build a server certificate verifier, allowing control over the root certificates to use as
     /// trust anchors, and to control how server certificate revocation checking is performed.
     ///
@@ -107,6 +122,7 @@ impl ServerCertVerifierBuilder {
             parse_crls(self.crls)?,
             self.revocation_check_depth,
             self.unknown_revocation_policy,
+            self.revocation_expiration_policy,
             self.supported_algs,
         )
         .into())
@@ -121,6 +137,7 @@ pub struct WebPkiServerVerifier {
     crls: Vec<CertRevocationList<'static>>,
     revocation_check_depth: RevocationCheckDepth,
     unknown_revocation_policy: UnknownStatusPolicy,
+    revocation_expiration_policy: ExpirationPolicy,
     supported: WebPkiSupportedAlgorithms,
 }
 
@@ -167,6 +184,7 @@ impl WebPkiServerVerifier {
             Vec::default(),
             RevocationCheckDepth::Chain,
             UnknownStatusPolicy::Allow,
+            ExpirationPolicy::Ignore,
             supported_algs,
         )
     }
@@ -187,13 +205,15 @@ impl WebPkiServerVerifier {
         crls: Vec<CertRevocationList<'static>>,
         revocation_check_depth: RevocationCheckDepth,
         unknown_revocation_policy: UnknownStatusPolicy,
+        revocation_expiration_policy: ExpirationPolicy,
         supported: WebPkiSupportedAlgorithms,
     ) -> Self {
         Self {
             roots: roots.into(),
             crls,
             revocation_check_depth,
             unknown_revocation_policy,
+            revocation_expiration_policy,
             supported,
         }
     }
@@ -234,6 +254,7 @@ impl ServerCertVerifier for WebPkiServerVerifier {
                     .unwrap()
                     .with_depth(self.revocation_check_depth)
                     .with_status_policy(self.unknown_revocation_policy)
+                    .with_expiration_policy(self.revocation_expiration_policy)
                     .build(),
             )
         };
@@ -413,4 +434,18 @@ test_for_each_provider! {
         println!("{:?}", builder);
         builder.build().unwrap();
     }
+
+    #[test]
+    fn test_server_verifier_enforce_expiration() {
+        // We should be able to build a server cert. verifier that allows unknown revocation
+        // status.
+        let builder = WebPkiServerVerifier::builder_with_provider(
+            test_roots(),
+            provider::default_provider().into(),
+        )
+        .enforce_revocation_expiration();
+        // The builder should be Debug.
+        println!("{:?}", builder);
+        builder.build().unwrap();
+    }
 }