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 briansmith#3.

src/der.rs

@@ -14,16 +14,69 @@
 
 use crate::{calendar, time, Error};
 pub use ring::io::{
-    der::{nested, Tag, CONSTRUCTED},
+    der::{CONSTRUCTED, CONTEXT_SPECIFIC},
     Positive,
 };
 
+#[allow(clippy::upper_case_acronyms)]
+#[derive(Clone, Copy, Eq, PartialEq)]
+#[repr(u8)]
+pub enum Tag {
+    Boolean = 0x01,
+    Integer = 0x02,
+    BitString = 0x03,
+    OctetString = 0x04,
+    Null = 0x05,
+    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 {
+    fn from(tag: Tag) -> Self {
+        tag as Self
+    }
+}
+
+impl From<Tag> for u8 {
+    fn from(tag: Tag) -> Self {
+        tag as Self
+    } // XXX: narrowing conversion.
+}
+
 #[inline(always)]
 pub 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 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/name/verify.rs

@@ -63,10 +63,7 @@ pub 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)?;
@@ -160,6 +157,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),
             ),
@@ -319,3 +320,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

@@ -53,6 +53,25 @@ pub fn netflix() {
     );
 }
 
+#[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");