end_entity: infallible name iteration

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 = GeneraDnsNameRef<'a>>` unconditionally, instead of
a `Result`. This allows the method to be alloc-free and 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

@@ -150,13 +150,14 @@ impl<'a> EndEntityCert<'a> {
         )
     }
 
-    /// Returns a list of the DNS names provided in the subject alternative names extension
+    /// Returns a list of the valid DNS names provided in the subject alternative names (SAN)
+    /// extension. Invalid names are skipped.
     ///
     /// This function must not be used to implement custom DNS name verification.
     /// Verification functions are already provided as `verify_is_valid_for_dns_name`
     /// and `verify_is_valid_for_at_least_one_dns_name`.
     #[cfg(feature = "alloc")]
-    pub fn dns_names(&'a self) -> Result<impl Iterator<Item = GeneralDnsNameRef<'a>>, Error> {
+    pub fn dns_names(&'a self) -> impl Iterator<Item = GeneralDnsNameRef<'a>> {
         subject_name::list_cert_dns_names(self)
     }
 }
@@ -214,9 +215,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

@@ -12,9 +12,6 @@
 // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-#[cfg(feature = "alloc")]
-use alloc::vec::Vec;
-
 #[cfg(feature = "alloc")]
 use super::dns_name::GeneralDnsNameRef;
 use super::dns_name::{self, DnsNameRef};
@@ -320,35 +317,23 @@ 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 = GeneralDnsNameRef<'names>>, Error> {
+) -> impl Iterator<Item = GeneralDnsNameRef<'names>> {
     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),
-            };
-
-            let presented_id = match name {
-                GeneralName::DnsName(presented) => presented,
-                _ => return None,
-            };
-
-            // if the name could be converted to a DNS name, add it; otherwise,
-            // keep going.
-            if let Ok(name) = GeneralDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) {
-                names.push(name)
-            }
+    NameIterator::new(Some(cert.subject), cert.subject_alt_name).filter_map(|result| {
+        let name = match result {
+            Ok(name) => name,
+            // Ignore invalid names, this is an informational function, not something used
+            // during validation.
+            Err(_) => return None,
+        };
 
-            None
-        });
+        let presented_id = match name {
+            GeneralName::DnsName(presented) => presented,
+            _ => return None,
+        };
 
-    match result {
-        Some(err) => Err(err),
-        _ => Ok(names.into_iter()),
-    }
+        GeneralDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()).ok()
+    })
 }
 
 // It is *not* valid to derive `Eq`, `PartialEq, etc. for this type. In

tests/integration.rs

@@ -346,5 +346,5 @@ fn expect_cert_dns_names<'name>(
         .into_iter()
         .map(|name| GeneralDnsNameRef::try_from_ascii_str(name).unwrap());
 
-    assert!(cert.dns_names().unwrap().eq(expected_names));
+    assert!(cert.dns_names().eq(expected_names));
 }