Fix name constraints check

There were two bugs. webpki didn't:

1. read the X.509 Name Constraints field in its entirety, nor

2. check the certificate subject against the constraints correctly

(1) is a simple fix, (2) requires reading the Common Name from the
certificate.

Requires lifting some DER parsing logic from ring to parse UTF8String
and Set fields. Ring doesn't support those and isn't likely to in the
near future, see briansmith/ring#1265.

Closes #3.

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/subject_name/verify.rs

@@ -99,10 +99,7 @@ pub(crate) fn check_name_constraints(
         if !inner.peek(subtrees_tag.into()) {
             return Ok(None);
         }
-        let subtrees = der::nested(inner, subtrees_tag, Error::BadDER, |tagged| {
-            der::expect_tag_and_get_value(tagged, der::Tag::Sequence)
-        })?;
-        Ok(Some(subtrees))
+        der::expect_tag_and_get_value(inner, subtrees_tag).map(Some)
     }
 
     let permitted_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed0)?;
@@ -205,6 +202,10 @@ fn check_presented_id_conforms_to_constraints_in_subtree(
                 dns_name::presented_id_matches_constraint(name, base).ok_or(Error::BadDER)
             }
 
+            (GeneralName::DirectoryName(name), GeneralName::DnsName(base)) => {
+                common_name(name).map(|cn| cn == base)
+            }
+
             (GeneralName::DirectoryName(name), GeneralName::DirectoryName(base)) => Ok(
                 presented_directory_name_matches_constraint(name, base, subtrees),
             ),
@@ -365,3 +366,18 @@ fn general_name<'a>(input: &mut untrusted::Reader<'a>) -> Result<GeneralName<'a>
     };
     Ok(name)
 }
+
+static COMMON_NAME: untrusted::Input = untrusted::Input::from(&[85, 4, 3]);
+
+fn common_name(input: untrusted::Input) -> Result<untrusted::Input, Error> {
+    let inner = &mut untrusted::Reader::new(input);
+    der::nested(inner, der::Tag::Set, Error::BadDER, |tagged| {
+        der::nested(tagged, der::Tag::Sequence, Error::BadDER, |tagged| {
+            let value = der::expect_tag_and_get_value(tagged, der::Tag::OID)?;
+            if value != COMMON_NAME {
+                return Err(Error::BadDER);
+            }
+            der::expect_tag_and_get_value(tagged, der::Tag::UTF8String)
+        })
+    })
+}

tests/integration.rs

@@ -104,6 +104,25 @@ pub fn cloudflare_dns() {
     check_addr("2606:4700:4700:0000:0000:0000:0000:6400");
 }
 
+#[cfg(feature = "alloc")]
+#[test]
+pub fn wpt() {
+    let ee: &[u8] = include_bytes!("wpt/ee.der");
+    let ca = include_bytes!("wpt/ca.der");
+
+    let anchors = vec![webpki::TrustAnchor::try_from_cert_der(ca).unwrap()];
+    let anchors = webpki::TLSServerTrustAnchors(&anchors);
+
+    #[allow(clippy::unreadable_literal)] // TODO: Make this clear.
+    let time = webpki::Time::from_seconds_since_unix_epoch(1619256684);
+
+    let cert = webpki::EndEntityCert::try_from(ee).unwrap();
+    assert_eq!(
+        Ok(()),
+        cert.verify_is_valid_tls_server_cert(ALL_SIGALGS, &anchors, &[], time)
+    );
+}
+
 #[test]
 pub fn ed25519() {
     let ee: &[u8] = include_bytes!("ed25519/ee.der");