Add tests for PrintableString, empty SEQUENCE CNs

This commit adds a pair of tests reproducing issue rustls#167,
where the `EndEntityCert::dns_names()` method returns an error
incorrectly on some certificate DER encodings. In particular,
`dns_names` fails if the CN is a `PrintableString`, or if it's an empty
`SEQUENCE`, rather than a `SEQUENCE` containing an empty `SET`.

The test for the `PrintableString` common name uses an end-entity
certificate generated using `rcgen`, while the test for empty `SEQUENCE`
CN required a hand-crafted DER using `ascii2der`. The text file that
generated the `ascii2der` cert is also included.

src/end_entity.rs

@@ -257,3 +257,59 @@ impl<'a> EndEntityCert<'a> {
         subject_name::list_cert_dns_names(self)
     }
 }
+
+#[cfg(feature = "alloc")]
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils;
+
+    // This test reproduces https://github.com/rustls/webpki/issues/167 --- an
+    // end-entity cert where the common name is a `PrintableString` rather than
+    // a `UTF8String` cannot iterate over its subject alternative names.
+    #[test]
+    fn printable_string_common_name() {
+        const DNS_NAME: &str = "test.example.com";
+
+        let issuer = test_utils::make_issuer("Test", None);
+
+        let ee_cert_der = {
+            let mut params = rcgen::CertificateParams::new(vec![DNS_NAME.to_string()]);
+            // construct a certificate that uses `PrintableString` as the
+            // common name value, rather than `UTF8String`.
+            params.distinguished_name.push(
+                rcgen::DnType::CommonName,
+                rcgen::DnValue::PrintableString("example.com".to_string()),
+            );
+            params.is_ca = rcgen::IsCa::ExplicitNoCa;
+            params.alg = test_utils::RCGEN_SIGNATURE_ALG;
+            let cert = rcgen::Certificate::from_params(params)
+                .expect("failed to make ee cert (this is a test bug)");
+            cert.serialize_der_with_signer(&issuer)
+                .expect("failed to serialize signed ee cert (this is a test bug)")
+        };
+
+        expect_dns_name(&ee_cert_der, DNS_NAME);
+    }
+
+    // This test reproduces https://github.com/rustls/webpki/issues/167 --- an
+    // end-entity cert where the common name is an empty SEQUENCE.
+    #[test]
+    fn empty_sequence_common_name() {
+        // handcrafted cert DER produced using `ascii2der`, since `rcgen` is
+        // unwilling to generate this particular weird cert.
+        let ee_cert_der = include_bytes!("../tests/misc/empty_sequence_common_name.der").as_slice();
+        expect_dns_name(ee_cert_der, "example.com");
+    }
+
+    fn expect_dns_name(der: &[u8], name: &str) {
+        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");
+        assert_eq!(names.next().map(<&str>::from), Some(name));
+        assert_eq!(names.next().map(<&str>::from), None);
+    }
+}

src/lib.rs

@@ -60,6 +60,10 @@ mod x509;
 
 #[allow(deprecated)]
 pub use trust_anchor::{TlsClientTrustAnchors, TlsServerTrustAnchors};
+
+#[cfg(test)]
+pub(crate) mod test_utils;
+
 pub use {
     cert::{Cert, EndEntityOrCa},
     crl::{BorrowedCertRevocationList, BorrowedRevokedCert, CertRevocationList, RevocationReason},

src/test_utils.rs

@@ -0,0 +1,36 @@
+/// Signature algorithm used by certificates generated using `make_issuer` and
+/// `make_end_entity`. This is exported as a constant so that tests can use the
+/// same algorithm when generating certificates using `rcgen`.
+#[cfg(feature = "alloc")]
+pub(crate) static RCGEN_SIGNATURE_ALG: &rcgen::SignatureAlgorithm = &rcgen::PKCS_ECDSA_P256_SHA256;
+
+#[cfg(feature = "alloc")]
+pub(crate) fn make_issuer(
+    org_name: impl Into<String>,
+    name_constraints: Option<rcgen::NameConstraints>,
+) -> rcgen::Certificate {
+    let mut ca_params = rcgen::CertificateParams::new(Vec::new());
+    ca_params
+        .distinguished_name
+        .push(rcgen::DnType::OrganizationName, org_name);
+    ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained);
+    ca_params.key_usages = vec![
+        rcgen::KeyUsagePurpose::KeyCertSign,
+        rcgen::KeyUsagePurpose::DigitalSignature,
+        rcgen::KeyUsagePurpose::CrlSign,
+    ];
+    ca_params.alg = RCGEN_SIGNATURE_ALG;
+    ca_params.name_constraints = name_constraints;
+    rcgen::Certificate::from_params(ca_params).unwrap()
+}
+
+#[cfg(feature = "alloc")]
+pub(crate) fn make_end_entity(issuer: &rcgen::Certificate) -> Vec<u8> {
+    let mut ee_params = rcgen::CertificateParams::new(vec!["example.com".to_string()]);
+    ee_params.is_ca = rcgen::IsCa::ExplicitNoCa;
+    ee_params.alg = RCGEN_SIGNATURE_ALG;
+    rcgen::Certificate::from_params(ee_params)
+        .unwrap()
+        .serialize_der_with_signer(issuer)
+        .unwrap()
+}

src/verify_cert.rs

