From 64fb1b7f3b8c271d65052e34cbcee2ffc2212ef4 Mon Sep 17 00:00:00 2001
From: Daniel McCarney <daniel@binaryparadox.net>
Date: Wed, 20 Sep 2023 11:15:21 -0400
Subject: [PATCH] cert: move `list_cert_dns_names` to `Cert`

This commit lifts the free-standing `list_cert_dns_names` helper from
the `subject_name` module to be associated with a `Cert`. Doing so also
requires making the `subject_name::NameIterator` and
`subject_name::WildcardDnsNameRef` `pub(crate)` visible.
---
 src/cert.rs                | 23 +++++++++++++++++++++++
 src/end_entity.rs          |  2 +-
 src/subject_name/mod.rs    |  3 ++-
 src/subject_name/verify.rs | 28 +++-------------------------
 4 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/src/cert.rs b/src/cert.rs
index 67de10f0..173f3deb 100644
--- a/src/cert.rs
+++ b/src/cert.rs
@@ -15,7 +15,9 @@
 use crate::der::{self, DerIterator, FromDer, Tag, CONSTRUCTED, CONTEXT_SPECIFIC};
 use crate::error::{DerTypeId, Error};
 use crate::signed_data::SignedData;
+use crate::subject_name::{GeneralName, NameIterator, WildcardDnsNameRef};
 use crate::x509::{remember_extension, set_extension_once, DistributionPointName, Extension};
+use crate::DnsNameRef;
 
 /// A parsed X509 certificate.
 pub struct Cert<'a> {
@@ -136,6 +138,27 @@ impl<'a> Cert<'a> {
         self.subject.as_slice_less_safe()
     }
 
+    pub(crate) fn valid_dns_names(&self) -> impl Iterator<Item = &str> {
+        NameIterator::new(Some(self.subject), self.subject_alt_name).filter_map(|result| {
+            let presented_id = match result.ok()? {
+                GeneralName::DnsName(presented) => presented,
+                _ => return None,
+            };
+
+            // if the name could be converted to a DNS name, return it; otherwise,
+            // keep going.
+            match DnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) {
+                Ok(dns_name) => Some(dns_name.as_str()),
+                Err(_) => {
+                    match WildcardDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) {
+                        Ok(wildcard_dns_name) => Some(wildcard_dns_name.as_str()),
+                        Err(_) => None,
+                    }
+                }
+            }
+        })
+    }
+
     /// Returns an iterator over the certificate's cRLDistributionPoints extension values, if any.
     pub(crate) fn crl_distribution_points(
         &self,
diff --git a/src/end_entity.rs b/src/end_entity.rs
index 76ff96f0..b36bb736 100644
--- a/src/end_entity.rs
+++ b/src/end_entity.rs
@@ -154,7 +154,7 @@ impl<'a> EndEntityCert<'a> {
     /// Checking that a certificate is valid for a given subject name should always be done with
     /// [EndEntityCert::verify_is_valid_for_subject_name].
     pub fn dns_names(&'a self) -> impl Iterator<Item = &'a str> {
-        subject_name::list_cert_dns_names(self)
+        self.inner.valid_dns_names()
     }
 }
 
diff --git a/src/subject_name/mod.rs b/src/subject_name/mod.rs
index fc90dc48..c3abf4ea 100644
--- a/src/subject_name/mod.rs
+++ b/src/subject_name/mod.rs
@@ -13,6 +13,7 @@
 // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 mod dns_name;
+pub(super) use dns_name::WildcardDnsNameRef;
 pub use dns_name::{DnsNameRef, InvalidDnsNameError};
 
 #[cfg(feature = "alloc")]
@@ -29,5 +30,5 @@ pub use ip_address::IpAddr;
 
 mod verify;
 pub(super) use verify::{
-    check_name_constraints, list_cert_dns_names, verify_cert_subject_name, GeneralName,
+    check_name_constraints, verify_cert_subject_name, GeneralName, NameIterator,
 };
diff --git a/src/subject_name/verify.rs b/src/subject_name/verify.rs
index ad2ec5d0..518fd062 100644
--- a/src/subject_name/verify.rs
+++ b/src/subject_name/verify.rs
@@ -12,7 +12,7 @@
 // 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, WildcardDnsNameRef};
+use super::dns_name::{self, DnsNameRef};
 use super::ip_address::{self, IpAddrRef};
 use super::name::SubjectNameRef;
 use crate::der::{self, FromDer};
@@ -258,13 +258,13 @@ enum Subtrees {
     ExcludedSubtrees,
 }
 
-struct NameIterator<'a> {
+pub(crate) struct NameIterator<'a> {
     subject_alt_name: Option<untrusted::Reader<'a>>,
     subject_directory_name: Option<untrusted::Input<'a>>,
 }
 
 impl<'a> NameIterator<'a> {
-    fn new(
+    pub(crate) fn new(
         subject: Option<untrusted::Input<'a>>,
         subject_alt_name: Option<untrusted::Input<'a>>,
     ) -> Self {
@@ -312,28 +312,6 @@ impl<'a> Iterator for NameIterator<'a> {
     }
 }
 
-pub(crate) fn list_cert_dns_names<'names>(
-    cert: &'names crate::EndEntityCert<'names>,
-) -> impl Iterator<Item = &'names str> {
-    let cert = &cert.inner();
-    NameIterator::new(Some(cert.subject), cert.subject_alt_name).filter_map(|result| {
-        let presented_id = match result.ok()? {
-            GeneralName::DnsName(presented) => presented,
-            _ => return None,
-        };
-
-        // if the name could be converted to a DNS name, return it; otherwise,
-        // keep going.
-        match DnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) {
-            Ok(dns_name) => Some(dns_name.as_str()),
-            Err(_) => match WildcardDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) {
-                Ok(wildcard_dns_name) => Some(wildcard_dns_name.as_str()),
-                Err(_) => None,
-            },
-        }
-    })
-}
-
 // It is *not* valid to derive `Eq`, `PartialEq, etc. for this type. In
 // particular, for the types of `GeneralName`s that we don't understand, we
 // don't even store the value. Also, the meaning of a `GeneralName` in a name