Skip to content

Commit

Permalink
verify_cert: check name constraints after sig. validation
Browse files Browse the repository at this point in the history
Prior to this commit parsing and processing certificate name constraints
was done before validating a chain of signatures to a known trust
anchor. This increases the attack surface of these features, allowing an
adversary to force webpki to process name constraints on a crafted
certificate without needing to have that certificate issued by a trusted
entity.

This commit moves the parsing and processing of name constraints to
after building and verifying the chain of signatures to reduce the
potential for mischief.
  • Loading branch information
cpu authored and briansmith committed Sep 30, 2023
1 parent 56a178d commit a7a0d41
Showing 1 changed file with 30 additions and 10 deletions.
40 changes: 30 additions & 10 deletions src/verify_cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,14 @@ fn build_chain_inner(
return Err(Error::UnknownIssuer.into());
}

let name_constraints = trust_anchor.name_constraints.map(untrusted::Input::from);

untrusted::read_all_optional(name_constraints, Error::BadDer, |value| {
name::check_name_constraints(value, cert)
})?;

let trust_anchor_spki = untrusted::Input::from(trust_anchor.spki);

// TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?;

check_signatures(supported_sig_algs, cert, trust_anchor_spki, budget)?;

check_signed_chain_name_constraints(cert, trust_anchor)?;

Ok(())
}) {
Ok(()) => {
Expand Down Expand Up @@ -179,10 +175,6 @@ fn build_chain_inner(
}
}

untrusted::read_all_optional(potential_issuer.name_constraints, Error::BadDer, |value| {
name::check_name_constraints(value, cert)
})?;

let next_sub_ca_count = match used_as_ca {
UsedAsCa::No => sub_ca_count,
UsedAsCa::Yes => sub_ca_count + 1,
Expand Down Expand Up @@ -230,6 +222,34 @@ fn check_signatures(
Ok(())
}

fn check_signed_chain_name_constraints(
cert_chain: &Cert,
trust_anchor: &TrustAnchor,
) -> Result<(), Error> {
let mut cert = cert_chain;
let mut name_constraints = trust_anchor
.name_constraints
.map(untrusted::Input::from);

loop {
untrusted::read_all_optional(name_constraints, Error::BadDer, |value| {
name::check_name_constraints(value, cert)
})?;

match &cert.ee_or_ca {
EndEntityOrCa::Ca(child_cert) => {
name_constraints = cert.name_constraints;
cert = child_cert;
}
EndEntityOrCa::EndEntity => {
break;
}
}
}

Ok(())
}

struct Budget {
signatures: usize,
build_chain_calls: usize,
Expand Down

0 comments on commit a7a0d41

Please sign in to comment.