diff --git a/src/error.rs b/src/error.rs index f3abcf70..5a8efe1c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -93,8 +93,8 @@ 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 name constraint comparisons has been reached. + MaximumNameConstraintComparisonsExceeded, /// The maximum number of internal path building calls has been reached. Path complexity is too great. MaximumPathBuildCallsExceeded, @@ -102,6 +102,9 @@ pub enum Error { /// 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, @@ -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 f303e9fd..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, @@ -97,11 +98,12 @@ 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, + budget: &mut Budget, ) -> Result<(), Error> { - let input = match input { + let constraints = match constraints { Some(input) => input, None => return Ok(()), }; @@ -116,8 +118,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 { @@ -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 b609e60a..7ed10156 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -184,18 +184,9 @@ 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) - })?; - // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; - check_signatures( + check_signed_chain( opts.supported_sig_algs, cert, trust_anchor, @@ -203,6 +194,13 @@ fn build_chain_inner( budget, )?; + check_signed_chain_name_constraints( + cert, + trust_anchor, + subject_common_name_contents, + budget, + )?; + Ok(()) }, ); @@ -236,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) - })?; - let next_sub_ca_count = match used_as_ca { UsedAsCa::No => sub_ca_count, UsedAsCa::Yes => sub_ca_count + 1, @@ -250,7 +244,7 @@ fn build_chain_inner( }) } -fn check_signatures( +fn check_signed_chain( supported_sig_algs: &[&dyn SignatureVerificationAlgorithm], cert_chain: &Cert, trust_anchor: &TrustAnchor, @@ -292,9 +286,41 @@ fn check_signatures( 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, + name_constraint_comparisons: usize, } impl Budget { @@ -315,6 +341,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 +364,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, } } } @@ -710,7 +749,8 @@ where match f(v) { Ok(()) => return Ok(()), err @ Err(Error::MaximumSignatureChecksExceeded) - | err @ Err(Error::MaximumPathBuildCallsExceeded) => return err, + | err @ Err(Error::MaximumPathBuildCallsExceeded) + | err @ Err(Error::MaximumNameConstraintComparisonsExceeded) => return err, Err(new_error) => error = error.most_specific(new_error), } } @@ -844,68 +884,23 @@ 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 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", 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(); + let intermediate = make_issuer("Bogus Subject", None); let intermediate_der = intermediate.serialize_der_with_signer(&issuer).unwrap(); intermediates.push(intermediate_der); 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 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] @@ -928,49 +923,149 @@ 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 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}"), 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(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; } - 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()); + verify_chain(ca_cert_der, intermediates, make_end_entity(&issuer)) + } + + #[test] + #[cfg(feature = "alloc")] + fn longest_allowed_path() { + assert_eq!(build_linear_chain(1), Ok(())); + assert_eq!(build_linear_chain(2), Ok(())); + assert_eq!(build_linear_chain(3), Ok(())); + assert_eq!(build_linear_chain(4), Ok(())); + assert_eq!(build_linear_chain(5), Ok(())); + assert_eq!(build_linear_chain(6), Ok(())); + } + + #[test] + #[cfg(feature = "alloc")] + fn path_too_long() { + 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_der).unwrap(); - let intermediates_der = intermediates + 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<'_>, + 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::>(); @@ -988,20 +1083,36 @@ mod tests { ) } - #[test] #[cfg(feature = "alloc")] - fn longest_allowed_path() { - assert_eq!(build_linear_chain(1), Ok(())); - assert_eq!(build_linear_chain(2), Ok(())); - assert_eq!(build_linear_chain(3), Ok(())); - assert_eq!(build_linear_chain(4), Ok(())); - assert_eq!(build_linear_chain(5), Ok(())); - assert_eq!(build_linear_chain(6), Ok(())); + 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 + .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; + ca_params.name_constraints = name_constraints; + rcgen::Certificate::from_params(ca_params).unwrap() } - #[test] #[cfg(feature = "alloc")] - fn path_too_long() { - assert_eq!(build_linear_chain(7), Err(Error::MaximumPathDepthExceeded)); + 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(), + ) } }