This applies GUIDs to certificate and key nicknames when

net/base/x509_util_nss.cc

Index: net/base/x509_util_nss.cc
diff --git a/net/base/x509_util_nss.cc b/net/base/x509_util_nss.cc
index fe3fb1731cb87297f228563892a69f6cbb8dc135..ed0927c0c0b09549a05ed0206a0dd2414f9515af 100644
--- a/net/base/x509_util_nss.cc
+++ b/net/base/x509_util_nss.cc
@@ -20,6 +20,10 @@
 #include "crypto/nss_util_internal.h"
 #include "crypto/rsa_private_key.h"
 #include "crypto/scoped_nss_types.h"
+#include "net/base/x509_certificate.h"
+#include "net/third_party/mozilla_security_manager/nsNSSCertTrust.h"
+
+namespace msm = mozilla_security_manager;
 
 namespace {
 
@@ -166,7 +170,6 @@ bool SignCertificate(
 
   return true;
 }
-

[wtc, 2011/12/08 00:07:43]

Nit: keep this blank line.  This blank line matches the
blank line at line 25 above.

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

Done.

 }  // namespace
 
 namespace net {
@@ -313,6 +316,156 @@ bool CreateOriginBoundCert(
   return true;
 }
 
+CertType GetCertType(const X509Certificate* cert) {
+  DCHECK(cert);
+  msm::nsNSSCertTrust trust(cert->os_cert_handle()->trust);
+  if (trust.HasAnyUser())
+    return USER_CERT;
+  if (trust.HasAnyCA() || CERT_IsCACert(cert->os_cert_handle(), NULL))
+    return CA_CERT;
+  if (trust.HasPeer(PR_TRUE, PR_FALSE, PR_FALSE))
+    return SERVER_CERT;
+  return UNKNOWN_CERT;
+}
+
+std::string GetDefaultCertificateLabel(const X509Certificate* cert) {
+  DCHECK(cert);
+  std::string result;
+
+#if defined(OS_CHROMEOS)
+  // When we have label support, we want to keep any existing label by default.
+  result = GetLabel(cert);
+  if (!result.empty())
+    return result;
+#endif

[wtc, 2011/12/08 00:07:43]

This behavior (lines 335-340) is difficult to document
precisely in prose.  Why is this behavior necessary for
Chrome OS.

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

We talked about this: I've removed this from the function, and instead I return
a default nickname if there is one.

+
+  switch (GetCertType(cert)) {
+    case CA_CERT: {
+      char *nickname = CERT_MakeCANickname(cert->os_cert_handle());

[wtc, 2011/12/08 00:07:43]

Nit: put '*' next to 'char'.

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

Done.

+      result = nickname;
+      PORT_Free(nickname);
+      break;
+    }
+    case USER_CERT: {
+      // Create a nickname for this user certificate.
+      //  We use the scheme used by Firefox:

[wtc, 2011/12/08 00:07:43]

Nit: extraneous space before "We".

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

Fixed.

+      // --> <subject's common name>'s <issuer's common name> ID.
+      // TODO(gspencer): internationalize this: it's wrong to
+      // hard code English.
+
+      std::string username, ca_name;
+      char* temp_username = CERT_GetCommonName(
+          &cert->os_cert_handle()->subject);
+      char* temp_ca_name = CERT_GetCommonName(&cert->os_cert_handle()->issuer);
+      if (temp_username) {
+        username = temp_username;
+        PORT_Free(temp_username);
+      }
+      if (temp_ca_name) {
+        ca_name = temp_ca_name;
+        PORT_Free(temp_ca_name);
+      }
+      result = username + "'s " + ca_name + " ID";
+      break;
+    }
+    case SERVER_CERT: {
+      result = cert->subject().GetDisplayName();
+      break;
+    }

[wtc, 2011/12/08 00:07:43]

Nit: this case doesn't need curly braces because it doesn't
use local variables.

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

Done.

+    case UNKNOWN_CERT:
+    default:
+      break;
+  }
+  return result;
+}
+
+#if defined(OS_CHROMEOS)
+bool SetLabel(X509Certificate* cert, const std::string& label) {
+  DCHECK(cert);
+  // If the slot isn't initialized, then do nothing.
+  if (!cert->os_cert_handle()->slot)
+    return true;
+
+  // First we set the nickname on the cert itself.  This doesn't
+  // work on production NSS yet (it's not implemented), but ChromeOS
+  // has a patched version that it will work on.
+  SECItem sec_label;
+  sec_label.type = siUTF8String;

[wtc, 2011/12/08 00:07:43]

PK11_WriteRawAttribute does not use the 'type' field of this
SECItem, so I suggest we just set it to the default siBuffer.

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

Done.

+  sec_label.data = reinterpret_cast<unsigned char*>(
+      const_cast<char*>(label.c_str()));
+  sec_label.len = label.size();
+  SECStatus srv = PK11_WriteRawAttribute(PK11_TypeCert,
+                                         cert->os_cert_handle(),
+                                         CKA_LABEL,
+                                         &sec_label);

[wtc, 2011/12/08 00:07:43]

IMPORTANT: my concern about this solution is that the NSS
CERTCertificate structure has a 'nickname' field:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certdb/c...

but your code does not update the 'nickname' field, so it
will become out of sync with the CKA_LABEL attribute of the
underlying PKCS #11 certificate object.

Perhaps this is why NSS does not support setting the
CKA_LABEL attribute on a PKCS #11 certificate object.

I think a better solution might be to only set the CKA_LABEL
attribute on the private key object.  Once you find the right
private key object by the label, NSS can find the corresponding
certificate object.  Will this solution work for you?

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

I've stopped trying to set the nickname later, and instead I rely on setting the
nickname at creation time for server and CA certs.  For client certs, I set the
nickname on the private key (which has a PK11 interface).
Yes, this is what I have adopted for client certificates.  Server and CA certs
have their nicknames set at creation time.

+  if (srv != SECSuccess)
+    LOG(WARNING) << "Unable to set certificate label to "
+                 << label;

[wtc, 2011/12/08 00:07:43]

Nit: please add curly braces because the statement is two
lines.

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

Done.

+
+  // As far as I can tell, there is no API for PKCS11 that allows one
+  // to get the valid public key (that has a valid PKCS11 slot and id)
+  // associated with a certificate.  So, instead, I extract the public
+  // key using CERT_ExtractPublicKey (which returns a key that has no
+  // slot or pkcs11 id set), and then we iterate through all of the
+  // existing public keys (which do have this information), and look
+  // for one that has the same DER encoding as the public key
+  // extracted from this certificate.  I then set that key's nickname
+  // to the given label.
+  SECKEYPublicKey* public_key = CERT_ExtractPublicKey(cert->os_cert_handle());
+  if (!public_key)
+    return false;
+
+  SECKEYPublicKeyList* pubkey_list =
+      PK11_ListPublicKeysInSlot(cert->os_cert_handle()->slot, NULL);
+
+  // If there are no public keys, that's OK.
+  if (pubkey_list) {
+    for (SECKEYPublicKeyListNode* node = PUBKEY_LIST_HEAD(pubkey_list);
+        !PUBKEY_LIST_END(node, pubkey_list);
+        node = PUBKEY_LIST_NEXT(node)) {
+      SECItem* der_encoded = PK11_DEREncodePublicKey(node->key);
+      if (SECITEM_CompareItem(
+          der_encoded,
+          &cert->os_cert_handle()->derPublicKey) == SECEqual)
+        PK11_SetPublicKeyNickname(node->key, label.c_str());
+      SECITEM_FreeItem(der_encoded, PR_TRUE);
+    }
+    SECKEY_DestroyPublicKeyList(pubkey_list);
+  }
+  SECKEY_DestroyPublicKey(public_key);
+
+  // Now set the nickname on the private key (if there is one)
+  SECKEYPrivateKey* private_key =
+      PK11_FindPrivateKeyFromCert(cert->os_cert_handle()->slot,
+                                  cert->os_cert_handle(),
+                                  NULL);
+  if (private_key) {
+    PK11_SetPrivateKeyNickname(private_key, label.c_str());
+    SECKEY_DestroyPrivateKey(private_key);
+  }
+  return true;
+}
+
+std::string GetLabel(const X509Certificate* cert) {
+  std::string result;
+  // This doesn't work on production NSS yet (it's not implemented), but
+  // ChromeOS has a patched version that it will work on.
+  SECItem sec_label;
+  SECStatus srv = PK11_ReadRawAttribute(PK11_TypeCert,
+                                        cert->os_cert_handle(),
+                                        CKA_LABEL,
+                                        &sec_label);

[wtc, 2011/12/08 00:07:43]

Why don't you just use the 'nickname' field of the NSS
CERTCertificate structure?  You can strip the "<token-name>:"
prefix if necessary.

[Greg Spencer (Chromium), 2011/12/09 18:51:38]

For server and CA certs, this works because I can supply the nickname at
creation time.  For client certs, it's not so simple.  I have switched to using
this model for server and CA certs, and for storing it on the private key for
client certs.

+  if (srv != SECSuccess) {
+    return result;
+  }
+
+  result = std::string(reinterpret_cast<char *>(sec_label.data), sec_label.len);
+  PORT_Free(sec_label.data);
+  return result;
+}
+#endif  // OS_CHROMEOS
+
+
 } // namespace x509_util
 
 } // namespace net