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

@@ -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,
 

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<untrusted::Input>,
     excluded_subtrees: Option<untrusted::Input>,
+    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<untrusted::Input>,
+    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<untrusted::Input>,
     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);

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:
             // <https://github.com/nss-dev/nss/blob/bb4a1d38dd9e92923525ac6b5ed0288479f3f3fc/lib/mozpkix/lib/pkixbuild.cpp#L381-L393>
             build_chain_calls: 200_000,
+
+            // This limit is taken from golang crypto/x509's default, see:
+            // <https://github.com/golang/go/blob/ac17bb6f13979f2ab9fcd45f0758b43ed72d0973/src/crypto/x509/verify.go#L588-L592>
+            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,
             _ => {}
         }
     }