diff --git a/src/verify_cert.rs b/src/verify_cert.rs index e72563bd..433c6e36 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -94,17 +94,18 @@ fn build_chain_inner( return Err(Error::UnknownIssuer); } - let name_constraints = trust_anchor.name_constraints.map(untrusted::Input::from); - untrusted::read_all_optional(name_constraints, Error::BadDer, |value| { - subject_name::check_name_constraints(value, cert, subject_common_name_contents, budget) - })?; - // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; let trust_anchor_spki = untrusted::Input::from(trust_anchor.spki); - check_signed_chain(supported_sig_algs, cert, trust_anchor_spki, budget)?; + check_signed_chain_name_constraints( + cert, + trust_anchor, + subject_common_name_contents, + budget, + )?; + Ok(()) }); @@ -112,7 +113,8 @@ fn build_chain_inner( match result { Ok(()) => return Ok(()), err @ Err(Error::MaximumSignatureChecksExceeded) - | err @ Err(Error::MaximumPathBuildCallsExceeded) => return err, + | err @ Err(Error::MaximumPathBuildCallsExceeded) + | err @ Err(Error::MaximumNameConstraintComparisonsExceeded) => return err, _ => {} }; @@ -142,10 +144,6 @@ fn build_chain_inner( } } - untrusted::read_all_optional(potential_issuer.name_constraints, Error::BadDer, |value| { - subject_name::check_name_constraints(value, cert, subject_common_name_contents, budget) - })?; - let next_sub_ca_count = match used_as_ca { UsedAsCa::No => sub_ca_count, UsedAsCa::Yes => sub_ca_count + 1, @@ -192,6 +190,37 @@ fn check_signed_chain( Ok(()) } +fn check_signed_chain_name_constraints( + cert_chain: &Cert, + trust_anchor: &TrustAnchor, + subject_common_name_contents: subject_name::SubjectCommonNameContents, + budget: &mut Budget, +) -> Result<(), Error> { + let mut cert = cert_chain; + let mut name_constraints = trust_anchor + .name_constraints + .as_ref() + .map(|der| untrusted::Input::from(der)); + + loop { + untrusted::read_all_optional(name_constraints, Error::BadDer, |value| { + subject_name::check_name_constraints(value, cert, subject_common_name_contents, budget) + })?; + + match &cert.ee_or_ca { + EndEntityOrCa::Ca(child_cert) => { + name_constraints = cert.name_constraints; + cert = child_cert; + } + EndEntityOrCa::EndEntity => { + break; + } + } + } + + Ok(()) +} + pub(crate) struct Budget { signatures: usize, build_chain_calls: usize, @@ -460,13 +489,13 @@ mod tests { intermediate_count: usize, trust_anchor_is_actual_issuer: TrustAnchorIsActualIssuer, ) -> Error { - let ca_cert = make_issuer("Bogus Subject"); + let ca_cert = make_issuer("Bogus Subject", None); let ca_cert_der = ca_cert.serialize_der().unwrap(); let mut intermediates = Vec::with_capacity(intermediate_count); let mut issuer = ca_cert; for _ in 0..intermediate_count { - let intermediate = make_issuer("Bogus Subject"); + let intermediate = make_issuer("Bogus Subject", None); let intermediate_der = intermediate.serialize_der_with_signer(&issuer).unwrap(); intermediates.push(intermediate_der); issuer = intermediate; @@ -499,13 +528,13 @@ mod tests { #[cfg(feature = "alloc")] fn build_linear_chain(chain_length: usize) -> Result<(), Error> { - let ca_cert = make_issuer(format!("Bogus Subject {chain_length}")); + let ca_cert = make_issuer(format!("Bogus Subject {chain_length}"), None); let ca_cert_der = ca_cert.serialize_der().unwrap(); let mut intermediates = Vec::with_capacity(chain_length); let mut issuer = ca_cert; for i in 0..chain_length { - let intermediate = make_issuer(format!("Bogus Subject {i}")); + let intermediate = make_issuer(format!("Bogus Subject {i}"), None); let intermediate_der = intermediate.serialize_der_with_signer(&issuer).unwrap(); intermediates.push(intermediate_der); issuer = intermediate; @@ -533,6 +562,98 @@ mod tests { assert_eq!(build_linear_chain(7), Err(Error::UnknownIssuer)); } + #[test] + #[cfg(feature = "alloc")] + fn name_constraint_budget() { + use crate::ECDSA_P256_SHA256; + use crate::{EndEntityCert, Time}; + + // Issue a trust anchor that imposes name constraints. The constraint should match + // the end entity certificate SAN. + let ca_cert = make_issuer( + "Constrained Root", + Some(rcgen::NameConstraints { + permitted_subtrees: vec![rcgen::GeneralSubtree::DnsName(".com".into())], + excluded_subtrees: vec![], + }), + ); + let ca_cert_der = ca_cert.serialize_der().unwrap(); + + // Create a series of intermediate issuers. We'll only use one in the actual built path, + // helping demonstrate that the name constraint budget is not expended checking certificates + // that are not part of the path we compute. + const NUM_INTERMEDIATES: usize = 5; + let mut intermediates = Vec::with_capacity(NUM_INTERMEDIATES); + for i in 0..NUM_INTERMEDIATES { + intermediates.push(make_issuer(format!("Intermediate {i}"), None)); + } + + // Each intermediate should be issued by the trust anchor. + let mut intermediates_der = Vec::with_capacity(NUM_INTERMEDIATES); + for intermediate in &intermediates { + intermediates_der.push(intermediate.serialize_der_with_signer(&ca_cert).unwrap()); + } + + // Create an end-entity cert that is issued by the last of the intermediates. + let ee_cert = make_end_entity(intermediates.last().unwrap()); + + let anchors = &[TrustAnchor::try_from_cert_der(&ca_cert_der).unwrap()]; + let time = Time::from_seconds_since_unix_epoch(0x1fed_f00d); + let cert = EndEntityCert::try_from(&ee_cert[..]).unwrap(); + let intermediates_der = intermediates_der + .iter() + .map(|x| x.as_ref()) + .collect::>(); + + // We use a custom budget to make it easier to write a test, otherwise it is tricky to + // stuff enough names/constraints into the potential chains while staying within the path + // depth limit and the build chain call limit. + let mut passing_budget = Budget { + // One comparison against the intermediate's distinguished name. + // One comparison against the EE's distinguished name. + // One comparison against the EE's SAN. + // = 3 total comparisons. + name_constraint_comparisons: 3, + ..Budget::default() + }; + + // Validation should succeed with the name constraint comparison budget allocated above. + // This shows that we're not consuming budget on unused intermediates: we didn't budget + // enough comparisons for that to pass the overall chain building. + build_chain_inner( + EKU_SERVER_AUTH, + &[&ECDSA_P256_SHA256], + anchors, + &intermediates_der, + cert.inner(), + time, + 0, + &mut passing_budget, + ) + .unwrap(); + + let mut failing_budget = Budget { + // See passing_budget: 2 comparisons is not sufficient. + name_constraint_comparisons: 2, + ..Budget::default() + }; + // Validation should fail when the budget is smaller than the number of comparisons performed + // on the validated path. This demonstrates we properly fail path building when too many + // name constraint comparisons occur. + let result = build_chain_inner( + EKU_SERVER_AUTH, + &[&ECDSA_P256_SHA256], + anchors, + &intermediates_der, + cert.inner(), + time, + 0, + &mut failing_budget, + ); + + assert_eq!(result, Err(Error::MaximumNameConstraintComparisonsExceeded)); + } + #[cfg(feature = "alloc")] fn verify_chain( trust_anchor_der: Vec, @@ -561,7 +682,10 @@ mod tests { } #[cfg(feature = "alloc")] - fn make_issuer(org_name: impl Into) -> rcgen::Certificate { + fn make_issuer( + org_name: impl Into, + name_constraints: Option, + ) -> rcgen::Certificate { let mut ca_params = rcgen::CertificateParams::new(Vec::new()); ca_params .distinguished_name @@ -573,6 +697,7 @@ mod tests { rcgen::KeyUsagePurpose::CrlSign, ]; ca_params.alg = &rcgen::PKCS_ECDSA_P256_SHA256; + ca_params.name_constraints = name_constraints; rcgen::Certificate::from_params(ca_params).unwrap() }