bug 1464520 - hard-code the builtin roots module name to avoid a dependency on l10n in nsNSSComponent r?jcj,fkiefer draft
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 25 May 2018 11:22:48 -0700
changeset 801279 042c15e1f58ec8314ef7808752386c1028ab2420
parent 801214 89d79c2258be1c420153e1adc54338b0165fc88e
push id111624
push userbmo:dkeeler@mozilla.com
push dateWed, 30 May 2018 00:11:29 +0000
reviewersjcj, fkiefer
bugs1464520
milestone62.0a1
bug 1464520 - hard-code the builtin roots module name to avoid a dependency on l10n in nsNSSComponent r?jcj,fkiefer 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
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/manager/ssl/PKCS11ModuleDB.cpp
security/manager/ssl/nsIPKCS11ModuleDB.idl
security/manager/ssl/nsNSSCertHelper.cpp
security/manager/ssl/nsNSSCertHelper.h
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/nsNSSComponent.h
security/manager/ssl/nsPKCS11Slot.cpp
security/manager/ssl/tests/unit/test_pkcs11_moduleDB.js
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -16,18 +16,19 @@
 #include "certdb.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Casting.h"
 #include "mozilla/Move.h"
 #include "mozilla/PodOperations.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Unused.h"
 #include "nsCRTGlue.h"
+#include "nsNSSCertHelper.h"
+#include "nsNSSCertValidity.h"
 #include "nsNSSCertificate.h"
-#include "nsNSSCertValidity.h"
 #include "nsServiceManagerUtils.h"
 #include "nsThreadUtils.h"
 #include "nss.h"
 #include "pk11pub.h"
 #include "pkix/Result.h"
 #include "pkix/pkix.h"
 #include "pkix/pkixnss.h"
 #include "prerror.h"
@@ -1174,23 +1175,23 @@ DisableMD5()
     0, NSS_USE_ALG_IN_CERT_SIGNATURE | NSS_USE_ALG_IN_CMS_SIGNATURE);
   NSS_SetAlgorithmPolicy(SEC_OID_PKCS1_MD5_WITH_RSA_ENCRYPTION,
     0, NSS_USE_ALG_IN_CERT_SIGNATURE | NSS_USE_ALG_IN_CMS_SIGNATURE);
   NSS_SetAlgorithmPolicy(SEC_OID_PKCS5_PBE_WITH_MD5_AND_DES_CBC,
     0, NSS_USE_ALG_IN_CERT_SIGNATURE | NSS_USE_ALG_IN_CMS_SIGNATURE);
 }
 
 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
   // fail to load our roots module. In these cases, deleting the "Root Certs"
   // module allows us to load the correct one. See bug 1406396.
   Unused << SECMOD_DeleteModule("Root Certs", &unusedModType);
 
   nsAutoCString fullLibraryPath;
@@ -1199,17 +1200,17 @@ LoadLoadableRoots(const nsCString& dir, 
     fullLibraryPath.AppendLiteral(FILE_PATH_SEPARATOR);
   }
   fullLibraryPath.Append(DLL_PREFIX "nssckbi" DLL_SUFFIX);
   // Escape the \ and " characters.
   fullLibraryPath.ReplaceSubstring("\\", "\\\\");
   fullLibraryPath.ReplaceSubstring("\"", "\\\"");
 
   nsAutoCString pkcs11ModuleSpec("name=\"");
