bug 857627 - 2/4: remove nsIX509Cert.nickname r?Cykesiopka,jcj draft
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 18 Nov 2016 13:12:29 -0800
changeset 446746 57fe50c26b19b9becaf01dab941ebc3f94432aca
parent 446745 00dbeb049b3c4b1fb9e915f3304954a8df46b4f2
child 446747 c98c247f2fc94221a3a0f65db32b50c560e876a5
push id37861
push userdkeeler@mozilla.com
push dateThu, 01 Dec 2016 19:20:01 +0000
reviewersCykesiopka, jcj
bugs857627
milestone53.0a1
bug 857627 - 2/4: remove nsIX509Cert.nickname r?Cykesiopka,jcj In general, any code that was using nsIX509Cert.nickname should be able to use the attribute displayName (if using nickname for display purposes) or the attribute dbKey (if using nickname as a unique identifier for a certificate). MozReview-Commit-ID: G9CfMJDfLqe
mobile/android/components/NSSDialogService.js
security/manager/locales/en-US/chrome/pipnss/pipnss.properties
security/manager/pki/resources/content/clientauthask.js
security/manager/ssl/nsCertTree.cpp
security/manager/ssl/nsIX509Cert.idl
security/manager/ssl/nsNSSCertificate.cpp
security/manager/ssl/nsNSSCertificateFakeTransport.cpp
security/manager/ssl/tests/mochitest/browser/browser_clientAuth_ui.js
security/manager/ssl/tests/unit/test_local_cert.js
security/manager/ssl/tests/unit/test_nsIX509Cert_utf8.js
security/manager/tools/dumpGoogleRoots.js
security/manager/tools/genHPKPStaticPins.js
--- a/mobile/android/components/NSSDialogService.js
+++ b/mobile/android/components/NSSDialogService.js
@@ -224,17 +224,17 @@ NSSDialogs.prototype = {
       this.formatString("clientAuthAsk.issuer", [issuerOrg]),
     ].join("<br/>");
 
     let certNickList = [];
     let certDetailsList = [];
     for (let i = 0; i < certList.length; i++) {
       let cert = certList.queryElementAt(i, Ci.nsIX509Cert);
       certNickList.push(this.formatString("clientAuthAsk.nickAndSerial",
-                                          [cert.nickname, cert.serialNumber]));
+                                          [cert.displayName, cert.serialNumber]));
       certDetailsList.push(this.getCertDetails(cert));
     }
 
     selectedIndex.value = 0;
     while (true) {
       let buttons = [
         this.getString("nssdialogs.ok.label"),
         this.getString("clientAuthAsk.viewCert.label"),
--- a/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
+++ b/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
@@ -279,17 +279,16 @@ certErrorExpiredNow=The certificate expi
 # LOCALIZATION NOTE (certErrorNotYetValidNow): Do not translate %1$S (date+time certificate will become valid) or %2$S (current date+time)
 certErrorNotYetValidNow=The certificate will not be valid until %1$S. The current time is %2$S.
 
 # LOCALIZATION NOTE (certErrorCodePrefix2): Do not translate <a id="errorCode" title="%1$S">%1$S</a>
 certErrorCodePrefix2=Error code: <a id="errorCode" title="%1$S">%1$S</a>
 
 P12DefaultNickname=Imported Certificate
 CertUnknown=Unknown
-CertNoNickname=(no nickname)
 CertNoEmailAddress=(no email address)
 CaCertExists=This certificate is already installed as a certificate authority.
 NotACACert=This is not a certificate authority certificate, so it can’t be imported into the certificate authority list.
 NotImportingUnverifiedCert=This certificate can’t be verified and will not be imported. The certificate issuer might be unknown or untrusted, the certificate might have expired or been revoked, or the certificate might not have been approved.
 UserCertIgnoredNoPrivateKey=This personal certificate can’t be installed because you do not own the corresponding private key which was created when the certificate was requested.
 UserCertImported=Your personal certificate has been installed. You should keep a backup copy of this certificate.
 CertOrgUnknown=(Unknown)
 CertNotStored=(Not Stored)
--- a/security/manager/pki/resources/content/clientauthask.js
+++ b/security/manager/pki/resources/content/clientauthask.js
@@ -84,17 +84,17 @@ function onLoad() {
 
   let selectElement = document.getElementById("nicknames");
   certArray = window.arguments[4].QueryInterface(Ci.nsIArray);
   for (let i = 0; i < certArray.length; i++) {
     let menuItemNode = document.createElement("menuitem");
     let cert = certArray.queryElementAt(i, Ci.nsIX509Cert);
     let nickAndSerial =
       bundle.getFormattedString("clientAuthNickAndSerial",
-                                [cert.nickname, cert.serialNumber]);
+                                [cert.displayName, cert.serialNumber]);
     menuItemNode.setAttribute("value", i);
     menuItemNode.setAttribute("label", nickAndSerial); // This is displayed.
     selectElement.firstChild.appendChild(menuItemNode);
     if (i == 0) {
       selectElement.selectedItem = menuItemNode;
     }
   }
 
--- a/security/manager/ssl/nsCertTree.cpp
+++ b/security/manager/ssl/nsCertTree.cpp
@@ -1070,40 +1070,19 @@ nsCertTree::GetCellText(int32_t row, nsI
     if (myString) {
       myString->GetData(_retval);
       return NS_OK;
     }
   }
 
   if (NS_LITERAL_STRING("certcol").Equals(colID)) {
     if (!cert) {
-      mNSSComponent->GetPIPNSSBundleString("CertNotStored", _retval);
-    }
-    else {
-      rv = cert->GetCommonName(_retval);
-      if (NS_FAILED(rv) || _retval.IsEmpty()) {
-        // kaie: I didn't invent the idea to cut off anything before 
-        //       the first colon. :-)
-        nsAutoString nick;
-        rv = cert->GetNickname(nick);
-        
-        nsAString::const_iterator start, end, end2;
-        nick.BeginReading(start);
-        nick.EndReading(end);
-        end2 = end;
-  
-        if (FindInReadable(NS_LITERAL_STRING(":"), start, end)) {
-          // found. end points to the first char after the colon,
-          // that's what we want.
-          _retval = Substring(end, end2);
-        }
-        else {
-          _retval = nick;
-        }
-      }
+      rv = mNSSComponent->GetPIPNSSBundleString("CertNotStored", _retval);
+    } else {
+      rv = cert->GetDisplayName(_retval);
     }
   } else if (NS_LITERAL_STRING("tokencol").Equals(colID) && cert) {
     rv = cert->GetTokenName(_retval);
   } else if (NS_LITERAL_STRING("emailcol").Equals(colID) && cert) {
     rv = cert->GetEmailAddress(_retval);
   } else if (NS_LITERAL_STRING("issuedcol").Equals(colID) && cert) {
     nsCOMPtr<nsIX509CertValidity> validity;
 
--- a/security/manager/ssl/nsIX509Cert.idl
+++ b/security/manager/ssl/nsIX509Cert.idl
@@ -24,21 +24,16 @@ interface nsICertVerificationListener;
  *       change this uuid you probably need a hack in nsBinaryInputStream to
  *       read the old uuid.  If you change the format of the object
  *       serialization then more complex changes will be needed.
  */
 [scriptable, uuid(bdc3979a-5422-4cd5-8589-696b6e96ea83)]
 interface nsIX509Cert : nsISupports {
 
   /**
-   *  A nickname for the certificate.
-   */
-  readonly attribute AString nickname;
-
-  /**
    *  The primary email address of the certificate, if present.
    */
   readonly attribute AString emailAddress;
 
   /**
    * Did this certificate ship with the platform as a built-in root?
    */
   readonly attribute bool isBuiltInRoot;
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -431,36 +431,16 @@ nsNSSCertificate::GetDisplayName(nsAStri
       return NS_OK;
     }
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsNSSCertificate::GetNickname(nsAString& aNickname)
-{
-  nsNSSShutDownPreventionLock locker;
-  if (isAlreadyShutDown())
-    return NS_ERROR_NOT_AVAILABLE;
-
-  if (mCert->nickname) {
-    CopyUTF8toUTF16(mCert->nickname, aNickname);
-  } else {
-    nsresult rv;
-    nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, &rv));
-    if (NS_FAILED(rv) || !nssComponent) {
-      return NS_ERROR_FAILURE;
-    }
-    nssComponent->GetPIPNSSBundleString("CertNoNickname", aNickname);
-  }
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 nsNSSCertificate::GetEmailAddress(nsAString& aEmailAddress)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   if (mCert->emailAddr) {
     CopyUTF8toUTF16(mCert->emailAddr, aEmailAddress);
--- a/security/manager/ssl/nsNSSCertificateFakeTransport.cpp
+++ b/security/manager/ssl/nsNSSCertificateFakeTransport.cpp
@@ -38,23 +38,16 @@ nsNSSCertificateFakeTransport::GetDbKey(
 NS_IMETHODIMP
 nsNSSCertificateFakeTransport::GetDisplayName(nsAString&)
 {
   NS_NOTREACHED("Unimplemented on content process");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
-nsNSSCertificateFakeTransport::GetNickname(nsAString&)
-{
-  NS_NOTREACHED("Unimplemented on content process");
-  return NS_ERROR_NOT_IMPLEMENTED;
-}
-
-NS_IMETHODIMP
 nsNSSCertificateFakeTransport::GetEmailAddress(nsAString&)
 {
   NS_NOTREACHED("Unimplemented on content process");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsNSSCertificateFakeTransport::GetEmailAddresses(uint32_t*, char16_t***)
--- a/security/manager/ssl/tests/mochitest/browser/browser_clientAuth_ui.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_clientAuth_ui.js
@@ -65,17 +65,17 @@ function checkDialogContents(win, notBef
   Assert.equal(win.document.getElementById("organization").textContent,
                `Organization: \u201C${TEST_ORG}\u201D`,
                "Actual and expected organization should be equal");
   Assert.equal(win.document.getElementById("issuer").textContent,
                `Issued Under: \u201C${TEST_ISSUER_ORG}\u201D`,
                "Actual and expected issuer organization should be equal");
 
   Assert.equal(win.document.getElementById("nicknames").label,
-               "test client certificate [03]",
+               "Mochitest client [03]",
                "Actual and expected selected cert nickname and serial should " +
                "be equal");
 
   let [subject, serialNum, validity, issuer, tokenName] =
     win.document.getElementById("details").value.split("\n");
   Assert.equal(subject, "Issued to: CN=Mochitest client",
                "Actual and expected subject should be equal");
   Assert.equal(serialNum, "Serial number: 03",
--- a/security/manager/ssl/tests/unit/test_local_cert.js
+++ b/security/manager/ssl/tests/unit/test_local_cert.js
@@ -44,21 +44,25 @@ function removeCert(nickname) {
   });
 }
 
 add_task(function* () {
   // No master password, so no prompt required here
   ok(!certService.loginPromptRequired);
 
   let certA = yield getOrCreateCert(gNickname);
-  equal(certA.nickname, gNickname);
+  // The local cert service implementation takes the given nickname and uses it
+  // as the common name for the certificate it creates. nsIX509Cert.displayName
+  // uses the common name if it is present, so these should match. Should either
+  // implementation change to do something else, this won't necessarily work.
+  equal(certA.displayName, gNickname);
 
   // Getting again should give the same cert
   let certB = yield getOrCreateCert(gNickname);
-  equal(certB.nickname, gNickname);
+  equal(certB.displayName, gNickname);
 
   // Should be matching instances
   ok(certA.equals(certB));
 
   // Check a few expected attributes
   ok(certA.isSelfSigned);
   equal(certA.certType, Ci.nsIX509Cert.USER_CERT);
 
--- a/security/manager/ssl/tests/unit/test_nsIX509Cert_utf8.js
+++ b/security/manager/ssl/tests/unit/test_nsIX509Cert_utf8.js
@@ -40,18 +40,16 @@ function run_test() {
     "MAowCAYGBACORgEBMA4GA1UdDwEB/wQEAwIE8DANBgkqhkiG9w0BAQUFAAOCAQEA" +
     "v2V+nnYYMIgabmmgHx49CtlZIHdGS3TuWKXw130xFhbXDnNhEbx3alaskNsvjQQR" +
     "Lqs1ZwKy58yynse+eJYHqenmHDACpAfVpCF9PXC/mDarVsoQw7NTcUpsAFhSd/zT" +
     "v9jIf3twECyxx/RVzONVcob7nPePESHiKoG4FbtcuUh0wSHvCmTwRIQqPDCIuHcF" +
     "StSt3Jr9iXcbXEhe4mSccOZ8N+r7Rv3ncKcevlRl7uFfDKDTyd43SZeRS/7J8KRf" +
     "hD/h2nawrCFwc5gJW10aLJGFL/mcS7ViAIT9HCVk23j4TuBjsVmnZ0VKxB5edux+" +
     "LIEqtU428UVHZWU/I5ngLw==");
 
-  equal(cert.nickname, "(no nickname)",
-        "Actual and expected nickname should match");
   equal(cert.emailAddress, "ludek.rasek@centrum.cz",
         "Actual and expected emailAddress should match");
   equal(cert.subjectName, "serialNumber=ICA - 10003769,SN=Rašek,name=Luděk Rašek,initials=LR,givenName=Luděk,E=ludek.rasek@centrum.cz,L=\"Pacov, Nádražní 769\",ST=Vysočina,CN=Luděk Rašek,C=CZ",
         "Actual and expected subjectName should match");
   equal(cert.commonName, "Luděk Rašek",
         "Actual and expected commonName should match");
   equal(cert.organization, "",
         "Actual and expected organization should match");
--- a/security/manager/tools/dumpGoogleRoots.js
+++ b/security/manager/tools/dumpGoogleRoots.js
@@ -53,30 +53,21 @@ function downloadRoots() {
     if (line == "-----BEGIN CERTIFICATE-----") {
       readingRoot = true;
     }
   }
   return roots;
 }
 
 function makeFormattedNickname(cert) {
-  if (cert.nickname.startsWith("Builtin Object Token:")) {
-    return `"${cert.nickname.substring("Builtin Object Token:".length)}"`;
+  if (cert.isBuiltInRoot) {
+    return `"${cert.displayName}"`;
   }
   // Otherwise, this isn't a built-in and we have to comment it out.
-  if (cert.commonName) {
-    return `// "${cert.commonName}"`;
-  }
-  if (cert.organizationalUnit) {
-    return `// "${cert.organizationalUnit}"`;
-  }
-  if (cert.organization) {
-    return `// "${cert.organization}"`;
-  }
-  throw new Error(`couldn't make nickname for ${cert.subjectName}`);
+  return `// "${cert.displayName}"`;
 }
 
 var roots = downloadRoots();
 var rootNicknames = [];
 for (var root of roots) {
   rootNicknames.push(makeFormattedNickname(root));
 }
 rootNicknames.sort(function(rootA, rootB) {
--- a/security/manager/tools/genHPKPStaticPins.js
+++ b/security/manager/tools/genHPKPStaticPins.js
@@ -23,17 +23,16 @@ var { 'classes': Cc, 'interfaces': Ci, '
 
 var { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
 var { FileUtils } = Cu.import("resource://gre/modules/FileUtils.jsm", {});
 var { Services } = Cu.import("resource://gre/modules/Services.jsm", {});
 
 var gCertDB = Cc["@mozilla.org/security/x509certdb;1"]
                 .getService(Ci.nsIX509CertDB);
 
-const BUILT_IN_NICK_PREFIX = "Builtin Object Token:";
 const SHA256_PREFIX = "sha256/";
 const GOOGLE_PIN_PREFIX = "GOOGLE_PIN_";
 
 // Pins expire in 14 weeks (6 weeks on Beta + 8 weeks on stable)
 const PINNING_MINIMUM_REQUIRED_MAX_AGE = 60 * 60 * 24 * 7 * 14;
 
 const FILE_HEADER = "/* This Source Code Form is subject to the terms of the Mozilla Public\n" +
 " * License, v. 2.0. If a copy of the MPL was not distributed with this\n" +
@@ -397,17 +396,17 @@ function loadNSSCertinfo(extraCertificat
   let enumerator = allCerts.getEnumerator();
   let certNameToSKD = {};
   let certSKDToName = {};
   while (enumerator.hasMoreElements()) {
     let cert = enumerator.getNext().QueryInterface(Ci.nsIX509Cert);
     if (!isCertBuiltIn(cert)) {
       continue;
     }
-    let name = cert.nickname.substr(BUILT_IN_NICK_PREFIX.length);
+    let name = cert.displayName;
     let SKD = cert.sha256SubjectPublicKeyInfoDigest;
     certNameToSKD[name] = SKD;
     certSKDToName[SKD] = name;
   }
 
   for (let cert of extraCertificates) {
     let name = cert.commonName;
     let SKD = cert.sha256SubjectPublicKeyInfoDigest;