bug 1406396 - work around NSS utils potentially loading spurious root cert modules r=mgoodwin

NSS command-line utilities may add a built-in root certificate module with the
name "Root Certs" if run on a profile that has a copy of the module file (which
is an unexpected configuration in general for Firefox). This can cause breakage.
To work around this, PSM now simply deletes any module named "Root Certs" at
startup. In an effort to prevent PSM from deleting unrelated modules
coincidentally named "Root Certs", we also prevent the user from using the
Firefox UI to name modules "Root Certs".

MozReview-Commit-ID: ABja3wpShO9

--HG--
extra : rebase_source : cfc62fb3fabf491a72f009601f3ec6973244642e

security/certverifier/NSSCertDBTrustDomain.cpp

@@ -1216,6 +1216,12 @@ LoadLoadableRoots(const nsCString& dir, const nsCString& modNameUTF8)
   // the return value would be detrimental in that case.
   int unusedModType;
   Unused << SECMOD_DeleteModule(modNameUTF8.get(), &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;
   if (!dir.IsEmpty()) {

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

@@ -185,3 +185,8 @@ addExceptionCheckingShort=Checking Information
 addExceptionCheckingLong2=Attempting to identify this site…
 addExceptionNoCertShort=No Information Available
 addExceptionNoCertLong2=Unable to obtain identification status for this site.
+
+# Load Module Dialog
+loadModuleHelp_emptyModuleName=The module name cannot be empty.
+# LOCALIZATION NOTE(loadModuleHelp_rootCertsModuleName): Do not translate 'Root Certs'
+loadModuleHelp_rootCertsModuleName=‘Root Certs‘ is reserved and cannot be used as the module name.

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

@@ -51,3 +51,20 @@ function onDialogAccept() {
 
   return true;
 }
+
+function validateModuleName() {
+  let bundle = document.getElementById("pippki_bundle");
+  let name = document.getElementById("device_name").value;
+  let helpText = document.getElementById("helpText");
+  helpText.value = "";
+  let dialogNode = document.querySelector("dialog");
+  dialogNode.removeAttribute("buttondisabledaccept");
+  if (name == "") {
+    helpText.value = bundle.getString("loadModuleHelp_emptyModuleName");
+    dialogNode.setAttribute("buttondisabledaccept", true);
+  }
+  if (name == "Root Certs") {
+    helpText.value = bundle.getString("loadModuleHelp_rootCertsModuleName");
+    dialogNode.setAttribute("buttondisabledaccept", true);
+  }
+}

security/manager/pki/resources/content/load_device.xul

@@ -29,7 +29,8 @@
   <hbox align="center">
     <label value="&loaddevice.modname2;" accesskey="&loaddevice.modname2.accesskey;"
            control="device_name"/>
-    <textbox id="device_name" flex="1" value="&loaddevice.modname.default;"/>
+    <textbox id="device_name" flex="1" value="&loaddevice.modname.default;"
+             onchange="validateModuleName();"/>
   </hbox>
   <hbox align="center">
     <label value="&loaddevice.filename2;" accesskey="&loaddevice.filename2.accesskey;"
@@ -39,5 +40,6 @@
             accesskey="&loaddevice.browse.accesskey;"
             oncommand="onBrowseBtnPress();"/>
   </hbox>
+  <label id="helpText" value=""/>
 
 </dialog>

security/manager/ssl/PKCS11ModuleDB.cpp

@@ -114,6 +114,17 @@ PKCS11ModuleDB::AddModule(const nsAString& aModuleName,
     return NS_ERROR_INVALID_ARG;
   }
 
+  // "Root Certs" is the name some NSS command-line utilities will give the
+  // roots module if they decide to load it when there happens to be a
+  // `DLL_PREFIX "nssckbi" DLL_SUFFIX` file in the directory being operated on.
+  // This causes failures, so as a workaround, the PSM initialization code will
+  // unconditionally remove any module named "Root Certs". We should prevent the
+  // user from adding an unrelated module named "Root Certs" in the first place
+  // so PSM doesn't delete it. See bug 1406396.
+  if (aModuleName.EqualsLiteral("Root Certs")) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
+
   // 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();

security/manager/ssl/tests/mochitest/browser/browser_loadPKCS11Module_ui.js

@@ -231,3 +231,33 @@ add_task(async function testCancel() {
 
   await BrowserTestUtils.windowClosed(win);
 });
+
+async function testModuleNameHelper(moduleName, acceptButtonShouldBeDisabled) {
+  let win = await openLoadModuleDialog();
+  resetCallCounts();
+
+  info(`Setting Module Name to '${moduleName}'`);
+  let moduleNameBox = win.document.getElementById("device_name");
+  moduleNameBox.value = moduleName;
+  // this makes this not a great test, but it's the easiest way to simulate this
+  moduleNameBox.onchange();
+
+  let dialogNode = win.document.querySelector("dialog");
+  Assert.equal(dialogNode.getAttribute("buttondisabledaccept"),
+               acceptButtonShouldBeDisabled ? "true" : "", // it's a string
+               `dialog accept button should ${acceptButtonShouldBeDisabled ? "" : "not "}be disabled`);
+
+  return BrowserTestUtils.closeWindow(win);
+}
+
+add_task(async function testEmptyModuleName() {
+  await testModuleNameHelper("", true);
+});
+
+add_task(async function testReservedModuleName() {
+  await testModuleNameHelper("Root Certs", true);
+});
+
+add_task(async function testAcceptableModuleName() {
+  await testModuleNameHelper("Some Module Name", false);
+});

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

@@ -0,0 +1,28 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/publicdomain/zero/1.0/
+"use strict";
+
+// Tests that adding modules with invalid names are prevented.
+
+// Ensure that the appropriate initialization has happened.
+do_get_profile();
+
+const gModuleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
+                    .getService(Ci.nsIPKCS11ModuleDB);
+
+function run_test() {
+  let libraryFile = Services.dirsvc.get("CurWorkD", Ci.nsIFile);
+  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");
+  throws(() => pkcs11ModuleDB.addModule("", libraryFile.path, 0, 0),
+         /NS_ERROR_ILLEGAL_VALUE/,
+         "Adding a module with an empty name should fail.");
+}

security/manager/ssl/tests/unit/xpcshell-smartcards.ini

@@ -7,6 +7,7 @@ support-files =
 
 [test_pkcs11_insert_remove.js]
 [test_pkcs11_module.js]
+[test_pkcs11_moduleDB.js]
 [test_pkcs11_no_events_after_removal.js]
 [test_pkcs11_safe_mode.js]
 skip-if = coverage # bug 1336728