Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/actions/fetch-vectors/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
183 changes: 174 additions & 9 deletions src/rust/cryptography-x509-verification/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -98,15 +102,23 @@ impl<B: CryptoOps> 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,
}
}

Expand All @@ -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> {
Expand Down Expand Up @@ -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<Item = &'a VerificationCertificate<'chain, B>> + '_ {
// 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::<AuthorityKeyIdentifier<'_, Asn1Read>>().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: <https://github.com/golang/go/blob/d00c67f297e/src/crypto/x509/cert_pool.go#L136>
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(
Expand Down Expand Up @@ -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<ValidationError<'_, B>> = 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.
Expand All @@ -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(
Expand Down Expand Up @@ -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() {
Expand All @@ -528,4 +595,102 @@ mod tests {
ValidationError::<PublicKeyErrorOps>::new(ValidationErrorKind::FatalError("oops"));
assert_eq!(err.to_string(), "fatal error: oops");
}

/// A `CryptoOps` whose public key extraction and signature verification
/// always succeed, so that `valid_issuer` can be driven to completion
/// without real cryptographic material.
struct NullOps;

impl CryptoOps for NullOps {
type Key = ();
type Err = ();
type CertificateExtra = ();
type PolicyExtra = ();

fn public_key(&self, _cert: &Certificate<'_>) -> Result<Self::Key, Self::Err> {
Ok(())
}

fn verify_signed_by(
&self,
_cert: &Certificate<'_>,
_key: &Self::Key,
) -> Result<(), Self::Err> {
Ok(())
}

fn clone_public_key(_key: &Self::Key) -> Self::Key {}

fn clone_extra(_extra: &Self::CertificateExtra) -> Self::CertificateExtra {}
}

#[test]
fn test_clone() {
assert_eq!(NullOps::clone_public_key(&()), ());
assert_eq!(NullOps::clone_extra(&()), ());
}
Comment on lines +627 to +631
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need NullOps for the coverage below, which means we need to "test" the empty trait impl bodies we add.

(We already do this for another test-only impl, PulicKeyErrorOps.)


// 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::<Certificate<'_>>(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::<NullOps>::new(&ca, ());
let intermediates = [VerificationCertificate::<NullOps>::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::<NullOps>(None, &ca_exts, false)
.ok()
.unwrap();
let err = builder
.build_chain_inner(&working, u8::MAX, &ca_exts, name_chain, &mut budget)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This single line is the coverage-bearing one.

.unwrap_err();

assert!(matches!(
err.kind,
ValidationErrorKind::Other(msg) if msg.contains("current depth calculation overflowed")
));
}
}
9 changes: 8 additions & 1 deletion src/rust/cryptography-x509-verification/src/policy/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
Expand Down
Loading