-  pkcs11ModuleSpec.Append(modNameUTF8);
+  pkcs11ModuleSpec.Append(kRootModuleName);
   pkcs11ModuleSpec.AppendLiteral("\" library=\"");
   pkcs11ModuleSpec.Append(fullLibraryPath);
   pkcs11ModuleSpec.AppendLiteral("\"");
 
   UniqueSECMODModule rootsModule(
     SECMOD_LoadUserModule(const_cast<char*>(pkcs11ModuleSpec.get()), nullptr,
                           false));
   if (!rootsModule) {
@@ -1219,20 +1220,19 @@ LoadLoadableRoots(const nsCString& dir, 
   if (!rootsModule->loaded) {
     return false;
   }
 
   return true;
 }
 
 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());
   }
 }
 
 nsresult
 DefaultServerNicknameForCert(const CERTCertificate* cert,
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -44,23 +44,21 @@ void DisableMD5();
 
 /**
  * Loads root certificates from a module.
  *
  * @param dir
  *        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);
 
 void SaveIntermediateCerts(const UniqueCERTCertList& certList);
 
 class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain
 {
--- a/security/manager/ssl/PKCS11ModuleDB.cpp
+++ b/security/manager/ssl/PKCS11ModuleDB.cpp
@@ -5,37 +5,63 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "PKCS11ModuleDB.h"
 
 #include "ScopedNSSTypes.h"
 #include "mozilla/Telemetry.h"
 #include "nsCRTGlue.h"
 #include "nsIMutableArray.h"
+#include "nsNSSCertHelper.h"
 #include "nsNSSComponent.h"
 #include "nsNativeCharsetUtils.h"
 #include "nsPKCS11Slot.h"
 #include "nsServiceManagerUtils.h"
 
 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)
 {
   if (aModuleName.IsEmpty()) {
     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;
   }
 
   return NS_OK;
 }
 
 // Given a PKCS#11 module, determines an appropriate name to identify it for the
@@ -90,29 +116,33 @@ PKCS11ModuleDB::AddModule(const nsAStrin
 
   // There appears to be a deadlock if we try to load modules concurrently, so
   // just wait until the loadable roots module has been loaded.
   nsresult rv = BlockUntilLoadableRootsLoaded();
   if (NS_FAILED(rv)) {
     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;
   }
 
   nsAutoString scalarKey;
   GetModuleNameForTelemetry(module.get(), scalarKey);
   // Scalar keys must be between 0 and 70 characters (exclusive).
   // GetModuleNameForTelemetry takes care of keys that are too long.
@@ -122,27 +152,32 @@ PKCS11ModuleDB::AddModule(const nsAStrin
   if (scalarKey.Length() > 0) {
     Telemetry::ScalarSet(Telemetry::ScalarID::SECURITY_PKCS11_MODULES_LOADED,
                          scalarKey, true);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
-PKCS11ModuleDB::FindModuleByName(const nsACString& name,
+PKCS11ModuleDB::FindModuleByName(const nsAString& name,
                          /*out*/ nsIPKCS11Module** _retval)
 {
   NS_ENSURE_ARG_POINTER(_retval);
 
   nsresult rv = BlockUntilLoadableRootsLoaded();
   if (NS_FAILED(rv)) {
     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;
   }
 
   nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(mod.get());
   module.forget(_retval);
   return NS_OK;
 }
--- a/security/manager/ssl/nsIPKCS11ModuleDB.idl
+++ b/security/manager/ssl/nsIPKCS11ModuleDB.idl
@@ -22,17 +22,17 @@ interface nsIPKCS11ModuleDB : nsISupport
 
   [must_use]
   void addModule(in AString moduleName,
                  in AString libraryFullPath,
                  in long cryptoMechanismFlags,
                  in long cipherFlags);
 
   [must_use]
-  nsIPKCS11Module findModuleByName(in AUTF8String name);
+  nsIPKCS11Module findModuleByName(in AString name);
 
   [must_use]
   nsISimpleEnumerator listModules();
 
   [must_use]
   readonly attribute boolean canToggleFIPS;
 
   [must_use]
--- a/security/manager/ssl/nsNSSCertHelper.cpp
+++ b/security/manager/ssl/nsNSSCertHelper.cpp
@@ -21,16 +21,23 @@
 #include "nsNSSCertificate.h"
 #include "nsReadableUtils.h"
 #include "nsServiceManagerUtils.h"
 #include "prerror.h"
 #include "secder.h"
 
 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)
 {
   nsCOMPtr<nsIStringBundleService> bundleService(
     do_GetService(NS_STRINGBUNDLE_CONTRACTID));
   if (!bundleService) {
     return NS_ERROR_NOT_AVAILABLE;
   }
--- a/security/manager/ssl/nsNSSCertHelper.h
+++ b/security/manager/ssl/nsNSSCertHelper.h
@@ -7,16 +7,19 @@
 
 #ifndef INET6_ADDRSTRLEN
 #define INET6_ADDRSTRLEN 46
 #endif
 
 #include "certt.h"
 #include "nsString.h"
 
+extern const char* kRootModuleName;
+extern const size_t kRootModuleNameLen;
+
 uint32_t
 getCertType(CERTCertificate* cert);
 
 nsresult
 GetCertFingerprintByOidTag(CERTCertificate* nsscert, SECOidTag aOidTag,
                            nsCString& fp);
 
 void
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -243,36 +243,23 @@ nsNSSComponent::PIPBundleFormatStringFro
   }
   return rv;
 }
 
 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;
 }
 
 NS_IMETHODIMP
 nsNSSComponent::GetNSSBundleString(const char* name, nsAString& outString)
 {
   MutexAutoLock lock(mMutex);
   nsresult rv = NS_ERROR_FAILURE;
 
@@ -972,35 +959,32 @@ nsNSSComponent::TrustLoaded3rdPartyRoots
   }
   return NS_OK;
 }
 #endif // XP_WIN
 
 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);
   }
 
   ~LoadLoadableRootsTask() = default;
 
   nsresult Dispatch();
 
 private:
   NS_IMETHOD Run() override;
   nsresult LoadLoadableRoots();
   RefPtr<nsNSSComponent> mNSSComponent;
   nsCOMPtr<nsIThread> mThread;
