bug 1464520 - hard-code the builtin roots module name to avoid a dependency on l10n in nsNSSComponent r=fkiefer,jcj

nsNSSComponent startup and shutdown would be simpler if there were no direct
dependencies on localized strings. This patch removes a dependency on the
localized name of the builtin roots module by hard-coding the name internally
and then mapping it to/from the localized version as appropriate.

MozReview-Commit-ID: 30kbpWFYbzm

--HG--
extra : rebase_source : 3d384af5a9fa45d5ac1f78e1fcb0dd9e4b94267d

security/certverifier/NSSCertDBTrustDomain.cpp

@@ -21,8 +21,9 @@
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Unused.h"
 #include "nsCRTGlue.h"
-#include "nsNSSCertificate.h"
+#include "nsNSSCertHelper.h"
 #include "nsNSSCertValidity.h"
+#include "nsNSSCertificate.h"
 #include "nsServiceManagerUtils.h"
 #include "nsThreadUtils.h"
 #include "nss.h"
@@ -1179,13 +1180,13 @@ DisableMD5()
 }
 
 bool
-LoadLoadableRoots(const nsCString& dir, const nsCString& modNameUTF8)
+LoadLoadableRoots(const nsCString& dir)
 {
   // If a module exists with the same name, make a best effort attempt to delete
   // it. Note that it isn't possible to delete the internal module, so checking
   // the return value would be detrimental in that case.
   int unusedModType;
-  Unused << SECMOD_DeleteModule(modNameUTF8.get(), &unusedModType);
+  Unused << SECMOD_DeleteModule(kRootModuleName, &unusedModType);
   // Some NSS command-line utilities will load a roots module under the name
   // "Root Certs" if there happens to be a `DLL_PREFIX "nssckbi" DLL_SUFFIX`
   // file in the directory being operated on. In some cases this can cause us to
@@ -1204,7 +1205,7 @@ LoadLoadableRoots(const nsCString& dir, const nsCString& modNameUTF8)
   fullLibraryPath.ReplaceSubstring("\"", "\\\"");
 
   nsAutoCString pkcs11ModuleSpec("name=\"");
-  pkcs11ModuleSpec.Append(modNameUTF8);
+  pkcs11ModuleSpec.Append(kRootModuleName);
   pkcs11ModuleSpec.AppendLiteral("\" library=\"");
   pkcs11ModuleSpec.Append(fullLibraryPath);
   pkcs11ModuleSpec.AppendLiteral("\"");
@@ -1224,10 +1225,9 @@ LoadLoadableRoots(const nsCString& dir, const nsCString& modNameUTF8)
 }
 
 void
