Resolve certificate references in ONC by PEM.

chromeos/network/onc/onc_certificate_importer.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/network/onc/onc_certificate_importer.h"
 
 #include <cert.h>
 #include <keyhi.h>
 #include <pk11pub.h>
 
@@ skipping 15 matching lines @@
 
 namespace chromeos {
 namespace onc {
 
 CertificateImporter::CertificateImporter(bool allow_trust_imports)
     : allow_trust_imports_(allow_trust_imports) {
 }
 
 CertificateImporter::ParseResult CertificateImporter::ParseAndStoreCertificates(
     const base::ListValue& certificates,
-    net::CertificateList* onc_trusted_certificates) {
+    net::CertificateList* onc_trusted_certificates,
+    CertsByGUID* imported_server_and_ca_certs) {
   size_t successful_imports = 0;
   for (size_t i = 0; i < certificates.GetSize(); ++i) {
     const base::DictionaryValue* certificate = NULL;
     certificates.GetDictionary(i, &certificate);
     DCHECK(certificate != NULL);
 
     VLOG(2) << "Parsing certificate at index " << i << ": " << *certificate;
 
-    if (!ParseAndStoreCertificate(*certificate, onc_trusted_certificates)) {
+    if (!ParseAndStoreCertificate(*certificate, onc_trusted_certificates,
+                                  imported_server_and_ca_certs)) {
       ONC_LOG_ERROR(
           base::StringPrintf("Cannot parse certificate at index %zu", i));
     } else {
       VLOG(2) << "Successfully imported certificate at index " << i;
       ++successful_imports;
     }
   }
 
   if (successful_imports == certificates.GetSize()) {
     return IMPORT_OK;
@@ skipping 57 matching lines @@
     // delete could fail, but still...  The other choice is to return
     // failure immediately, but that doesn't seem to do what is intended.
     if (!net::NSSCertDatabase::GetInstance()->DeleteCertAndKey(iter->get()))
       result = false;
   }
   return result;
 }
 
 bool CertificateImporter::ParseAndStoreCertificate(
     const base::DictionaryValue& certificate,
-    net::CertificateList* onc_trusted_certificates) {
+    net::CertificateList* onc_trusted_certificates,
+    CertsByGUID* imported_server_and_ca_certs) {
   // Get out the attributes of the given certificate.
   std::string guid;
   certificate.GetStringWithoutPathExpansion(certificate::kGUID, &guid);
   DCHECK(!guid.empty());
 
   bool remove = false;
   if (certificate.GetBooleanWithoutPathExpansion(kRemove, &remove) && remove) {
     if (!DeleteCertAndKeyByNickname(guid)) {
       ONC_LOG_ERROR("Unable to delete certificate");
       return false;
     } else {
       return true;
     }
   }
 
   // Not removing, so let's get the data we need to add this certificate.
   std::string cert_type;
   certificate.GetStringWithoutPathExpansion(certificate::kType, &cert_type);
   if (cert_type == certificate::kServer ||
       cert_type == certificate::kAuthority) {
-    return ParseServerOrCaCertificate(
-        cert_type, guid, certificate, onc_trusted_certificates);
+    return ParseServerOrCaCertificate(cert_type, guid, certificate,
+                                      onc_trusted_certificates,
+                                      imported_server_and_ca_certs);
   } else if (cert_type == certificate::kClient) {
     return ParseClientCertificate(guid, certificate);
   }
 
   NOTREACHED();
   return false;
 }
 
 bool CertificateImporter::ParseServerOrCaCertificate(
     const std::string& cert_type,
     const std::string& guid,
     const base::DictionaryValue& certificate,
-    net::CertificateList* onc_trusted_certificates) {
+    net::CertificateList* onc_trusted_certificates,
+    CertsByGUID* imported_server_and_ca_certs) {
   bool web_trust_flag = false;
   const base::ListValue* trust_list = NULL;
   if (certificate.GetListWithoutPathExpansion(certificate::kTrustBits,
                                               &trust_list)) {
     for (base::ListValue::const_iterator it = trust_list->begin();
          it != trust_list->end(); ++it) {
       std::string trust_type;
       if (!(*it)->GetAsString(&trust_type))
         NOTREACHED();
 
@@ skipping 22 matching lines @@
   if (!certificate.GetStringWithoutPathExpansion(certificate::kX509,
                                                  &x509_data) ||
       x509_data.empty()) {
     ONC_LOG_ERROR(
         "Certificate missing appropriate certificate data for type: " +
         cert_type);
     return false;
   }
 
   scoped_refptr<net::X509Certificate> x509_cert =
-      DecodePEMCertificate(x509_data, guid);
+      DecodePEMCertificate(x509_data);
   if (!x509_cert.get()) {
     ONC_LOG_ERROR("Unable to create certificate from PEM encoding, type: " +
                   cert_type);
     return false;
   }
 
-  // Due to a mismatch regarding cert identity between NSS (cert identity is
-  // determined by the raw bytes) and ONC (cert identity is determined by
-  // GUIDs), we have to special-case a number of situations here:
-  //
-  // a) The cert bits we're trying to insert are already present in the NSS cert
-  //    store. This is indicated by the isperm bit in CERTCertificateStr. Since
-  //    we might have to update the nick name, we just delete the existing cert
-  //    and reimport the cert bits.
-  // b) NSS gives us an actual temporary certificate. In this case, there is no
-  //    identical certificate known to NSS, so we can safely import the
-  //    certificate. The GUID being imported may still be on a different
-  //    certificate, and we could jump through hoops to reimport the existing
-  //    certificate with a different nickname. However, that would mean lots of
-  //    effort for a case that's pretty much illegal (reusing GUIDs contradicts
-  //    the intention of GUIDs), so we just report an error.
-  //
-  // TODO(mnissler, gspencer): We should probably switch to a mode where we
-  // keep our own database for mapping GUIDs to certs in order to enable several
-  // GUIDs to map to the same cert. See http://crosbug.com/26073.
+  net::NSSCertDatabase::TrustBits trust = import_with_ssl_trust ?
+                                          net::NSSCertDatabase::TRUSTED_SSL :
+                                          net::NSSCertDatabase::TRUST_DEFAULT;
+
   net::NSSCertDatabase* cert_database = net::NSSCertDatabase::GetInstance();
   if (x509_cert->os_cert_handle()->isperm) {
-    if (!cert_database->DeleteCertAndKey(x509_cert.get())) {
-      ONC_LOG_ERROR("Unable to delete X509 certificate.");
+    VLOG(1) << "Certificate is already installed.";
+    if (!cert_database->SetCertTrust(

[Mattias Nissler (ping if slow), 2013/06/24 12:45:09]

Not sure we should be doing this. What difference does it make btw.?

[pneubeck (no reviews), 2013/06/24 15:35:41]

Good point.
This is required only to modify trust by user ONC import (through
about:net-internals).
We could remove this if that feature wouldn't be required (no idea if it is).
Instead, I ensure now that we don't try to call this function during policy
import (allow_trust_imports_ = false).

+            x509_cert,
+            cert_type == certificate::kServer ? net::SERVER_CERT : net::CA_CERT,
+            trust)) {
+      ONC_LOG_ERROR("Certificate of type " + cert_type +
+                    " was already present, but trust couldn't be set.");
+    }
+  } else {
+    net::CertificateList cert_list;
+    cert_list.push_back(x509_cert);
+    net::NSSCertDatabase::ImportCertFailureList failures;
+    bool success = false;
+    if (cert_type == certificate::kServer)
+      success = cert_database->ImportServerCert(cert_list, trust, &failures);
+    else  // Authority cert
+      success = cert_database->ImportCACerts(cert_list, trust, &failures);
+
+    if (!failures.empty()) {
+      ONC_LOG_ERROR(
+          base::StringPrintf("Error ( %s ) importing %s certificate",
+                             net::ErrorToString(failures[0].net_error),
+                             cert_type.c_str()));
       return false;
     }
 
-    // Reload the cert here to get an actual temporary cert instance.
-    x509_cert = DecodePEMCertificate(x509_data, guid);
-    if (!x509_cert.get()) {
-      ONC_LOG_ERROR("Unable to create X509 certificate from bytes.");
+    if (!success) {
+      ONC_LOG_ERROR("Unknown error importing " + cert_type + " certificate.");
       return false;
     }
-    DCHECK(!x509_cert->os_cert_handle()->isperm);
-    DCHECK(x509_cert->os_cert_handle()->istemp);
-  }
-
-  // Make sure the GUID is not already taken. Note that for the reimport case we
-  // have removed the existing cert above.
-  net::CertificateList certs;
-  ListCertsWithNickname(guid, &certs);
-  if (!certs.empty()) {
-    ONC_LOG_ERROR("Certificate GUID is already in use: " + guid);
-    return false;
-  }
-
-  net::CertificateList cert_list;
-  cert_list.push_back(x509_cert);
-  net::NSSCertDatabase::ImportCertFailureList failures;
-  bool success = false;
-  net::NSSCertDatabase::TrustBits trust = import_with_ssl_trust ?
-                                          net::NSSCertDatabase::TRUSTED_SSL :
-                                          net::NSSCertDatabase::TRUST_DEFAULT;
-  if (cert_type == certificate::kServer) {
-    success = cert_database->ImportServerCert(cert_list, trust, &failures);
-  } else {  // Authority cert
-    success = cert_database->ImportCACerts(cert_list, trust, &failures);
-  }
-
-  if (!failures.empty()) {
-    ONC_LOG_ERROR(base::StringPrintf("Error ( %s ) importing %s certificate",
-                                     net::ErrorToString(failures[0].net_error),
-                                     cert_type.c_str()));
-    return false;
-  }
-  if (!success) {
-    ONC_LOG_ERROR("Unknown error importing " + cert_type + " certificate.");
-    return false;
   }
 
   if (web_trust_flag && onc_trusted_certificates)
     onc_trusted_certificates->push_back(x509_cert);
 
+  if (imported_server_and_ca_certs)
+    (*imported_server_and_ca_certs)[guid] = x509_cert;
+
   return true;
 }
 
 bool CertificateImporter::ParseClientCertificate(
     const std::string& guid,
     const base::DictionaryValue& certificate) {
   std::string pkcs12_data;
   if (!certificate.GetStringWithoutPathExpansion(certificate::kPKCS12,
                                                  &pkcs12_data) ||
       pkcs12_data.empty()) {
@@ skipping 44 matching lines @@
     PK11_SetPrivateKeyNickname(private_key, const_cast<char*>(guid.c_str()));
     SECKEY_DestroyPrivateKey(private_key);
   } else {
     ONC_LOG_WARNING("Unable to find private key for certificate.");
   }
   return true;
 }
 
 }  // namespace onc
 }  // namespace chromeos