-  nsCString mRootModuleName;
 };
 
 nsresult
 LoadLoadableRootsTask::Dispatch()
 {
   // Can't add 'this' as the event to run, since mThread may not be set yet
   nsresult rv = NS_NewNamedThread("LoadRoots", getter_AddRefs(mThread),
                                   nullptr,
@@ -1281,40 +1265,26 @@ LoadLoadableRootsTask::LoadLoadableRoots
   // As a last resort, this will cause the library loading code to use the OS'
   // default library search path.
   nsAutoCString emptyString;
   if (!possibleCKBILocations.append(Move(emptyString))) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   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;
     }
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("could not load loadable roots"));
   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()
 {
   nsAutoString manufacturerID;
   nsAutoString libraryDescription;
   nsAutoString tokenDescription;
   nsAutoString privateTokenDescription;
   nsAutoString slotDescription;
@@ -2201,33 +2171,18 @@ nsNSSComponent::InitializeNSS()
     if (SSL_ConfigServerSessionIDCache(1000, 0, 0, nullptr) != SECSuccess) {
       return NS_ERROR_FAILURE;
     }
 
     // Set dynamic options from prefs. This has to run after
     // 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;
     }
 
     mNSSInitialized = true;
     return NS_OK;
   }
@@ -2253,17 +2208,17 @@ nsNSSComponent::ShutdownNSS()
     // We can only run SSL_ShutdownServerSessionIDCache once (the rest of
     // these operations are idempotent).
     SSL_ClearSessionCache();
     // TLSServerSocket may be run with the session cache enabled. This ensures
     // those resources are cleaned up.
     Unused << SSL_ShutdownServerSessionIDCache();
   }
 
-  UnloadLoadableRoots(lock);
+  ::mozilla::psm::UnloadLoadableRoots();
 
 #ifdef XP_WIN
   mFamilySafetyRoot = nullptr;
   mEnterpriseRoots = nullptr;
 #endif
 
   PK11_SetPasswordFunc((PK11PasswordFunc)nullptr);
 
--- a/security/manager/ssl/nsNSSComponent.h
+++ b/security/manager/ssl/nsNSSComponent.h
@@ -157,19 +157,16 @@ public:
 
 protected:
   virtual ~nsNSSComponent();
 
 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();
   nsresult InitializePIPNSSBundle();
   nsresult ConfigureInternalPKCS11Token();
   nsresult RegisterObservers();
 
   void MaybeEnableFamilySafetyCompatibility();
--- a/security/manager/ssl/nsPKCS11Slot.cpp
+++ b/security/manager/ssl/nsPKCS11Slot.cpp
@@ -180,21 +180,44 @@ nsPKCS11Slot::GetStatus(uint32_t* _retva
 NS_IMPL_ISUPPORTS(nsPKCS11Module, nsIPKCS11Module)
 
 nsPKCS11Module::nsPKCS11Module(SECMODModule* module)
 {
   MOZ_ASSERT(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
 nsPKCS11Module::GetLibName(/*out*/ nsACString& libName)
 {
   if (mModule->dllName) {
     libName = mModule->dllName;
   } else {
--- a/security/manager/ssl/tests/unit/test_pkcs11_moduleDB.js
+++ b/security/manager/ssl/tests/unit/test_pkcs11_moduleDB.js
@@ -16,13 +16,22 @@ function run_test() {
   libraryFile.append("pkcs11testmodule");
   libraryFile.append(ctypes.libraryName("pkcs11testmodule"));
   ok(libraryFile.exists(), "The pkcs11testmodule file should exist");
 
   let pkcs11ModuleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
                          .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.");
 }