From 019064f495d49ffa2090b22744c9a68fc85d0c31 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 31 Aug 2023 11:34:12 -0400 Subject: [PATCH 1/8] subject_name: clarify var name in check_name_constraints --- src/subject_name/verify.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/subject_name/verify.rs b/src/subject_name/verify.rs index f303e9fd..058da6f7 100644 --- a/src/subject_name/verify.rs +++ b/src/subject_name/verify.rs @@ -97,11 +97,11 @@ pub(crate) fn verify_cert_subject_name( // https://tools.ietf.org/html/rfc5280#section-4.2.1.10 pub(crate) fn check_name_constraints( - input: Option<&mut untrusted::Reader>, + constraints: Option<&mut untrusted::Reader>, subordinate_certs: &Cert, subject_common_name_contents: SubjectCommonNameContents, ) -> Result<(), Error> { - let input = match input { + let constraints = match constraints { Some(input) => input, None => return Ok(()), }; @@ -116,8 +116,8 @@ pub(crate) fn check_name_constraints( der::expect_tag(inner, subtrees_tag).map(Some) } - let permitted_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed0)?; - let excluded_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed1)?; + let permitted_subtrees = parse_subtrees(constraints, der::Tag::ContextSpecificConstructed0)?; + let excluded_subtrees = parse_subtrees(constraints, der::Tag::ContextSpecificConstructed1)?; let mut child = subordinate_certs; loop { From a170ca8f3f52692f5593c5f3510a164691df4aa9 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 1 Sep 2023 10:07:00 -0400 Subject: [PATCH 2/8] verify_cert: check_signatures -> check_signed_chain This commit renames the `check_signatures` fn, as it is doing more than simply verifying signatures. It also checks revocation status w/ CRLs when appropriate. --- src/verify_cert.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index b609e60a..1f3bdccb 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -195,7 +195,7 @@ fn build_chain_inner( // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; - check_signatures( + check_signed_chain( opts.supported_sig_algs, cert, trust_anchor, @@ -250,7 +250,7 @@ fn build_chain_inner( }) } -fn check_signatures( +fn check_signed_chain( supported_sig_algs: &[&dyn SignatureVerificationAlgorithm], cert_chain: &Cert, trust_anchor: &TrustAnchor, From 681dc197ed96fb9bb1b3b91d9ae8222fba3304f1 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 5 Sep 2023 11:11:05 -0400 Subject: [PATCH 3/8] error: alpha sort --- src/error.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/error.rs b/src/error.rs index f3abcf70..a7fda114 100644 --- a/src/error.rs +++ b/src/error.rs @@ -93,15 +93,15 @@ pub enum Error { /// invalid labels. MalformedNameConstraint, - /// The maximum number of signature checks has been reached. Path complexity is too great. - MaximumSignatureChecksExceeded, - /// The maximum number of internal path building calls has been reached. Path complexity is too great. MaximumPathBuildCallsExceeded, /// The path search was terminated because it became too deep. MaximumPathDepthExceeded, + /// The maximum number of signature checks has been reached. Path complexity is too great. + MaximumSignatureChecksExceeded, + /// The certificate violates one or more name constraints. NameConstraintViolation, From 85406c500f7b51b77b0e35924e2b06317e4fce04 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 6 Sep 2023 10:20:38 -0400 Subject: [PATCH 4/8] verify_cert: pull out `make_issuer` test helper --- src/verify_cert.rs | 55 +++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 1f3bdccb..d0b8ee1c 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -849,28 +849,13 @@ mod tests { let alg = &rcgen::PKCS_ECDSA_P256_SHA256; - let make_issuer = || { - let mut ca_params = rcgen::CertificateParams::new(Vec::new()); - ca_params - .distinguished_name - .push(rcgen::DnType::OrganizationName, "Bogus Subject"); - ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); - ca_params.key_usages = vec![ - rcgen::KeyUsagePurpose::KeyCertSign, - rcgen::KeyUsagePurpose::DigitalSignature, - rcgen::KeyUsagePurpose::CrlSign, - ]; - ca_params.alg = alg; - rcgen::Certificate::from_params(ca_params).unwrap() - }; - - let ca_cert = make_issuer(); + let ca_cert = make_issuer("Bogus Subject"); let ca_cert_der = CertificateDer::from(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(); + let intermediate = make_issuer("Bogus Subject"); let intermediate_der = intermediate.serialize_der_with_signer(&issuer).unwrap(); intermediates.push(intermediate_der); issuer = intermediate; @@ -933,29 +918,13 @@ mod tests { let alg = &rcgen::PKCS_ECDSA_P256_SHA256; - let make_issuer = |index: usize| { - let mut ca_params = rcgen::CertificateParams::new(Vec::new()); - ca_params.distinguished_name.push( - rcgen::DnType::OrganizationName, - format!("Bogus Subject {index}"), - ); - ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); - ca_params.key_usages = vec![ - rcgen::KeyUsagePurpose::KeyCertSign, - rcgen::KeyUsagePurpose::DigitalSignature, - rcgen::KeyUsagePurpose::CrlSign, - ]; - ca_params.alg = alg; - rcgen::Certificate::from_params(ca_params).unwrap() - }; - - let ca_cert = make_issuer(chain_length); + let ca_cert = make_issuer(format!("Bogus Subject {chain_length}")); let ca_cert_der = CertificateDer::from(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(i); + let intermediate = make_issuer(format!("Bogus Subject {i}")); let intermediate_der = intermediate.serialize_der_with_signer(&issuer).unwrap(); intermediates.push(intermediate_der); issuer = intermediate; @@ -1004,4 +973,20 @@ mod tests { fn path_too_long() { assert_eq!(build_linear_chain(7), Err(Error::MaximumPathDepthExceeded)); } + + #[cfg(feature = "alloc")] + fn make_issuer(org_name: impl Into) -> rcgen::Certificate { + let mut ca_params = rcgen::CertificateParams::new(Vec::new()); + ca_params + .distinguished_name + .push(rcgen::DnType::OrganizationName, org_name); + ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); + ca_params.key_usages = vec![ + rcgen::KeyUsagePurpose::KeyCertSign, + rcgen::KeyUsagePurpose::DigitalSignature, + rcgen::KeyUsagePurpose::CrlSign, + ]; + ca_params.alg = &rcgen::PKCS_ECDSA_P256_SHA256; + rcgen::Certificate::from_params(ca_params).unwrap() + } } From f5269ba2c9df50c47f0c484c93857f530b291ca1 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 6 Sep 2023 10:24:01 -0400 Subject: [PATCH 5/8] verify_cert: pull out `make_end_entity` test helper --- src/verify_cert.rs | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index d0b8ee1c..cfe9abf0 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -847,8 +847,6 @@ mod tests { use crate::{extract_trust_anchor, ECDSA_P256_SHA256}; use crate::{EndEntityCert, Time}; - let alg = &rcgen::PKCS_ECDSA_P256_SHA256; - let ca_cert = make_issuer("Bogus Subject"); let ca_cert_der = CertificateDer::from(ca_cert.serialize_der().unwrap()); @@ -861,14 +859,9 @@ mod tests { issuer = intermediate; } - let mut ee_params = rcgen::CertificateParams::new(vec!["example.com".to_string()]); - ee_params.is_ca = rcgen::IsCa::ExplicitNoCa; - ee_params.alg = alg; - let ee_cert = rcgen::Certificate::from_params(ee_params).unwrap(); - let ee_cert_der = CertificateDer::from(ee_cert.serialize_der_with_signer(&issuer).unwrap()); - let anchors = &[extract_trust_anchor(&ca_cert_der).unwrap()]; let time = Time::from_seconds_since_unix_epoch(0x1fed_f00d); + let ee_cert_der = make_end_entity(&issuer); let cert = EndEntityCert::try_from(&ee_cert_der).unwrap(); let mut intermediates_der = intermediates .iter() @@ -916,8 +909,6 @@ mod tests { use crate::{extract_trust_anchor, ECDSA_P256_SHA256}; use crate::{EndEntityCert, Time}; - let alg = &rcgen::PKCS_ECDSA_P256_SHA256; - let ca_cert = make_issuer(format!("Bogus Subject {chain_length}")); let ca_cert_der = CertificateDer::from(ca_cert.serialize_der().unwrap()); @@ -930,14 +921,9 @@ mod tests { issuer = intermediate; } - let mut ee_params = rcgen::CertificateParams::new(vec!["example.com".to_string()]); - ee_params.is_ca = rcgen::IsCa::ExplicitNoCa; - ee_params.alg = alg; - let ee_cert = rcgen::Certificate::from_params(ee_params).unwrap(); - let ee_cert_der = CertificateDer::from(ee_cert.serialize_der_with_signer(&issuer).unwrap()); - let anchors = &[extract_trust_anchor(&ca_cert_der).unwrap()]; let time = Time::from_seconds_since_unix_epoch(0x1fed_f00d); + let ee_cert_der = make_end_entity(&issuer); let cert = EndEntityCert::try_from(&ee_cert_der).unwrap(); let intermediates_der = intermediates .iter() @@ -989,4 +975,17 @@ mod tests { ca_params.alg = &rcgen::PKCS_ECDSA_P256_SHA256; rcgen::Certificate::from_params(ca_params).unwrap() } + + #[cfg(feature = "alloc")] + fn make_end_entity(issuer: &rcgen::Certificate) -> CertificateDer<'static> { + let mut ee_params = rcgen::CertificateParams::new(vec!["example.com".to_string()]); + ee_params.is_ca = rcgen::IsCa::ExplicitNoCa; + ee_params.alg = &rcgen::PKCS_ECDSA_P256_SHA256; + CertificateDer::from( + rcgen::Certificate::from_params(ee_params) + .unwrap() + .serialize_der_with_signer(issuer) + .unwrap(), + ) + } } From 9c6d55ef3cb91623bd181a9c3e56389f4205668f Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 6 Sep 2023 10:27:06 -0400 Subject: [PATCH 6/8] verify_cert: pull out `verify_chain` test helper --- src/verify_cert.rs | 81 +++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 48 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index cfe9abf0..30f73085 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -844,9 +844,6 @@ mod tests { intermediate_count: usize, trust_anchor_is_actual_issuer: TrustAnchorIsActualIssuer, ) -> Error { - use crate::{extract_trust_anchor, ECDSA_P256_SHA256}; - use crate::{EndEntityCert, Time}; - let ca_cert = make_issuer("Bogus Subject"); let ca_cert_der = CertificateDer::from(ca_cert.serialize_der().unwrap()); @@ -859,31 +856,11 @@ mod tests { issuer = intermediate; } - let anchors = &[extract_trust_anchor(&ca_cert_der).unwrap()]; - let time = Time::from_seconds_since_unix_epoch(0x1fed_f00d); - let ee_cert_der = make_end_entity(&issuer); - let cert = EndEntityCert::try_from(&ee_cert_der).unwrap(); - let mut intermediates_der = intermediates - .iter() - .map(|x| CertificateDer::from(x.as_ref())) - .collect::>(); - if let TrustAnchorIsActualIssuer::No = trust_anchor_is_actual_issuer { - intermediates_der.pop(); + intermediates.pop(); } - build_chain( - &ChainOptions { - eku: KeyUsage::server_auth(), - supported_sig_algs: &[ECDSA_P256_SHA256], - trust_anchors: anchors, - intermediate_certs: &intermediates_der, - revocation: None, - }, - cert.inner(), - time, - ) - .unwrap_err() + verify_chain(ca_cert_der, intermediates, make_end_entity(&issuer)).unwrap_err() } #[test] @@ -906,9 +883,6 @@ mod tests { #[cfg(feature = "alloc")] fn build_linear_chain(chain_length: usize) -> Result<(), Error> { - use crate::{extract_trust_anchor, ECDSA_P256_SHA256}; - use crate::{EndEntityCert, Time}; - let ca_cert = make_issuer(format!("Bogus Subject {chain_length}")); let ca_cert_der = CertificateDer::from(ca_cert.serialize_der().unwrap()); @@ -921,26 +895,7 @@ mod tests { issuer = intermediate; } - let anchors = &[extract_trust_anchor(&ca_cert_der).unwrap()]; - let time = Time::from_seconds_since_unix_epoch(0x1fed_f00d); - let ee_cert_der = make_end_entity(&issuer); - let cert = EndEntityCert::try_from(&ee_cert_der).unwrap(); - let intermediates_der = intermediates - .iter() - .map(|x| CertificateDer::from(x.as_ref())) - .collect::>(); - - build_chain( - &ChainOptions { - eku: KeyUsage::server_auth(), - supported_sig_algs: &[ECDSA_P256_SHA256], - trust_anchors: anchors, - intermediate_certs: &intermediates_der, - revocation: None, - }, - cert.inner(), - time, - ) + verify_chain(ca_cert_der, intermediates, make_end_entity(&issuer)) } #[test] @@ -960,6 +915,36 @@ mod tests { assert_eq!(build_linear_chain(7), Err(Error::MaximumPathDepthExceeded)); } + #[cfg(feature = "alloc")] + fn verify_chain( + trust_anchor: CertificateDer<'_>, + intermediates_der: Vec>, + ee_cert: CertificateDer<'_>, + ) -> Result<(), Error> { + use crate::{extract_trust_anchor, ECDSA_P256_SHA256}; + use crate::{EndEntityCert, Time}; + + let anchors = &[extract_trust_anchor(&trust_anchor).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| CertificateDer::from(x.as_ref())) + .collect::>(); + + build_chain( + &ChainOptions { + eku: KeyUsage::server_auth(), + supported_sig_algs: &[ECDSA_P256_SHA256], + trust_anchors: anchors, + intermediate_certs: &intermediates_der, + revocation: None, + }, + cert.inner(), + time, + ) + } + #[cfg(feature = "alloc")] fn make_issuer(org_name: impl Into) -> rcgen::Certificate { let mut ca_params = rcgen::CertificateParams::new(Vec::new()); From b6d47787c160b2d07ecece5927a93e49651278d8 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 5 Sep 2023 11:28:26 -0400 Subject: [PATCH 7/8] verify_cert: budget for name constraint comparisons This commit updates the name constraint validation done during path building to apply a budget for the maximum allowed number of name constraint checks. We use the same limit that golang crypto/x509 applies by default: 250,000 comparisons. Note: this commit applies the budget during path building in a manner that means certificates _not_ part of the built path can consume comparisons from the budget even though they will not be present in the complete validated path. Similarly name constraints are evaluated before signatures, meaning a certificate that doesn't verify to a trusted root still has its constraints parsed and evaluated. A subsequent commit will adjust these shortcomings. --- src/error.rs | 4 ++++ src/subject_name/verify.rs | 14 +++++++++++++- src/verify_cert.rs | 24 ++++++++++++++++++++++-- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/error.rs b/src/error.rs index a7fda114..5a8efe1c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -93,6 +93,9 @@ pub enum Error { /// invalid labels. MalformedNameConstraint, + /// The maximum number of name constraint comparisons has been reached. + MaximumNameConstraintComparisonsExceeded, + /// The maximum number of internal path building calls has been reached. Path complexity is too great. MaximumPathBuildCallsExceeded, @@ -248,6 +251,7 @@ impl Error { // Special case errors - not subject to ranking. Error::MaximumSignatureChecksExceeded => 0, Error::MaximumPathBuildCallsExceeded => 0, + Error::MaximumNameConstraintComparisonsExceeded => 0, // Default catch all error - should be renamed in the future. Error::UnknownIssuer => 0, diff --git a/src/subject_name/verify.rs b/src/subject_name/verify.rs index 058da6f7..011f9309 100644 --- a/src/subject_name/verify.rs +++ b/src/subject_name/verify.rs @@ -23,6 +23,7 @@ use super::name::SubjectNameRef; use crate::cert::{Cert, EndEntityOrCa}; use crate::der::{self, FromDer}; use crate::error::{DerTypeId, Error}; +use crate::verify_cert::Budget; pub(crate) fn verify_cert_dns_name( cert: &crate::EndEntityCert, @@ -100,6 +101,7 @@ pub(crate) fn check_name_constraints( constraints: Option<&mut untrusted::Reader>, subordinate_certs: &Cert, subject_common_name_contents: SubjectCommonNameContents, + budget: &mut Budget, ) -> Result<(), Error> { let constraints = match constraints { Some(input) => input, @@ -132,7 +134,12 @@ pub(crate) fn check_name_constraints( Err(err) => return Some(Err(err)), }; - check_presented_id_conforms_to_constraints(name, permitted_subtrees, excluded_subtrees) + check_presented_id_conforms_to_constraints( + name, + permitted_subtrees, + excluded_subtrees, + budget, + ) }); if let Some(Err(err)) = result { @@ -154,6 +161,7 @@ fn check_presented_id_conforms_to_constraints( name: GeneralName, permitted_subtrees: Option, excluded_subtrees: Option, + budget: &mut Budget, ) -> Option> { let subtrees = [ (Subtrees::PermittedSubtrees, permitted_subtrees), @@ -173,6 +181,10 @@ fn check_presented_id_conforms_to_constraints( let mut has_permitted_subtrees_match = false; let mut has_permitted_subtrees_mismatch = false; while !constraints.at_end() { + if let Err(e) = budget.consume_name_constraint_comparison() { + return Some(Err(e)); + } + // http://tools.ietf.org/html/rfc5280#section-4.2.1.10: "Within this // profile, the minimum and maximum fields are not used with any name // forms, thus, the minimum MUST be zero, and maximum MUST be absent." diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 30f73085..452b69c4 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -190,7 +190,12 @@ fn build_chain_inner( .map(|der| untrusted::Input::from(der.as_ref())); untrusted::read_all_optional(name_constraints, Error::BadDer, |value| { - subject_name::check_name_constraints(value, cert, subject_common_name_contents) + subject_name::check_name_constraints( + value, + cert, + subject_common_name_contents, + budget, + ) })?; // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; @@ -237,7 +242,7 @@ 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) + subject_name::check_name_constraints(value, cert, subject_common_name_contents, budget) })?; let next_sub_ca_count = match used_as_ca { @@ -295,6 +300,7 @@ fn check_signed_chain( pub struct Budget { signatures: usize, build_chain_calls: usize, + name_constraint_comparisons: usize, } impl Budget { @@ -315,6 +321,15 @@ impl Budget { .ok_or(Error::MaximumPathBuildCallsExceeded)?; Ok(()) } + + #[inline] + pub(crate) fn consume_name_constraint_comparison(&mut self) -> Result<(), Error> { + self.name_constraint_comparisons = self + .name_constraint_comparisons + .checked_sub(1) + .ok_or(Error::MaximumNameConstraintComparisonsExceeded)?; + Ok(()) + } } impl Default for Budget { @@ -329,6 +344,10 @@ impl Default for Budget { // This limit is taken from NSS libmozpkix, see: // build_chain_calls: 200_000, + + // This limit is taken from golang crypto/x509's default, see: + // + name_constraint_comparisons: 250_000, } } } @@ -711,6 +730,7 @@ where Ok(()) => return Ok(()), err @ Err(Error::MaximumSignatureChecksExceeded) | err @ Err(Error::MaximumPathBuildCallsExceeded) => return err, + err @ Err(Error::MaximumNameConstraintComparisonsExceeded) => return err, Err(new_error) => error = error.most_specific(new_error), } } From a90f075b5b4a9191709666c7cd5c2dbab4d4e5a0 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 5 Sep 2023 13:45:11 -0400 Subject: [PATCH 8/8] verify_cert: name constraint checking on verified chain This commit updates the path building process such that name constraints are only evaluated against a complete path where signatures on the chain have been checked successfully to a trust anchor. This avoids: * Parsing name constraints before signatures are validated. * Evaluating name constraints and consuming name constraint comparison budget for certificates that are not part of the built path. In the future it could be possible to interleave the name constraint checking with the signature checking, however the logic for this is more complicated. For an initial fix let's prefer a simpler solution that walks the built + validated path to check name constraints from the trust anchor to the end entity certificate. --- src/verify_cert.rs | 172 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 147 insertions(+), 25 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 452b69c4..7ed10156 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -184,20 +184,6 @@ fn build_chain_inner( return Err(Error::UnknownIssuer); } - let name_constraints = trust_anchor - .name_constraints - .as_ref() - .map(|der| untrusted::Input::from(der.as_ref())); - - 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)?; check_signed_chain( @@ -208,6 +194,13 @@ fn build_chain_inner( budget, )?; + check_signed_chain_name_constraints( + cert, + trust_anchor, + subject_common_name_contents, + budget, + )?; + Ok(()) }, ); @@ -241,10 +234,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, @@ -297,6 +286,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.as_ref())); + + 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 struct Budget { signatures: usize, build_chain_calls: usize, @@ -729,8 +749,8 @@ where match f(v) { Ok(()) => return Ok(()), err @ Err(Error::MaximumSignatureChecksExceeded) - | err @ Err(Error::MaximumPathBuildCallsExceeded) => return err, - err @ Err(Error::MaximumNameConstraintComparisonsExceeded) => return err, + | err @ Err(Error::MaximumPathBuildCallsExceeded) + | err @ Err(Error::MaximumNameConstraintComparisonsExceeded) => return err, Err(new_error) => error = error.most_specific(new_error), } } @@ -864,13 +884,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 = CertificateDer::from(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; @@ -903,13 +923,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 = CertificateDer::from(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; @@ -935,6 +955,104 @@ mod tests { assert_eq!(build_linear_chain(7), Err(Error::MaximumPathDepthExceeded)); } + #[test] + #[cfg(feature = "alloc")] + fn name_constraint_budget() { + use crate::{extract_trust_anchor, 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 = CertificateDer::from(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 = &[extract_trust_anchor(&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| CertificateDer::from(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( + &ChainOptions { + eku: KeyUsage::server_auth(), + supported_sig_algs: &[ECDSA_P256_SHA256], + trust_anchors: anchors, + intermediate_certs: &intermediates_der, + revocation: None, + }, + 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( + &ChainOptions { + eku: KeyUsage::server_auth(), + supported_sig_algs: &[ECDSA_P256_SHA256], + trust_anchors: anchors, + intermediate_certs: &intermediates_der, + revocation: None, + }, + cert.inner(), + time, + 0, + &mut failing_budget, + ); + + assert_eq!(result, Err(Error::MaximumNameConstraintComparisonsExceeded)); + } + #[cfg(feature = "alloc")] fn verify_chain( trust_anchor: CertificateDer<'_>, @@ -966,7 +1084,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 @@ -978,6 +1099,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() }