end_entity: make dns_names infallible

The purpose of the `dns_names` helper on an `EndEntityCert` is to
provide users the opportunity to get information on the dNSName SAN
values in a certificate for **non-validation** purposes. Checking that
a certificate is valid for a particular name should always be done with
`verify_is_valid_for_at_least_one_dns_name`.

With that use-case in mind, we can make the `dns_names` helper easier
for consumers to use by filtering out invalid general names, returning
an `Iterator<Item = &'a str>` unconditionally, instead of
a `Result`. This better matches the updated name validation semantics
where we ignore `MalformedDnsIdentifier` errors to continue to try to
find a valid name to validate against.

Author: cpu

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].
     #[cfg(feature = "alloc")]
-    pub fn dns_names(&'a self) -> Result<impl Iterator<Item = &'a str>, Error> {
+    pub fn dns_names(&'a self) -> impl Iterator<Item = &'a str> {
         subject_name::list_cert_dns_names(self)
     }
 }
@@ -212,9 +212,7 @@ mod tests {
         let cert =
             EndEntityCert::try_from(der).expect("should parse end entity certificate correctly");
 
-        let mut names = cert
-            .dns_names()
-            .expect("should get all DNS names correctly for end entity cert");
+        let mut names = cert.dns_names();
         assert_eq!(names.next().map(<&str>::from), Some(name));
         assert_eq!(names.next().map(<&str>::from), None);
     }

src/subject_name/verify.rs

@@ -320,42 +320,36 @@ impl<'a> Iterator for NameIterator<'a> {
 #[cfg(feature = "alloc")]
 pub(crate) fn list_cert_dns_names<'names>(
     cert: &'names crate::EndEntityCert<'names>,
-) -> Result<impl Iterator<Item = &'names str>, Error> {
+) -> impl Iterator<Item = &'names str> {
     let cert = &cert.inner();
     let mut names = Vec::new();
 
-    let result =
-        NameIterator::new(Some(cert.subject), cert.subject_alt_name).find_map(&mut |result| {
-            let name = match result {
-                Ok(name) => name,
-                Err(err) => return Some(err),
-            };
+    NameIterator::new(Some(cert.subject), cert.subject_alt_name).for_each(|result| {
+        let name = match result {
+            Ok(name) => name,
+            Err(_) => return,
+        };
 
-            let presented_id = match name {
-                GeneralName::DnsName(presented) => presented,
-                _ => return None,
-            };
+        let presented_id = match name {
+            GeneralName::DnsName(presented) => presented,
+            _ => return,
+        };
 
-            let dns_name = DnsNameRef::try_from_ascii(presented_id.as_slice_less_safe())
-                .map(GeneralDnsNameRef::DnsName)
-                .or_else(|_| {
-                    WildcardDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe())
-                        .map(GeneralDnsNameRef::Wildcard)
-                });
-
-            // if the name could be converted to a DNS name, add it; otherwise,
-            // keep going.
-            if let Ok(name) = dns_name {
-                names.push(name.into())
-            }
+        let dns_name = DnsNameRef::try_from_ascii(presented_id.as_slice_less_safe())
+            .map(GeneralDnsNameRef::DnsName)
+            .or_else(|_| {
+                WildcardDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe())
+                    .map(GeneralDnsNameRef::Wildcard)
+            });
 
-            None
-        });
+        // if the name could be converted to a DNS name, add it; otherwise,
+        // keep going.
+        if let Ok(name) = dns_name {
+            names.push(name.into())
+        }
+    });
 
-    match result {
-        Some(err) => Err(err),
-        _ => Ok(names.into_iter()),
-    }
+    names.into_iter()
 }
 
 // It is *not* valid to derive `Eq`, `PartialEq, etc. for this type. In

tests/integration.rs

@@ -340,5 +340,5 @@ fn expect_cert_dns_names<'name>(
     let cert = webpki::EndEntityCert::try_from(&der)
         .expect("should parse end entity certificate correctly");
 
-    assert!(cert.dns_names().unwrap().eq(expected_names))
+    assert!(cert.dns_names().eq(expected_names))
 }