-UnloadLoadableRoots(const char* modNameUTF8)
+UnloadLoadableRoots()
 {
-  MOZ_ASSERT(modNameUTF8);
-  UniqueSECMODModule rootsModule(SECMOD_FindModule(modNameUTF8));
+  UniqueSECMODModule rootsModule(SECMOD_FindModule(kRootModuleName));
 
   if (rootsModule) {
     SECMOD_UnloadUserModule(rootsModule.get());

security/certverifier/NSSCertDBTrustDomain.h

@@ -49,13 +49,11 @@ void DisableMD5();
  *        The path to the directory containing the NSS builtin roots module.
  *        Usually the same as the path to the other NSS shared libraries.
  *        If empty, the (library) path will be searched.
- * @param modNameUTF8
- *        The UTF-8 name to give the module for display purposes.
  * @return true if the roots were successfully loaded, false otherwise.
  */
-bool LoadLoadableRoots(const nsCString& dir, const nsCString& modNameUTF8);
+bool LoadLoadableRoots(const nsCString& dir);
 
-void UnloadLoadableRoots(const char* modNameUTF8);
+void UnloadLoadableRoots();
 
 nsresult DefaultServerNicknameForCert(const CERTCertificate* cert,
                               /*out*/ nsCString& nickname);

security/manager/ssl/PKCS11ModuleDB.cpp

@@ -10,6 +10,7 @@
 #include "mozilla/Telemetry.h"
 #include "nsCRTGlue.h"
 #include "nsIMutableArray.h"
+#include "nsNSSCertHelper.h"
 #include "nsNSSComponent.h"
 #include "nsNativeCharsetUtils.h"
 #include "nsPKCS11Slot.h"
@@ -19,6 +20,27 @@ namespace mozilla { namespace psm {
 
 NS_IMPL_ISUPPORTS(PKCS11ModuleDB, nsIPKCS11ModuleDB)
 
+// Convert the UTF16 name of the module as it appears to the user to the
+// internal representation. For most modules this just involves converting from
+// UTF16 to UTF8. For the builtin root module, it also involves mapping from the
+// localized name to the internal, non-localized name.
+static nsresult
+NormalizeModuleNameIn(const nsAString& moduleNameIn, nsCString& moduleNameOut)
+{
+  nsAutoString localizedRootModuleName;
+  nsresult rv = GetPIPNSSBundleString("RootCertModuleName",
+                                      localizedRootModuleName);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  if (moduleNameIn.Equals(localizedRootModuleName)) {
+    moduleNameOut.Assign(kRootModuleName);
+    return NS_OK;
+  }
+  moduleNameOut.Assign(NS_ConvertUTF16toUTF8(moduleNameIn));
+  return NS_OK;
+}
+
 // Delete a PKCS11 module from the user's profile.
 NS_IMETHODIMP
 PKCS11ModuleDB::DeleteModule(const nsAString& aModuleName)
@@ -27,10 +49,14 @@ PKCS11ModuleDB::DeleteModule(const nsAString& aModuleName)
     return NS_ERROR_INVALID_ARG;
   }
 
-  NS_ConvertUTF16toUTF8 moduleName(aModuleName);
+  nsAutoCString moduleNameNormalized;
+  nsresult rv = NormalizeModuleNameIn(aModuleName, moduleNameNormalized);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
   // modType is an output variable. We ignore it.
   int32_t modType;
-  SECStatus srv = SECMOD_DeleteModule(moduleName.get(), &modType);
+  SECStatus srv = SECMOD_DeleteModule(moduleNameNormalized.get(), &modType);
   if (srv != SECSuccess) {
     return NS_ERROR_FAILURE;
   }
@@ -95,19 +121,23 @@ PKCS11ModuleDB::AddModule(const nsAString& aModuleName,
     return rv;
   }
 
-  NS_ConvertUTF16toUTF8 moduleName(aModuleName);
+  nsAutoCString moduleNameNormalized;
+  rv = NormalizeModuleNameIn(aModuleName, moduleNameNormalized);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
   nsCString fullPath;
   // NSS doesn't support Unicode path.  Use native charset
   NS_CopyUnicodeToNative(aLibraryFullPath, fullPath);
   uint32_t mechFlags = SECMOD_PubMechFlagstoInternal(aCryptoMechanismFlags);
   uint32_t cipherFlags = SECMOD_PubCipherFlagstoInternal(aCipherFlags);
-  SECStatus srv = SECMOD_AddNewModule(moduleName.get(), fullPath.get(),
-                                      mechFlags, cipherFlags);
+  SECStatus srv = SECMOD_AddNewModule(moduleNameNormalized.get(),
+                                      fullPath.get(), mechFlags, cipherFlags);
   if (srv != SECSuccess) {
     return NS_ERROR_FAILURE;
   }
 
-  UniqueSECMODModule module(SECMOD_FindModule(moduleName.get()));
+  UniqueSECMODModule module(SECMOD_FindModule(moduleNameNormalized.get()));
   if (!module) {
     return NS_ERROR_FAILURE;
   }
@@ -127,7 +157,7 @@ PKCS11ModuleDB::AddModule(const nsAString& aModuleName,
 }
 
 NS_IMETHODIMP
-PKCS11ModuleDB::FindModuleByName(const nsACString& name,
+PKCS11ModuleDB::FindModuleByName(const nsAString& name,
                          /*out*/ nsIPKCS11Module** _retval)
 {
   NS_ENSURE_ARG_POINTER(_retval);
@@ -137,7 +167,12 @@ PKCS11ModuleDB::FindModuleByName(const nsACString& name,
     return rv;
   }
 
-  UniqueSECMODModule mod(SECMOD_FindModule(PromiseFlatCString(name).get()));
+  nsAutoCString moduleNameNormalized;
+  rv = NormalizeModuleNameIn(name, moduleNameNormalized);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  UniqueSECMODModule mod(SECMOD_FindModule(moduleNameNormalized.get()));
   if (!mod) {
     return NS_ERROR_FAILURE;
   }

security/manager/ssl/nsIPKCS11ModuleDB.idl

@@ -27,7 +27,7 @@ interface nsIPKCS11ModuleDB : nsISupports
                  in long cipherFlags);
 
   [must_use]
-  nsIPKCS11Module findModuleByName(in AUTF8String name);
+  nsIPKCS11Module findModuleByName(in AString name);
 
   [must_use]
   nsISimpleEnumerator listModules();

security/manager/ssl/nsNSSCertHelper.cpp

@@ -26,6 +26,13 @@
 
 using namespace mozilla;
 
+// To avoid relying on localized strings in PSM, we hard-code the root module
+// name internally. When we display it to the user in the list of modules in the
+// front-end, we look up the localized value and display that instead of this.
+const char* kRootModuleName = "Builtin Roots Module";
+const size_t kRootModuleNameLen = strlen(kRootModuleName);
+
+
 static nsresult
 GetPIPNSSBundle(nsIStringBundle** pipnssBundle)
 {

security/manager/ssl/nsNSSCertHelper.h

@@ -12,6 +12,9 @@
 #include "certt.h"
 #include "nsString.h"
 
+extern const char* kRootModuleName;
+extern const size_t kRootModuleNameLen;
+
 uint32_t
 getCertType(CERTCertificate* cert);
 

security/manager/ssl/nsNSSComponent.cpp

@@ -229,26 +229,13 @@ NS_IMETHODIMP
 nsNSSComponent::GetPIPNSSBundleString(const char* name, nsAString& outString)
 {
   MutexAutoLock lock(mMutex);
-  return GetPIPNSSBundleStringLocked(name, outString, lock);
-}
 
-nsresult
-nsNSSComponent::GetPIPNSSBundleStringLocked(
-  const char* name, nsAString& outString, const MutexAutoLock& /*proofOfLock*/)
-{
-  nsresult rv = NS_ERROR_FAILURE;
-
-  outString.SetLength(0);
+  outString.Truncate();
   if (mPIPNSSBundle && name) {
-    nsAutoString result;
-    rv = mPIPNSSBundle->GetStringFromName(name, result);
-    if (NS_SUCCEEDED(rv)) {
-      outString = result;
-      rv = NS_OK;
-    }
+    return mPIPNSSBundle->GetStringFromName(name, outString);
   }
 
-  return rv;
+  return NS_ERROR_FAILURE;
 }
 
 #ifdef XP_WIN
@@ -939,11 +926,9 @@ nsNSSComponent::TrustLoaded3rdPartyRoots()
 class LoadLoadableRootsTask final : public Runnable
 {
 public:
-  explicit LoadLoadableRootsTask(nsNSSComponent* nssComponent,
-                                 nsCString&& rootModuleName)
+  explicit LoadLoadableRootsTask(nsNSSComponent* nssComponent)
     : Runnable("LoadLoadableRootsTask")
     , mNSSComponent(nssComponent)
-    , mRootModuleName(Move(rootModuleName))
   {
     MOZ_ASSERT(nssComponent);
   }
@@ -957,7 +942,6 @@ private:
   nsresult LoadLoadableRoots();
   RefPtr<nsNSSComponent> mNSSComponent;
   nsCOMPtr<nsIThread> mThread;
-  nsCString mRootModuleName;
 };
 
 nsresult
@@ -1248,7 +1232,7 @@ LoadLoadableRootsTask::LoadLoadableRoots()
   }
 
   for (const auto& possibleCKBILocation : possibleCKBILocations) {
-    if (mozilla::psm::LoadLoadableRoots(possibleCKBILocation, mRootModuleName)) {
+    if (mozilla::psm::LoadLoadableRoots(possibleCKBILocation)) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("loaded CKBI from %s",
                                             possibleCKBILocation.get()));
       return NS_OK;
@@ -1258,20 +1242,6 @@ LoadLoadableRootsTask::LoadLoadableRoots()
   return NS_ERROR_FAILURE;
 }
 
-void
-nsNSSComponent::UnloadLoadableRoots(const MutexAutoLock& proofOfLock)
-{
-  nsresult rv;
-  nsAutoString modName;
-  rv = GetPIPNSSBundleStringLocked("RootCertModuleName", modName, proofOfLock);
-  if (NS_FAILED(rv)) {
-    return;
-  }
-
-  NS_ConvertUTF16toUTF8 modNameUTF8(modName);
-  ::mozilla::psm::UnloadLoadableRoots(modNameUTF8.get());
-}
-
 nsresult
 nsNSSComponent::ConfigureInternalPKCS11Token()
 {
@@ -2160,23 +2130,8 @@ nsNSSComponent::InitializeNSS()
     // SSL_ConfigServerSessionIDCache.
     setValidationOptions(true, lock);
 
-    nsAutoString rootModuleName;
-    rv = GetPIPNSSBundleStringLocked("RootCertModuleName", rootModuleName,
-                                     lock);
-    if (NS_FAILED(rv)) {
-      // When running Cpp unit tests on Android, this will fail because string
-      // bundles aren't available (see bug 1311077, bug 1228175 comment 12, and
-      // bug 929655). Because the module name is really only for display
-      // purposes, we can just hard-code the value here. Furthermore, if we want
-      // to be able to stop using string bundles in PSM in this way, we'll have
-      // to hard-code the string and only use the localized version when
-      // displaying it to the user, so this is a step in that direction anyway.
-      rootModuleName.AssignLiteral("Builtin Roots Module");
-    }
-    NS_ConvertUTF16toUTF8 rootModuleNameUTF8(rootModuleName);
-
     RefPtr<LoadLoadableRootsTask> loadLoadableRootsTask(
-      new LoadLoadableRootsTask(this, Move(rootModuleNameUTF8)));
+      new LoadLoadableRootsTask(this));
     rv = loadLoadableRootsTask->Dispatch();
     if (NS_FAILED(rv)) {
       return rv;
@@ -2212,7 +2167,7 @@ nsNSSComponent::ShutdownNSS()
     Unused << SSL_ShutdownServerSessionIDCache();
   }
 
-  UnloadLoadableRoots(lock);
+  ::mozilla::psm::UnloadLoadableRoots();
 
 #ifdef XP_WIN
   mFamilySafetyRoot = nullptr;

security/manager/ssl/nsNSSComponent.h

@@ -150,9 +150,6 @@ private:
   nsresult InitializeNSS();
   void ShutdownNSS();
 
-  nsresult GetPIPNSSBundleStringLocked(const char* name, nsAString& outString,
-                                       const mozilla::MutexAutoLock& proofOfLock);
-  void UnloadLoadableRoots(const mozilla::MutexAutoLock& proofOfLock);
   void setValidationOptions(bool isInitialSetting,
                             const mozilla::MutexAutoLock& proofOfLock);
   nsresult setEnabledTLSVersions();

security/manager/ssl/nsPKCS11Slot.cpp

@@ -185,11 +185,34 @@ nsPKCS11Module::nsPKCS11Module(SECMODModule* module)
   mModule.reset(SECMOD_ReferenceModule(module));
 }
 
+// Convert the UTF8 internal name of the module to how it should appear to the
+// user. In most cases this involves simply passing back the module's name.
+// However, the builtin roots module has a non-localized name internally that we
+// must map to the localized version when we display it to the user.
+static nsresult
+NormalizeModuleNameOut(const char* moduleNameIn, nsACString& moduleNameOut)
+{
+  // Easy case: this isn't the builtin roots module.
+  if (strnlen(moduleNameIn, kRootModuleNameLen + 1) != kRootModuleNameLen ||
+      strncmp(kRootModuleName, moduleNameIn, kRootModuleNameLen) != 0) {
+    moduleNameOut.Assign(moduleNameIn);
+    return NS_OK;
+  }
+
+  nsAutoString localizedRootModuleName;
+  nsresult rv = GetPIPNSSBundleString("RootCertModuleName",
+                                      localizedRootModuleName);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  moduleNameOut.Assign(NS_ConvertUTF16toUTF8(localizedRootModuleName));
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsPKCS11Module::GetName(/*out*/ nsACString& name)
 {
-  name = mModule->commonName;
-  return NS_OK;
+  return NormalizeModuleNameOut(mModule->commonName, name);
 }
 
 NS_IMETHODIMP

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

@@ -21,8 +21,17 @@ function run_test() {
                          .getService(Ci.nsIPKCS11ModuleDB);
   throws(() => pkcs11ModuleDB.addModule("Root Certs", libraryFile.path, 0, 0),
          /NS_ERROR_ILLEGAL_VALUE/,
-         "Adding a module named 'Root Certs' should fail");
+         "Adding a module named 'Root Certs' should fail.");
   throws(() => pkcs11ModuleDB.addModule("", libraryFile.path, 0, 0),
          /NS_ERROR_ILLEGAL_VALUE/,
          "Adding a module with an empty name should fail.");
+
+  let bundle =
+    Services.strings.createBundle("chrome://pipnss/locale/pipnss.properties");
+  let rootsModuleName = bundle.GetStringFromName("RootCertModuleName");
+  let rootsModule = pkcs11ModuleDB.findModuleByName(rootsModuleName);
+  notEqual(rootsModule, null,
+           "Should be able to find builtin roots module by localized name.");
+  equal(rootsModule.name, rootsModuleName,
+        "Builtin roots module should have correct localized name.");
 }