diff --git a/.github/actions/fetch-vectors/action.yml b/.github/actions/fetch-vectors/action.yml index c698872a0ad2..4a9a9c87b4bc 100644 --- a/.github/actions/fetch-vectors/action.yml +++ b/.github/actions/fetch-vectors/action.yml @@ -17,6 +17,6 @@ runs: with: repository: "C2SP/x509-limbo" path: "x509-limbo" - # Latest commit on the x509-limbo main branch, as of Jun 16, 2026. - ref: "9d41359a05d811ee3de8026ff21d7ddecbd0f3eb" # x509-limbo-ref + # Latest commit on the x509-limbo main branch, as of Jun 07, 2026. + ref: "f9e9186820670e4c27edc59d7860dd3804b867b0" # x509-limbo-ref persist-credentials: false diff --git a/src/cryptography/hazmat/bindings/_rust/x509.pyi b/src/cryptography/hazmat/bindings/_rust/x509.pyi index 34a726e167dc..792de5bcd52a 100644 --- a/src/cryptography/hazmat/bindings/_rust/x509.pyi +++ b/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: ... diff --git a/src/cryptography/x509/verification.py b/src/cryptography/x509/verification.py index 2db4324d9615..9776f6b006c6 100644 --- a/src/cryptography/x509/verification.py +++ b/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 diff --git a/src/rust/cryptography-x509-verification/src/certificate.rs b/src/rust/cryptography-x509-verification/src/certificate.rs index 4c9fc256b5f8..ac7af1f21ce2 100644 --- a/src/rust/cryptography-x509-verification/src/certificate.rs +++ b/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()); } } diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index 059f62e33d87..9cd95085c39e 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/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 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::::new( + ValidationErrorKind::RevocationNotDetermined("oops".to_owned()), + ); + assert_eq!( + err.to_string(), + "unable to determine revocation status: oops" + ); + let err = ValidationError::::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 { - 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, diff --git a/src/rust/cryptography-x509-verification/src/ops.rs b/src/rust/cryptography-x509-verification/src/ops.rs index c539d2eaf015..7bbfcad57fd7 100644 --- a/src/rust/cryptography-x509-verification/src/ops.rs +++ b/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; + /// 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 { + 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(); diff --git a/src/rust/cryptography-x509-verification/src/policy/mod.rs b/src/rust/cryptography-x509-verification/src/policy/mod.rs index 8e4f450fbc62..7709069ea6c2 100644 --- a/src/rust/cryptography-x509-verification/src/policy/mod.rs +++ b/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 = 1950..2050; diff --git a/src/rust/cryptography-x509-verification/src/revocation.rs b/src/rust/cryptography-x509-verification/src/revocation.rs new file mode 100644 index 000000000000..755270245997 --- /dev/null +++ b/src/rust/cryptography-x509-verification/src/revocation.rs @@ -0,0 +1,315 @@ +// This file is dual licensed under the terms of the Apache License, Version +// 2.0, and the BSD License. See the LICENSE file in the root of this repository +// for complete details. + +use std::collections::{HashMap, HashSet}; + +use cryptography_x509::{ + certificate::Certificate, + common::Asn1Read, + crl::{CertificateRevocationList, IssuingDistributionPoint}, + extensions::{ + BasicConstraints, DistributionPointName, Extensions, KeyUsage, SequenceOfDistributionPoints, + }, + name::{GeneralName, Name}, + oid, +}; + +use crate::{ + ops::{CryptoOps, VerificationCertificate}, + policy::Policy, + ValidationError, ValidationErrorKind, ValidationResult, +}; + +fn validate_distribution_point_uris<'a>( + set: &mut HashSet<&'a str>, + dp: Option>, +) -> bool { + // The other match case here is nameRelativeToCRLIssuer, and RFC 5280 4.2.1.13 has a salient + // recommendation on the subject: + // + // > Conforming CAs SHOULD NOT use nameRelativeToCRLIssuer to specify distribution point names. + let Some(DistributionPointName::FullName(idp_names)) = dp else { + return false; + }; + + // CABF 7.2.2.1, 7.1.2.11.2: names in iDPs and CDPs must be of type uniformResourceIdentifier. + for name in idp_names { + match name { + GeneralName::UniformResourceIdentifier(ref uri) => set.insert(uri.0), + _ => return false, + }; + } + + true +} + +/// Verifies that the scope of the CRL (iDP) matches the certificate. +fn cert_dp_matches_idp<'a>( + cert_exts: &Extensions<'a>, + idp: IssuingDistributionPoint<'a, Asn1Read>, +) -> bool { + let cert_bc_ca = cert_exts + .get_extension(&oid::BASIC_CONSTRAINTS_OID) + .and_then(|e| e.value::().ok()) + .map(|bc| bc.ca) + .unwrap_or(false); + + // Check iDP following 5280 6.3.3(b)(2). + + // If onlyContainsUserCerts is asserted in the iDP CRL extension, verify that the certificate + // does not include the basic constraints extension with the cA boolean asserted. + if idp.only_contains_user_certs && cert_bc_ca { + return false; + } + + // If onlyContainsCACerts is asserted in the iDP CRL extension, verify that the certificate + // includes the basic constraints extension with the cA boolean asserted. + if idp.only_contains_ca_certs && !cert_bc_ca { + return false; + } + + // Verify that onlyContainsAttributeCerts is not asserted. + if idp.only_contains_attribute_certs { + return false; + } + + // Now, check that the iDP URIs match those in the certificate. + let mut idp_uris = HashSet::new(); + if !validate_distribution_point_uris(&mut idp_uris, idp.distribution_point) { + return false; + }; + + let Some(cert_dps) = cert_exts + .get_extension(&oid::CRL_DISTRIBUTION_POINTS_OID) + .and_then(|ext| { + ext.value::>() + .ok() + }) + else { + return false; + }; + + // NOTE(tnytown): a faster way to do this might be to short-circuit when we find a matching name + // in a CDP; in the interest of strictness, we process every DP and its associated names to + // ensure that the certificate has the correct shape. + let mut dp_uris = HashSet::new(); + for dp in cert_dps { + if !validate_distribution_point_uris(&mut dp_uris, dp.distribution_point) { + return false; + }; + } + + !idp_uris.is_disjoint(&dp_uris) +} + +/// A CRL alongside a map containing revocation information to enable fast lookups. +struct CrlMeta<'a> { + crl: &'a CertificateRevocationList<'a>, + serials: HashSet>, +} + +impl<'a> CrlMeta<'a> { + fn load(crl: &'a CertificateRevocationList<'a>) -> Option { + // We only interpret X.509 v2 CRLs. + if crl.tbs_cert_list.version != Some(1) { + return None; + } + + let crl_exts = crl.extensions().ok()?; + + // 5280 5.2.3: CRLNumber is required. + crl_exts.get_extension(&oid::CRL_NUMBER_OID)?; + + // Check bits on the iDP extension. CABF 7.2.2.1 specifies that full and complete CRLs may omit + // this extension; do nothing if it is missing. + if let Some(ref idp) = crl_exts + .get_extension(&oid::ISSUING_DISTRIBUTION_POINT_OID) + .and_then(|ext| ext.value::>().ok()) + { + // Don't support reason code partitioning yet. + if idp.only_some_reasons.is_some() { + return None; + } + + // CABF 7.2 disallows iCRLs. + if idp.indirect_crl { + return None; + } + } + + // Check for any unrecognized critical extensions. + for ext in crl_exts.iter() { + if !ext.critical { + continue; + } + + match ext.extn_id { + oid::ISSUING_DISTRIBUTION_POINT_OID => (), + _ => return None, + } + } + + let mut serials = HashSet::new(); + let Some(crl_entries) = crl + .tbs_cert_list + .revoked_certificates + .as_ref() + .map(|e| e.unwrap_read().clone()) + else { + // Allow empty CRLs. + return Some(Self { crl, serials }); + }; + + // Check extensions on CRL entries while populating the set. + for entry in crl_entries.into_iter() { + for ext in entry.extensions().ok()?.iter() { + // We don't recognize any critical extensions. + if ext.critical { + return None; + } + } + + // Insert and check for duplicates. + if !serials.insert(entry.user_certificate) { + return None; + } + } + + Some(Self { crl, serials }) + } +} + +pub struct CrlRevocationChecker<'a> { + by_issuer: HashMap, CrlMeta<'a>>, +} + +impl<'a> CrlRevocationChecker<'a> { + pub fn is_revoked<'chain, B: CryptoOps>( + &self, + cert: &VerificationCertificate<'chain, B>, + policy: &Policy<'_, B>, + ) -> ValidationResult<'chain, bool, B> { + let cert = cert.certificate(); + // Get the CRL out of our map of verified CRLs keyed by issuer. + let CrlMeta { crl, serials } = + self.by_issuer + .get(&cert.tbs_cert.issuer) + .ok_or(ValidationError::new( + ValidationErrorKind::RevocationNotDetermined( + "applicable CRL not found for certificate".to_owned(), + ), + ))?; + + // Perform a scoping check on the iDP if it exists. + if let Some(idp) = crl + .extensions() + .ok() + .and_then(|exts| exts.get_extension(&oid::ISSUING_DISTRIBUTION_POINT_OID)) + .and_then(|ext| ext.value::>().ok()) + { + let cert_exts = cert.extensions()?; + if !cert_dp_matches_idp(&cert_exts, idp) { + return Err(ValidationError::new( + ValidationErrorKind::RevocationNotDetermined( + "applicable CRL not correctly scoped to certificate".to_owned(), + ), + )); + } + } + + // Check CRL against policy time. + policy.permits_crl(crl)?; + + Ok(serials.contains(&cert.tbs_cert.serial)) + } + + /// Constructs a new revocation checker backed by CRLs in accordance with the RFC 5280 and CABF + /// CRL profiles. + /// + /// Accepts issuers and their associated CRLs. Each [`CrlRevocationChecker`] instance + /// must abide by the following constraints: + /// - Must contain at most one CRL per issuer. + /// - CRLs must not be partitioned by reason code. + /// - CRLs must be direct: the CRL's issuer must match the issuer of its revokees. + /// + /// In other words, each CRL should be authoritative for its issuer for the duration of the + /// CRL's effective window. + pub fn new( + ops: B, + crls: impl IntoIterator, &'a CertificateRevocationList<'a>)>, + ) -> Option { + let mut by_issuer = HashMap::new(); + + for (issuer, crl) in crls { + // 5280 4.2.1.3 says to check keyUsage.cRLSign if keyUsage is present. + // Diverge a little bit: CABF (and our policy engine) requires keyUsage on issuers, + // so don't allow for missing keyUsage here either. + let key_usage_set = issuer + .extensions() + .ok()? + .get_extension(&oid::KEY_USAGE_OID) + .and_then(|ku| ku.value::>().ok()) + .map(|ku| ku.crl_sign()) + .unwrap_or(false); + if !key_usage_set { + return None; + } + + let key = ops.public_key(issuer).ok()?; + ops.verify_crl_signed_by(crl, &key).ok()?; + + let meta = CrlMeta::load(crl)?; + let issuer_name = issuer.tbs_cert.subject.clone(); + + // Fail if we've already processed a CRL for this issuer. + if by_issuer.insert(issuer_name, meta).is_some() { + return None; + } + } + + Some(Self { by_issuer }) + } +} + +#[cfg(test)] +mod tests { + use super::CrlRevocationChecker; + use crate::{ + certificate::tests::ca_pem, + ops::tests::{cert, crl, NullOps}, + revocation::CrlMeta, + }; + + fn crl_pem() -> pem::Pem { + // From vectors/cryptography_vectors/x509/custom/crl_bad_version.pem + pem::parse( + "-----BEGIN X509 CRL----- +MIIBpzCBkAIBAjANBgkqhkiG9w0BAQsFADBOMQswCQYDVQQGEwJVUzETMBEGA1UE +CAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNTW91bnRhaW4gVmlldzESMBAGA1UECgwJ +Qm9yaW5nU1NMFw0xNjA5MjYxNTEwNTVaFw0xNjEwMjYxNTEwNTVaoA4wDDAKBgNV +HRQEAwIBATANBgkqhkiG9w0BAQsFAAOCAQEAnrBKKgvd9x9zwK9rtUvVeFeJ7+LN +ZEAc+a5oxpPNEsJx6hXoApYEbzXMxuWBQoCs5iEBycSGudct21L+MVf27M38KrWo +eOkq0a2siqViQZO2Fb/SUFR0k9zb8xl86Zf65lgPplALun0bV/HT7MJcl04Tc4os +dsAReBs5nqTGNEd5AlC1iKHvQZkM//MD51DspKnDpsDiUVi54h9C1SpfZmX8H2Vv +diyu0fZ/bPAM3VAGawatf/SyWfBMyKpoPXEG39oAzmjjOj8en82psn7m474IGaho +/vBbhl1ms5qQiLYPjm4YELtnXQoFyC72tBjbdFd/ZE9k4CNKDbxFUXFbkw== +-----END X509 CRL-----", + ) + .unwrap() + } + + // Limbo tests exercise from Python which hits the check in load_der_x509_crl, missing the + // additional check in the Rust surface. + #[test] + fn rejects_unsupported_crl_version() { + let issuer_pem = ca_pem(); + let issuer = cert(&issuer_pem); + let crl_pem = crl_pem(); + let crl = crl(&crl_pem); + + assert_eq!(crl.tbs_cert_list.version, Some(2)); + assert!(CrlMeta::load(&crl).is_none()); + assert!(CrlRevocationChecker::new(NullOps, [(&issuer, &crl)]).is_none()); + } +} diff --git a/src/rust/cryptography-x509/src/crl.rs b/src/rust/cryptography-x509/src/crl.rs index 77b303a0c989..4816e8852320 100644 --- a/src/rust/cryptography-x509/src/crl.rs +++ b/src/rust/cryptography-x509/src/crl.rs @@ -4,6 +4,7 @@ use crate::certificate::SerialNumber; use crate::common::Asn1Operation; +use crate::extensions::{DuplicateExtensionsError, Extensions}; use crate::{common, extensions, name}; pub type ReasonFlags<'a, Op> = Option<::OwnedBitString<'a>>; @@ -15,6 +16,12 @@ pub struct CertificateRevocationList<'a> { pub signature_value: asn1::BitString<'a>, } +impl<'a> CertificateRevocationList<'a> { + pub fn extensions(&self) -> Result, DuplicateExtensionsError> { + self.tbs_cert_list.extensions() + } +} + pub type RevokedCertificates<'a> = Option< common::Asn1ReadableOrWritable< asn1::SequenceOf<'a, RevokedCertificate<'a>>, @@ -34,6 +41,12 @@ pub struct TBSCertList<'a> { pub raw_crl_extensions: Option>, } +impl<'a> TBSCertList<'a> { + pub fn extensions(&self) -> Result, DuplicateExtensionsError> { + Extensions::from_raw_extensions(self.raw_crl_extensions.as_ref()) + } +} + #[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Eq, Hash, Clone)] pub struct RevokedCertificate<'a> { pub user_certificate: SerialNumber<'a>, @@ -41,6 +54,12 @@ pub struct RevokedCertificate<'a> { pub raw_crl_entry_extensions: Option>, } +impl<'a> RevokedCertificate<'a> { + pub fn extensions(&self) -> Result, DuplicateExtensionsError> { + Extensions::from_raw_extensions(self.raw_crl_entry_extensions.as_ref()) + } +} + #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct IssuingDistributionPoint<'a, Op: Asn1Operation> { #[explicit(0)] diff --git a/src/rust/cryptography-x509/src/extensions.rs b/src/rust/cryptography-x509/src/extensions.rs index 8a9df2a16482..336ac596e956 100644 --- a/src/rust/cryptography-x509/src/extensions.rs +++ b/src/rust/cryptography-x509/src/extensions.rs @@ -171,6 +171,9 @@ pub struct MSCertificateTemplate { pub minor_version: Option, } +pub type SequenceOfDistributionPoints<'a, Op> = + ::SequenceOfVec<'a, DistributionPoint<'a, Op>>; + #[derive(asn1::Asn1Read, asn1::Asn1Write)] pub struct DistributionPoint<'a, Op: Asn1Operation> { #[explicit(0)] diff --git a/src/rust/src/lib.rs b/src/rust/src/lib.rs index 1fa3cc48a093..62f8d4f88f48 100644 --- a/src/rust/src/lib.rs +++ b/src/rust/src/lib.rs @@ -183,8 +183,9 @@ mod _rust { use crate::x509::sct::Sct; #[pymodule_export] use crate::x509::verify::{ - PolicyBuilder, PyClientVerifier, PyCriticality, PyExtensionPolicy, PyPolicy, - PyServerVerifier, PyStore, PyVerifiedClient, VerificationError, + PolicyBuilder, PyClientVerifier, PyCriticality, PyCrlRevocationChecker, + PyExtensionPolicy, PyPolicy, PyServerVerifier, PyStore, PyVerifiedClient, + VerificationError, }; } diff --git a/src/rust/src/x509/crl.rs b/src/rust/src/x509/crl.rs index 8f51be6916b4..985d42a68fc1 100644 --- a/src/rust/src/x509/crl.rs +++ b/src/rust/src/x509/crl.rs @@ -10,6 +10,7 @@ use cryptography_x509::crl::{ }; use cryptography_x509::extensions::{Extension, IssuerAlternativeName}; use cryptography_x509::{name, oid}; +use cryptography_x509_verification::ops::CryptoOps; use pyo3::types::{PyAnyMethods, PyListMethods, PySliceMethods}; use crate::asn1::{ @@ -17,6 +18,7 @@ use crate::asn1::{ }; use crate::backend::hashes::Hash; use crate::error::{CryptographyError, CryptographyResult}; +use crate::x509::verify::PyCryptoOps; use crate::x509::{certificate, extensions, sign}; use crate::{exceptions, types, x509}; @@ -426,14 +428,10 @@ impl CertificateRevocationList { // being an invalid signature. sign::identify_public_key_type(py, public_key.clone())?; - Ok(sign::verify_signature_with_signature_algorithm( - py, - public_key, - &slf.owned.borrow_dependent().signature_algorithm, - slf.owned.borrow_dependent().signature_value.as_bytes(), - &asn1::write_single(&slf.owned.borrow_dependent().tbs_cert_list)?, - ) - .is_ok()) + let ops = PyCryptoOps {}; + Ok(ops + .verify_crl_signed_by(slf.owned.borrow_dependent(), &public_key.unbind()) + .is_ok()) } } diff --git a/src/rust/src/x509/verify/mod.rs b/src/rust/src/x509/verify/mod.rs index 3fce53d6a4b4..dffdbdf97e98 100644 --- a/src/rust/src/x509/verify/mod.rs +++ b/src/rust/src/x509/verify/mod.rs @@ -3,6 +3,7 @@ // for complete details. use cryptography_x509::certificate::Certificate; +use cryptography_x509::crl::CertificateRevocationList; use cryptography_x509::extensions::SubjectAlternativeName; use cryptography_x509::oid::SUBJECT_ALTERNATIVE_NAME_OID; use cryptography_x509_verification::ops::{CryptoOps, VerificationCertificate}; @@ -13,6 +14,8 @@ use pyo3::types::{PyAnyMethods, PyListMethods}; mod extension_policy; mod policy; +mod revocation; +pub(crate) use crate::x509::verify::revocation::PyCrlRevocationChecker; pub(crate) use extension_policy::{PyCriticality, PyExtensionPolicy}; pub(crate) use policy::PyPolicy; @@ -39,6 +42,22 @@ impl CryptoOps for PyCryptoOps { }) } + fn verify_crl_signed_by( + &self, + crl: &CertificateRevocationList<'_>, + key: &Self::Key, + ) -> Result<(), Self::Err> { + pyo3::Python::attach(|py| -> CryptographyResult<()> { + sign::verify_signature_with_signature_algorithm( + py, + key.bind(py).clone(), + &crl.signature_algorithm, + crl.signature_value.as_bytes(), + &asn1::write_single(&crl.tbs_cert_list)?, + ) + }) + } + fn verify_signed_by(&self, cert: &Certificate<'_>, key: &Self::Key) -> Result<(), Self::Err> { pyo3::Python::attach(|py| -> CryptographyResult<()> { sign::verify_signature_with_signature_algorithm( @@ -87,6 +106,7 @@ pub(crate) struct PolicyBuilder { max_chain_depth: Option, ca_ext_policy: Option>, ee_ext_policy: Option>, + revocation_checker: Option>, } impl PolicyBuilder { @@ -97,6 +117,7 @@ impl PolicyBuilder { max_chain_depth: self.max_chain_depth, ca_ext_policy: self.ca_ext_policy.as_ref().map(|p| p.clone_ref(py)), ee_ext_policy: self.ee_ext_policy.as_ref().map(|p| p.clone_ref(py)), + revocation_checker: self.revocation_checker.as_ref().map(|p| p.clone_ref(py)), } } } @@ -111,6 +132,7 @@ impl PolicyBuilder { max_chain_depth: None, ca_ext_policy: None, ee_ext_policy: None, + revocation_checker: None, } } @@ -170,6 +192,19 @@ impl PolicyBuilder { }) } + fn revocation_checker( + &self, + py: pyo3::Python<'_>, + revocation_checker: pyo3::Py, + ) -> CryptographyResult { + policy_builder_set_once_check!(self, revocation_checker, "revocation checker"); + + Ok(PolicyBuilder { + revocation_checker: Some(revocation_checker), + ..self.py_clone(py) + }) + } + fn build_client_verifier(&self, py: pyo3::Python<'_>) -> CryptographyResult { let store = match self.store.as_ref() { Some(s) => s.clone_ref(py), @@ -209,6 +244,7 @@ impl PolicyBuilder { Ok(PyClientVerifier { py_policy: pyo3::Py::new(py, py_policy)?, + revocation_checker: self.revocation_checker.as_ref().map(|c| c.clone_ref(py)), store, }) } @@ -266,6 +302,7 @@ impl PolicyBuilder { Ok(PyServerVerifier { py_policy: pyo3::Py::new(py, py_policy)?, + revocation_checker: self.revocation_checker.as_ref().map(|c| c.clone_ref(py)), store, }) } @@ -314,6 +351,8 @@ pub(crate) struct PyClientVerifier { #[pyo3(get, name = "policy")] py_policy: pyo3::Py, #[pyo3(get)] + revocation_checker: Option>, + #[pyo3(get)] store: pyo3::Py, } @@ -332,6 +371,10 @@ impl PyClientVerifier { intermediates: Vec>, ) -> CryptographyResult { let policy = Policy::new(self.as_policy_def(), self.py_policy.clone_ref(py)); + let revocation_checker = self + .revocation_checker + .as_ref() + .map(|c| c.get().raw.borrow_dependent()); let store = self.store.get(); let intermediates = intermediates @@ -345,6 +388,7 @@ impl PyClientVerifier { &v, &intermediates, &policy, + revocation_checker, store.raw.borrow_dependent(), ) .or_else(|e| handle_validation_error(py, e))?; @@ -386,6 +430,8 @@ pub(crate) struct PyServerVerifier { #[pyo3(get, name = "policy")] py_policy: pyo3::Py, #[pyo3(get)] + revocation_checker: Option>, + #[pyo3(get)] store: pyo3::Py, } @@ -404,6 +450,10 @@ impl PyServerVerifier { intermediates: Vec>, ) -> CryptographyResult> { let policy = Policy::new(self.as_policy_def(), self.py_policy.clone_ref(py)); + let revocation_checker = self + .revocation_checker + .as_ref() + .map(|c| c.get().raw.borrow_dependent()); let store = self.store.get(); let intermediates = intermediates @@ -417,6 +467,7 @@ impl PyServerVerifier { &v, &intermediates, &policy, + revocation_checker, store.raw.borrow_dependent(), ) .or_else(|e| handle_validation_error(py, e))?; diff --git a/src/rust/src/x509/verify/revocation.rs b/src/rust/src/x509/verify/revocation.rs new file mode 100644 index 000000000000..72b7d8694eed --- /dev/null +++ b/src/rust/src/x509/verify/revocation.rs @@ -0,0 +1,55 @@ +use cryptography_x509_verification::revocation::CrlRevocationChecker; + +use crate::x509::{certificate::Certificate, crl::CertificateRevocationList, verify::PyCryptoOps}; + +self_cell::self_cell!( + pub(crate) struct RawPyCrlRevocationChecker { + owner: Vec<(pyo3::Py, pyo3::Py)>, + + #[covariant] + dependent: CrlRevocationChecker, + } +); + +// NO-COVERAGE-START +#[pyo3::pyclass( + frozen, + module = "cryptography.hazmat.bindings._rust.x509", + name = "CRLRevocationChecker" +)] +// NO-COVERAGE-END +pub(crate) struct PyCrlRevocationChecker { + pub(crate) raw: RawPyCrlRevocationChecker, +} + +#[pyo3::pymethods] +impl PyCrlRevocationChecker { + #[new] + fn new( + issuers_to_crls: Vec<(pyo3::Py, pyo3::Py)>, + ) -> pyo3::PyResult { + if issuers_to_crls.is_empty() { + return Err(pyo3::exceptions::PyValueError::new_err( + "can't create an empty CRL revocation checker", + )); + } + + let raw = RawPyCrlRevocationChecker::try_new(issuers_to_crls, |v| { + CrlRevocationChecker::new( + PyCryptoOps {}, + v.iter().map(|i| { + ( + i.0.get().raw.borrow_dependent(), + i.1.get().owned.borrow_dependent(), + ) + }), + ) + .ok_or_else(|| { + pyo3::exceptions::PyValueError::new_err( + "Failed to process CRLs. Ensure that CRLs and issuers match", + ) + }) + })?; + Ok(Self { raw }) + } +} diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py index 97b092bc3520..e9357cf1c32a 100644 --- a/tests/x509/verification/test_limbo.py +++ b/tests/x509/verification/test_limbo.py @@ -12,9 +12,10 @@ import pytest from cryptography import x509 -from cryptography.x509 import load_pem_x509_certificate +from cryptography.x509 import load_pem_x509_certificate, load_pem_x509_crl from cryptography.x509.verification import ( ClientVerifier, + CRLRevocationChecker, PolicyBuilder, ServerVerifier, Store, @@ -43,8 +44,6 @@ "rfc5280-incompatible-with-webpki", # We do not support policy constraints. "has-policy-constraints", - # We don't yet support CRLs - "has-crl", } LIMBO_SKIP_TESTCASES = { @@ -95,6 +94,8 @@ "webpki::cn::utf8-vs-punycode-mismatch", "webpki::cn::not-in-san", "webpki::cn::case-mismatch", + # We fail on chain building for this case. + "crl::issuer-no-keyusage-extension", } @@ -110,6 +111,19 @@ def _get_limbo_peer(expected_peer): return x509.RFC822Name(value) +def _crl_revocation_checker(builder, issuer_candidates, crls) -> PolicyBuilder: + issuers_by_name = {} + for cert in issuer_candidates: + assert cert.subject not in issuers_by_name + issuers_by_name[cert.subject] = cert + + issuer_crls = [] + for crl in crls: + issuer_crls.append((issuers_by_name[crl.issuer], crl)) + + return builder.revocation_checker(CRLRevocationChecker(issuer_crls)) + + def _limbo_testcase(id_, testcase): if id_ in LIMBO_SKIP_TESTCASES: pytest.skip(f"explicitly skipped testcase: {id_}") @@ -156,8 +170,15 @@ def _limbo_testcase(id_, testcase): # Some tests exercise invalid leaf SANs, which get caught before # validation even begins. try: + crls = [ + load_pem_x509_crl(crl.encode()) for crl in testcase["crls"] + ] + if crls: + builder = _crl_revocation_checker( + builder, [*trusted_certs, *untrusted_intermediates], crls + ) verifier = builder.build_server_verifier(peer_name) - except ValueError: + except Exception: assert not should_pass return else: diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index 8bdabf0ff967..48b807ed19a3 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -19,6 +19,7 @@ from cryptography.x509.oid import NameOID from cryptography.x509.verification import ( Criticality, + CRLRevocationChecker, ExtensionPolicy, Policy, PolicyBuilder, @@ -39,6 +40,23 @@ def dummy_store() -> Store: return Store([cert]) +def crl_vector() -> tuple[x509.Certificate, x509.CertificateRevocationList]: + crl_issuer = _load_cert( + os.path.join("x509", "PKITS_data", "certs", "GoodCACert.crt"), + x509.load_der_x509_certificate, + ) + crl = _load_cert( + os.path.join("x509", "PKITS_data", "crls", "GoodCACRL.crl"), + x509.load_der_x509_crl, + ) + return crl_issuer, crl + + +def dummy_revocation_checker() -> CRLRevocationChecker: + crl_issuer, crl = crl_vector() + return CRLRevocationChecker([(crl_issuer, crl)]) + + class TestPolicyBuilder: def test_time_already_set(self): with pytest.raises(ValueError): @@ -54,6 +72,11 @@ def test_max_chain_depth_already_set(self): with pytest.raises(ValueError): PolicyBuilder().max_chain_depth(8).max_chain_depth(9) + def test_revocation_checker_already_set(self): + with pytest.raises(ValueError): + rc = dummy_revocation_checker() + PolicyBuilder().revocation_checker(rc).revocation_checker(rc) + def test_ipaddress_subject(self): verifier = ( PolicyBuilder() @@ -119,6 +142,18 @@ def test_build_server_verifier_missing_store(self): PolicyBuilder().build_server_verifier(DNSName("cryptography.io")) +class TestCRLRevocationChecker: + def test_crl_revocation_checker_rejects_empty_list(self): + with pytest.raises(ValueError): + CRLRevocationChecker([]) + + def test_crl_revocation_checker_rejects_duplicate_crls(self): + crl_issuer, crl = crl_vector() + + with pytest.raises(ValueError, match="Failed to process CRLs"): + CRLRevocationChecker([(crl_issuer, crl), (crl_issuer, crl)]) + + class TestStore: def test_store_rejects_empty_list(self): with pytest.raises(ValueError): @@ -179,6 +214,32 @@ def test_verify(self): assert x509.DNSName("cryptography.io") in verified_client.subjects assert len(verified_client.subjects) == 2 + def test_verify_crl_checker(self): + crl_issuer, _crl = crl_vector() + leaf = _load_cert( + os.path.join( + "x509", "PKITS_data", "certs", "InvalidRevokedEETest3EE.crt" + ), + x509.load_der_x509_certificate, + ) + validation_time = datetime.datetime.fromisoformat( + "2024-01-01T00:00:00+00:00" + ) + + builder = ( + PolicyBuilder().store(Store([crl_issuer])).time(validation_time) + ) + # PKITS EE cert doesn't have SAN, relax validator to test CRL behavior + builder = builder.extension_policies( + ca_policy=ExtensionPolicy.webpki_defaults_ca(), + ee_policy=ExtensionPolicy.permit_all(), + ) + builder = builder.revocation_checker(dummy_revocation_checker()) + verifier = builder.build_client_verifier() + + with pytest.raises(VerificationError, match="certificate revoked"): + verifier.verify(leaf, []) + def test_verify_fails_renders_oid(self): leaf = _load_cert( os.path.join("x509", "custom", "ekucrit-testuser-cert.pem"),