bug 1466942 - avoid l10n string bundles in nsNSSComponent initialization r=fkiefer

Before this patch, nsNSSComponent initialization would call PK11_ConfigurePKCS11
with some localized strings, which contributed to startup time. Also,
PK11_UnconfigurePKCS11 was never called, so the memory allocated to these
strings would stick around forever. This patch addresses both of these problems
by not calling PK11_ConfigurePKCS11. This means that some properties of NSS'
internal "PKCS#11 slots/tokens" have to be localized when displaying them to the
user.

MozReview-Commit-ID: BbAgbgpFfFG

--HG--
extra : rebase_source : b633da8fea683675d0c0514a378954332afeb024

security/manager/locales/en-US/chrome/pipnss/pipnss.properties

@@ -56,10 +56,6 @@ Fips140TokenDescription=Software Security Device (FIPS)
 # conversion to UTF-8.
 # length_limit = 64 bytes
 Fips140SlotDescription=FIPS 140 Cryptographic, Key and Certificate Services
-# LOCALIZATION NOTE (InternalToken): string limit is 32 bytes after conversion
-# to UTF-8.
-# length_limit = 32 bytes
-InternalToken=Software Security Device
 
 VerifySSLClient=SSL Client Certificate
 VerifySSLServer=SSL Server Certificate

security/manager/pki/resources/content/device_manager.js

