diff --git a/src/error.rs b/src/error.rs index bdd3569e..2b7183a4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -63,6 +63,9 @@ pub enum Error { /// and as recommended by RFC6125. MalformedExtensions, + /// 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, diff --git a/src/subject_name/verify.rs b/src/subject_name/verify.rs index 1dd81cad..ea3faecd 100644 --- a/src/subject_name/verify.rs +++ b/src/subject_name/verify.rs @@ -19,7 +19,9 @@ use super::{ }; use crate::{ cert::{Cert, EndEntityOrCa}, - der, Error, + der, + verify_cert::Budget, + Error, }; pub(crate) fn verify_cert_dns_name( @@ -33,7 +35,7 @@ pub(crate) fn verify_cert_dns_name( cert.subject_alt_name, SubjectCommonNameContents::Ignore, Err(Error::CertNotValidForName), - &|name| { + &mut |name| { if let GeneralName::DnsName(presented_id) = name { match dns_name::presented_id_matches_reference_id(presented_id, dns_name) { Some(true) => return NameIteration::Stop(Ok(())), @@ -67,7 +69,7 @@ pub(crate) fn verify_cert_subject_name( cert.inner().subject_alt_name, SubjectCommonNameContents::Ignore, Err(Error::CertNotValidForName), - &|name| { + &mut |name| { if let GeneralName::IpAddress(presented_id) = name { match ip_address::presented_id_matches_reference_id(presented_id, ip_address) { Ok(true) => return NameIteration::Stop(Ok(())), @@ -84,11 +86,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( - constraints: Option<&mut untrusted::Reader>, + input: Option<&mut untrusted::Reader>, subordinate_certs: &Cert, subject_common_name_contents: SubjectCommonNameContents, + budget: &mut Budget, ) -> Result<(), Error> { - let constraints = match constraints { + let input = match input { Some(input) => input, None => { return Ok(()); @@ -105,8 +108,8 @@ pub(crate) fn check_name_constraints( der::expect_tag_and_get_value(inner, subtrees_tag).map(Some) } - let permitted_subtrees = parse_subtrees(constraints, der::Tag::ContextSpecificConstructed0)?; - let excluded_subtrees = parse_subtrees(constraints, der::Tag::ContextSpecificConstructed1)?; + let permitted_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed0)?; + let excluded_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed1)?; let mut child = subordinate_certs; loop { @@ -115,11 +118,12 @@ pub(crate) fn check_name_constraints( child.subject_alt_name, subject_common_name_contents, Ok(()), - &|name| { + &mut |name| { check_presented_id_conforms_to_constraints( name, permitted_subtrees, excluded_subtrees, + budget, ) }, )?; @@ -139,11 +143,13 @@ fn check_presented_id_conforms_to_constraints( name: GeneralName, permitted_subtrees: Option, excluded_subtrees: Option, + budget: &mut Budget, ) -> NameIteration { match check_presented_id_conforms_to_constraints_in_subtree( name, Subtrees::PermittedSubtrees, permitted_subtrees, + budget, ) { stop @ NameIteration::Stop(..) => { return stop; @@ -155,6 +161,7 @@ fn check_presented_id_conforms_to_constraints( name, Subtrees::ExcludedSubtrees, excluded_subtrees, + budget, ) } @@ -168,6 +175,7 @@ fn check_presented_id_conforms_to_constraints_in_subtree( name: GeneralName, subtrees: Subtrees, constraints: Option, + budget: &mut Budget, ) -> NameIteration { let mut constraints = match constraints { Some(constraints) => untrusted::Reader::new(constraints), @@ -180,6 +188,10 @@ fn check_presented_id_conforms_to_constraints_in_subtree( let mut has_permitted_subtrees_mismatch = false; while !constraints.at_end() { + if let Err(e) = budget.consume_name_constraint_comparison() { + return NameIteration::Stop(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." @@ -307,7 +319,7 @@ fn iterate_names( subject_alt_name: Option, subject_common_name_contents: SubjectCommonNameContents, result_if_never_stopped_early: Result<(), Error>, - f: &dyn Fn(GeneralName) -> NameIteration, + f: &mut dyn FnMut(GeneralName) -> NameIteration, ) -> Result<(), Error> { if let Some(subject_alt_name) = subject_alt_name { let mut subject_alt_name = untrusted::Reader::new(subject_alt_name); diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 55b4adbe..e72563bd 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -95,9 +95,8 @@ fn build_chain_inner( } 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) + subject_name::check_name_constraints(value, cert, subject_common_name_contents, budget) })?; // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; @@ -144,7 +143,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 { @@ -196,6 +195,7 @@ fn check_signed_chain( pub(crate) struct Budget { signatures: usize, build_chain_calls: usize, + name_constraint_comparisons: usize, } impl Budget { @@ -216,6 +216,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 { @@ -230,6 +239,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, } } } @@ -423,7 +436,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, _ => {} } }