diff --git a/.github/actions/fetch-vectors/action.yml b/.github/actions/fetch-vectors/action.yml index 69e92183bfb3..7c0402f527d1 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 May 21, 2026. - ref: "feb7caccc1afaa9d7e63ee8d7f81e6ce8b199510" # x509-limbo-ref + # Latest commit on the x509-limbo main branch, as of June 6, 2026. + ref: "8e1700f79ade7df528c483aa62410c769c828a4d" # x509-limbo-ref persist-credentials: false diff --git a/src/rust/cryptography-x509-verification/src/lib.rs b/src/rust/cryptography-x509-verification/src/lib.rs index 60c89d4945a5..7ee3bb085019 100644 --- a/src/rust/cryptography-x509-verification/src/lib.rs +++ b/src/rust/cryptography-x509-verification/src/lib.rs @@ -18,10 +18,14 @@ use std::vec; use asn1::ObjectIdentifier; use cryptography_x509::common::Asn1Read; use cryptography_x509::extensions::{ - DuplicateExtensionsError, Extensions, NameConstraints, SubjectAlternativeName, + AuthorityKeyIdentifier, DuplicateExtensionsError, Extensions, NameConstraints, + SubjectAlternativeName, }; use cryptography_x509::name::GeneralName; -use cryptography_x509::oid::{NAME_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID}; +use cryptography_x509::oid::{ + AUTHORITY_KEY_IDENTIFIER_OID, NAME_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID, + SUBJECT_KEY_IDENTIFIER_OID, +}; use crate::certificate::cert_is_self_issued; use crate::ops::{CryptoOps, VerificationCertificate}; @@ -98,15 +102,23 @@ impl Display for ValidationError<'_, B> { struct Budget { name_constraint_checks: usize, + signature_checks: usize, } impl Budget { - // Same limit as other validators + // The maximum number of name constraint checks performed when attempting + // path construction. This is the same limit as other validators. const DEFAULT_NAME_CONSTRAINT_CHECK_LIMIT: usize = 1 << 20; + // The maximum number of signature verifications performed when attempting + // path construction. The is similar to other validators: + // both Go and rustls-webpki pick 100. + const DEFAULT_SIGNATURE_CHECK_LIMIT: usize = 1 << 7; + fn new() -> Budget { Budget { name_constraint_checks: Self::DEFAULT_NAME_CONSTRAINT_CHECK_LIMIT, + signature_checks: Self::DEFAULT_SIGNATURE_CHECK_LIMIT, } } @@ -119,6 +131,15 @@ impl Budget { })?; Ok(()) } + + fn signature_check<'chain, B: CryptoOps>(&mut self) -> ValidationResult<'chain, (), B> { + self.signature_checks = self.signature_checks.checked_sub(1).ok_or_else(|| { + ValidationError::new(ValidationErrorKind::FatalError( + "Exceeded maximum signature check limit", + )) + })?; + Ok(()) + } } struct NameChain<'a, 'chain> { @@ -341,18 +362,57 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { } } + /// Identify and return potential issuers for `cert`, considering + /// candidates from both the trusted store and untrusted intermediate set. + /// Trusted candidates are returned before untrusted intermediate + /// candidates, and both groups are opportunisitically ordered by + /// "likeliness" in terms of AKI/SKI match. fn potential_issuers( &self, cert: &'a VerificationCertificate<'chain, B>, - ) -> impl Iterator> + '_ { - // TODO: Optimizations: - // * Search by AKI and other identifiers? - self.store + cert_extensions: &Extensions<'chain>, + ) -> Vec<&'a VerificationCertificate<'chain, B>> { + let mut candidates: Vec<&'a VerificationCertificate<'chain, B>> = self + .store .get_by_subject(&cert.certificate().tbs_cert.issuer) .iter() .chain(self.intermediates.iter().filter(|&candidate| { candidate.certificate().subject() == cert.certificate().issuer() })) + .collect(); + + let want_kid: Option<&[u8]> = cert_extensions + .get_extension(&AUTHORITY_KEY_IDENTIFIER_OID) + .and_then(|ext| ext.value::>().ok()) + .and_then(|aki| aki.key_identifier); + + // This mirrors Go's `findPotentialParents`: we have a global + // signature budget, so we want to bucket candidates by likeliness + // to avoid wasting budget on (potentially adversarial) name collisions. + // + // Observe that we use a stable sort to preserve trusted candidates + // before untrusted candidates in each likeliness bucket. In other + // words, we always try a likely trusted candidate over an equally + // likely untrusted one. + // + // See: + candidates.sort_by_key(|candidate| { + let have_kid: Option<&[u8]> = + candidate.certificate().extensions().ok().and_then(|exts| { + exts.get_extension(&SUBJECT_KEY_IDENTIFIER_OID) + .and_then(|ext| ext.value::<&[u8]>().ok()) + }); + + match (want_kid, have_kid) { + // cert AKID matches candidate SKID, highest likelihood. + (Some(want), Some(have)) if want == have => 0, + // cert AKID and candidate SKID don't match, lowest likelihood. + (Some(_), Some(_)) => 2, + // cert AKID and/or candidate SKID is not present, medium likelihood. + _ => 1u8, + } + }); + candidates } fn build_chain_inner( @@ -385,7 +445,8 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { // Otherwise, we collect a list of potential issuers for this cert, // and continue with the first that verifies. let mut last_err: Option> = None; - for issuing_cert_candidate in self.potential_issuers(working_cert) { + for issuing_cert_candidate in self.potential_issuers(working_cert, working_cert_extensions) + { // A candidate issuer is said to verify if it both // signs for the working certificate and conforms to the // policy. @@ -395,6 +456,7 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { working_cert, current_depth, &issuer_extensions, + budget, ) { Ok(_) => { match self.build_chain_inner( @@ -503,10 +565,15 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> { #[cfg(test)] mod tests { use asn1::ParseError; + use cryptography_x509::certificate::Certificate; use cryptography_x509::oid::SUBJECT_ALTERNATIVE_NAME_OID; use crate::certificate::tests::PublicKeyErrorOps; - use crate::{ValidationError, ValidationErrorKind}; + use crate::ops::{CryptoOps, VerificationCertificate}; + use crate::policy::{Policy, PolicyDefinition, Subject}; + use crate::trust_store::Store; + use crate::types::DNSName; + use crate::{Budget, ChainBuilder, NameChain, ValidationError, ValidationErrorKind}; #[test] fn test_validationerror_display() { @@ -528,4 +595,102 @@ mod tests { 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(&()), ()); + } + + // A self-issued ("looping") CA certificate that is its own issuer. + fn looping_ca_pem() -> pem::Pem { + pem::parse( + "-----BEGIN CERTIFICATE----- +MIIBcjCCARmgAwIBAgIBATAKBggqhkjOPQQDAjAhMR8wHQYDVQQDDBZsb29waW5n +IHNlbGYtc2lnbmVkIENBMB4XDTIzMTIzMTAwMDAwMFoXDTI0MDEzMTAwMDAwMFow +ITEfMB0GA1UEAwwWbG9vcGluZyBzZWxmLXNpZ25lZCBDQTBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABKAoXUGnHdfXJbSXjRjeW+PCVHmlo4KEki69N5pJUA0QyQMR +v9ySOMnWf3Ea7TR4g3zdguwTP7LdpSku3uR1QkmjQjBAMA8GA1UdEwEB/wQFMAMB +Af8wDgYDVR0PAQH/BAQDAgGGMB0GA1UdDgQWBBR23MGdG1Ma9iR+3CxKTafD/OE0 +dTAKBggqhkjOPQQDAgNHADBEAiA4RCr07KfZdM16VfGNZAQFjvC60SWIU3RRVY/L +qolIOwIgCaIgj9ipK0Q0p+45UJiq+L/ncrxsweJkFq/UYubzhX0= +-----END CERTIFICATE-----", + ) + .unwrap() + } + + /// Exercises our pathlen overflow error scenario. + /// + /// This condition is logically unreachable from Python, since + /// we unconditionally limit signature checks to a number smaller + /// than `u8::MAX`, meaning that we always exhaust the signature budget + /// before potentially exhausting the pathlen budget. + /// + /// To test that directly, we manually lift the signature budget + /// and start our pathlen state right at `u8::MAX`, guaranteeing + /// an overflow on the immediate chain building step. + #[test] + fn test_build_chain_inner_depth_overflow() { + let pem = looping_ca_pem(); + let ca = asn1::parse_single::>(pem.contents()).unwrap(); + let ca_exts = ca.extensions().ok().unwrap(); + + // The same self-issued CA is both the working certificate and its own + // (only) candidate issuer, so the search recurses on itself. + let working = VerificationCertificate::::new(&ca, ()); + let intermediates = [VerificationCertificate::::new(&ca, ())]; + let store: Store<'_, NullOps> = Store::new([]); + + let subject = Subject::DNS(DNSName::new("example.com").unwrap()); + let time = asn1::DateTime::new(2024, 1, 1, 0, 0, 0).unwrap(); + let policy_def = + 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 mut budget = Budget { + name_constraint_checks: usize::MAX, + signature_checks: usize::MAX, + }; + + let name_chain = NameChain::new::(None, &ca_exts, false) + .ok() + .unwrap(); + let err = builder + .build_chain_inner(&working, u8::MAX, &ca_exts, name_chain, &mut budget) + .unwrap_err(); + + assert!(matches!( + err.kind, + ValidationErrorKind::Other(msg) if msg.contains("current depth calculation overflowed") + )); + } } diff --git a/src/rust/cryptography-x509-verification/src/policy/mod.rs b/src/rust/cryptography-x509-verification/src/policy/mod.rs index ca33de4f4f78..b327f9af9c91 100644 --- a/src/rust/cryptography-x509-verification/src/policy/mod.rs +++ b/src/rust/cryptography-x509-verification/src/policy/mod.rs @@ -29,7 +29,9 @@ pub use crate::policy::extension::{ PresentExtensionValidatorCallback, }; use crate::types::{DNSName, DNSPattern, IPAddress}; -use crate::{ValidationError, ValidationErrorKind, ValidationResult, VerificationCertificate}; +use crate::{ + Budget, ValidationError, ValidationErrorKind, ValidationResult, VerificationCertificate, +}; // RSA key constraints, as defined in CA/B 6.1.5. const WEBPKI_MINIMUM_RSA_MODULUS: usize = 2048; @@ -505,6 +507,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> { child: &VerificationCertificate<'chain, B>, current_depth: u8, issuer_extensions: &Extensions<'_>, + budget: &mut Budget, ) -> ValidationResult<'chain, (), B> { // The issuer needs to be a valid CA at the current depth. self.permits_ca(issuer, current_depth, issuer_extensions) @@ -565,6 +568,10 @@ impl<'a, B: CryptoOps> Policy<'a, B> { } } + // Charge the (potentially expensive) signature verification against the + // budget before performing it, bounding the total work an attacker can + // force during chain building. + budget.signature_check()?; if self.ops.verify_signed_by(child.certificate(), pk).is_err() { return Err(ValidationError::new(ValidationErrorKind::Other( "signature does not match".to_string(),