From a5212763ab5f698bcb2d2a970da056790c707a0e Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Wed, 11 Mar 2026 20:57:25 +0000 Subject: [PATCH 01/16] verification: add skeleton for revocation checking --- .../hazmat/bindings/_rust/x509.pyi | 6 + src/cryptography/x509/verification.py | 13 +++ .../src/certificate.rs | 33 +++++- .../cryptography-x509-verification/src/lib.rs | 14 ++- .../cryptography-x509-verification/src/ops.rs | 14 +++ .../src/revocation.rs | 68 ++++++++++++ src/rust/src/lib.rs | 5 +- src/rust/src/types.rs | 3 + src/rust/src/x509/crl.rs | 4 +- src/rust/src/x509/verify/mod.rs | 104 ++++++++++++++++++ src/rust/src/x509/verify/revocation.rs | 39 +++++++ 11 files changed, 297 insertions(+), 6 deletions(-) create mode 100644 src/rust/cryptography-x509-verification/src/revocation.rs create mode 100644 src/rust/src/x509/verify/revocation.rs diff --git a/src/cryptography/hazmat/bindings/_rust/x509.pyi b/src/cryptography/hazmat/bindings/_rust/x509.pyi index 196a3c60f7ed..29002e8c4b3e 100644 --- a/src/cryptography/hazmat/bindings/_rust/x509.pyi +++ b/src/cryptography/hazmat/bindings/_rust/x509.pyi @@ -217,6 +217,9 @@ class PolicyBuilder: def extension_policies( self, *, ca_policy: ExtensionPolicy, ee_policy: ExtensionPolicy ) -> PolicyBuilder: ... + def revocation_checker( + self, revocation_checker: x509.verification.RevocationChecker + ) -> PolicyBuilder: ... def build_client_verifier(self) -> ClientVerifier: ... def build_server_verifier( self, subject: x509.verification.Subject @@ -278,6 +281,9 @@ class ExtensionPolicy: validator: PresentExtensionValidatorCallback[T] | None, ) -> ExtensionPolicy: ... +class CRLRevocationChecker: + def __init__(self, crls: list[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..45ef075a41f1 100644 --- a/src/cryptography/x509/verification.py +++ b/src/cryptography/x509/verification.py @@ -4,17 +4,20 @@ from __future__ import annotations +import abc import typing from cryptography.hazmat.bindings._rust import x509 as rust_x509 from cryptography.x509.general_name import DNSName, IPAddress __all__ = [ + "CRLRevocationChecker", "ClientVerifier", "Criticality", "ExtensionPolicy", "Policy", "PolicyBuilder", + "RevocationChecker", "ServerVerifier", "Store", "Subject", @@ -32,3 +35,13 @@ ExtensionPolicy = rust_x509.ExtensionPolicy Criticality = rust_x509.Criticality VerificationError = rust_x509.VerificationError +CRLRevocationChecker = rust_x509.CRLRevocationChecker + + +class RevocationChecker(metaclass=abc.ABCMeta): + """ + An interface for revocation checkers. + """ + + +RevocationChecker.register(CRLRevocationChecker) diff --git a/src/rust/cryptography-x509-verification/src/certificate.rs b/src/rust/cryptography-x509-verification/src/certificate.rs index 4c9fc256b5f8..22ecb476c703 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() { @@ -42,6 +43,25 @@ Xw4nMqk= .unwrap() } + 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 b995a03505f2..057b68fe1326 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; @@ -26,6 +27,7 @@ use cryptography_x509::oid::{NAME_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID} use crate::certificate::cert_is_self_issued; use crate::ops::{CryptoOps, VerificationCertificate}; use crate::policy::Policy; +use crate::revocation::RevocationChecker; use crate::trust_store::Store; use crate::types::{ DNSConstraint, DNSPattern, IPAddress, IPConstraint, RFC822Constraint, RFC822Name, @@ -40,6 +42,7 @@ pub enum ValidationErrorKind<'chain, B: CryptoOps> { reason: &'static str, }, FatalError(&'static str), + RevocationNotDetermined(String), Other(String), } @@ -91,6 +94,9 @@ impl Display for ValidationError<'_, B> { write!(f, "invalid extension: {oid}: {reason}") } ValidationErrorKind::FatalError(err) => write!(f, "fatal error: {err}"), + ValidationErrorKind::RevocationNotDetermined(err) => { + write!(f, "unable to determine revocation status: {err}") + } ValidationErrorKind::Other(err) => write!(f, "{err}"), } } @@ -272,9 +278,10 @@ pub fn verify<'chain, B: CryptoOps>( leaf: &VerificationCertificate<'chain, B>, intermediates: &[VerificationCertificate<'chain, B>], policy: &Policy<'_, B>, + revocation_checker: Option>, 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) @@ -283,6 +290,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>, store: &'a Store<'chain, B>, } @@ -309,11 +317,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>, store: &'a Store<'chain, B>, ) -> Self { Self { intermediates, policy, + revocation_checker, store, } } @@ -340,6 +350,8 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { name_chain: NameChain<'_, 'chain>, budget: &mut Budget, ) -> ValidationResult<'chain, Chain<'chain, B>, B> { + let _revocation_checker = &self.revocation_checker; + if let Some(nc) = working_cert_extensions.get_extension(&NAME_CONSTRAINTS_OID) { name_chain.evaluate_constraints(&nc.value()?, budget)?; } diff --git a/src/rust/cryptography-x509-verification/src/ops.rs b/src/rust/cryptography-x509-verification/src/ops.rs index c539d2eaf015..07e201c666f7 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,6 +109,7 @@ pub trait CryptoOps { #[cfg(test)] pub(crate) mod tests { use cryptography_x509::certificate::Certificate; + use cryptography_x509::crl::CertificateRevocationList; use super::VerificationCertificate; use crate::certificate::tests::PublicKeyErrorOps; @@ -129,6 +139,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/revocation.rs b/src/rust/cryptography-x509-verification/src/revocation.rs new file mode 100644 index 000000000000..af6a61899022 --- /dev/null +++ b/src/rust/cryptography-x509-verification/src/revocation.rs @@ -0,0 +1,68 @@ +// 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 cryptography_x509::crl::CertificateRevocationList; + +use crate::{ + ops::{CryptoOps, VerificationCertificate}, + policy::Policy, + ValidationResult, +}; + +trait CheckRevocation { + fn is_revoked( + &self, + cert: &VerificationCertificate<'_, B>, + issuer: &VerificationCertificate<'_, B>, + policy: &Policy<'_, B>, + ) -> ValidationResult<'_, bool, B>; +} + +pub struct CrlRevocationChecker<'a> { + crls: Vec<&'a CertificateRevocationList<'a>>, +} + +impl<'a, B: CryptoOps> CheckRevocation for CrlRevocationChecker<'a> { + fn is_revoked( + &self, + cert: &VerificationCertificate<'_, B>, + issuer: &VerificationCertificate<'_, B>, + policy: &Policy<'_, B>, + ) -> ValidationResult<'_, bool, B> { + let _crls = &self.crls; + let _cert = cert; + let _issuer = issuer; + let _policy = policy; + + Ok(false) + } +} + +impl<'a> CrlRevocationChecker<'a> { + pub fn new(crls: impl IntoIterator>) -> Self { + Self { + crls: crls.into_iter().collect(), + } + } +} + +/// Wrapper for revocation checkers that dispatches `is_revoked` calls to the inner implementation. +/// This allows us to avoid trait object-based polymorphism in the verifier. +pub enum RevocationChecker<'a> { + Crl(&'a CrlRevocationChecker<'a>), +} + +impl RevocationChecker<'_> { + /// Checks the revocation status of the leaf of the provided chain. + pub fn is_revoked( + &self, + cert: &VerificationCertificate<'_, B>, + issuer: &VerificationCertificate<'_, B>, + policy: &Policy<'_, B>, + ) -> ValidationResult<'_, bool, B> { + match self { + RevocationChecker::Crl(c) => c.is_revoked(cert, issuer, policy), + } + } +} diff --git a/src/rust/src/lib.rs b/src/rust/src/lib.rs index e32468df9671..4ff0c0243a92 100644 --- a/src/rust/src/lib.rs +++ b/src/rust/src/lib.rs @@ -179,8 +179,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/types.rs b/src/rust/src/types.rs index 12e5a7e16200..03fa6df2e616 100644 --- a/src/rust/src/types.rs +++ b/src/rust/src/types.rs @@ -324,6 +324,9 @@ pub static ASN1_TYPE_BMP_STRING: LazyPyImport = pub static ASN1_TYPE_UNIVERSAL_STRING: LazyPyImport = LazyPyImport::new("cryptography.x509.name", &["_ASN1Type", "UniversalString"]); +pub static REVOCATION_CHECKER: LazyPyImport = + LazyPyImport::new("cryptography.x509.verification", &["RevocationChecker"]); + pub static PKCS7_OPTIONS: LazyPyImport = LazyPyImport::new( "cryptography.hazmat.primitives.serialization.pkcs7", &["PKCS7Options"], diff --git a/src/rust/src/x509/crl.rs b/src/rust/src/x509/crl.rs index 791513ca8ad1..4aff2e9be18a 100644 --- a/src/rust/src/x509/crl.rs +++ b/src/rust/src/x509/crl.rs @@ -72,7 +72,7 @@ pub(crate) fn load_pem_x509_crl( } self_cell::self_cell!( - struct OwnedCertificateRevocationList { + pub(crate) struct OwnedCertificateRevocationList { owner: pyo3::Py, #[covariant] dependent: RawCertificateRevocationList, @@ -81,7 +81,7 @@ self_cell::self_cell!( #[pyo3::pyclass(frozen, module = "cryptography.hazmat.bindings._rust.x509")] pub(crate) struct CertificateRevocationList { - owned: OwnedCertificateRevocationList, + pub(crate) owned: OwnedCertificateRevocationList, revoked_certs: pyo3::sync::PyOnceLock>, cached_extensions: pyo3::sync::PyOnceLock>, diff --git a/src/rust/src/x509/verify/mod.rs b/src/rust/src/x509/verify/mod.rs index 3fce53d6a4b4..48c8ceab7e88 100644 --- a/src/rust/src/x509/verify/mod.rs +++ b/src/rust/src/x509/verify/mod.rs @@ -3,16 +3,20 @@ // 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}; use cryptography_x509_verification::policy::{Policy, PolicyDefinition, Subject}; +use cryptography_x509_verification::revocation::RevocationChecker; use cryptography_x509_verification::trust_store::Store; use cryptography_x509_verification::types::{DNSName, IPAddress}; 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 +43,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 +107,7 @@ pub(crate) struct PolicyBuilder { max_chain_depth: Option, ca_ext_policy: Option>, ee_ext_policy: Option>, + revocation_checker: Option>, } impl PolicyBuilder { @@ -97,6 +118,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 +133,7 @@ impl PolicyBuilder { max_chain_depth: None, ca_ext_policy: None, ee_ext_policy: None, + revocation_checker: None, } } @@ -170,6 +193,30 @@ impl PolicyBuilder { }) } + fn revocation_checker( + &self, + py: pyo3::Python<'_>, + revocation_checker: pyo3::Py, + ) -> CryptographyResult { + policy_builder_set_once_check!(self, revocation_checker, "revocation checker"); + + if !revocation_checker + .bind(py) + .is_instance(&types::REVOCATION_CHECKER.get(py)?)? + { + return Err(CryptographyError::from( + pyo3::exceptions::PyTypeError::new_err( + "revocation_checker must be a registered RevocationChecker", + ), + )); + } + + 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 +256,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 +314,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, }) } @@ -293,6 +342,12 @@ self_cell::self_cell!( } ); +/// This enum provides heterogenously typed ownership of revocation checker pyclasses for +/// `PyClientVerifier::verify` and `PyServerVerifier::verify`. +enum RevocationCheckerOwner { + Crl(pyo3::Py), +} + #[pyo3::pyclass( frozen, name = "VerifiedClient", @@ -314,6 +369,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 +389,15 @@ impl PyClientVerifier { intermediates: Vec>, ) -> CryptographyResult { let policy = Policy::new(self.as_policy_def(), self.py_policy.clone_ref(py)); + let revocation_checker_owner = self + .revocation_checker + .as_ref() + .map(|c| build_revocation_checker_owner(py, c)) + .transpose()?; + let revocation_checker = revocation_checker_owner + .as_ref() + .map(build_revocation_checker) + .transpose()?; let store = self.store.get(); let intermediates = intermediates @@ -345,6 +411,7 @@ impl PyClientVerifier { &v, &intermediates, &policy, + revocation_checker, store.raw.borrow_dependent(), ) .or_else(|e| handle_validation_error(py, e))?; @@ -386,6 +453,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 +473,15 @@ impl PyServerVerifier { intermediates: Vec>, ) -> CryptographyResult> { let policy = Policy::new(self.as_policy_def(), self.py_policy.clone_ref(py)); + let revocation_checker_owner = self + .revocation_checker + .as_ref() + .map(|c| build_revocation_checker_owner(py, c)) + .transpose()?; + let revocation_checker = revocation_checker_owner + .as_ref() + .map(build_revocation_checker) + .transpose()?; let store = self.store.get(); let intermediates = intermediates @@ -417,6 +495,7 @@ impl PyServerVerifier { &v, &intermediates, &policy, + revocation_checker, store.raw.borrow_dependent(), ) .or_else(|e| handle_validation_error(py, e))?; @@ -476,6 +555,31 @@ fn build_subject<'a>( } } +fn build_revocation_checker_owner( + py: pyo3::Python<'_>, + checker: &pyo3::Py, +) -> pyo3::PyResult { + let checker = checker.bind(py); + if let Ok(checker) = checker.extract::>() { + return Ok(RevocationCheckerOwner::Crl(checker)); + } + + Err(pyo3::exceptions::PyTypeError::new_err( + "unsupported revocation checker type", + )) +} + +fn build_revocation_checker<'a>( + checker: &'a RevocationCheckerOwner, +) -> pyo3::PyResult> { + match checker { + RevocationCheckerOwner::Crl(crl) => { + let inner = crl.get().raw.borrow_dependent(); + Ok(RevocationChecker::Crl(inner)) + } + } +} + fn handle_validation_error( py: pyo3::Python<'_>, e: cryptography_x509_verification::ValidationError<'_, PyCryptoOps>, diff --git a/src/rust/src/x509/verify/revocation.rs b/src/rust/src/x509/verify/revocation.rs new file mode 100644 index 000000000000..b764fef18076 --- /dev/null +++ b/src/rust/src/x509/verify/revocation.rs @@ -0,0 +1,39 @@ +use cryptography_x509_verification::revocation::CrlRevocationChecker; + +use crate::x509::crl::CertificateRevocationList; + +self_cell::self_cell!( + pub(crate) struct RawPyCrlRevocationChecker { + owner: Vec>, + + #[covariant] + dependent: CrlRevocationChecker, + } +); + +#[pyo3::pyclass( + frozen, + module = "cryptography.hazmat.bindings._rust.x509", + name = "CRLRevocationChecker" +)] +pub(crate) struct PyCrlRevocationChecker { + pub(crate) raw: RawPyCrlRevocationChecker, +} + +#[pyo3::pymethods] +impl PyCrlRevocationChecker { + #[new] + fn new(crls: Vec>) -> pyo3::PyResult { + if crls.is_empty() { + return Err(pyo3::exceptions::PyValueError::new_err( + "can't create an empty CRL revocation checker", + )); + } + + Ok(Self { + raw: RawPyCrlRevocationChecker::new(crls, |v| { + CrlRevocationChecker::new(v.iter().map(|i| i.get().owned.borrow_dependent())) + }), + }) + } +} From 280da61f8599e19821056f3f06c6f597e4ff34b0 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Wed, 18 Mar 2026 19:19:13 +0000 Subject: [PATCH 02/16] test CRLRevocationChecker with limbo --- src/rust/cryptography-x509-verification/src/lib.rs | 8 ++++++++ tests/x509/verification/test_limbo.py | 8 +++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index 057b68fe1326..3dbf05758aa3 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/src/rust/cryptography-x509-verification/src/lib.rs @@ -513,6 +513,14 @@ mod tests { "invalid extension: 2.5.29.17: duplicate extension" ); + let err = ValidationError::::new( + ValidationErrorKind::RevocationNotDetermined("no matching CRL found".to_owned()), + ); + assert_eq!( + err.to_string(), + "unable to determine revocation status: no matching CRL found" + ); + let err = ValidationError::::new(ValidationErrorKind::FatalError("oops")); assert_eq!(err.to_string(), "fatal error: oops"); diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py index 5c037556934b..c723325971ef 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 = { @@ -132,6 +131,7 @@ def _limbo_testcase(id_, testcase): peer_certificate = load_pem_x509_certificate( testcase["peer_certificate"].encode() ) + crls = [load_pem_x509_crl(crl.encode()) for crl in testcase["crls"]] validation_time = testcase["validation_time"] validation_time = ( datetime.datetime.fromisoformat(validation_time) @@ -142,6 +142,8 @@ def _limbo_testcase(id_, testcase): should_pass = testcase["expected_result"] == "SUCCESS" builder = PolicyBuilder().store(Store(trusted_certs)) + if crls: + builder = builder.revocation_checker(CRLRevocationChecker(crls)) if validation_time is not None: builder = builder.time(validation_time) if max_chain_depth is not None: From 533f582df1f196d71680ce6b61adc4ea8a4d0af3 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Thu, 19 Mar 2026 20:05:27 +0000 Subject: [PATCH 03/16] reintroduce trait object, Python ABC --- .../hazmat/bindings/_rust/x509.pyi | 4 +- src/cryptography/x509/verification.py | 12 +++- .../cryptography-x509-verification/src/lib.rs | 6 +- .../src/revocation.rs | 22 +----- src/rust/src/types.rs | 3 - src/rust/src/x509/verify/mod.rs | 70 +++---------------- src/rust/src/x509/verify/revocation.rs | 63 ++++++++++++++--- 7 files changed, 82 insertions(+), 98 deletions(-) diff --git a/src/cryptography/hazmat/bindings/_rust/x509.pyi b/src/cryptography/hazmat/bindings/_rust/x509.pyi index 29002e8c4b3e..83d6d8204e64 100644 --- a/src/cryptography/hazmat/bindings/_rust/x509.pyi +++ b/src/cryptography/hazmat/bindings/_rust/x509.pyi @@ -218,7 +218,9 @@ class PolicyBuilder: self, *, ca_policy: ExtensionPolicy, ee_policy: ExtensionPolicy ) -> PolicyBuilder: ... def revocation_checker( - self, revocation_checker: x509.verification.RevocationChecker + self, + revocation_checker: x509.verification.CRLRevocationChecker + | x509.verification.RevocationChecker, ) -> PolicyBuilder: ... def build_client_verifier(self) -> ClientVerifier: ... def build_server_verifier( diff --git a/src/cryptography/x509/verification.py b/src/cryptography/x509/verification.py index 45ef075a41f1..6053c4c97a0c 100644 --- a/src/cryptography/x509/verification.py +++ b/src/cryptography/x509/verification.py @@ -8,6 +8,7 @@ import typing from cryptography.hazmat.bindings._rust import x509 as rust_x509 +from cryptography.x509 import Certificate from cryptography.x509.general_name import DNSName, IPAddress __all__ = [ @@ -38,10 +39,15 @@ CRLRevocationChecker = rust_x509.CRLRevocationChecker -class RevocationChecker(metaclass=abc.ABCMeta): +class RevocationChecker(rust_x509.RevocationChecker, metaclass=abc.ABCMeta): """ An interface for revocation checkers. """ - -RevocationChecker.register(CRLRevocationChecker) + @abc.abstractmethod + def is_revoked( + self, leaf: Certificate, issuer: Certificate, policy: Policy + ) -> bool: + """ + Returns whether the certificate is revoked or not. + """ diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index 3dbf05758aa3..2fec9b668046 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/src/rust/cryptography-x509-verification/src/lib.rs @@ -278,7 +278,7 @@ pub fn verify<'chain, B: CryptoOps>( leaf: &VerificationCertificate<'chain, B>, intermediates: &[VerificationCertificate<'chain, B>], policy: &Policy<'_, B>, - revocation_checker: Option>, + revocation_checker: Option<&'_ RevocationChecker<'_, B>>, store: &Store<'chain, B>, ) -> ValidationResult<'chain, Chain<'chain, B>, B> { let builder = ChainBuilder::new(intermediates, policy, revocation_checker, store); @@ -290,7 +290,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>, + revocation_checker: Option<&'a RevocationChecker<'a, B>>, store: &'a Store<'chain, B>, } @@ -317,7 +317,7 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { fn new( intermediates: &'a [VerificationCertificate<'chain, B>], policy: &'a Policy<'a, B>, - revocation_checker: Option>, + revocation_checker: Option<&'a RevocationChecker<'a, B>>, store: &'a Store<'chain, B>, ) -> Self { Self { diff --git a/src/rust/cryptography-x509-verification/src/revocation.rs b/src/rust/cryptography-x509-verification/src/revocation.rs index af6a61899022..5a8918669804 100644 --- a/src/rust/cryptography-x509-verification/src/revocation.rs +++ b/src/rust/cryptography-x509-verification/src/revocation.rs @@ -10,7 +10,7 @@ use crate::{ ValidationResult, }; -trait CheckRevocation { +pub trait CheckRevocation { fn is_revoked( &self, cert: &VerificationCertificate<'_, B>, @@ -47,22 +47,6 @@ impl<'a> CrlRevocationChecker<'a> { } } -/// Wrapper for revocation checkers that dispatches `is_revoked` calls to the inner implementation. -/// This allows us to avoid trait object-based polymorphism in the verifier. -pub enum RevocationChecker<'a> { - Crl(&'a CrlRevocationChecker<'a>), -} +pub type RevocationChecker<'a, B> = dyn CheckRevocation + Send + Sync + 'a; -impl RevocationChecker<'_> { - /// Checks the revocation status of the leaf of the provided chain. - pub fn is_revoked( - &self, - cert: &VerificationCertificate<'_, B>, - issuer: &VerificationCertificate<'_, B>, - policy: &Policy<'_, B>, - ) -> ValidationResult<'_, bool, B> { - match self { - RevocationChecker::Crl(c) => c.is_revoked(cert, issuer, policy), - } - } -} +impl<'a, B: CryptoOps> RevocationChecker<'a, B> {} diff --git a/src/rust/src/types.rs b/src/rust/src/types.rs index 03fa6df2e616..12e5a7e16200 100644 --- a/src/rust/src/types.rs +++ b/src/rust/src/types.rs @@ -324,9 +324,6 @@ pub static ASN1_TYPE_BMP_STRING: LazyPyImport = pub static ASN1_TYPE_UNIVERSAL_STRING: LazyPyImport = LazyPyImport::new("cryptography.x509.name", &["_ASN1Type", "UniversalString"]); -pub static REVOCATION_CHECKER: LazyPyImport = - LazyPyImport::new("cryptography.x509.verification", &["RevocationChecker"]); - pub static PKCS7_OPTIONS: LazyPyImport = LazyPyImport::new( "cryptography.hazmat.primitives.serialization.pkcs7", &["PKCS7Options"], diff --git a/src/rust/src/x509/verify/mod.rs b/src/rust/src/x509/verify/mod.rs index 48c8ceab7e88..debf610c26b7 100644 --- a/src/rust/src/x509/verify/mod.rs +++ b/src/rust/src/x509/verify/mod.rs @@ -8,7 +8,6 @@ use cryptography_x509::extensions::SubjectAlternativeName; use cryptography_x509::oid::SUBJECT_ALTERNATIVE_NAME_OID; use cryptography_x509_verification::ops::{CryptoOps, VerificationCertificate}; use cryptography_x509_verification::policy::{Policy, PolicyDefinition, Subject}; -use cryptography_x509_verification::revocation::RevocationChecker; use cryptography_x509_verification::trust_store::Store; use cryptography_x509_verification::types::{DNSName, IPAddress}; use pyo3::types::{PyAnyMethods, PyListMethods}; @@ -17,6 +16,7 @@ mod extension_policy; mod policy; mod revocation; pub(crate) use crate::x509::verify::revocation::PyCrlRevocationChecker; +use crate::x509::verify::revocation::{build_rust_revocation_checker, PyRevocationChecker}; pub(crate) use extension_policy::{PyCriticality, PyExtensionPolicy}; pub(crate) use policy::PyPolicy; @@ -107,7 +107,7 @@ pub(crate) struct PolicyBuilder { max_chain_depth: Option, ca_ext_policy: Option>, ee_ext_policy: Option>, - revocation_checker: Option>, + revocation_checker: Option>, } impl PolicyBuilder { @@ -196,21 +196,10 @@ impl PolicyBuilder { fn revocation_checker( &self, py: pyo3::Python<'_>, - revocation_checker: pyo3::Py, + revocation_checker: pyo3::Py, ) -> CryptographyResult { policy_builder_set_once_check!(self, revocation_checker, "revocation checker"); - if !revocation_checker - .bind(py) - .is_instance(&types::REVOCATION_CHECKER.get(py)?)? - { - return Err(CryptographyError::from( - pyo3::exceptions::PyTypeError::new_err( - "revocation_checker must be a registered RevocationChecker", - ), - )); - } - Ok(PolicyBuilder { revocation_checker: Some(revocation_checker), ..self.py_clone(py) @@ -342,12 +331,6 @@ self_cell::self_cell!( } ); -/// This enum provides heterogenously typed ownership of revocation checker pyclasses for -/// `PyClientVerifier::verify` and `PyServerVerifier::verify`. -enum RevocationCheckerOwner { - Crl(pyo3::Py), -} - #[pyo3::pyclass( frozen, name = "VerifiedClient", @@ -369,7 +352,7 @@ pub(crate) struct PyClientVerifier { #[pyo3(get, name = "policy")] py_policy: pyo3::Py, #[pyo3(get)] - revocation_checker: Option>, + revocation_checker: Option>, #[pyo3(get)] store: pyo3::Py, } @@ -389,15 +372,10 @@ impl PyClientVerifier { intermediates: Vec>, ) -> CryptographyResult { let policy = Policy::new(self.as_policy_def(), self.py_policy.clone_ref(py)); - let revocation_checker_owner = self + let revocation_checker = self .revocation_checker .as_ref() - .map(|c| build_revocation_checker_owner(py, c)) - .transpose()?; - let revocation_checker = revocation_checker_owner - .as_ref() - .map(build_revocation_checker) - .transpose()?; + .map(|c| build_rust_revocation_checker(py, c).unwrap_or(c)); let store = self.store.get(); let intermediates = intermediates @@ -453,7 +431,7 @@ pub(crate) struct PyServerVerifier { #[pyo3(get, name = "policy")] py_policy: pyo3::Py, #[pyo3(get)] - revocation_checker: Option>, + revocation_checker: Option>, #[pyo3(get)] store: pyo3::Py, } @@ -473,15 +451,10 @@ impl PyServerVerifier { intermediates: Vec>, ) -> CryptographyResult> { let policy = Policy::new(self.as_policy_def(), self.py_policy.clone_ref(py)); - let revocation_checker_owner = self + let revocation_checker = self .revocation_checker .as_ref() - .map(|c| build_revocation_checker_owner(py, c)) - .transpose()?; - let revocation_checker = revocation_checker_owner - .as_ref() - .map(build_revocation_checker) - .transpose()?; + .map(|c| build_rust_revocation_checker(py, c).unwrap_or(c)); let store = self.store.get(); let intermediates = intermediates @@ -555,31 +528,6 @@ fn build_subject<'a>( } } -fn build_revocation_checker_owner( - py: pyo3::Python<'_>, - checker: &pyo3::Py, -) -> pyo3::PyResult { - let checker = checker.bind(py); - if let Ok(checker) = checker.extract::>() { - return Ok(RevocationCheckerOwner::Crl(checker)); - } - - Err(pyo3::exceptions::PyTypeError::new_err( - "unsupported revocation checker type", - )) -} - -fn build_revocation_checker<'a>( - checker: &'a RevocationCheckerOwner, -) -> pyo3::PyResult> { - match checker { - RevocationCheckerOwner::Crl(crl) => { - let inner = crl.get().raw.borrow_dependent(); - Ok(RevocationChecker::Crl(inner)) - } - } -} - fn handle_validation_error( py: pyo3::Python<'_>, e: cryptography_x509_verification::ValidationError<'_, PyCryptoOps>, diff --git a/src/rust/src/x509/verify/revocation.rs b/src/rust/src/x509/verify/revocation.rs index b764fef18076..39f5078ffddd 100644 --- a/src/rust/src/x509/verify/revocation.rs +++ b/src/rust/src/x509/verify/revocation.rs @@ -1,6 +1,11 @@ -use cryptography_x509_verification::revocation::CrlRevocationChecker; +use cryptography_x509_verification::{ + ops::VerificationCertificate, + policy::Policy, + revocation::{CheckRevocation, CrlRevocationChecker, RevocationChecker}, + ValidationResult, +}; -use crate::x509::crl::CertificateRevocationList; +use crate::x509::{crl::CertificateRevocationList, verify::PyCryptoOps}; self_cell::self_cell!( pub(crate) struct RawPyCrlRevocationChecker { @@ -14,7 +19,8 @@ self_cell::self_cell!( #[pyo3::pyclass( frozen, module = "cryptography.hazmat.bindings._rust.x509", - name = "CRLRevocationChecker" + name = "CRLRevocationChecker", + extends = PyRevocationChecker, )] pub(crate) struct PyCrlRevocationChecker { pub(crate) raw: RawPyCrlRevocationChecker, @@ -23,17 +29,58 @@ pub(crate) struct PyCrlRevocationChecker { #[pyo3::pymethods] impl PyCrlRevocationChecker { #[new] - fn new(crls: Vec>) -> pyo3::PyResult { + fn new( + crls: Vec>, + ) -> pyo3::PyResult<(Self, PyRevocationChecker)> { if crls.is_empty() { return Err(pyo3::exceptions::PyValueError::new_err( "can't create an empty CRL revocation checker", )); } - Ok(Self { - raw: RawPyCrlRevocationChecker::new(crls, |v| { - CrlRevocationChecker::new(v.iter().map(|i| i.get().owned.borrow_dependent())) - }), + Ok(( + Self { + raw: RawPyCrlRevocationChecker::new(crls, |v| { + CrlRevocationChecker::new(v.iter().map(|i| i.get().owned.borrow_dependent())) + }), + }, + PyRevocationChecker {}, + )) + } +} + +/// A marker class that Rust and Python revocation checkers subclass from. +#[pyo3::pyclass( + subclass, + frozen, + module = "cryptography.hazmat.bindings._rust.x509", + name = "RevocationChecker" +)] +pub(crate) struct PyRevocationChecker; + +impl CheckRevocation for pyo3::Py { + fn is_revoked( + &self, + _cert: &VerificationCertificate<'_, PyCryptoOps>, + _issuer: &VerificationCertificate<'_, PyCryptoOps>, + _policy: &Policy<'_, PyCryptoOps>, + ) -> ValidationResult<'_, bool, PyCryptoOps> { + pyo3::Python::attach(|py| { + let _self = self.bind(py); + todo!("self_.call_method w/ is_revoked ...") }) } } + +pub(crate) fn build_rust_revocation_checker<'a>( + py: pyo3::Python<'a>, + checker: &'a pyo3::Py, +) -> pyo3::PyResult<&'a RevocationChecker<'a, PyCryptoOps>> { + if let Ok(crl) = checker.cast_bound::(py) { + return Ok(crl.get().raw.borrow_dependent()); + } + + Err(pyo3::exceptions::PyTypeError::new_err( + "not a Rust RevocationChecker", + )) +} From 98bd2db0ee354fa6298347d9513e396355f4c0a5 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Thu, 19 Mar 2026 20:14:19 +0000 Subject: [PATCH 04/16] fix circular import --- src/cryptography/x509/verification.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cryptography/x509/verification.py b/src/cryptography/x509/verification.py index 6053c4c97a0c..f478370f6869 100644 --- a/src/cryptography/x509/verification.py +++ b/src/cryptography/x509/verification.py @@ -8,7 +8,6 @@ import typing from cryptography.hazmat.bindings._rust import x509 as rust_x509 -from cryptography.x509 import Certificate from cryptography.x509.general_name import DNSName, IPAddress __all__ = [ @@ -46,7 +45,10 @@ class RevocationChecker(rust_x509.RevocationChecker, metaclass=abc.ABCMeta): @abc.abstractmethod def is_revoked( - self, leaf: Certificate, issuer: Certificate, policy: Policy + self, + leaf: rust_x509.Certificate, + issuer: rust_x509.Certificate, + policy: Policy, ) -> bool: """ Returns whether the certificate is revoked or not. From dfe51b6b7d8f553ce5da4389824793dbd1400e21 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Thu, 19 Mar 2026 20:21:41 +0000 Subject: [PATCH 05/16] correctly export PyRevocationChecker --- src/rust/src/lib.rs | 4 ++-- src/rust/src/x509/verify/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rust/src/lib.rs b/src/rust/src/lib.rs index 4ff0c0243a92..0f918cbe5799 100644 --- a/src/rust/src/lib.rs +++ b/src/rust/src/lib.rs @@ -180,8 +180,8 @@ mod _rust { #[pymodule_export] use crate::x509::verify::{ PolicyBuilder, PyClientVerifier, PyCriticality, PyCrlRevocationChecker, - PyExtensionPolicy, PyPolicy, PyServerVerifier, PyStore, PyVerifiedClient, - VerificationError, + PyExtensionPolicy, PyPolicy, PyRevocationChecker, PyServerVerifier, PyStore, + PyVerifiedClient, VerificationError, }; } diff --git a/src/rust/src/x509/verify/mod.rs b/src/rust/src/x509/verify/mod.rs index debf610c26b7..625868d88994 100644 --- a/src/rust/src/x509/verify/mod.rs +++ b/src/rust/src/x509/verify/mod.rs @@ -15,8 +15,7 @@ use pyo3::types::{PyAnyMethods, PyListMethods}; mod extension_policy; mod policy; mod revocation; -pub(crate) use crate::x509::verify::revocation::PyCrlRevocationChecker; -use crate::x509::verify::revocation::{build_rust_revocation_checker, PyRevocationChecker}; +pub(crate) use crate::x509::verify::revocation::{PyCrlRevocationChecker, PyRevocationChecker}; pub(crate) use extension_policy::{PyCriticality, PyExtensionPolicy}; pub(crate) use policy::PyPolicy; @@ -27,6 +26,7 @@ use crate::types; use crate::x509::certificate::Certificate as PyCertificate; use crate::x509::common::{datetime_now, py_to_datetime}; use crate::x509::sign; +use crate::x509::verify::revocation::build_rust_revocation_checker; #[derive(Clone)] pub(crate) struct PyCryptoOps {} From e5e4ba935acbcbf84e2f074a0579b97ab83b2bb6 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Fri, 20 Mar 2026 16:11:52 +0000 Subject: [PATCH 06/16] hook revocation up to verification --- .../hazmat/bindings/_rust/x509.pyi | 2 + .../cryptography-x509-verification/src/lib.rs | 14 ++++- .../src/revocation.rs | 18 +++--- src/rust/src/x509/verify/mod.rs | 4 +- src/rust/src/x509/verify/revocation.rs | 57 ++++++++++++++----- tests/x509/verification/test_limbo.py | 4 ++ tests/x509/verification/test_verification.py | 48 ++++++++++++++++ 7 files changed, 119 insertions(+), 28 deletions(-) diff --git a/src/cryptography/hazmat/bindings/_rust/x509.pyi b/src/cryptography/hazmat/bindings/_rust/x509.pyi index 83d6d8204e64..25544a783b1d 100644 --- a/src/cryptography/hazmat/bindings/_rust/x509.pyi +++ b/src/cryptography/hazmat/bindings/_rust/x509.pyi @@ -283,6 +283,8 @@ class ExtensionPolicy: validator: PresentExtensionValidatorCallback[T] | None, ) -> ExtensionPolicy: ... +class RevocationChecker: ... + class CRLRevocationChecker: def __init__(self, crls: list[x509.CertificateRevocationList]) -> None: ... diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index 2fec9b668046..76c875abdb5a 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/src/rust/cryptography-x509-verification/src/lib.rs @@ -350,8 +350,6 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { name_chain: NameChain<'_, 'chain>, budget: &mut Budget, ) -> ValidationResult<'chain, Chain<'chain, B>, B> { - let _revocation_checker = &self.revocation_checker; - if let Some(nc) = working_cert_extensions.get_extension(&NAME_CONSTRAINTS_OID) { name_chain.evaluate_constraints(&nc.value()?, budget)?; } @@ -422,6 +420,18 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { budget, ) { Ok(mut chain) => { + if let Some(revocation_checker) = self.revocation_checker { + if revocation_checker.is_revoked( + working_cert, + issuing_cert_candidate, + self.policy, + )? { + return Err(ValidationError::new(ValidationErrorKind::Other( + "certificate revoked".to_string(), + ))); + } + } + chain.push(working_cert.clone()); return Ok(chain); } diff --git a/src/rust/cryptography-x509-verification/src/revocation.rs b/src/rust/cryptography-x509-verification/src/revocation.rs index 5a8918669804..72467d6c9dd6 100644 --- a/src/rust/cryptography-x509-verification/src/revocation.rs +++ b/src/rust/cryptography-x509-verification/src/revocation.rs @@ -11,12 +11,12 @@ use crate::{ }; pub trait CheckRevocation { - fn is_revoked( + fn is_revoked<'chain>( &self, - cert: &VerificationCertificate<'_, B>, - issuer: &VerificationCertificate<'_, B>, + cert: &VerificationCertificate<'chain, B>, + issuer: &VerificationCertificate<'chain, B>, policy: &Policy<'_, B>, - ) -> ValidationResult<'_, bool, B>; + ) -> ValidationResult<'chain, bool, B>; } pub struct CrlRevocationChecker<'a> { @@ -24,12 +24,12 @@ pub struct CrlRevocationChecker<'a> { } impl<'a, B: CryptoOps> CheckRevocation for CrlRevocationChecker<'a> { - fn is_revoked( + fn is_revoked<'chain>( &self, - cert: &VerificationCertificate<'_, B>, - issuer: &VerificationCertificate<'_, B>, + cert: &VerificationCertificate<'chain, B>, + issuer: &VerificationCertificate<'chain, B>, policy: &Policy<'_, B>, - ) -> ValidationResult<'_, bool, B> { + ) -> ValidationResult<'chain, bool, B> { let _crls = &self.crls; let _cert = cert; let _issuer = issuer; @@ -48,5 +48,3 @@ impl<'a> CrlRevocationChecker<'a> { } pub type RevocationChecker<'a, B> = dyn CheckRevocation + Send + Sync + 'a; - -impl<'a, B: CryptoOps> RevocationChecker<'a, B> {} diff --git a/src/rust/src/x509/verify/mod.rs b/src/rust/src/x509/verify/mod.rs index 625868d88994..26f37437cb25 100644 --- a/src/rust/src/x509/verify/mod.rs +++ b/src/rust/src/x509/verify/mod.rs @@ -375,7 +375,7 @@ impl PyClientVerifier { let revocation_checker = self .revocation_checker .as_ref() - .map(|c| build_rust_revocation_checker(py, c).unwrap_or(c)); + .map(|c| build_rust_revocation_checker(py, c)); let store = self.store.get(); let intermediates = intermediates @@ -454,7 +454,7 @@ impl PyServerVerifier { let revocation_checker = self .revocation_checker .as_ref() - .map(|c| build_rust_revocation_checker(py, c).unwrap_or(c)); + .map(|c| build_rust_revocation_checker(py, c)); let store = self.store.get(); let intermediates = intermediates diff --git a/src/rust/src/x509/verify/revocation.rs b/src/rust/src/x509/verify/revocation.rs index 39f5078ffddd..41cf4e0106dd 100644 --- a/src/rust/src/x509/verify/revocation.rs +++ b/src/rust/src/x509/verify/revocation.rs @@ -2,10 +2,13 @@ use cryptography_x509_verification::{ ops::VerificationCertificate, policy::Policy, revocation::{CheckRevocation, CrlRevocationChecker, RevocationChecker}, - ValidationResult, + ValidationError, ValidationErrorKind, ValidationResult, }; -use crate::x509::{crl::CertificateRevocationList, verify::PyCryptoOps}; +use crate::x509::{ + crl::CertificateRevocationList, + verify::{PyCryptoOps, VerificationError}, +}; self_cell::self_cell!( pub(crate) struct RawPyCrlRevocationChecker { @@ -58,29 +61,55 @@ impl PyCrlRevocationChecker { )] pub(crate) struct PyRevocationChecker; +#[pyo3::pymethods] +impl PyRevocationChecker { + #[new] + pub fn new(_cls: pyo3::Py) -> Self { + Self + } +} + impl CheckRevocation for pyo3::Py { - fn is_revoked( + fn is_revoked<'chain>( &self, - _cert: &VerificationCertificate<'_, PyCryptoOps>, - _issuer: &VerificationCertificate<'_, PyCryptoOps>, - _policy: &Policy<'_, PyCryptoOps>, - ) -> ValidationResult<'_, bool, PyCryptoOps> { + cert: &VerificationCertificate<'chain, PyCryptoOps>, + issuer: &VerificationCertificate<'chain, PyCryptoOps>, + policy: &Policy<'_, PyCryptoOps>, + ) -> ValidationResult<'chain, bool, PyCryptoOps> { pyo3::Python::attach(|py| { - let _self = self.bind(py); - todo!("self_.call_method w/ is_revoked ...") + self.call_method1( + py, + pyo3::intern!(py, "is_revoked"), + (cert.extra(), issuer.extra(), &policy.extra), + ) + .map_err(|e| { + let kind = if e.is_instance_of::(py) { + ValidationErrorKind::RevocationNotDetermined::(e.to_string()) + } else { + ValidationErrorKind::FatalError::("the revocation checker threw an exception while checking revocation status") + }; + + ValidationError::new(kind) + })? + .extract(py) + .map_err(|_e| { + ValidationError::new(ValidationErrorKind::FatalError::( + "the revocation checker returned an invalid non-bool result", + )) + }) }) } } +/// Retrieves the underlying native RevocationChecker from the PyRevocationChecker if it exists. pub(crate) fn build_rust_revocation_checker<'a>( py: pyo3::Python<'a>, checker: &'a pyo3::Py, -) -> pyo3::PyResult<&'a RevocationChecker<'a, PyCryptoOps>> { +) -> &'a RevocationChecker<'a, PyCryptoOps> { if let Ok(crl) = checker.cast_bound::(py) { - return Ok(crl.get().raw.borrow_dependent()); + return crl.get().raw.borrow_dependent(); } - Err(pyo3::exceptions::PyTypeError::new_err( - "not a Rust RevocationChecker", - )) + // this isn't a Rust-native revocation checker, fallthrough. + checker } diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py index c723325971ef..d2d97177ace7 100644 --- a/tests/x509/verification/test_limbo.py +++ b/tests/x509/verification/test_limbo.py @@ -166,6 +166,10 @@ def _limbo_testcase(id_, testcase): assert testcase["extended_key_usage"] == ["clientAuth"] verifier = builder.build_client_verifier() + # XX(tnytown): xfail until we implement the CRL revocation checker. + if crls: + pytest.xfail() + if should_pass: if isinstance(verifier, ServerVerifier): built_chain = verifier.verify( diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index 1d846a118485..a001531250ff 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -16,9 +16,11 @@ from cryptography.x509.general_name import DNSName, IPAddress from cryptography.x509.verification import ( Criticality, + CRLRevocationChecker, ExtensionPolicy, Policy, PolicyBuilder, + RevocationChecker, Store, VerificationError, ) @@ -27,6 +29,16 @@ WEBPKI_MINIMUM_RSA_MODULUS = 2048 +class DummyRevocationChecker(RevocationChecker): + def __init__(self, returns: bool) -> None: + self._returns = returns + + def is_revoked( + self, cert: x509.Certificate, issuer: x509.Certificate, policy: Policy + ) -> bool: + return self._returns + + @lru_cache(maxsize=1) def dummy_store() -> Store: cert = _load_cert( @@ -51,6 +63,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 = DummyRevocationChecker(False) + PolicyBuilder().revocation_checker(rc).revocation_checker(rc) + def test_ipaddress_subject(self): verifier = ( PolicyBuilder() @@ -116,6 +133,12 @@ def test_build_server_verifier_missing_store(self): PolicyBuilder().build_server_verifier(DNSName("cryptography.io")) +class TestCRLRevocationChecker: + def test_revocation_checker_rejects_empty_list(self): + with pytest.raises(ValueError): + CRLRevocationChecker([]) + + class TestStore: def test_store_rejects_empty_list(self): with pytest.raises(ValueError): @@ -151,6 +174,7 @@ def test_verify(self): builder = builder.time(validation_time).max_chain_depth( max_chain_depth ) + builder = builder.revocation_checker(DummyRevocationChecker(False)) verifier = builder.build_client_verifier() assert verifier.policy.subject is None @@ -176,6 +200,30 @@ def test_verify(self): assert x509.DNSName("cryptography.io") in verified_client.subjects assert len(verified_client.subjects) == 2 + def test_verify_fails_revoked(self): + # expires 2018-11-16 01:15:03 UTC + leaf = _load_cert( + os.path.join("x509", "cryptography.io.pem"), + x509.load_pem_x509_certificate, + ) + + ca = _load_cert( + os.path.join("x509", "rapidssl_sha256_ca_g3.pem"), + x509.load_pem_x509_certificate, + ) + store = Store([ca]) + + validation_time = datetime.datetime.fromisoformat( + "2018-11-16T00:00:00+00:00" + ) + + builder = PolicyBuilder().store(store).time(validation_time) + builder = builder.revocation_checker(DummyRevocationChecker(True)) + verifier = builder.build_client_verifier() + + with pytest.raises(VerificationError): + verifier.verify(leaf, []) + def test_verify_fails_renders_oid(self): leaf = _load_cert( os.path.join("x509", "custom", "ekucrit-testuser-cert.pem"), From c9863a5766f6459d031837df6cdf08708bd52cdf Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Sat, 21 Mar 2026 01:20:37 +0000 Subject: [PATCH 07/16] more limbo harness hax --- tests/x509/verification/test_limbo.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py index d2d97177ace7..887301e82743 100644 --- a/tests/x509/verification/test_limbo.py +++ b/tests/x509/verification/test_limbo.py @@ -166,9 +166,14 @@ def _limbo_testcase(id_, testcase): assert testcase["extended_key_usage"] == ["clientAuth"] verifier = builder.build_client_verifier() - # XX(tnytown): xfail until we implement the CRL revocation checker. + # XX(tnytown): just run the verifier without caring about its results for + # CRL tests. this is just to give us a preview of coverage through limbo if crls: - pytest.xfail() + try: + verifier.verify(peer_certificate, untrusted_intermediates) + except Exception as _e: + pass + return if should_pass: if isinstance(verifier, ServerVerifier): From 0b64bb5a9891c1677be366956003a502b4aa09f6 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 20:30:02 +0000 Subject: [PATCH 08/16] rework error states for Python revocation checkers --- src/cryptography/x509/verification.py | 5 +-- .../cryptography-x509-verification/src/lib.rs | 6 ++-- src/rust/src/x509/crl.rs | 14 ++++---- src/rust/src/x509/verify/revocation.rs | 32 +++++++++---------- tests/x509/verification/test_verification.py | 30 ++++++++++++++--- 5 files changed, 54 insertions(+), 33 deletions(-) diff --git a/src/cryptography/x509/verification.py b/src/cryptography/x509/verification.py index f478370f6869..a4c3eda44bf9 100644 --- a/src/cryptography/x509/verification.py +++ b/src/cryptography/x509/verification.py @@ -49,7 +49,8 @@ def is_revoked( leaf: rust_x509.Certificate, issuer: rust_x509.Certificate, policy: Policy, - ) -> bool: + ) -> bool | None: """ - Returns whether the certificate is revoked or not. + Returns whether the certificate is revoked. If the revocation status + cannot be determined, the revocation checker may return None. """ diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index 76c875abdb5a..bc7d8a6ba73a 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/src/rust/cryptography-x509-verification/src/lib.rs @@ -42,7 +42,7 @@ pub enum ValidationErrorKind<'chain, B: CryptoOps> { reason: &'static str, }, FatalError(&'static str), - RevocationNotDetermined(String), + RevocationNotDetermined, Other(String), } @@ -94,8 +94,8 @@ impl Display for ValidationError<'_, B> { write!(f, "invalid extension: {oid}: {reason}") } ValidationErrorKind::FatalError(err) => write!(f, "fatal error: {err}"), - ValidationErrorKind::RevocationNotDetermined(err) => { - write!(f, "unable to determine revocation status: {err}") + ValidationErrorKind::RevocationNotDetermined => { + write!(f, "unable to determine revocation status") } ValidationErrorKind::Other(err) => write!(f, "{err}"), } diff --git a/src/rust/src/x509/crl.rs b/src/rust/src/x509/crl.rs index 4aff2e9be18a..18504d4760ba 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}; @@ -422,14 +424,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/revocation.rs b/src/rust/src/x509/verify/revocation.rs index 41cf4e0106dd..d658569c27db 100644 --- a/src/rust/src/x509/verify/revocation.rs +++ b/src/rust/src/x509/verify/revocation.rs @@ -52,13 +52,15 @@ impl PyCrlRevocationChecker { } } -/// A marker class that Rust and Python revocation checkers subclass from. +// NO-COVERAGE-START #[pyo3::pyclass( subclass, frozen, module = "cryptography.hazmat.bindings._rust.x509", name = "RevocationChecker" )] +// NO-COVERAGE-END +/// A marker class that Rust and Python revocation checkers subclass from. pub(crate) struct PyRevocationChecker; #[pyo3::pymethods] @@ -77,26 +79,24 @@ impl CheckRevocation for pyo3::Py { policy: &Policy<'_, PyCryptoOps>, ) -> ValidationResult<'chain, bool, PyCryptoOps> { pyo3::Python::attach(|py| { - self.call_method1( + let result = self.call_method1( py, pyo3::intern!(py, "is_revoked"), (cert.extra(), issuer.extra(), &policy.extra), ) - .map_err(|e| { - let kind = if e.is_instance_of::(py) { - ValidationErrorKind::RevocationNotDetermined::(e.to_string()) - } else { - ValidationErrorKind::FatalError::("the revocation checker threw an exception while checking revocation status") - }; - - ValidationError::new(kind) - })? - .extract(py) .map_err(|_e| { - ValidationError::new(ValidationErrorKind::FatalError::( - "the revocation checker returned an invalid non-bool result", - )) - }) + ValidationError::new(ValidationErrorKind::FatalError::("the revocation checker raised an exception")) + })?; + + if result.is_none(py) { + ValidationError::new(ValidationErrorKind::RevocationNotDetermined); + } else { + result.extract(py).map_err(|_e| { + ValidationError::new(ValidationErrorKind::FatalError::( + "the revocation checker must return one of True, False, or None", + )) + }) + } }) } } diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index a001531250ff..7f5985adc706 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -30,12 +30,14 @@ class DummyRevocationChecker(RevocationChecker): - def __init__(self, returns: bool) -> None: + def __init__(self, returns, raises: Exception | None = None) -> None: self._returns = returns def is_revoked( self, cert: x509.Certificate, issuer: x509.Certificate, policy: Policy ) -> bool: + if self.raises is not None: + raise self.raises return self._returns @@ -200,7 +202,25 @@ def test_verify(self): assert x509.DNSName("cryptography.io") in verified_client.subjects assert len(verified_client.subjects) == 2 - def test_verify_fails_revoked(self): + @pytest.mark.parametrize( + ("returns", "raises", "error"), + [ + (True, None, "certificate revoked"), + ( + "Truthy", + None, + "fatal error: the revocation checker must return one of " + "True, False, or None", + ), + ( + None, + Exception("some exception"), + "fatal error: the revocation checker raised an exception", + ), + (None, None, "unable to determine revocation status"), + ], + ) + def test_verify_fails_revocation(self, returns, raises, error): # expires 2018-11-16 01:15:03 UTC leaf = _load_cert( os.path.join("x509", "cryptography.io.pem"), @@ -218,10 +238,12 @@ def test_verify_fails_revoked(self): ) builder = PolicyBuilder().store(store).time(validation_time) - builder = builder.revocation_checker(DummyRevocationChecker(True)) + builder = builder.revocation_checker( + DummyRevocationChecker(returns, raises) + ) verifier = builder.build_client_verifier() - with pytest.raises(VerificationError): + with pytest.raises(VerificationError, match=error): verifier.verify(leaf, []) def test_verify_fails_renders_oid(self): From 9d1e5a9403427f8b9ac297dacee2d634f50297b7 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 20:31:30 +0000 Subject: [PATCH 09/16] get rid of limbo CRL tests for now --- tests/x509/verification/test_limbo.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py index 887301e82743..5c037556934b 100644 --- a/tests/x509/verification/test_limbo.py +++ b/tests/x509/verification/test_limbo.py @@ -12,10 +12,9 @@ import pytest from cryptography import x509 -from cryptography.x509 import load_pem_x509_certificate, load_pem_x509_crl +from cryptography.x509 import load_pem_x509_certificate from cryptography.x509.verification import ( ClientVerifier, - CRLRevocationChecker, PolicyBuilder, ServerVerifier, Store, @@ -44,6 +43,8 @@ "rfc5280-incompatible-with-webpki", # We do not support policy constraints. "has-policy-constraints", + # We don't yet support CRLs + "has-crl", } LIMBO_SKIP_TESTCASES = { @@ -131,7 +132,6 @@ def _limbo_testcase(id_, testcase): peer_certificate = load_pem_x509_certificate( testcase["peer_certificate"].encode() ) - crls = [load_pem_x509_crl(crl.encode()) for crl in testcase["crls"]] validation_time = testcase["validation_time"] validation_time = ( datetime.datetime.fromisoformat(validation_time) @@ -142,8 +142,6 @@ def _limbo_testcase(id_, testcase): should_pass = testcase["expected_result"] == "SUCCESS" builder = PolicyBuilder().store(Store(trusted_certs)) - if crls: - builder = builder.revocation_checker(CRLRevocationChecker(crls)) if validation_time is not None: builder = builder.time(validation_time) if max_chain_depth is not None: @@ -166,15 +164,6 @@ def _limbo_testcase(id_, testcase): assert testcase["extended_key_usage"] == ["clientAuth"] verifier = builder.build_client_verifier() - # XX(tnytown): just run the verifier without caring about its results for - # CRL tests. this is just to give us a preview of coverage through limbo - if crls: - try: - verifier.verify(peer_certificate, untrusted_intermediates) - except Exception as _e: - pass - return - if should_pass: if isinstance(verifier, ServerVerifier): built_chain = verifier.verify( From c8fd39d6b98483fddc4015ff1c3c262961ac087e Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 20:34:12 +0000 Subject: [PATCH 10/16] oops --- src/rust/src/x509/verify/revocation.rs | 28 ++++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/rust/src/x509/verify/revocation.rs b/src/rust/src/x509/verify/revocation.rs index d658569c27db..9be98d18e570 100644 --- a/src/rust/src/x509/verify/revocation.rs +++ b/src/rust/src/x509/verify/revocation.rs @@ -5,10 +5,7 @@ use cryptography_x509_verification::{ ValidationError, ValidationErrorKind, ValidationResult, }; -use crate::x509::{ - crl::CertificateRevocationList, - verify::{PyCryptoOps, VerificationError}, -}; +use crate::x509::{crl::CertificateRevocationList, verify::PyCryptoOps}; self_cell::self_cell!( pub(crate) struct RawPyCrlRevocationChecker { @@ -79,17 +76,22 @@ impl CheckRevocation for pyo3::Py { policy: &Policy<'_, PyCryptoOps>, ) -> ValidationResult<'chain, bool, PyCryptoOps> { pyo3::Python::attach(|py| { - let result = self.call_method1( - py, - pyo3::intern!(py, "is_revoked"), - (cert.extra(), issuer.extra(), &policy.extra), - ) - .map_err(|_e| { - ValidationError::new(ValidationErrorKind::FatalError::("the revocation checker raised an exception")) - })?; + let result = self + .call_method1( + py, + pyo3::intern!(py, "is_revoked"), + (cert.extra(), issuer.extra(), &policy.extra), + ) + .map_err(|_e| { + ValidationError::new(ValidationErrorKind::FatalError::( + "the revocation checker raised an exception", + )) + })?; if result.is_none(py) { - ValidationError::new(ValidationErrorKind::RevocationNotDetermined); + Err(ValidationError::new( + ValidationErrorKind::RevocationNotDetermined, + )) } else { result.extract(py).map_err(|_e| { ValidationError::new(ValidationErrorKind::FatalError::( From 5342e5ca85143fb86dac431c8b72c8db0555e9b7 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 21:12:10 +0000 Subject: [PATCH 11/16] accept args on subclasses of PyRevocationChecker --- src/rust/src/x509/verify/revocation.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rust/src/x509/verify/revocation.rs b/src/rust/src/x509/verify/revocation.rs index 9be98d18e570..d339e9434781 100644 --- a/src/rust/src/x509/verify/revocation.rs +++ b/src/rust/src/x509/verify/revocation.rs @@ -63,7 +63,8 @@ pub(crate) struct PyRevocationChecker; #[pyo3::pymethods] impl PyRevocationChecker { #[new] - pub fn new(_cls: pyo3::Py) -> Self { + #[pyo3(signature = (*_args))] + pub fn new(_args: &pyo3::Bound<'_, pyo3::PyAny>) -> Self { Self } } From 530c3fda2690b4b009b6a3506dc104995f2ba675 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 21:22:13 +0000 Subject: [PATCH 12/16] fixup tests --- src/rust/cryptography-x509-verification/src/lib.rs | 7 +++---- tests/x509/verification/test_verification.py | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index bc7d8a6ba73a..636a12671aa2 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/src/rust/cryptography-x509-verification/src/lib.rs @@ -523,12 +523,11 @@ mod tests { "invalid extension: 2.5.29.17: duplicate extension" ); - let err = ValidationError::::new( - ValidationErrorKind::RevocationNotDetermined("no matching CRL found".to_owned()), - ); + let err = + ValidationError::::new(ValidationErrorKind::RevocationNotDetermined); assert_eq!( err.to_string(), - "unable to determine revocation status: no matching CRL found" + "unable to determine revocation status" ); let err = diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index 7f5985adc706..36d29206756c 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -30,7 +30,7 @@ class DummyRevocationChecker(RevocationChecker): - def __init__(self, returns, raises: Exception | None = None) -> None: + def __init__(self, returns, raises=None) -> None: self._returns = returns def is_revoked( From aed68872b326f4fcb703745ee5bfa78fa597ccf2 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 21:23:36 +0000 Subject: [PATCH 13/16] fixup tests --- src/rust/cryptography-x509-verification/src/lib.rs | 5 +---- tests/x509/verification/test_verification.py | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index 636a12671aa2..03e04727d4d8 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/src/rust/cryptography-x509-verification/src/lib.rs @@ -525,10 +525,7 @@ mod tests { let err = ValidationError::::new(ValidationErrorKind::RevocationNotDetermined); - assert_eq!( - err.to_string(), - "unable to determine revocation status" - ); + assert_eq!(err.to_string(), "unable to determine revocation status"); let err = ValidationError::::new(ValidationErrorKind::FatalError("oops")); diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index 36d29206756c..7604f1741804 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -32,6 +32,7 @@ class DummyRevocationChecker(RevocationChecker): def __init__(self, returns, raises=None) -> None: self._returns = returns + self._raises = raises def is_revoked( self, cert: x509.Certificate, issuer: x509.Certificate, policy: Policy From 33412e0106b8427d038cc5b236eac6b5155b93e9 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 21:30:47 +0000 Subject: [PATCH 14/16] fixup tests --- tests/x509/verification/test_verification.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index 7604f1741804..2f09f1ac432b 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -37,8 +37,8 @@ def __init__(self, returns, raises=None) -> None: def is_revoked( self, cert: x509.Certificate, issuer: x509.Certificate, policy: Policy ) -> bool: - if self.raises is not None: - raise self.raises + if self._raises is not None: + raise self._raises return self._returns From a9ff0503412bd8e0a5ef5a37573d419dd305e7bb Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 22:05:51 +0000 Subject: [PATCH 15/16] exercise CRL checker in verification tests --- .../src/revocation.rs | 6 ++-- tests/x509/verification/test_verification.py | 30 ++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/rust/cryptography-x509-verification/src/revocation.rs b/src/rust/cryptography-x509-verification/src/revocation.rs index 72467d6c9dd6..91200a4457f8 100644 --- a/src/rust/cryptography-x509-verification/src/revocation.rs +++ b/src/rust/cryptography-x509-verification/src/revocation.rs @@ -7,7 +7,7 @@ use cryptography_x509::crl::CertificateRevocationList; use crate::{ ops::{CryptoOps, VerificationCertificate}, policy::Policy, - ValidationResult, + ValidationError, ValidationErrorKind, ValidationResult, }; pub trait CheckRevocation { @@ -35,7 +35,9 @@ impl<'a, B: CryptoOps> CheckRevocation for CrlRevocationChecker<'a> { let _issuer = issuer; let _policy = policy; - Ok(false) + Err(ValidationError::new(ValidationErrorKind::FatalError( + "unimplemented", + ))) } } diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index 2f09f1ac432b..323adbb8ab94 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -137,7 +137,7 @@ def test_build_server_verifier_missing_store(self): class TestCRLRevocationChecker: - def test_revocation_checker_rejects_empty_list(self): + def test_crl_revocation_checker_rejects_empty_list(self): with pytest.raises(ValueError): CRLRevocationChecker([]) @@ -203,6 +203,34 @@ 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): + # expires 2018-11-16 01:15:03 UTC + leaf = _load_cert( + os.path.join("x509", "cryptography.io.pem"), + x509.load_pem_x509_certificate, + ) + + ca = _load_cert( + os.path.join("x509", "rapidssl_sha256_ca_g3.pem"), + x509.load_pem_x509_certificate, + ) + crl = _load_cert( + os.path.join("x509", "custom", "crl_empty.pem"), + x509.load_pem_x509_crl, + ) + store = Store([ca]) + + validation_time = datetime.datetime.fromisoformat( + "2018-11-16T00:00:00+00:00" + ) + + builder = PolicyBuilder().store(store).time(validation_time) + builder = builder.revocation_checker(CRLRevocationChecker([crl])) + verifier = builder.build_client_verifier() + + with pytest.raises(VerificationError, match="unimplemented"): + verifier.verify(leaf, []) + @pytest.mark.parametrize( ("returns", "raises", "error"), [ From f2d21efa30589f4c7c59a66d40ab130b3e645b25 Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Tue, 24 Mar 2026 22:40:24 +0000 Subject: [PATCH 16/16] rearrange tests for coverage --- tests/x509/verification/test_verification.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index 323adbb8ab94..3ae3349cd202 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -177,7 +177,6 @@ def test_verify(self): builder = builder.time(validation_time).max_chain_depth( max_chain_depth ) - builder = builder.revocation_checker(DummyRevocationChecker(False)) verifier = builder.build_client_verifier() assert verifier.policy.subject is None @@ -234,6 +233,7 @@ def test_verify_crl_checker(self): @pytest.mark.parametrize( ("returns", "raises", "error"), [ + (False, None, None), (True, None, "certificate revoked"), ( "Truthy", @@ -249,7 +249,7 @@ def test_verify_crl_checker(self): (None, None, "unable to determine revocation status"), ], ) - def test_verify_fails_revocation(self, returns, raises, error): + def test_verify_revocation(self, returns, raises, error): # expires 2018-11-16 01:15:03 UTC leaf = _load_cert( os.path.join("x509", "cryptography.io.pem"), @@ -272,7 +272,10 @@ def test_verify_fails_revocation(self, returns, raises, error): ) verifier = builder.build_client_verifier() - with pytest.raises(VerificationError, match=error): + if error is not None: + with pytest.raises(VerificationError, match=error): + verifier.verify(leaf, []) + else: verifier.verify(leaf, []) def test_verify_fails_renders_oid(self):