This applies GUIDs to certificate and key nicknames when

net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp

Index: net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp
diff --git a/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp b/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp
index 7781bf76a7a5e5e663e1874e79093000eb5448ed..a306028a37a9737d92ef1ad2d282a56a5a8d0a8d 100644
--- a/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp
+++ b/net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp
@@ -57,7 +57,7 @@ namespace {
 //
 // For the NSS PKCS#12 library, must convert PRUnichars (shorts) to
 // a buffer of octets.  Must handle byte order correctly.
-// TODO: Is there a mozilla way to do this?  In the string lib?
+// TODO: Is there a Mozilla way to do this?  In the string lib?
 void unicodeToItem(const PRUnichar *uni, SECItem *item)
 {
   int len = 0;
@@ -138,7 +138,7 @@ pip_ucs2_ascii_conversion_fn(PRBool toUnicode,
                              PRBool swapBytes)
 {
   CHECK_GE(maxOutBufLen, inBufLen);
-  // do a no-op, since I've already got unicode.  Hah!
+  // do a no-op, since I've already got Unicode.  Hah!

[wtc, 2011/11/29 23:13:57]

In general it's not necessary to fix minor comment problems
in third-party code.  We have forked these files, so this is
fine.

   *outBufLen = inBufLen;
   memcpy(outBuf, inBuf, inBufLen);
   return PR_TRUE;
@@ -151,7 +151,8 @@ nsPKCS12Blob_ImportHelper(const char* pkcs12_data,
                           const string16& password,
                           bool is_extractable,
                           bool try_zero_length_secitem,
-                          PK11SlotInfo *slot)
+                          PK11SlotInfo *slot,
+                          net::CertificateList* imported_certs)
 {
   DCHECK(pkcs12_data);
   DCHECK(slot);
@@ -159,6 +160,10 @@ nsPKCS12Blob_ImportHelper(const char* pkcs12_data,
   SECStatus srv = SECSuccess;
   SEC_PKCS12DecoderContext *dcx = NULL;
   SECItem unicodePw;
+  SECItem attribute_value;
+  CK_BBOOL attribute_data = CK_FALSE;
+  const SEC_PKCS12DecoderItem* decoder_item = NULL;
+
   unicodePw.type = siBuffer;
   unicodePw.len = 0;
   unicodePw.data = NULL;
@@ -188,57 +193,68 @@ nsPKCS12Blob_ImportHelper(const char* pkcs12_data,
   // validate bags
   srv = SEC_PKCS12DecoderValidateBags(dcx, nickname_collision);
   if (srv) goto finish;
-  // import cert and key
+  // import certificate and key
   srv = SEC_PKCS12DecoderImportBags(dcx);
   if (srv) goto finish;
 
-  if (!is_extractable) {
-    SECItem attribute_value;
-    CK_BBOOL attribute_data = CK_FALSE;
-    attribute_value.data = &attribute_data;
-    attribute_value.len = sizeof(attribute_data);
+  attribute_value.data = &attribute_data;
+  attribute_value.len = sizeof(attribute_data);
 
-    srv = SEC_PKCS12DecoderIterateInit(dcx);
-    if (srv) goto finish;
+  srv = SEC_PKCS12DecoderIterateInit(dcx);
+  if (srv) goto finish;
+
+  if (imported_certs)
+    imported_certs->clear();
+
+  // Collect the list of decoded certificates, and mark private keys
+  // non-extractable if needed.
+  while (SEC_PKCS12DecoderIterateNext(dcx, &decoder_item) == SECSuccess) {
+    if (decoder_item->type != SEC_OID_PKCS12_V1_CERT_BAG_ID)
+      continue;
+    if (!decoder_item->hasKey)
+      continue;

[wtc, 2011/11/29 23:13:57]

BUG: I would expect that a certificate without a corresponding
private key should also be added to imported_certs.

[Greg Spencer (Chromium), 2011/12/02 18:50:07]

Oooh.   Nice catch. Done.

+
+    // Once we have determined that the imported certificate has an
+    // associated private key too, only then can we mark the key as
+    // non-extractable.
+    CERTCertificate* cert = PK11_FindCertFromDERCertItem(
+        slot, decoder_item->der,
+        NULL);  // wincx
+    if (!cert) {
+      LOG(ERROR) << "Could not grab a handle to the certificate in the slot "
+                 << "from the corresponding PKCS#12 DER certificate.";
+      continue;
+    }
+
+    // Add the cert to the list
+    if (imported_certs) {
+      // Empty list of intermediates.
+      net::X509Certificate::OSCertHandles intermediates;
+      imported_certs->push_back(
+          net::X509Certificate::CreateFromHandle(cert, intermediates));
+    }
 
-    const SEC_PKCS12DecoderItem* decoder_item = NULL;
     // Iterate through all the imported PKCS12 items and mark any accompanying
-    // private keys as unextractable.
-    while (SEC_PKCS12DecoderIterateNext(dcx, &decoder_item) == SECSuccess) {
-      if (decoder_item->type != SEC_OID_PKCS12_V1_CERT_BAG_ID)
-        continue;
-      if (!decoder_item->hasKey)
-        continue;
-
-      // Once we have determined that the imported certificate has an
-      // associated private key too, only then can we mark the key as
-      // unextractable.
-      CERTCertificate* cert = PK11_FindCertFromDERCertItem(
-          slot, decoder_item->der,
-          NULL);  // wincx
-      if (!cert) {
-        LOG(ERROR) << "Could not grab a handle to the certificate in the slot "
-                   << "from the corresponding PKCS#12 DER certificate.";
-        continue;
-      }
+    // private keys as non-extractable.
+    if (!is_extractable) {
       SECKEYPrivateKey* privKey = PK11_FindPrivateKeyFromCert(slot, cert,
                                                               NULL);  // wincx
-      CERT_DestroyCertificate(cert);
       if (privKey) {
-        // Mark the private key as unextractable.
+        // Mark the private key as non-extractable.
         srv = PK11_WriteRawAttribute(PK11_TypePrivKey, privKey, CKA_EXTRACTABLE,
                                      &attribute_value);
         SECKEY_DestroyPrivateKey(privKey);
         if (srv) {
           LOG(ERROR) << "Could not set CKA_EXTRACTABLE attribute on private "
                      << "key.";
+          CERT_DestroyCertificate(cert);
           break;
         }
       }
     }
+    CERT_DestroyCertificate(cert);
     if (srv) goto finish;
   }
-
   import_result = net::OK;
 finish:
   // If srv != SECSuccess, NSS probably set a specific error code.
@@ -335,10 +351,12 @@ int nsPKCS12Blob_Import(PK11SlotInfo* slot,
                         const char* pkcs12_data,
                         size_t pkcs12_len,
                         const string16& password,
-                        bool is_extractable) {
+                        bool is_extractable,
+                        net::CertificateList* imported_certs) {
 
   int rv = nsPKCS12Blob_ImportHelper(pkcs12_data, pkcs12_len, password,
-                                     is_extractable, false, slot);
+                                     is_extractable, false, slot,
+                                     imported_certs);
 
   // When the user entered a zero length password:
   //   An empty password should be represented as an empty
@@ -350,7 +368,7 @@ int nsPKCS12Blob_Import(PK11SlotInfo* slot,
   //   flavors.
   if (rv == net::ERR_PKCS12_IMPORT_BAD_PASSWORD && password.empty()) {
     rv = nsPKCS12Blob_ImportHelper(pkcs12_data, pkcs12_len, password,
-                                   is_extractable, true, slot);
+                                   is_extractable, true, slot, imported_certs);
   }
   return rv;
 }