Refactor name verification flow

src/end_entity.rs

@@ -18,7 +18,7 @@ use pki_types::{CertificateDer, SignatureVerificationAlgorithm, TrustAnchor, Uni
 
 use crate::crl::RevocationOptions;
 use crate::error::Error;
-use crate::subject_name::{self, SubjectNameRef};
+use crate::subject_name::{NameIterator, SubjectNameRef};
 use crate::verify_cert::{self, KeyUsage};
 use crate::{cert, signed_data};
 
@@ -109,7 +109,16 @@ impl<'a> EndEntityCert<'a> {
         &self,
         subject_name: SubjectNameRef,
     ) -> Result<(), Error> {
-        subject_name::verify_cert_subject_name(self, subject_name)
+        match subject_name {
+            SubjectNameRef::DnsName(dns_name) => dns_name.verify_dns_names(NameIterator::new(
+                Some(self.inner.subject),
+                self.inner.subject_alt_name,
+            )),
+            // IP addresses are not compared against the subject field;
+            // only against Subject Alternative Names.
+            SubjectNameRef::IpAddress(ip_address) => ip_address
+                .verify_ip_address_names(NameIterator::new(None, self.inner.subject_alt_name)),
+        }
     }
 
     /// Verifies the signature `signature` of message `msg` using the

src/subject_name/dns_name.rs

@@ -16,6 +16,7 @@
 use alloc::string::String;
 use core::fmt::Write;
 
+use super::verify::{GeneralName, NameIterator};
 use crate::Error;
 
 /// A DNS Name suitable for use in the TLS Server Name Indication (SNI)
@@ -84,6 +85,29 @@
         Self::try_from_ascii(dns_name.as_bytes())
     }
 