@@ -363,7 +363,7 @@ function changePassword() {
 function showTokenInfo() {
   var selected_token = selected_slot.getToken();
   AddInfoRow(bundle.getString("devinfo_label"),
-             selected_token.tokenLabel, "tok_label");
+             selected_token.tokenName, "tok_label");
   AddInfoRow(bundle.getString("devinfo_manID"),
              selected_token.tokenManID, "tok_manID");
   AddInfoRow(bundle.getString("devinfo_serialnum"),

security/manager/ssl/nsIPK11Token.idl

@@ -15,8 +15,6 @@ interface nsIPK11Token : nsISupports
   [must_use]
   readonly attribute AUTF8String tokenName;
   [must_use]
-  readonly attribute AUTF8String tokenLabel;
-  [must_use]
   readonly attribute boolean isInternalKeyToken;
   /**
    * Manufacturer ID of the token.

security/manager/ssl/nsNSSCertHelper.cpp

@@ -65,6 +65,18 @@ GetPIPNSSBundleString(const char* stringName, nsAString& result)
   return pipnssBundle->GetStringFromName(stringName, result);
 }
 
+nsresult
+GetPIPNSSBundleString(const char* stringName, nsACString& result)
+{
+  nsAutoString tmp;
+  nsresult rv = GetPIPNSSBundleString(stringName, tmp);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  result.Assign(NS_ConvertUTF16toUTF8(tmp));
+  return NS_OK;
+}
+
 nsresult
 PIPBundleFormatStringFromName(const char* stringName, const char16_t** params,
                               uint32_t numParams, nsAString& result)

security/manager/ssl/nsNSSCertHelper.h

@@ -29,6 +29,8 @@ LossyUTF8ToUTF16(const char* str, uint32_t len, /*out*/ nsAString& result);
 nsresult
 GetPIPNSSBundleString(const char* stringName, nsAString& result);
 nsresult
+GetPIPNSSBundleString(const char* stringName, nsACString& result);
+nsresult
 PIPBundleFormatStringFromName(const char* stringName, const char16_t** params,
                               uint32_t numParams, nsAString& result);
 

security/manager/ssl/nsNSSCertificate.cpp

@@ -671,28 +671,22 @@ nsNSSCertificate::GetSha1Fingerprint(nsAString& _sha1Fingerprint)
 NS_IMETHODIMP
 nsNSSCertificate::GetTokenName(nsAString& aTokenName)
 {
-  aTokenName.Truncate();
-  if (mCert) {
-    // HACK alert
-    // When the trust of a builtin cert is modified, NSS copies it into the
-    // cert db.  At this point, it is now "managed" by the user, and should
-    // not be listed with the builtins.  However, in the collection code
-    // used by PK11_ListCerts, the cert is found in the temp db, where it
-    // has been loaded from the token.  Though the trust is correct (grabbed
-    // from the cert db), the source is wrong.  I believe this is a safe
-    // way to work around this.
-    if (mCert->slot) {
-      char* token = PK11_GetTokenName(mCert->slot);
-      if (token) {
-        aTokenName = NS_ConvertUTF8toUTF16(token);
-      }
-    } else {
-      nsAutoString tok;
-      if (NS_SUCCEEDED(GetPIPNSSBundleString("InternalToken", tok))) {
-        aTokenName = tok;
-      }
-    }
+  MOZ_ASSERT(mCert);
+  if (!mCert) {
+    return NS_ERROR_FAILURE;
   }
+  UniquePK11SlotInfo internalSlot(PK11_GetInternalSlot());
+  if (!internalSlot) {
+    return NS_ERROR_FAILURE;
+  }
+  nsCOMPtr<nsIPK11Token> token(
+    new nsPK11Token(mCert->slot ? mCert->slot : internalSlot.get()));
+  nsAutoCString tmp;
+  nsresult rv = token->GetTokenName(tmp);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  aTokenName.Assign(NS_ConvertUTF8toUTF16(tmp));
   return NS_OK;
 }
 

security/manager/ssl/nsNSSComponent.cpp

@@ -1229,55 +1229,6 @@ LoadLoadableRootsTask::LoadLoadableRoots()
   return NS_ERROR_FAILURE;
 }
 
-nsresult
-nsNSSComponent::ConfigureInternalPKCS11Token()
-{
-  nsAutoString manufacturerID;
-  nsAutoString libraryDescription;
-  nsAutoString tokenDescription;
-  nsAutoString privateTokenDescription;
-  nsAutoString slotDescription;
-  nsAutoString privateSlotDescription;
-  nsAutoString fips140SlotDescription;
-  nsAutoString fips140TokenDescription;
-
-  nsresult rv;
-  rv = GetPIPNSSBundleString("ManufacturerID", manufacturerID);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = GetPIPNSSBundleString("LibraryDescription", libraryDescription);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = GetPIPNSSBundleString("TokenDescription", tokenDescription);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = GetPIPNSSBundleString("PrivateTokenDescription", privateTokenDescription);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = GetPIPNSSBundleString("SlotDescription", slotDescription);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = GetPIPNSSBundleString("PrivateSlotDescription", privateSlotDescription);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = GetPIPNSSBundleString("Fips140SlotDescription", fips140SlotDescription);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = GetPIPNSSBundleString("Fips140TokenDescription", fips140TokenDescription);
-  if (NS_FAILED(rv)) return rv;
-
-  PK11_ConfigurePKCS11(NS_ConvertUTF16toUTF8(manufacturerID).get(),
-                       NS_ConvertUTF16toUTF8(libraryDescription).get(),
-                       NS_ConvertUTF16toUTF8(tokenDescription).get(),
-                       NS_ConvertUTF16toUTF8(privateTokenDescription).get(),
-                       NS_ConvertUTF16toUTF8(slotDescription).get(),
-                       NS_ConvertUTF16toUTF8(privateSlotDescription).get(),
-                       NS_ConvertUTF16toUTF8(fips140SlotDescription).get(),
-                       NS_ConvertUTF16toUTF8(fips140TokenDescription).get(),
-                       0, 0);
-  return NS_OK;
-}
-
 // Table of pref names and SSL cipher ID
 typedef struct {
   const char* pref;
@@ -1934,14 +1885,6 @@ nsNSSComponent::InitializeNSS()
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("NSS Initialization beginning\n"));
 
-  // The call to ConfigureInternalPKCS11Token needs to be done before NSS is initialized,
-  // but affects only static data.
-  // If we could assume i18n will not change between profiles, one call per application
-  // run were sufficient. As I can't predict what happens in the future, let's repeat
-  // this call for every re-init of NSS.
-
-  ConfigureInternalPKCS11Token();
-
   nsAutoCString profileStr;
   nsresult rv = GetNSSProfilePath(profileStr);
   if (NS_FAILED(rv)) {

security/manager/ssl/nsNSSComponent.h

@@ -147,7 +147,6 @@ private:
   void setValidationOptions(bool isInitialSetting,
                             const mozilla::MutexAutoLock& proofOfLock);
   nsresult setEnabledTLSVersions();
-  nsresult ConfigureInternalPKCS11Token();
   nsresult RegisterObservers();
 
   void MaybeEnableFamilySafetyCompatibility();

security/manager/ssl/nsPK11TokenDB.cpp

@@ -27,15 +27,34 @@ nsPK11Token::nsPK11Token(PK11SlotInfo* slot)
 {
   MOZ_ASSERT(slot);
   mSlot.reset(PK11_ReferenceSlot(slot));
+  mIsInternalCryptoToken = PK11_IsInternal(mSlot.get()) &&
+                           !PK11_IsInternalKeySlot(mSlot.get());
+  mIsInternalKeyToken = PK11_IsInternalKeySlot(mSlot.get());
   mSeries = PK11_GetSlotSeries(slot);
-
   Unused << refreshTokenInfo();
 }
 
 nsresult
 nsPK11Token::refreshTokenInfo()
 {
-  mTokenName = PK11_GetTokenName(mSlot.get());
+  if (mIsInternalCryptoToken) {
+    nsresult rv;
+    if (PK11_IsFIPS()) {
+      rv = GetPIPNSSBundleString("Fips140TokenDescription", mTokenName);
+    } else {
+      rv = GetPIPNSSBundleString("TokenDescription", mTokenName);
+    }
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+  } else if (mIsInternalKeyToken) {
+    nsresult rv = GetPIPNSSBundleString("PrivateTokenDescription", mTokenName);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+  } else {
+    mTokenName.Assign(PK11_GetTokenName(mSlot.get()));
+  }
 
   CK_TOKEN_INFO tokInfo;
   nsresult rv = MapSECStatus(PK11_GetTokenInfo(mSlot.get(), &tokInfo));
@@ -43,18 +62,20 @@ nsPK11Token::refreshTokenInfo()
     return rv;
   }
 
-  // Set the Label field
-  const char* ccLabel = mozilla::BitwiseCast<char*, CK_UTF8CHAR*>(tokInfo.label);
-  mTokenLabel.Assign(ccLabel, strnlen(ccLabel, sizeof(tokInfo.label)));
-  mTokenLabel.Trim(" ", false, true);
-
   // Set the Manufacturer field
-  const char* ccManID =
-    mozilla::BitwiseCast<char*, CK_UTF8CHAR*>(tokInfo.manufacturerID);
-  mTokenManufacturerID.Assign(
-    ccManID,
-    strnlen(ccManID, sizeof(tokInfo.manufacturerID)));
-  mTokenManufacturerID.Trim(" ", false, true);
+  if (mIsInternalCryptoToken || mIsInternalKeyToken) {
+    rv = GetPIPNSSBundleString("ManufacturerID", mTokenManufacturerID);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+  } else {
+    const char* ccManID =
+      mozilla::BitwiseCast<char*, CK_UTF8CHAR*>(tokInfo.manufacturerID);
+    mTokenManufacturerID.Assign(
+      ccManID,
+      strnlen(ccManID, sizeof(tokInfo.manufacturerID)));
+    mTokenManufacturerID.Trim(" ", false, true);
+  }
 
   // Set the Hardware Version field
   mTokenHWVersion.Truncate();
@@ -100,17 +121,11 @@ nsPK11Token::GetTokenName(/*out*/ nsACString& tokenName)
   return GetAttributeHelper(mTokenName, tokenName);
 }
 
-NS_IMETHODIMP
-nsPK11Token::GetTokenLabel(/*out*/ nsACString& tokenLabel)
-{
-  return GetAttributeHelper(mTokenLabel, tokenLabel);
-}
-
 NS_IMETHODIMP
 nsPK11Token::GetIsInternalKeyToken(/*out*/ bool* _retval)
 {
   NS_ENSURE_ARG_POINTER(_retval);
-  *_retval = PK11_IsInternalKeySlot(mSlot.get());
+  *_retval = mIsInternalKeyToken;
   return NS_OK;
 }
 

security/manager/ssl/nsPK11TokenDB.h

@@ -32,12 +32,15 @@ private:
   nsresult refreshTokenInfo();
 
   nsCString mTokenName;
-  nsCString mTokenLabel;
   nsCString mTokenManufacturerID;
   nsCString mTokenHWVersion;
   nsCString mTokenFWVersion;
   nsCString mTokenSerialNum;
   mozilla::UniquePK11SlotInfo mSlot;
+  // True if this is the "PKCS#11 token" that provides cryptographic functions.
+  bool mIsInternalCryptoToken;
+  // True if this is the "PKCS#11 token" where private keys are stored.
+  bool mIsInternalKeyToken;
   int mSeries;
   nsCOMPtr<nsIInterfaceRequestor> mUIContext;
   nsresult GetAttributeHelper(const nsACString& attribute,

security/manager/ssl/nsPKCS11Slot.cpp

@@ -27,6 +27,9 @@ nsPKCS11Slot::nsPKCS11Slot(PK11SlotInfo* slot)
 {
   MOZ_ASSERT(slot);
   mSlot.reset(PK11_ReferenceSlot(slot));
+  mIsInternalCryptoSlot = PK11_IsInternal(mSlot.get()) &&
+                          !PK11_IsInternalKeySlot(mSlot.get());
+  mIsInternalKeySlot = PK11_IsInternalKeySlot(mSlot.get());
   mSeries = PK11_GetSlotSeries(slot);
   Unused << refreshSlotInfo();
 }
@@ -41,18 +44,42 @@ nsPKCS11Slot::refreshSlotInfo()
   }
 
   // Set the Description field
-  const char* ccDesc =
-    mozilla::BitwiseCast<char*, CK_UTF8CHAR*>(slotInfo.slotDescription);
-  mSlotDesc.Assign(ccDesc, strnlen(ccDesc, sizeof(slotInfo.slotDescription)));
-  mSlotDesc.Trim(" ", false, true);
+  if (mIsInternalCryptoSlot) {
+    nsresult rv;
+    if (PK11_IsFIPS()) {
+      rv = GetPIPNSSBundleString("Fips140SlotDescription", mSlotDesc);
+    } else {
+      rv = GetPIPNSSBundleString("SlotDescription", mSlotDesc);
+    }
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+  } else if (mIsInternalKeySlot) {
+    rv = GetPIPNSSBundleString("PrivateSlotDescription", mSlotDesc);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+  } else {
+    const char* ccDesc =
+      mozilla::BitwiseCast<char*, CK_UTF8CHAR*>(slotInfo.slotDescription);
+    mSlotDesc.Assign(ccDesc, strnlen(ccDesc, sizeof(slotInfo.slotDescription)));
+    mSlotDesc.Trim(" ", false, true);
+  }
 
   // Set the Manufacturer field
-  const char* ccManID =
-    mozilla::BitwiseCast<char*, CK_UTF8CHAR*>(slotInfo.manufacturerID);
-  mSlotManufacturerID.Assign(
-    ccManID,
-    strnlen(ccManID, sizeof(slotInfo.manufacturerID)));
-  mSlotManufacturerID.Trim(" ", false, true);
+  if (mIsInternalCryptoSlot || mIsInternalKeySlot) {
+    rv = GetPIPNSSBundleString("ManufacturerID", mSlotManufacturerID);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+  } else {
+    const char* ccManID =
+      mozilla::BitwiseCast<char*, CK_UTF8CHAR*>(slotInfo.manufacturerID);
+    mSlotManufacturerID.Assign(
+      ccManID,
+      strnlen(ccManID, sizeof(slotInfo.manufacturerID)));
+    mSlotManufacturerID.Trim(" ", false, true);
+  }
 
   // Set the Hardware Version field
   mSlotHWVersion.Truncate();
@@ -87,19 +114,16 @@ nsPKCS11Slot::GetAttributeHelper(const nsACString& attribute,
 NS_IMETHODIMP
 nsPKCS11Slot::GetName(/*out*/ nsACString& name)
 {
-  // |csn| is non-owning.
-  char* csn = PK11_GetSlotName(mSlot.get());
-  if (csn && *csn) {
-    name = csn;
-  } else if (PK11_HasRootCerts(mSlot.get())) {
-    // This is a workaround to an Root Module bug - the root certs module has
-    // no slot name.  Not bothering to localize, because this is a workaround
-    // and for now all the slot names returned by NSS are char * anyway.
-    name = NS_LITERAL_CSTRING("Root Certificates");
-  } else {
-    // same as above, this is a catch-all
-    name = NS_LITERAL_CSTRING("Unnamed Slot");
+  if (mIsInternalCryptoSlot) {
+    if (PK11_IsFIPS()) {
+      return GetPIPNSSBundleString("Fips140TokenDescription", name);
+    }
+    return GetPIPNSSBundleString("TokenDescription", name);
   }
+  if (mIsInternalKeySlot) {
+    return GetPIPNSSBundleString("PrivateTokenDescription", name);
+  }
+  name.Assign(PK11_GetSlotName(mSlot.get()));
 
   return NS_OK;
 }
@@ -152,7 +176,17 @@ nsPKCS11Slot::GetTokenName(/*out*/ nsACString& tokenName)
     }
   }
 
-  tokenName = PK11_GetTokenName(mSlot.get());
+  if (mIsInternalCryptoSlot) {
+    if (PK11_IsFIPS()) {
+      return GetPIPNSSBundleString("Fips140TokenDescription", tokenName);
+    }
+    return GetPIPNSSBundleString("TokenDescription", tokenName);
+  }
+  if (mIsInternalKeySlot) {
+    return GetPIPNSSBundleString("PrivateTokenDescription", tokenName);
+  }
+
+  tokenName.Assign(PK11_GetTokenName(mSlot.get()));
   return NS_OK;
 }
 