@@ -604,6 +604,9 @@ where
 mod tests {
     use super::*;
 
+    #[cfg(feature = "alloc")]
+    use crate::test_utils::{make_end_entity, make_issuer};
+
     #[test]
     fn eku_key_purpose_id() {
         assert!(ExtendedKeyUsage::RequiredIfPresent(EKU_SERVER_AUTH)
@@ -819,36 +822,4 @@ mod tests {
             time,
         )
     }
-
-    #[cfg(feature = "alloc")]
-    fn make_issuer(
-        org_name: impl Into<String>,
-        name_constraints: Option<rcgen::NameConstraints>,
-    ) -> rcgen::Certificate {
-        let mut ca_params = rcgen::CertificateParams::new(Vec::new());
-        ca_params
-            .distinguished_name
-            .push(rcgen::DnType::OrganizationName, org_name);
-        ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained);
-        ca_params.key_usages = vec![
-            rcgen::KeyUsagePurpose::KeyCertSign,
-            rcgen::KeyUsagePurpose::DigitalSignature,
-            rcgen::KeyUsagePurpose::CrlSign,
-        ];
-        ca_params.alg = &rcgen::PKCS_ECDSA_P256_SHA256;
-        ca_params.name_constraints = name_constraints;
-        rcgen::Certificate::from_params(ca_params).unwrap()
-    }
-
-    #[cfg(feature = "alloc")]
-    fn make_end_entity(issuer: &rcgen::Certificate) -> Vec<u8> {
-        let mut ee_params = rcgen::CertificateParams::new(vec!["example.com".to_string()]);
-        ee_params.is_ca = rcgen::IsCa::ExplicitNoCa;
-        ee_params.alg = &rcgen::PKCS_ECDSA_P256_SHA256;
-
-        rcgen::Certificate::from_params(ee_params)
-            .unwrap()
-            .serialize_der_with_signer(issuer)
-            .unwrap()
-    }
 }

tests/misc/empty_sequence_common_name.der.txt

@@ -0,0 +1,104 @@
+SEQUENCE {
+  SEQUENCE {
+    [0] {
+      INTEGER { 2 }
+    }
+    INTEGER { `7214c2cb574538a4d63af28bb25140821cd3c528` }
+    SEQUENCE {
+      # ecdsa-with-SHA256
+      OBJECT_IDENTIFIER { 1.2.840.10045.4.3.2 }
+    }
+    SEQUENCE {
+      SET {
+        SEQUENCE {
+          # organizationUnitName
+          OBJECT_IDENTIFIER { 2.5.4.11 }
+          PrintableString { "None" }
+        }
+      }
+    }
+    SEQUENCE {
+      UTCTime { "230906172100Z" }
+      UTCTime { "330903172100Z" }
+    }
+    SEQUENCE {}
+    SEQUENCE {
+      SEQUENCE {
+        # ecPublicKey
+        OBJECT_IDENTIFIER { 1.2.840.10045.2.1 }
+        # secp256r1
+        OBJECT_IDENTIFIER { 1.2.840.10045.3.1.7 }
+      }
+      BIT_STRING { `00` `04681e0d5d4e66ff6f0cc3a9f0e1ba6114d647e9dfc2ee23418d56ad58edfb78cfe4ec8dadf856fde5a890799753bcd2a9380896701b7c13e49ec2e097f8ee3421` }
+    }
+    [3] {
+      SEQUENCE {
+        SEQUENCE {
+          # keyUsage
+          OBJECT_IDENTIFIER { 2.5.29.15 }
+          BOOLEAN { TRUE }
+          OCTET_STRING {
+            BIT_STRING { b`101` }
+          }
+        }
+        SEQUENCE {
+          # extKeyUsage
+          OBJECT_IDENTIFIER { 2.5.29.37 }
+          OCTET_STRING {
+            SEQUENCE {
+              # serverAuth
+              OBJECT_IDENTIFIER { 1.3.6.1.5.5.7.3.1 }
+              # clientAuth
+              OBJECT_IDENTIFIER { 1.3.6.1.5.5.7.3.2 }
+            }
+          }
+        }
+        SEQUENCE {
+          # basicConstraints
+          OBJECT_IDENTIFIER { 2.5.29.19 }
+          BOOLEAN { TRUE }
+          OCTET_STRING {
+            SEQUENCE {}
+          }
+        }
+        SEQUENCE {
+          # subjectKeyIdentifier
+          OBJECT_IDENTIFIER { 2.5.29.14 }
+          OCTET_STRING {
+            OCTET_STRING { `8f8c61be7ce4c34689e2618034581e34ef88b24c` }
+          }
+        }
+        SEQUENCE {
+          # authorityKeyIdentifier
+          OBJECT_IDENTIFIER { 2.5.29.35 }
+          OCTET_STRING {
+            SEQUENCE {
+              [0 PRIMITIVE] { `c9a3daf11d035f6eab28e2ea195c677568b0adea` }
+            }
+          }
+        }
+        SEQUENCE {
+          # subjectAltName
+          OBJECT_IDENTIFIER { 2.5.29.17 }
+          BOOLEAN { TRUE }
+          OCTET_STRING {
+            SEQUENCE {
+              [2 PRIMITIVE] { "example.com" }
+            }
+          }
+        }
+      }
+    }
+  }
+  SEQUENCE {
+    # ecdsa-with-SHA256
+    OBJECT_IDENTIFIER { 1.2.840.10045.4.3.2 }
+  }
+  BIT_STRING {
+    `00`
+    SEQUENCE {
+      INTEGER { `00f1891fc58e16c37752bf64d58999f703c8d458b1d22d8291292ce2ad87042990` }
+      INTEGER { `00a6f16e39c83ba001f7f6aae73aaad0aea46d596800746887fe4567a4ffe88f9b` }
+    }
+  }
+}