diff --git a/src/error.rs b/src/error.rs index 987d0a4d..74e2e3fc 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 signature checks has been reached. Path complexity is too great. MaximumSignatureChecksExceeded, @@ -238,8 +241,9 @@ impl Error { Error::BadDerTime => 2, Error::BadDer => 1, - // Special case error - not subject to ranking. - Error::MaximumSignatureChecksExceeded => 0, + // Special case errors - not subject to ranking. + Error::MaximumSignatureChecksExceeded + | 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..ee3d80de 100644 --- a/src/subject_name/verify.rs +++ b/src/subject_name/verify.rs @@ -97,15 +97,17 @@ 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(()), }; + let mut comparisons = 0_usize; + fn parse_subtrees<'b>( inner: &mut untrusted::Reader<'b>, subtrees_tag: der::Tag, @@ -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, + &mut comparisons, + ) }); 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, + comparisons: &mut usize, ) -> Option> { let subtrees = [ (Subtrees::PermittedSubtrees, permitted_subtrees), @@ -173,6 +181,11 @@ 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() { + *comparisons += 1; + if *comparisons > 250_000 { + return Some(Err(Error::MaximumNameConstraintComparisonsExceeded)); + } + // 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 8a7403bf..aa534603 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -664,7 +664,8 @@ where for v in values { match f(v) { Ok(()) => return Ok(()), - err @ Err(Error::MaximumSignatureChecksExceeded) => return err, + err @ Err(Error::MaximumSignatureChecksExceeded) + | err @ Err(Error::MaximumNameConstraintComparisonsExceeded) => return err, Err(new_error) => error = error.most_specific(new_error), } } diff --git a/tests/generate.py b/tests/generate.py index 0c0746aa..a5a15342 100755 --- a/tests/generate.py +++ b/tests/generate.py @@ -558,6 +558,15 @@ def tls_server_certs(force: bool) -> None: ], ) + generate_tls_server_cert_test( + output, + "name_constraints_abuse", + expected_error="MaximumNameConstraintComparisonsExceeded", + valid_names=["a"], + sans=[x509.DNSName("a")] * 13000, + permitted_subtrees=[x509.DNSName("a")] * 6000, + ) + def signatures(force: bool) -> None: rsa_pub_exponent: int = 0x10001 diff --git a/tests/tls_server_certs.rs b/tests/tls_server_certs.rs index 06c84604..6004b2e1 100644 --- a/tests/tls_server_certs.rs +++ b/tests/tls_server_certs.rs @@ -364,3 +364,13 @@ fn invalid_dns_name_matching() { let ca = include_bytes!("tls_server_certs/invalid_dns_name_matching.ca.der"); assert_eq!(check_cert(ee, ca, &["dns.example.com"], &[]), Ok(())); } + +#[test] +fn name_constraints_abuse() { + let ee = include_bytes!("tls_server_certs/name_constraints_abuse.ee.der"); + let ca = include_bytes!("tls_server_certs/name_constraints_abuse.ca.der"); + assert_eq!( + check_cert(ee, ca, &["a"], &[]), + Err(webpki::Error::MaximumNameConstraintComparisonsExceeded) + ); +} diff --git a/tests/tls_server_certs/name_constraints_abuse.ca.der b/tests/tls_server_certs/name_constraints_abuse.ca.der new file mode 100644 index 00000000..167324ad Binary files /dev/null and b/tests/tls_server_certs/name_constraints_abuse.ca.der differ diff --git a/tests/tls_server_certs/name_constraints_abuse.ee.der b/tests/tls_server_certs/name_constraints_abuse.ee.der new file mode 100644 index 00000000..d701e26d Binary files /dev/null and b/tests/tls_server_certs/name_constraints_abuse.ee.der differ