verification: implement CRL revocation checking

Author: tnytown

src/cryptography/hazmat/bindings/_rust/x509.pyi

@@ -218,6 +218,10 @@ class PolicyBuilder:
     def extension_policies(
         self, *, ca_policy: ExtensionPolicy, ee_policy: ExtensionPolicy
     ) -> PolicyBuilder: ...
+    def revocation_checker(
+        self,
+        revocation_checker: x509.verification.CRLRevocationChecker,
+    ) -> PolicyBuilder: ...
     def build_client_verifier(self) -> ClientVerifier: ...
     def build_server_verifier(
         self, subject: x509.verification.Subject
@@ -279,6 +283,12 @@ class ExtensionPolicy:
         validator: PresentExtensionValidatorCallback[T] | None,
     ) -> ExtensionPolicy: ...
 
+class CRLRevocationChecker:
+    def __init__(
+        self,
+        crls: list[tuple[x509.Certificate, x509.CertificateRevocationList]],
+    ) -> None: ...
+
 class VerifiedClient:
     @property
     def subjects(self) -> list[x509.GeneralName] | None: ...

src/cryptography/x509/verification.py

@@ -10,6 +10,7 @@
 from cryptography.x509.general_name import DNSName, IPAddress
 
 __all__ = [
+    "CRLRevocationChecker",
     "ClientVerifier",
     "Criticality",
     "ExtensionPolicy",
@@ -32,3 +33,4 @@
 ExtensionPolicy = rust_x509.ExtensionPolicy
 Criticality = rust_x509.Criticality
 VerificationError = rust_x509.VerificationError
+CRLRevocationChecker = rust_x509.CRLRevocationChecker

src/rust/cryptography-x509-verification/src/certificate.rs

@@ -14,8 +14,9 @@ pub(crate) fn cert_is_self_issued(cert: &Certificate<'_>) -> bool {
 pub(crate) mod tests {
     use super::cert_is_self_issued;
     use crate::certificate::Certificate;
-    use crate::ops::tests::{cert, v1_cert_pem};
+    use crate::ops::tests::{cert, crl, v1_cert_pem};
     use crate::ops::CryptoOps;
+    use cryptography_x509::crl::CertificateRevocationList;
 
     #[test]
     fn test_certificate_v1() {
@@ -25,7 +26,7 @@ pub(crate) mod tests {
         assert!(!cert_is_self_issued(&cert));
     }
 
-    fn ca_pem() -> pem::Pem {
+    pub(crate) fn ca_pem() -> pem::Pem {
         // From vectors/cryptography_vectors/x509/custom/ca/ca.pem
         pem::parse(
             "-----BEGIN CERTIFICATE-----
@@ -42,6 +43,25 @@ Xw4nMqk=
         .unwrap()
     }
 
+    pub(crate) fn crl_pem() -> pem::Pem {
+        // From vectors/cryptography_vectors/x509/custom/crl_empty.pem
+        pem::parse(
+            "-----BEGIN X509 CRL-----
+MIIBxTCBrgIBATANBgkqhkiG9w0BAQUFADBhMQswCQYDVQQGEwJVUzERMA8GA1UE
+CAwISWxsaW5vaXMxEDAOBgNVBAcMB0NoaWNhZ28xETAPBgNVBAoMCHI1MDkgTExD
+MRowGAYDVQQDDBFyNTA5IENSTCBEZWxlZ2F0ZRcNMTUxMjIwMjM0NDQ3WhcNMTUx
+MjI4MDA0NDQ3WqAZMBcwCgYDVR0UBAMCAQEwCQYDVR0jBAIwADANBgkqhkiG9w0B
+AQUFAAOCAQEAXebqoZfEVAC4NcSEB5oGqUviUn/AnY6TzB6hUe8XC7yqEkBcyTgk
+G1Zq+b+T/5X1ewTldvuUqv19WAU/Epbbu4488PoH5qMV8Aii2XcotLJOR9OBANp0
+Yy4ir/n6qyw8kM3hXJloE+xgkELhd5JmKCnlXihM1BTl7Xp7jyKeQ86omR+DhItb
+CU+9RoqOK9Hm087Z7RurXVrz5RKltQo7VLCp8VmrxFwfALCZENXGEQ+g5VkvoCjc
+ph5jqOSyzp7aZy1pnLE/6U6V32ItskrwqA+x4oj2Wvzir/Q23y2zYfqOkuq4fTd2
+lWW+w5mB167fIWmd6efecDn1ZqbdECDPUg==
+-----END X509 CRL-----",
+        )
+        .unwrap()
+    }
+
     #[test]
     fn test_certificate_ca() {
         let cert_pem = ca_pem();
@@ -62,6 +82,14 @@ Xw4nMqk=
             Err(())
         }
 
+        fn verify_crl_signed_by(
+            &self,
+            _crl: &CertificateRevocationList<'_>,
+            _key: &Self::Key,
+        ) -> Result<(), Self::Err> {
+            Ok(())
+        }
+
         fn verify_signed_by(
             &self,
             _cert: &Certificate<'_>,
@@ -98,9 +126,12 @@ Xw4nMqk=
         // Just to get coverage on the `PublicKeyErrorOps` helper.
         let cert_pem = ca_pem();
         let cert = cert(&cert_pem);
+        let crl_pem = crl_pem();
+        let crl = crl(&crl_pem);
         let ops = PublicKeyErrorOps {};
 
         assert!(ops.public_key(&cert).is_err());
         assert!(ops.verify_signed_by(&cert, &()).is_ok());
+        assert!(ops.verify_crl_signed_by(&crl, &()).is_ok());
     }
 }

src/rust/cryptography-x509-verification/src/lib.rs

@@ -9,6 +9,7 @@
 pub mod certificate;
 pub mod ops;
 pub mod policy;
+pub mod revocation;
 pub mod trust_store;
 pub mod types;
 
@@ -30,6 +31,7 @@ use cryptography_x509::oid::{
 use crate::certificate::cert_is_self_issued;
 use crate::ops::{CryptoOps, VerificationCertificate};
 use crate::policy::Policy;
+use crate::revocation::CrlRevocationChecker;
 use crate::trust_store::Store;
 use crate::types::{
     DNSConstraint, DNSPattern, IPAddress, IPConstraint, RFC822Constraint, RFC822Name,
@@ -44,6 +46,7 @@ pub enum ValidationErrorKind<'chain, B: CryptoOps> {
         reason: &'static str,
     },
     FatalError(&'static str),
+    RevocationNotDetermined(String),
     Other(String),
 }
 
@@ -95,6 +98,9 @@ impl<B: CryptoOps> Display for ValidationError<'_, B> {
                 write!(f, "invalid extension: {oid}: {reason}")
             }
             ValidationErrorKind::FatalError(err) => write!(f, "fatal error: {err}"),
+            ValidationErrorKind::RevocationNotDetermined(reason) => {
+                write!(f, "unable to determine revocation status: {reason}")
+            }
             ValidationErrorKind::Other(err) => write!(f, "{err}"),
         }
     }
@@ -306,9 +312,10 @@ pub fn verify<'chain, B: CryptoOps>(
     leaf: &VerificationCertificate<'chain, B>,
     intermediates: &[VerificationCertificate<'chain, B>],
     policy: &Policy<'_, B>,
+    revocation_checker: Option<&'_ CrlRevocationChecker<'_>>,
     store: &Store<'chain, B>,
 ) -> ValidationResult<'chain, Chain<'chain, B>, B> {
-    let builder = ChainBuilder::new(intermediates, policy, store);
+    let builder = ChainBuilder::new(intermediates, policy, revocation_checker, store);
 
     let mut budget = Budget::new();
     builder.build_chain(leaf, &mut budget)
@@ -317,6 +324,7 @@ pub fn verify<'chain, B: CryptoOps>(
 struct ChainBuilder<'a, 'chain, B: CryptoOps> {
     intermediates: &'a [VerificationCertificate<'chain, B>],
     policy: &'a Policy<'a, B>,
+    revocation_checker: Option<&'a CrlRevocationChecker<'a>>,
     store: &'a Store<'chain, B>,
 }
 
@@ -353,11 +361,13 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
     fn new(
         intermediates: &'a [VerificationCertificate<'chain, B>],
         policy: &'a Policy<'a, B>,
+        revocation_checker: Option<&'a CrlRevocationChecker<'a>>,
         store: &'a Store<'chain, B>,
     ) -> Self {
         Self {
             intermediates,
             policy,
+            revocation_checker,
             store,
         }
     }
@@ -464,6 +474,14 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
             return Ok(vec![working_cert.clone()]);
         }
 
+        if let Some(revocation_checker) = self.revocation_checker {
+            if revocation_checker.is_revoked(working_cert, self.policy)? {
+                return Err(ValidationError::new(ValidationErrorKind::Other(
+                    "certificate revoked".to_string(),
+                )));
+            }
+        }
+
         // Check that our current depth does not exceed our policy-configured
         // max depth. We do this after the root set check, since the depth
         // only measures the intermediate chain's length, not the root or leaf.
@@ -599,7 +617,8 @@ mod tests {
     use cryptography_x509::certificate::Certificate;
     use cryptography_x509::oid::SUBJECT_ALTERNATIVE_NAME_OID;
 
-    use crate::certificate::tests::PublicKeyErrorOps;
+    use crate::certificate::tests::{crl_pem, PublicKeyErrorOps};
+    use crate::ops::tests::{crl, NullOps};
     use crate::ops::{CryptoOps, VerificationCertificate};
     use crate::policy::{Policy, PolicyDefinition, Subject};
     use crate::trust_store::Store;
@@ -622,43 +641,27 @@ mod tests {
             "invalid extension: 2.5.29.17: duplicate extension"
         );
 
+        let err = ValidationError::<PublicKeyErrorOps>::new(
+            ValidationErrorKind::RevocationNotDetermined("oops".to_owned()),
+        );
+        assert_eq!(
+            err.to_string(),
+            "unable to determine revocation status: oops"
+        );
+
         let err =
             ValidationError::<PublicKeyErrorOps>::new(ValidationErrorKind::FatalError("oops"));
         assert_eq!(err.to_string(), "fatal error: oops");
     }
 
-    /// A `CryptoOps` whose public key extraction and signature verification
-    /// always succeed, so that `valid_issuer` can be driven to completion
-    /// without real cryptographic material.
-    struct NullOps;
-
-    impl CryptoOps for NullOps {
-        type Key = ();
-        type Err = ();
-        type CertificateExtra = ();
-        type PolicyExtra = ();
-
-        fn public_key(&self, _cert: &Certificate<'_>) -> Result<Self::Key, Self::Err> {
-            Ok(())
-        }
-
-        fn verify_signed_by(
-            &self,
-            _cert: &Certificate<'_>,
-            _key: &Self::Key,
-        ) -> Result<(), Self::Err> {
-            Ok(())
-        }
-
-        fn clone_public_key(_key: &Self::Key) -> Self::Key {}
-
-        fn clone_extra(_extra: &Self::CertificateExtra) -> Self::CertificateExtra {}
-    }
-
     #[test]
     fn test_clone() {
         assert_eq!(NullOps::clone_public_key(&()), ());
         assert_eq!(NullOps::clone_extra(&()), ());
+
+        let crl_pem = crl_pem();
+        let crl = crl(&crl_pem);
+        assert!(NullOps.verify_crl_signed_by(&crl, &()).is_ok());
     }
 
     // A self-issued ("looping") CA certificate that is its own issuer.
@@ -706,7 +709,7 @@ qolIOwIgCaIgj9ipK0Q0p+45UJiq+L/ncrxsweJkFq/UYubzhX0=
             PolicyDefinition::server(NullOps, subject, time, Some(u8::MAX), None, None).unwrap();
         let policy = Policy::new(&policy_def, ());
 
-        let builder = ChainBuilder::new(&intermediates, &policy, &store);
+        let builder = ChainBuilder::new(&intermediates, &policy, None, &store);
         let mut budget = Budget {
             name_constraint_checks: usize::MAX,
             signature_checks: usize::MAX,

src/rust/cryptography-x509-verification/src/ops.rs

@@ -5,6 +5,7 @@
 use std::sync::OnceLock;
 
 use cryptography_x509::certificate::Certificate;
+use cryptography_x509::crl::CertificateRevocationList;
 
 pub struct VerificationCertificate<'a, B: CryptoOps> {
     cert: &'a Certificate<'a>,
@@ -86,6 +87,14 @@ pub trait CryptoOps {
     /// if the key is malformed.
     fn public_key(&self, cert: &Certificate<'_>) -> Result<Self::Key, Self::Err>;
 
+    /// Verifies the signature on `CertificateRevocationList` using
+    /// the given `Key`.
+    fn verify_crl_signed_by(
+        &self,
+        crl: &CertificateRevocationList<'_>,
+        key: &Self::Key,
+    ) -> Result<(), Self::Err>;
+
     /// Verifies the signature on `Certificate` using the given
     /// `Key`.
     fn verify_signed_by(&self, cert: &Certificate<'_>, key: &Self::Key) -> Result<(), Self::Err>;
@@ -100,10 +109,47 @@ pub trait CryptoOps {
 #[cfg(test)]
 pub(crate) mod tests {
     use cryptography_x509::certificate::Certificate;
+    use cryptography_x509::crl::CertificateRevocationList;
 
-    use super::VerificationCertificate;
+    use super::{CryptoOps, VerificationCertificate};
     use crate::certificate::tests::PublicKeyErrorOps;
 
+    /// A `CryptoOps` whose public key extraction and signature verification
+    /// always succeed, so that `valid_issuer` can be driven to completion
+    /// without real cryptographic material.
+    pub(crate) struct NullOps;
+
+    impl CryptoOps for NullOps {
+        type Key = ();
+        type Err = ();
+        type CertificateExtra = ();
+        type PolicyExtra = ();
+
+        fn public_key(&self, _cert: &Certificate<'_>) -> Result<Self::Key, Self::Err> {
+            Ok(())
+        }
+
+        fn verify_crl_signed_by(
+            &self,
+            _crl: &CertificateRevocationList<'_>,
+            _key: &Self::Key,
+        ) -> Result<(), Self::Err> {
+            Ok(())
+        }
+
+        fn verify_signed_by(
+            &self,
+            _cert: &Certificate<'_>,
+            _key: &Self::Key,
+        ) -> Result<(), Self::Err> {
+            Ok(())
+        }
+
+        fn clone_public_key(_key: &Self::Key) -> Self::Key {}
+
+        fn clone_extra(_extra: &Self::CertificateExtra) -> Self::CertificateExtra {}
+    }
+
     pub(crate) fn v1_cert_pem() -> pem::Pem {
         pem::parse(
             "
@@ -129,6 +175,10 @@ zl9HYIMxATFyqSiD9jsx
         asn1::parse_single(cert_pem.contents()).unwrap()
     }
 
+    pub(crate) fn crl(crl_pem: &pem::Pem) -> CertificateRevocationList<'_> {
+        asn1::parse_single(crl_pem.contents()).unwrap()
+    }
+
     #[test]
     fn test_verification_certificate_debug() {
         let p = v1_cert_pem();

src/rust/cryptography-x509-verification/src/policy/mod.rs

@@ -16,6 +16,7 @@ use cryptography_x509::common::{
     PSS_SHA256_HASH_ALG, PSS_SHA256_MASK_GEN_ALG, PSS_SHA384_HASH_ALG, PSS_SHA384_MASK_GEN_ALG,
     PSS_SHA512_HASH_ALG, PSS_SHA512_MASK_GEN_ALG,
 };
+use cryptography_x509::crl::CertificateRevocationList;
 use cryptography_x509::extensions::{BasicConstraints, Extensions, SubjectAlternativeName};
 use cryptography_x509::name::GeneralName;
 use cryptography_x509::oid::{
@@ -580,9 +581,35 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
 
         Ok(())
     }
+
+    pub(crate) fn permits_crl<'chain>(
+        &self,
+        crl: &CertificateRevocationList<'_>,
+    ) -> ValidationResult<'chain, (), B> {
+        let this_update = crl.tbs_cert_list.this_update.as_datetime();
+        permits_validity_date(&crl.tbs_cert_list.this_update)?;
+
+        // 5280 5: CRLs MUST include the nextUpdate field
+        let next_update = if let Some(next_update) = crl.tbs_cert_list.next_update.as_ref() {
+            permits_validity_date(next_update)?;
+            next_update.as_datetime()
+        } else {
+            return Err(ValidationError::new(ValidationErrorKind::Other(
+                "CRL missing required nextUpdate field".to_string(),
+            )));
+        };
+
+        if &self.validation_time < this_update || &self.validation_time > next_update {
+            return Err(ValidationError::new(ValidationErrorKind::Other(
+                "CRL is not in effect at validation time".to_string(),
+            )));
+        }
+
+        Ok(())
+    }
 }
 
-fn permits_validity_date<'chain, B: CryptoOps>(
+pub(crate) fn permits_validity_date<'chain, B: CryptoOps>(
     validity_date: &Time,
 ) -> ValidationResult<'chain, (), B> {
     const GENERALIZED_DATE_INVALIDITY_RANGE: Range<u16> = 1950..2050;