Fix the CertDatabaseNSSTest hierarchy tests

A number of CertDatabaseNSSTests were changed in 2012 to test
the wrong thing, after a certificate expired, rather than to
DISABLE the test.

This updates all of the expectations to match what they were
intended to test, using regular test certificates that can
be easily updated.

BUG=111029

Change-Id: If7fcb67a20f52fe551da53ea809274ee5775a8bf
Reviewed-on: https://chromium-review.googlesource.com/1012452
Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551598}

net/BUILD.gn

@@ -2438,9 +2438,6 @@ bundle_data("test_support_bundle_data") {
     "data/ssl/certificates/diginotar_public_ca_2025.pem",
     "data/ssl/certificates/diginotar_root_ca.pem",
     "data/ssl/certificates/diginotar_services_1024_ca.pem",
-    "data/ssl/certificates/dod_ca_13_cert.der",
-    "data/ssl/certificates/dod_ca_17_cert.der",
-    "data/ssl/certificates/dod_root_ca_2_cert.der",
     "data/ssl/certificates/duplicate_cn_1.p12",
     "data/ssl/certificates/duplicate_cn_1.pem",
     "data/ssl/certificates/duplicate_cn_2.p12",
@@ -2550,7 +2547,6 @@ bundle_data("test_support_bundle_data") {
     "data/ssl/certificates/websocket_cacert.pem",
     "data/ssl/certificates/websocket_client_cert.p12",
     "data/ssl/certificates/wildcard.pem",
-    "data/ssl/certificates/www_us_army_mil_cert.der",
     "data/ssl/certificates/x509_verify_results.chain.pem",
   ]
   outputs = [

net/cert/nss_cert_database.cc

@@ -186,10 +186,15 @@ CERTCertificate* NSSCertDatabase::FindRootInList(
   CERTCertificate* certn_2 = certificates[certificates.size() - 2].get();
   CERTCertificate* certn_1 = certificates[certificates.size() - 1].get();
 
-  if (CERT_CompareName(&cert1->issuer, &cert0->subject) == SECEqual)
+  // Using CERT_CompareName is an alternative, except that it is broken until
+  // NSS 3.32 (see https://bugzilla.mozilla.org/show_bug.cgi?id=1361197 ).
+  if (SECITEM_CompareItem(&cert1->derIssuer, &cert0->derSubject) == SECEqual)
     return cert0;
-  if (CERT_CompareName(&certn_2->issuer, &certn_1->subject) == SECEqual)
+
+  if (SECITEM_CompareItem(&certn_2->derIssuer, &certn_1->derSubject) ==
+      SECEqual) {
     return certn_1;
+  }
 
   LOG(WARNING) << "certificate list is not a hierarchy";
   return cert0;
@@ -225,6 +230,7 @@ bool NSSCertDatabase::ImportCACerts(
     ImportCertFailureList* not_imported) {
   crypto::ScopedPK11Slot slot(GetPublicSlot());
   CERTCertificate* root = FindRootInList(certificates);
+
   bool success = psm::ImportCACerts(slot.get(), certificates, root, trust_bits,
                                     not_imported);
   if (success)
@@ -332,7 +338,7 @@ bool NSSCertDatabase::IsUntrusted(const CERTCertificate* cert) const {
   // Self-signed certificates that don't have any trust bits set are untrusted.
   // Other certificates that don't have any trust bits set may still be trusted
   // if they chain up to a trust anchor.
-  if (CERT_CompareName(&cert->issuer, &cert->subject) == SECEqual) {
+  if (SECITEM_CompareItem(&cert->derIssuer, &cert->derSubject) == SECEqual) {
     return (nsstrust.sslFlags & kTrusted) == 0 &&
            (nsstrust.emailFlags & kTrusted) == 0 &&
            (nsstrust.objectSigningFlags & kTrusted) == 0;

net/cert/nss_cert_database_unittest.cc

@@ -394,9 +394,10 @@ TEST_F(CertDatabaseNSSTest, ImportCA_NotCACert) {
 
 TEST_F(CertDatabaseNSSTest, ImportCACertHierarchy) {
   ScopedCERTCertificateList certs;
-  ASSERT_TRUE(ReadCertIntoList("dod_root_ca_2_cert.der", &certs));
-  ASSERT_TRUE(ReadCertIntoList("dod_ca_17_cert.der", &certs));
-  ASSERT_TRUE(ReadCertIntoList("www_us_army_mil_cert.der", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-D-by-D.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-C-by-D.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-B-by-C.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-A-by-B.pem", &certs));
 
   // Import it.
   NSSCertDatabase::ImportCertFailureList failed;
@@ -408,21 +409,20 @@ TEST_F(CertDatabaseNSSTest, ImportCACertHierarchy) {
       certs, NSSCertDatabase::TRUSTED_SSL | NSSCertDatabase::TRUSTED_EMAIL,
       &failed));
 
-  ASSERT_EQ(2U, failed.size());
-  EXPECT_EQ("DOD CA-17", GetSubjectCN(failed[0].certificate.get()));
-  EXPECT_THAT(failed[0].net_error,
-              IsError(ERR_FAILED));  // The certificate expired.
-  EXPECT_EQ("www.us.army.mil", GetSubjectCN(failed[1].certificate.get()));
-  EXPECT_THAT(failed[1].net_error, IsError(ERR_IMPORT_CA_CERT_NOT_CA));
+  ASSERT_EQ(1U, failed.size());
+  EXPECT_EQ("127.0.0.1", GetSubjectCN(failed[0].certificate.get()));
+  EXPECT_THAT(failed[0].net_error, IsError(ERR_IMPORT_CA_CERT_NOT_CA));
 
   ScopedCERTCertificateList cert_list = ListCerts();
-  ASSERT_EQ(1U, cert_list.size());
-  EXPECT_EQ("DoD Root CA 2", GetSubjectCN(cert_list[0].get()));
+  ASSERT_EQ(3U, cert_list.size());
+  EXPECT_EQ("B CA - Multi-root", GetSubjectCN(cert_list[0].get()));
+  EXPECT_EQ("D Root CA - Multi-root", GetSubjectCN(cert_list[1].get()));
+  EXPECT_EQ("C CA - Multi-root", GetSubjectCN(cert_list[2].get()));
 }
 
 TEST_F(CertDatabaseNSSTest, ImportCACertHierarchyDupeRoot) {
   ScopedCERTCertificateList certs;
-  ASSERT_TRUE(ReadCertIntoList("dod_root_ca_2_cert.der", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-D-by-D.pem", &certs));
 
   // First import just the root.
   NSSCertDatabase::ImportCertFailureList failed;
@@ -433,10 +433,11 @@ TEST_F(CertDatabaseNSSTest, ImportCACertHierarchyDupeRoot) {
   EXPECT_EQ(0U, failed.size());
   ScopedCERTCertificateList cert_list = ListCerts();
   ASSERT_EQ(1U, cert_list.size());
-  EXPECT_EQ("DoD Root CA 2", GetSubjectCN(cert_list[0].get()));
+  EXPECT_EQ("D Root CA - Multi-root", GetSubjectCN(cert_list[0].get()));
 
-  ASSERT_TRUE(ReadCertIntoList("dod_ca_17_cert.der", &certs));
-  ASSERT_TRUE(ReadCertIntoList("www_us_army_mil_cert.der", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-C-by-D.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-B-by-C.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-A-by-B.pem", &certs));
 
   // Now import with the other certs in the list too.  Even though the root is
   // already present, we should still import the rest.
@@ -445,24 +446,24 @@ TEST_F(CertDatabaseNSSTest, ImportCACertHierarchyDupeRoot) {
       certs, NSSCertDatabase::TRUSTED_SSL | NSSCertDatabase::TRUSTED_EMAIL,
       &failed));
 
-  ASSERT_EQ(3U, failed.size());
-  EXPECT_EQ("DoD Root CA 2", GetSubjectCN(failed[0].certificate.get()));
+  ASSERT_EQ(2U, failed.size());
+  EXPECT_EQ("D Root CA - Multi-root",
+            GetSubjectCN(failed[0].certificate.get()));
   EXPECT_THAT(failed[0].net_error, IsError(ERR_IMPORT_CERT_ALREADY_EXISTS));
-  EXPECT_EQ("DOD CA-17", GetSubjectCN(failed[1].certificate.get()));
-  EXPECT_THAT(failed[1].net_error,
-              IsError(ERR_FAILED));  // The certificate expired.
-  EXPECT_EQ("www.us.army.mil", GetSubjectCN(failed[2].certificate.get()));
-  EXPECT_THAT(failed[2].net_error, IsError(ERR_IMPORT_CA_CERT_NOT_CA));
+  EXPECT_EQ("127.0.0.1", GetSubjectCN(failed[1].certificate.get()));
+  EXPECT_THAT(failed[1].net_error, IsError(ERR_IMPORT_CA_CERT_NOT_CA));
 
   cert_list = ListCerts();
-  ASSERT_EQ(1U, cert_list.size());
-  EXPECT_EQ("DoD Root CA 2", GetSubjectCN(cert_list[0].get()));
+  ASSERT_EQ(3U, cert_list.size());
+  EXPECT_EQ("B CA - Multi-root", GetSubjectCN(cert_list[0].get()));
+  EXPECT_EQ("D Root CA - Multi-root", GetSubjectCN(cert_list[1].get()));
+  EXPECT_EQ("C CA - Multi-root", GetSubjectCN(cert_list[2].get()));
 }
 
 TEST_F(CertDatabaseNSSTest, ImportCACertHierarchyUntrusted) {
   ScopedCERTCertificateList certs;
-  ASSERT_TRUE(ReadCertIntoList("dod_root_ca_2_cert.der", &certs));
-  ASSERT_TRUE(ReadCertIntoList("dod_ca_17_cert.der", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-D-by-D.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-C-by-D.pem", &certs));
 
   // Import it.
   NSSCertDatabase::ImportCertFailureList failed;
@@ -470,21 +471,21 @@ TEST_F(CertDatabaseNSSTest, ImportCACertHierarchyUntrusted) {
                                       &failed));
 
   ASSERT_EQ(1U, failed.size());
-  EXPECT_EQ("DOD CA-17", GetSubjectCN(failed[0].certificate.get()));
+  EXPECT_EQ("C CA - Multi-root", GetSubjectCN(failed[0].certificate.get()));
   // TODO(mattm): should check for net error equivalent of
   // SEC_ERROR_UNTRUSTED_ISSUER
   EXPECT_THAT(failed[0].net_error, IsError(ERR_FAILED));
 
   ScopedCERTCertificateList cert_list = ListCerts();
   ASSERT_EQ(1U, cert_list.size());
-  EXPECT_EQ("DoD Root CA 2", GetSubjectCN(cert_list[0].get()));
+  EXPECT_EQ("D Root CA - Multi-root", GetSubjectCN(cert_list[0].get()));
 }
 
 TEST_F(CertDatabaseNSSTest, ImportCACertHierarchyTree) {
   ScopedCERTCertificateList certs;
-  ASSERT_TRUE(ReadCertIntoList("dod_root_ca_2_cert.der", &certs));
-  ASSERT_TRUE(ReadCertIntoList("dod_ca_13_cert.der", &certs));
-  ASSERT_TRUE(ReadCertIntoList("dod_ca_17_cert.der", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-E-by-E.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-C-by-E.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-F-by-E.pem", &certs));
 
   // Import it.
   NSSCertDatabase::ImportCertFailureList failed;
@@ -492,17 +493,11 @@ TEST_F(CertDatabaseNSSTest, ImportCACertHierarchyTree) {
       certs, NSSCertDatabase::TRUSTED_SSL | NSSCertDatabase::TRUSTED_EMAIL,
       &failed));
 
-  EXPECT_EQ(2U, failed.size());
-  EXPECT_EQ("DOD CA-13", GetSubjectCN(failed[0].certificate.get()));
-  EXPECT_THAT(failed[0].net_error,
-              IsError(ERR_FAILED));  // The certificate expired.
-  EXPECT_EQ("DOD CA-17", GetSubjectCN(failed[1].certificate.get()));
-  EXPECT_THAT(failed[1].net_error,
-              IsError(ERR_FAILED));  // The certificate expired.
-
   ScopedCERTCertificateList cert_list = ListCerts();
-  ASSERT_EQ(1U, cert_list.size());
-  EXPECT_EQ("DoD Root CA 2", GetSubjectCN(cert_list[0].get()));
+  ASSERT_EQ(3U, cert_list.size());
+  EXPECT_EQ("F CA - Multi-root", GetSubjectCN(cert_list[0].get()));
+  EXPECT_EQ("C CA - Multi-root", GetSubjectCN(cert_list[1].get()));
+  EXPECT_EQ("E Root CA - Multi-root", GetSubjectCN(cert_list[2].get()));
 }
 
 TEST_F(CertDatabaseNSSTest, ImportCACertNotHierarchy) {
@@ -510,23 +505,21 @@ TEST_F(CertDatabaseNSSTest, ImportCACertNotHierarchy) {
       GetTestCertsDirectory(), "root_ca_cert.pem",
       X509Certificate::FORMAT_AUTO);
   ASSERT_EQ(1U, certs.size());
-  ASSERT_TRUE(ReadCertIntoList("dod_ca_13_cert.der", &certs));
-  ASSERT_TRUE(ReadCertIntoList("dod_ca_17_cert.der", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-F-by-E.pem", &certs));
+  ASSERT_TRUE(ReadCertIntoList("multi-root-C-by-E.pem", &certs));
 
   // Import it.
   NSSCertDatabase::ImportCertFailureList failed;
-  EXPECT_TRUE(cert_db_->ImportCACerts(certs,
-                                      NSSCertDatabase::TRUSTED_SSL |
-                                          NSSCertDatabase::TRUSTED_EMAIL |
-                                          NSSCertDatabase::TRUSTED_OBJ_SIGN,
-                                      &failed));
+  EXPECT_TRUE(cert_db_->ImportCACerts(
+      certs, NSSCertDatabase::TRUSTED_SSL | NSSCertDatabase::TRUSTED_EMAIL,
+      &failed));
 
   ASSERT_EQ(2U, failed.size());
   // TODO(mattm): should check for net error equivalent of
   // SEC_ERROR_UNKNOWN_ISSUER
-  EXPECT_EQ("DOD CA-13", GetSubjectCN(failed[0].certificate.get()));
+  EXPECT_EQ("F CA - Multi-root", GetSubjectCN(failed[0].certificate.get()));
   EXPECT_THAT(failed[0].net_error, IsError(ERR_FAILED));
-  EXPECT_EQ("DOD CA-17", GetSubjectCN(failed[1].certificate.get()));
+  EXPECT_EQ("C CA - Multi-root", GetSubjectCN(failed[1].certificate.get()));
   EXPECT_THAT(failed[1].net_error, IsError(ERR_FAILED));
 
   ScopedCERTCertificateList cert_list = ListCerts();

net/data/ssl/certificates/README

@@ -21,11 +21,6 @@ unit tests.
 - foaf.me.chromium-test-cert.der : A client certificate for a FOAF.ME identity
      created for testing.
 
-- www_us_army_mil_cert.der
-- dod_ca_17_cert.der
-- dod_root_ca_2_cert.der :
-     A certificate chain used for testing certificate imports
-
 - unosoft_hu_cert : Certificate used by X509CertificateTest.UnoSoftCertParsing.
 
 - google_diginotar.pem