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 4d0cbba commit 519bcb6
Showing 1 changed file with 28 additions and 10 deletions.
38 changes: 28 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,32 @@ 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 519bcb6

Please sign in to comment.