Make chromeos certificate_helper functions use NSS types directly.

Bug: 671420
Change-Id: I289163e10dd8690991ecfd41ee8901eb613ee820
Reviewed-on: https://chromium-review.googlesource.com/622241
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499258}

chromeos/network/certificate_helper.cc

@@ -4,6 +4,7 @@
 
 #include "chromeos/network/certificate_helper.h"
 
+#include <cert.h>
 #include <certdb.h>
 #include <pk11pub.h>
 #include <secport.h>
@@ -29,7 +30,7 @@ std::string Stringize(char* nss_text, const std::string& alternative_text) {
   return !s.empty() ? s : alternative_text;
 }
 
-std::string GetNickname(net::X509Certificate::OSCertHandle cert_handle) {
+std::string GetNickname(CERTCertificate* cert_handle) {
   if (!cert_handle->nickname)
     return std::string();
   std::string name = cert_handle->nickname;
@@ -43,7 +44,7 @@ std::string GetNickname(net::X509Certificate::OSCertHandle cert_handle) {
 
 }  // namespace
 
-net::CertType GetCertType(net::X509Certificate::OSCertHandle cert_handle) {
+net::CertType GetCertType(CERTCertificate* cert_handle) {
   CERTCertTrust trust = {0};
   CERT_GetCertTrust(cert_handle, &trust);
 
@@ -63,28 +64,26 @@ net::CertType GetCertType(net::X509Certificate::OSCertHandle cert_handle) {
   return net::OTHER_CERT;
 }
 
-std::string GetCertTokenName(net::X509Certificate::OSCertHandle cert_handle) {
+std::string GetCertTokenName(CERTCertificate* cert_handle) {
   std::string token;
   if (cert_handle->slot)
     token = PK11_GetTokenName(cert_handle->slot);
   return token;
 }
 
-std::string GetIssuerCommonName(net::X509Certificate::OSCertHandle cert_handle,
+std::string GetIssuerCommonName(CERTCertificate* cert_handle,
                                 const std::string& alternative_text) {
   return Stringize(CERT_GetCommonName(&cert_handle->issuer), alternative_text);
 }
 
-std::string GetCertNameOrNickname(
-    net::X509Certificate::OSCertHandle cert_handle) {
+std::string GetCertNameOrNickname(CERTCertificate* cert_handle) {
   std::string name = GetCertAsciiNameOrNickname(cert_handle);
   if (!name.empty())
     name = base::UTF16ToUTF8(url_formatter::IDNToUnicode(name));
   return name;
 }
 
-std::string GetCertAsciiNameOrNickname(
-    net::X509Certificate::OSCertHandle cert_handle) {
+std::string GetCertAsciiNameOrNickname(CERTCertificate* cert_handle) {
   std::string alternative_text = GetNickname(cert_handle);
   return Stringize(CERT_GetCommonName(&cert_handle->subject), alternative_text);
 }

chromeos/network/certificate_helper.h

@@ -10,39 +10,35 @@
 
 #include "chromeos/chromeos_export.h"
 #include "net/cert/cert_type.h"
-#include "net/cert/x509_certificate.h"
 
 namespace chromeos {
 namespace certificate {
 
 // Selected functions from chrome/common/net/x509_certificate_model.cc
 
 // Decodes the certificate type from |cert|.
-CHROMEOS_EXPORT net::CertType GetCertType(
-    net::X509Certificate::OSCertHandle cert_handle);
+CHROMEOS_EXPORT net::CertType GetCertType(CERTCertificate* cert_handle);
 
 // Extracts the token name from |cert|->slot if it exists or returns an empty
 // string.
-CHROMEOS_EXPORT std::string GetCertTokenName(
-    net::X509Certificate::OSCertHandle cert_handle);
+CHROMEOS_EXPORT std::string GetCertTokenName(CERTCertificate* cert_handle);
 
 // Returns the common name for |cert_handle|->issuer or |alternative_text| if
 // the common name is missing or empty.
 CHROMEOS_EXPORT std::string GetIssuerCommonName(
-    net::X509Certificate::OSCertHandle cert_handle,
+    CERTCertificate* cert_handle,
     const std::string& alternative_text);
 
 // Returns the common name for |cert_handle|->subject converted to unicode,
 // or |cert_handle|->nickname if the subject name is unavailable or empty.
 // NOTE: Unlike x509_certificate_model::GetCertNameOrNickname in src/chrome,
 // the raw unformated name or nickname will not be included in the returned
 // value (see GetCertAsciiNameOrNickname instead).
-CHROMEOS_EXPORT std::string GetCertNameOrNickname(
-    net::X509Certificate::OSCertHandle cert_handle);
+CHROMEOS_EXPORT std::string GetCertNameOrNickname(CERTCertificate* cert_handle);
 
 // Returns the unformated ASCII common name for |cert_handle|->subject.
 CHROMEOS_EXPORT std::string GetCertAsciiNameOrNickname(
-    net::X509Certificate::OSCertHandle cert_handle);
+    CERTCertificate* cert_handle);
 
 }  // namespace certificate
 }  // namespace chromeos

chromeos/network/certificate_helper_unittest.cc

@@ -13,34 +13,32 @@
 namespace chromeos {
 
 TEST(CertificateHelperTest, GetCertNameOrNickname) {
-  scoped_refptr<net::X509Certificate> cert(net::ImportCertFromFile(
+  net::ScopedCERTCertificate cert(net::ImportCERTCertificateFromFile(
       net::GetTestCertsDirectory(), "root_ca_cert.pem"));
   ASSERT_TRUE(cert.get());
-  EXPECT_EQ("Test Root CA",
-            certificate::GetCertNameOrNickname(cert->os_cert_handle()));
+  EXPECT_EQ("Test Root CA", certificate::GetCertNameOrNickname(cert.get()));
 
-  scoped_refptr<net::X509Certificate> punycode_cert(net::ImportCertFromFile(
+  net::ScopedCERTCertificate punycode_cert(net::ImportCERTCertificateFromFile(
       net::GetTestCertsDirectory(), "punycodetest.pem"));
   ASSERT_TRUE(punycode_cert.get());
-  EXPECT_EQ("xn--wgv71a119e.com", certificate::GetCertAsciiNameOrNickname(
-                                      punycode_cert->os_cert_handle()));
-  EXPECT_EQ("日本語.com", certificate::GetCertNameOrNickname(
-                              punycode_cert->os_cert_handle()));
+  EXPECT_EQ("xn--wgv71a119e.com",
+            certificate::GetCertAsciiNameOrNickname(punycode_cert.get()));
+  EXPECT_EQ("日本語.com",
+            certificate::GetCertNameOrNickname(punycode_cert.get()));
 
-  scoped_refptr<net::X509Certificate> no_cn_cert(net::ImportCertFromFile(
+  net::ScopedCERTCertificate no_cn_cert(net::ImportCERTCertificateFromFile(
       net::GetTestCertsDirectory(), "no_subject_common_name_cert.pem"));
   ASSERT_TRUE(no_cn_cert.get());
   // Temp cert has no nickname.
-  EXPECT_EQ("",
-            certificate::GetCertNameOrNickname(no_cn_cert->os_cert_handle()));
+  EXPECT_EQ("", certificate::GetCertNameOrNickname(no_cn_cert.get()));
 }
 
 TEST(CertificateHelperTest, GetTypeCA) {
-  scoped_refptr<net::X509Certificate> cert(net::ImportCertFromFile(
+  net::ScopedCERTCertificate cert(net::ImportCERTCertificateFromFile(
       net::GetTestCertsDirectory(), "root_ca_cert.pem"));
   ASSERT_TRUE(cert.get());
 
-  EXPECT_EQ(net::CA_CERT, certificate::GetCertType(cert->os_cert_handle()));
+  EXPECT_EQ(net::CA_CERT, certificate::GetCertType(cert.get()));
 
   crypto::ScopedTestNSSDB test_nssdb;
   net::NSSCertDatabase db(crypto::ScopedPK11Slot(PK11_ReferenceSlot(
@@ -53,19 +51,19 @@ TEST(CertificateHelperTest, GetTypeCA) {
   EXPECT_TRUE(db.SetCertTrust(cert.get(), net::CA_CERT,
                               net::NSSCertDatabase::DISTRUSTED_SSL));
 
-  EXPECT_EQ(net::CA_CERT, certificate::GetCertType(cert->os_cert_handle()));
+  EXPECT_EQ(net::CA_CERT, certificate::GetCertType(cert.get()));
 }
 
 TEST(CertificateHelperTest, GetTypeServer) {
-  scoped_refptr<net::X509Certificate> cert(net::ImportCertFromFile(
+  net::ScopedCERTCertificate cert(net::ImportCERTCertificateFromFile(
       net::GetTestCertsDirectory(), "google.single.der"));
   ASSERT_TRUE(cert.get());
 
   // Test mozilla_security_manager::GetCertType with server certs and default
   // trust.  Currently this doesn't work.
   // TODO(mattm): make mozilla_security_manager::GetCertType smarter so we can
   // tell server certs even if they have no trust bits set.
-  EXPECT_EQ(net::OTHER_CERT, certificate::GetCertType(cert->os_cert_handle()));
+  EXPECT_EQ(net::OTHER_CERT, certificate::GetCertType(cert.get()));
 
   crypto::ScopedTestNSSDB test_nssdb;
   net::NSSCertDatabase db(crypto::ScopedPK11Slot(PK11_ReferenceSlot(
@@ -77,13 +75,13 @@ TEST(CertificateHelperTest, GetTypeServer) {
   EXPECT_TRUE(db.SetCertTrust(cert.get(), net::SERVER_CERT,
                               net::NSSCertDatabase::TRUSTED_SSL));
 
-  EXPECT_EQ(net::SERVER_CERT, certificate::GetCertType(cert->os_cert_handle()));
+  EXPECT_EQ(net::SERVER_CERT, certificate::GetCertType(cert.get()));
 
   // Test GetCertType with server certs and explicit distrust.
   EXPECT_TRUE(db.SetCertTrust(cert.get(), net::SERVER_CERT,
                               net::NSSCertDatabase::DISTRUSTED_SSL));
 
-  EXPECT_EQ(net::SERVER_CERT, certificate::GetCertType(cert->os_cert_handle()));
+  EXPECT_EQ(net::SERVER_CERT, certificate::GetCertType(cert.get()));
 }
 
 }  // namespace chromeos