security/manager/ssl/nsPKCS11Slot.h

@@ -27,6 +27,10 @@ protected:
 
 private:
   mozilla::UniquePK11SlotInfo mSlot;
+  // True if this is the "PKCS#11 slot" that provides cryptographic functions.
+  bool mIsInternalCryptoSlot;
+  // True if this is the "PKCS#11 slot" where private keys are stored.
+  bool mIsInternalKeySlot;
   nsCString mSlotDesc;
   nsCString mSlotManufacturerID;
   nsCString mSlotHWVersion;

security/manager/ssl/tests/unit/test_certDB_import.js

@@ -125,6 +125,11 @@ function run_test() {
   let emailArray = getCertAsByteArray("test_certDB_import/emailEE.pem");
   gCertDB.importEmailCertificate(emailArray, emailArray.length,
                                  gInterfaceRequestor);
-  notEqual(findCertByEmailAddress(TEST_EMAIL_ADDRESS), null,
-           "E-mail cert should now be found in the database");
+  let emailCert = findCertByEmailAddress(TEST_EMAIL_ADDRESS);
+  notEqual(emailCert, null, "E-mail cert should now be found in the database");
+  let bundle =
+    Services.strings.createBundle("chrome://pipnss/locale/pipnss.properties");
+  equal(emailCert.tokenName,
+        bundle.GetStringFromName("PrivateTokenDescription"),
+        "cert's tokenName should be the expected localized value");
 }

