Only check DirectoryName as part of name constraints

rustls · Jan 20, 2025 · 0b058ba · 0b058ba
1 parent 340a07a
commit 0b058ba
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 32 deletions.
diff --git a/src/cert.rs b/src/cert.rs
@@ -139,7 +139,7 @@ impl<'a> Cert<'a> {
     ///
     /// [EndEntityCert::verify_is_valid_for_subject_name]: crate::EndEntityCert::verify_is_valid_for_subject_name
     pub fn valid_dns_names(&self) -> impl Iterator<Item = &str> {
-        NameIterator::new(self.subject_alt_name, true).filter_map(|result| {
+        NameIterator::new(self.subject_alt_name).filter_map(|result| {
             let presented_id = match result.ok()? {
                 GeneralName::DnsName(presented) => presented,
                 _ => return None,

diff --git a/src/subject_name/dns_name.rs b/src/subject_name/dns_name.rs
@@ -26,7 +26,7 @@ use crate::error::{Error, InvalidNameContext};
 
 pub(crate) fn verify_dns_names(reference: &DnsName<'_>, cert: &Cert<'_>) -> Result<(), Error> {
     let dns_name = untrusted::Input::from(reference.as_ref().as_bytes());
-    let result = NameIterator::new(cert.subject_alt_name, true).find_map(|result| {
+    let result = NameIterator::new(cert.subject_alt_name).find_map(|result| {
         let name = match result {
             Ok(name) => name,
             Err(err) => return Some(Err(err)),
@@ -58,7 +58,7 @@ pub(crate) fn verify_dns_names(reference: &DnsName<'_>, cert: &Cert<'_>) -> Resu
     {
         Err(Error::CertNotValidForName(InvalidNameContext {
             expected: ServerName::DnsName(reference.to_owned()),
-            presented: NameIterator::new(cert.subject_alt_name, true)
+            presented: NameIterator::new(cert.subject_alt_name)
                 .filter_map(|result| Some(format!("{:?}", result.ok()?)))
                 .collect(),
         }))

diff --git a/src/subject_name/ip_address.rs b/src/subject_name/ip_address.rs
@@ -29,7 +29,7 @@ pub(crate) fn verify_ip_address_names(reference: &IpAddr, cert: &Cert<'_>) -> Re
         IpAddr::V6(ip) => untrusted::Input::from(ip.as_ref()),
     };
 
-    let result = NameIterator::new(cert.subject_alt_name, false).find_map(|result| {
+    let result = NameIterator::new(cert.subject_alt_name).find_map(|result| {
         let name = match result {
             Ok(name) => name,
             Err(err) => return Some(Err(err)),
@@ -58,7 +58,7 @@ pub(crate) fn verify_ip_address_names(reference: &IpAddr, cert: &Cert<'_>) -> Re
     {
         Err(Error::CertNotValidForName(InvalidNameContext {
             expected: ServerName::from(*reference),
-            presented: NameIterator::new(cert.subject_alt_name, false)
+            presented: NameIterator::new(cert.subject_alt_name)
                 .filter_map(|result| Some(format!("{:?}", result.ok()?)))
                 .collect(),
         }))

diff --git a/src/subject_name/mod.rs b/src/subject_name/mod.rs
@@ -16,7 +16,6 @@
 use alloc::string::String;
 #[cfg(feature = "alloc")]
 use core::fmt;
-use core::mem;
 
 use crate::der::{self, FromDer};
 use crate::error::{DerTypeId, Error};
@@ -54,7 +53,7 @@ pub(crate) fn check_name_constraints(
     let excluded_subtrees = parse_subtrees(constraints, der::Tag::ContextSpecificConstructed1)?;
 
     for path in path.iter() {
-        let result = NameIterator::new(path.cert.subject_alt_name, true).find_map(|result| {
+        let result = NameIterator::new(path.cert.subject_alt_name).find_map(|result| {
             let name = match result {
                 Ok(name) => name,
                 Err(err) => return Some(Err(err)),
@@ -71,6 +70,17 @@ pub(crate) fn check_name_constraints(
         if let Some(Err(err)) = result {
             return Err(err);
         }
+
+        let result = check_presented_id_conforms_to_constraints(
+            GeneralName::DirectoryName,
+            permitted_subtrees,
+            excluded_subtrees,
+            budget,
+        );
+
+        if let Some(Err(err)) = result {
+            return Err(err);
+        }
     }
 
     Ok(())
@@ -203,17 +213,12 @@ enum Subtrees {
 
 pub(crate) struct NameIterator<'a> {
     subject_alt_name: Option<untrusted::Reader<'a>>,
-    directory_name: bool,
 }
 
 impl<'a> NameIterator<'a> {
-    pub(crate) fn new(
-        subject_alt_name: Option<untrusted::Input<'a>>,
-        directory_name: bool,
-    ) -> Self {
+    pub(crate) fn new(subject_alt_name: Option<untrusted::Input<'a>>) -> Self {
         Self {
             subject_alt_name: subject_alt_name.map(untrusted::Reader::new),
-            directory_name,
         }
     }
 }
@@ -238,17 +243,12 @@ impl<'a> Iterator for NameIterator<'a> {
 
                 // Make sure we don't yield any items after this error.
                 self.subject_alt_name = None;
-                self.directory_name = false;
                 return Some(Err(err));
             } else {
                 self.subject_alt_name = None;
             }
         }
 
-        if mem::take(&mut self.directory_name) {
-            return Some(Ok(GeneralName::DirectoryName));
-        }
-
         None
     }
 }

diff --git a/tests/generate.py b/tests/generate.py
@@ -296,10 +296,6 @@ def generate_tls_server_cert_test(
             presented_names_str += f'"IpAddress({san.value})"'
 
     ip_addr_sans = all(isinstance(san, x509.IPAddress) for san in (sans or []))
-    if expected_error is None and not (ip_addr_sans and subject_common_name is None):
-        if presented_names_str:
-            presented_names_str += ", "
-        presented_names_str += '"DirectoryName"'
 
     print(
         """

diff --git a/tests/tls_server_certs.rs b/tests/tls_server_certs.rs
@@ -73,7 +73,7 @@ fn no_name_constraints() {
             ca,
             &["dns.example.com"],
             &["subject.example.com"],
-            &["DnsName(\"dns.example.com\")", "DirectoryName"]
+            &["DnsName(\"dns.example.com\")"]
         ),
         Ok(())
     );
@@ -91,8 +91,7 @@ fn additional_dns_labels() {
             &["subject.example.com"],
             &[
                 "DnsName(\"host1.example.com\")",
-                "DnsName(\"host2.example.com\")",
-                "DirectoryName"
+                "DnsName(\"host2.example.com\")"
             ]
         ),
         Ok(())
@@ -114,7 +113,7 @@ fn allow_subject_common_name() {
     let ee = include_bytes!("tls_server_certs/allow_subject_common_name.ee.der");
     let ca = include_bytes!("tls_server_certs/allow_subject_common_name.ca.der");
     assert_eq!(
-        check_cert(ee, ca, &[], &["allowed.example.com"], &["DirectoryName"]),
+        check_cert(ee, ca, &[], &["allowed.example.com"], &[]),
         Ok(())
     );
 }
@@ -129,7 +128,7 @@ fn allow_dns_san() {
             ca,
             &["allowed.example.com"],
             &[],
-            &["DnsName(\"allowed.example.com\")", "DirectoryName"]
+            &["DnsName(\"allowed.example.com\")"]
         ),
         Ok(())
     );
@@ -145,7 +144,7 @@ fn allow_dns_san_and_subject_common_name() {
             ca,
             &["allowed-san.example.com"],
             &["allowed-cn.example.com"],
-            &["DnsName(\"allowed-san.example.com\")", "DirectoryName"]
+            &["DnsName(\"allowed-san.example.com\")"]
         ),
         Ok(())
     );
@@ -207,7 +206,7 @@ fn we_ignore_constraints_on_names_that_do_not_appear_in_cert() {
             ca,
             &["notexample.com"],
             &["example.com"],
-            &["DnsName(\"notexample.com\")", "DirectoryName"]
+            &["DnsName(\"notexample.com\")"]
         ),
         Ok(())
     );
@@ -223,7 +222,7 @@ fn wildcard_san_accepted_if_in_subtree() {
             ca,
             &["bob.example.com", "jane.example.com"],
             &["example.com", "uh.oh.example.com"],
-            &["DnsName(\"*.example.com\")", "DirectoryName"]
+            &["DnsName(\"*.example.com\")"]
         ),
         Ok(())
     );
@@ -399,8 +398,7 @@ fn invalid_dns_name_matching() {
             &[],
             &[
                 "DnsName(\"{invalid}.example.com\")",
-                "DnsName(\"dns.example.com\")",
-                "DirectoryName"
+                "DnsName(\"dns.example.com\")"
             ]
         ),
         Ok(())