Merge branch 'jbp-improve-name-constraints-testing' into main

Author: ctz

README.md

@@ -70,6 +70,11 @@ Changelog
   - Allow verification of certificates with IP address subjectAltNames.
     `EndEntityCert::verify_is_valid_for_subject_name` was added, and
     `EndEntityCert::verify_is_valid_for_dns_name` was removed.
+  - Make `Error` type non-exhaustive.
+  - Reject non-contiguous netmasks in IP address name constraints.
+  - Name constraints of type dNSName and iPAddress now work and are tested.
+    directoryName name constraints are not implemented and will prevent
+    path building where they appear.
 * 0.22.0 (2021-04-10) - last upstream release of `webpki` crate.
 
 

src/der.rs

@@ -14,16 +14,71 @@
 
 use crate::{calendar, time, Error};
 pub(crate) use ring::io::{
-    der::{nested, Tag},
+    der::{CONSTRUCTED, CONTEXT_SPECIFIC},
     Positive,
 };
 
+// Copied (and extended) from ring's src/der.rs
+#[allow(clippy::upper_case_acronyms)]
+#[derive(Clone, Copy, Eq, PartialEq)]
+#[repr(u8)]
+pub(crate) enum Tag {
+    Boolean = 0x01,
+    Integer = 0x02,
+    BitString = 0x03,
+    OctetString = 0x04,
+    OID = 0x06,
+    UTF8String = 0x0C,
+    Sequence = CONSTRUCTED | 0x10, // 0x30
+    Set = CONSTRUCTED | 0x11,      // 0x31
+    UTCTime = 0x17,
+    GeneralizedTime = 0x18,
+
+    #[allow(clippy::identity_op)]
+    ContextSpecificConstructed0 = CONTEXT_SPECIFIC | CONSTRUCTED | 0,
+    ContextSpecificConstructed1 = CONTEXT_SPECIFIC | CONSTRUCTED | 1,
+    ContextSpecificConstructed3 = CONTEXT_SPECIFIC | CONSTRUCTED | 3,
+}
+
+impl From<Tag> for usize {
+    #[allow(clippy::as_conversions)]
+    fn from(tag: Tag) -> Self {
+        tag as Self
+    }
+}
+
+impl From<Tag> for u8 {
+    #[allow(clippy::as_conversions)]
+    fn from(tag: Tag) -> Self {
+        tag as Self
+    } // XXX: narrowing conversion.
+}
+
 #[inline(always)]
 pub(crate) fn expect_tag_and_get_value<'a>(
     input: &mut untrusted::Reader<'a>,
     tag: Tag,
 ) -> Result<untrusted::Input<'a>, Error> {