+    pub(crate) fn verify_dns_names(&self, mut names: NameIterator<'_>) -> Result<(), Error> {
+        let dns_name = untrusted::Input::from(self.as_str().as_bytes());
+        names
+            .find_map(|result| {
+                let name = match result {
+                    Ok(name) => name,
+                    Err(err) => return Some(Err(err)),
+                };
+
+                let presented_id = match name {
+                    GeneralName::DnsName(presented) => presented,
+                    _ => return None,
+                };
+
+                match presented_id_matches_reference_id(presented_id, IdRole::Reference, dns_name) {
+                    Ok(true) => Some(Ok(())),
+                    Ok(false) | Err(Error::MalformedDnsIdentifier) => None,
+                    Err(e) => Some(Err(e)),
+                }
+            })
+            .unwrap_or(Err(Error::CertNotValidForName))
+    }
+
     /// Constructs a `DnsName` from this `DnsNameRef`
     #[cfg(feature = "alloc")]
     pub fn to_owned(&self) -> DnsName {
@@ -181,28 +205,6 @@
 #[cfg(feature = "std")]
 impl ::std::error::Error for InvalidDnsNameError {}
 
-pub(super) fn presented_id_matches_reference_id(
-    presented_dns_id: untrusted::Input,
-    reference_dns_id: untrusted::Input,
-) -> Result<bool, Error> {
-    presented_id_matches_reference_id_internal(
-        presented_dns_id,
-        IdRole::Reference,
-        reference_dns_id,
-    )
-}
-
-pub(super) fn presented_id_matches_constraint(
-    presented_dns_id: untrusted::Input,
-    reference_dns_id: untrusted::Input,
-) -> Result<bool, Error> {
-    presented_id_matches_reference_id_internal(
-        presented_dns_id,
-        IdRole::NameConstraint,
-        reference_dns_id,
-    )
-}
-
 // We assume that both presented_dns_id and reference_dns_id are encoded in
 // such a way that US-ASCII (7-bit) characters are encoded in one byte and no
 // encoding of a non-US-ASCII character contains a code point in the range
@@ -319,7 +321,7 @@
 // [4] Feedback on the lack of clarify in the definition that never got
 //     incorporated into the spec:
 //     https://www.ietf.org/mail-archive/web/pkix/current/msg21192.html
-fn presented_id_matches_reference_id_internal(
+pub(super) fn presented_id_matches_reference_id(
     presented_dns_id: untrusted::Input,
     reference_dns_id_role: IdRole,
     reference_dns_id: untrusted::Input,
@@ -465,7 +467,7 @@
 }
 
 #[derive(Clone, Copy, PartialEq)]
-enum IdRole {
+pub(super) enum IdRole {
     Reference,
     Presented,
     NameConstraint,
@@ -976,6 +978,7 @@
         for &(presented, reference, expected_result) in PRESENTED_MATCHES_REFERENCE {
             let actual_result = presented_id_matches_reference_id(
                 untrusted::Input::from(presented),
+                IdRole::Reference,
                 untrusted::Input::from(reference),
             );
             assert_eq!(
@@ -1050,8 +1053,9 @@
     #[test]
     fn presented_matches_constraint_test() {
         for &(presented, constraint, expected_result) in PRESENTED_MATCHES_CONSTRAINT {
-            let actual_result = presented_id_matches_constraint(
+            let actual_result = presented_id_matches_reference_id(
                 untrusted::Input::from(presented),
+                IdRole::NameConstraint,
                 untrusted::Input::from(constraint),
             );
             assert_eq!(

src/subject_name/ip_address.rs

@@ -15,6 +15,7 @@
 #[cfg(feature = "std")]
 use core::fmt::Write;
 
+use super::verify::{GeneralName, NameIterator};
 use crate::Error;
 
 #[cfg(feature = "alloc")]
@@ -50,6 +51,34 @@
     V6(&'a [u8], [u8; 16]),
 }
 
+impl<'a> IpAddrRef<'a> {
+    pub(crate) fn verify_ip_address_names(&self, mut names: NameIterator<'_>) -> Result<(), Error> {
+        let ip_address = match self {
+            IpAddrRef::V4(_, ref ip_address_octets) => untrusted::Input::from(ip_address_octets),
+            IpAddrRef::V6(_, ref ip_address_octets) => untrusted::Input::from(ip_address_octets),
+        };
+
+        names
+            .find_map(|result| {
+                let name = match result {
+                    Ok(name) => name,
+                    Err(err) => return Some(Err(err)),
+                };
+
+                let presented_id = match name {
+                    GeneralName::IpAddress(presented) => presented,
+                    _ => return None,
+                };
+
+                match presented_id_matches_reference_id(presented_id, ip_address) {
+                    true => Some(Ok(())),
+                    false => None,
+                }
+            })
+            .unwrap_or(Err(Error::CertNotValidForName))
+    }
+}
+
 #[cfg(feature = "alloc")]
 impl<'a> From<IpAddrRef<'a>> for IpAddr {
     fn from(ip_address: IpAddrRef<'a>) -> IpAddr {
@@ -196,7 +225,7 @@
 //   version 4, as specified in [RFC791], the octet string MUST contain
 //   exactly four octets.  For IP version 6, as specified in
 //   [RFC2460], the octet string MUST contain exactly sixteen octets.
-pub(super) fn presented_id_matches_reference_id(
+fn presented_id_matches_reference_id(
     presented_id: untrusted::Input,
     reference_id: untrusted::Input,
 ) -> bool {

src/subject_name/mod.rs

@@ -29,6 +29,4 @@ pub use ip_address::{AddrParseError, IpAddrRef};
 pub use ip_address::IpAddr;
 
 mod verify;
-pub(super) use verify::{
-    check_name_constraints, verify_cert_subject_name, GeneralName, NameIterator,
-};
+pub(super) use verify::{check_name_constraints, GeneralName, NameIterator};

src/subject_name/verify.rs

@@ -12,78 +12,12 @@
 // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-use super::dns_name::{self, DnsNameRef};
-use super::ip_address::{self, IpAddrRef};
-use super::name::SubjectNameRef;
+use super::dns_name::{self, IdRole};
+use super::ip_address;
 use crate::der::{self, FromDer};
 use crate::error::{DerTypeId, Error};
 use crate::verify_cert::{Budget, PathNode};
 
-pub(crate) fn verify_cert_dns_name(
-    cert: &crate::EndEntityCert,
-    dns_name: DnsNameRef,
-) -> Result<(), Error> {
-    let dns_name = untrusted::Input::from(dns_name.as_str().as_bytes());
-    NameIterator::new(Some(cert.subject), cert.subject_alt_name)
-        .find_map(|result| {
-            let name = match result {
-                Ok(name) => name,
-                Err(err) => return Some(Err(err)),
-            };
-
-            let presented_id = match name {
-                GeneralName::DnsName(presented) => presented,
-                _ => return None,
-            };
-
-            match dns_name::presented_id_matches_reference_id(presented_id, dns_name) {
-                Ok(true) => Some(Ok(())),
-                Ok(false) | Err(Error::MalformedDnsIdentifier) => None,
-                Err(e) => Some(Err(e)),
-            }
-        })
-        .unwrap_or(Err(Error::CertNotValidForName))
-}
-
-pub(crate) fn verify_cert_subject_name(
-    cert: &crate::EndEntityCert,
-    subject_name: SubjectNameRef,
-) -> Result<(), Error> {
-    let ip_address = match subject_name {
-        SubjectNameRef::DnsName(dns_name) => return verify_cert_dns_name(cert, dns_name),
-        SubjectNameRef::IpAddress(IpAddrRef::V4(_, ref ip_address_octets)) => {
-            untrusted::Input::from(ip_address_octets)
-        }
-        SubjectNameRef::IpAddress(IpAddrRef::V6(_, ref ip_address_octets)) => {
-            untrusted::Input::from(ip_address_octets)
-        }
-    };
-
-    NameIterator::new(
-        // IP addresses are not compared against the subject field;
-        // only against Subject Alternative Names.
-        None,
-        cert.subject_alt_name,
-    )
-    .find_map(|result| {
-        let name = match result {
-            Ok(name) => name,
-            Err(err) => return Some(Err(err)),
-        };
-
-        let presented_id = match name {
-            GeneralName::IpAddress(presented) => presented,
-            _ => return None,
-        };
-
-        match ip_address::presented_id_matches_reference_id(presented_id, ip_address) {
-            true => Some(Ok(())),
-            false => None,
-        }
-    })
-    .unwrap_or(Err(Error::CertNotValidForName))
-}
-
 // https://tools.ietf.org/html/rfc5280#section-4.2.1.10
 pub(crate) fn check_name_constraints(
     constraints: Option<&mut untrusted::Reader>,
@@ -174,7 +108,7 @@ fn check_presented_id_conforms_to_constraints(
 
             let matches = match (name, base) {
                 (GeneralName::DnsName(name), GeneralName::DnsName(base)) => {
-                    dns_name::presented_id_matches_constraint(name, base)
+                    dns_name::presented_id_matches_reference_id(name, IdRole::NameConstraint, base)
                 }
 
                 (GeneralName::DirectoryName(_), GeneralName::DirectoryName(_)) => Ok(