security/manager/ssl/tests/unit/test_pkcs11_slot.js

@@ -18,19 +18,23 @@ function find_slot_by_name(module, name) {
   return null;
 }
 
+function find_module_by_name(moduleDB, name) {
+  for (let slot of XPCOMUtils.IterSimpleEnumerator(moduleDB.listModules(),
+                                                   Ci.nsIPKCS11Module)) {
+    if (slot.name == name) {
+      return slot;
+    }
+  }
+  return null;
+}
+
 function run_test() {
   loadPKCS11TestModule(false);
 
   let moduleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
                    .getService(Ci.nsIPKCS11ModuleDB);
-  let testModule;
-  for (let module of XPCOMUtils.IterSimpleEnumerator(moduleDB.listModules(),
-                                                     Ci.nsIPKCS11Module)) {
-    if (module.name == "PKCS11 Test Module") {
-      testModule = module;
-      break;
-    }
-  }
+  let testModule = find_module_by_name(moduleDB, "PKCS11 Test Module");
+  notEqual(testModule, null, "should be able to find test module");
   let testSlot = find_slot_by_name(testModule, "Test PKCS11 Slot 二");
   notEqual(testSlot, null, "should be able to find 'Test PKCS11 Slot 二'");
 
@@ -51,8 +55,8 @@ function run_test() {
 
   let testToken = testSlot.getToken();
   notEqual(testToken, null, "getToken() should succeed");
-  equal(testToken.tokenLabel, "Test PKCS11 Tokeñ 2 Label",
-        "Spot check: the actual and expected test token labels should be equal");
+  equal(testToken.tokenName, "Test PKCS11 Tokeñ 2 Label",
+        "Spot check: the actual and expected test token names should be equal");
   ok(!testToken.isInternalKeyToken, "This token is not the internal key token");
 
   testSlot = find_slot_by_name(testModule, "Empty PKCS11 Slot");
@@ -60,4 +64,24 @@ function run_test() {
   equal(testSlot.tokenName, null, "Empty slot is empty");
   equal(testSlot.status, Ci.nsIPKCS11Slot.SLOT_NOT_PRESENT,
         "Actual and expected status should match");
+
+  let bundle =
+    Services.strings.createBundle("chrome://pipnss/locale/pipnss.properties");
+  let internalModule = find_module_by_name(moduleDB,
+                                           "NSS Internal PKCS #11 Module");
+  notEqual(internalModule, null, "should be able to find internal module");
+  let cryptoSlot = find_slot_by_name(
+    internalModule, bundle.GetStringFromName("TokenDescription"));
+  notEqual(cryptoSlot, "should be able to find internal crypto slot");
+  equal(cryptoSlot.desc, bundle.GetStringFromName("SlotDescription"),
+        "crypto slot should have expected 'desc'");
+  equal(cryptoSlot.manID, bundle.GetStringFromName("ManufacturerID"),
+        "crypto slot should have expected 'manID'");
+  let keySlot = find_slot_by_name(
+    internalModule, bundle.GetStringFromName("PrivateTokenDescription"));
+  notEqual(keySlot, "should be able to find internal key slot");
+  equal(keySlot.desc, bundle.GetStringFromName("PrivateSlotDescription"),
+        "key slot should have expected 'desc'");
+  equal(keySlot.manID, bundle.GetStringFromName("ManufacturerID"),
+        "key slot should have expected 'manID'");
 }

security/manager/ssl/tests/unit/test_pkcs11_token.js

@@ -24,8 +24,6 @@ function checkBasicAttributes(token) {
   let expectedTokenName = bundle.GetStringFromName("PrivateTokenDescription");
   equal(token.tokenName, expectedTokenName,
         "Actual and expected name should match");
-  equal(token.tokenLabel, expectedTokenName,
-        "Actual and expected label should match");
   equal(token.tokenManID, bundle.GetStringFromName("ManufacturerID"),
         "Actual and expected manufacturer ID should match");
   equal(token.tokenHWVersion, "0.0",