-    ring::io::der::expect_tag_and_get_value(input, tag).map_err(|_| Error::BadDER)
+    let (actual_tag, inner) = read_tag_and_get_value(input)?;
+    if usize::from(tag) != usize::from(actual_tag) {
+        return Err(Error::BadDER);
+    }
+    Ok(inner)
+}
+
+// TODO: investigate taking decoder as a reference to reduce generated code
+// size.
+pub(crate) fn nested<'a, F, R, E: Copy>(
+    input: &mut untrusted::Reader<'a>,
+    tag: Tag,
+    error: E,
+    decoder: F,
+) -> Result<R, E>
+where
+    F: FnOnce(&mut untrusted::Reader<'a>) -> Result<R, E>,
+{
+    let inner = expect_tag_and_get_value(input, tag).map_err(|_| error)?;
+    inner.read_all(error, decoder)
 }
 
 pub struct Value<'a> {

src/error.rs

@@ -16,6 +16,7 @@ use core::fmt;
 
 /// An error that occurs during certificate validation or name validation.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[non_exhaustive]
 pub enum Error {
     /// The encoding of some ASN.1 DER-encoded item is invalid.
     // TODO: Rename to `BadDer` in the next release.
@@ -97,6 +98,11 @@ pub enum Error {
     /// The signature algorithm for a signature is not in the set of supported
     /// signature algorithms given.
     UnsupportedSignatureAlgorithm,
+
+    /// A iPAddress name constraint was invalid:
+    /// - it had a sparse network mask (ie, cannot be written in CIDR form).
+    /// - it was too long or short
+    InvalidNetworkMaskConstraint,
 }
 
 impl fmt::Display for Error {

src/subject_name/dns_name.rs

@@ -792,11 +792,68 @@ mod tests {
                 untrusted::Input::from(reference),
             );
             assert_eq!(
-                actual_result,
-                expected_result,
-                "presented_dns_id_matches_reference_dns_id(\"{:?}\", IDRole::ReferenceID, \"{:?}\")",
-                presented,
-                reference
+                actual_result, expected_result,
+                "presented_id_matches_reference_id(\"{:?}\", \"{:?}\")",
+                presented, reference
+            );
+        }
+    }
+
+    // (presented_name, constraint, expected_matches)
+    const PRESENTED_MATCHES_CONSTRAINT: &[(&[u8], &[u8], Option<bool>)] = &[
+        // No absolute presented IDs allowed
+        (b".", b"", None),
+        (b"www.example.com.", b"", None),
+        (b"www.example.com.", b"www.example.com.", None),
+        // No absolute constraints allowed
+        (b"www.example.com", b".", None),
+        (b"www.example.com", b"www.example.com.", None),
+        // No wildcard in constraints allowed
+        (b"www.example.com", b"*.example.com", None),
+        // No empty presented IDs allowed
+        (b"", b"", None),
+        // Empty constraints match everything allowed
+        (b"example.com", b"", Some(true)),
+        (b"*.example.com", b"", Some(true)),
+        // Constraints that start with a dot
+        (b"www.example.com", b".example.com", Some(true)),
+        (b"www.example.com", b".EXAMPLE.COM", Some(true)),
+        (b"www.example.com", b".axample.com", Some(false)),
+        (b"www.example.com", b".xample.com", Some(false)),
+        (b"www.example.com", b".exampl.com", Some(false)),
+        (b"badexample.com", b".example.com", Some(false)),
+        // Constraints that do not start with a dot
+        (b"www.example.com", b"example.com", Some(true)),
+        (b"www.example.com", b"EXAMPLE.COM", Some(true)),
+        (b"www.example.com", b"axample.com", Some(false)),
+        (b"www.example.com", b"xample.com", Some(false)),
+        (b"www.example.com", b"exampl.com", Some(false)),
+        (b"badexample.com", b"example.com", Some(false)),
+        // Presented IDs with wildcard
+        (b"*.example.com", b".example.com", Some(true)),
+        (b"*.example.com", b"example.com", Some(true)),
+        (b"*.example.com", b"www.example.com", Some(true)),
+        (b"*.example.com", b"www.EXAMPLE.COM", Some(true)),
+        (b"*.example.com", b"www.axample.com", Some(false)),
+        (b"*.example.com", b".xample.com", Some(false)),
+        (b"*.example.com", b"xample.com", Some(false)),
+        (b"*.example.com", b".exampl.com", Some(false)),
+        (b"*.example.com", b"exampl.com", Some(false)),
+        // Matching IDs
+        (b"www.example.com", b"www.example.com", Some(true)),
+    ];
+
+    #[test]
+    fn presented_matches_constraint_test() {
+        for &(presented, constraint, expected_result) in PRESENTED_MATCHES_CONSTRAINT {
+            let actual_result = presented_id_matches_constraint(
+                untrusted::Input::from(presented),
+                untrusted::Input::from(constraint),
+            );
+            assert_eq!(
+                actual_result, expected_result,
+                "presented_id_matches_constraint(\"{:?}\", \"{:?}\")",
+                presented, constraint
             );
         }
     }