bug 986956 - only ever initialize NSS once per process r=Cykesiopka r=mgoodwin
authorDavid Keeler <dkeeler@mozilla.com>
Thu, 19 Nov 2015 13:31:52 -0800
changeset 308893 97419358e5899b3e6ee2e21e7d89106d90b8fa70
parent 308892 86efea9a2d3068929da0ce935605c60233b7830b
child 308894 bc44c9e55ef0e042c5a6c88bd241c08282b27f55
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCykesiopka, mgoodwin
bugs986956, 1216882
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 986956 - only ever initialize NSS once per process r=Cykesiopka r=mgoodwin As a consequence, if NSS is initialized when there is no profile directory, NSS will not persist changes. Other failures may occur (e.g. see bug 1216882).
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/tests/unit/test_pkcs11_safe_mode.js
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -921,18 +921,18 @@ GetNSSProfilePath(nsAutoCString& aProfil
     aProfilePath.Assign(dbDirOverride);
     return NS_OK;
   }
 
   nsCOMPtr<nsIFile> profileFile;
   nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
                                        getter_AddRefs(profileFile));
   if (NS_FAILED(rv)) {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Error,
-           ("Unable to get profile directory - continuing with no NSS DB\n"));
+    NS_WARNING("NSS will be initialized without a profile directory. "
+               "Some things may not work as expected.");
     return NS_OK;
   }
 
 #if defined(XP_WIN)
   // Native path will drop Unicode characters that cannot be mapped to system's
   // codepage, using short (canonical) path as workaround.
   nsCOMPtr<nsILocalFileWin> profileFileWin(do_QueryInterface(profileFile));
   if (!profileFileWin) {
@@ -945,16 +945,18 @@ GetNSSProfilePath(nsAutoCString& aProfil
   rv = profileFile->GetNativePath(aProfilePath);
 #endif
   if (NS_FAILED(rv)) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Error,
            ("Could not get native path for profile directory.\n"));
     return rv;
   }
 
+  MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+          ("NSS profile at '%s'\n", aProfilePath.get()));
   return NS_OK;
 }
 
 nsresult
 nsNSSComponent::InitializeNSS()
 {
   // Can be called both during init and profile change.
   // Needs mutex protection.
@@ -1234,57 +1236,25 @@ nsNSSComponent::RandomUpdate(void* entro
   if (!mNSSInitialized)
       return NS_ERROR_NOT_INITIALIZED;
 
   PK11_RandomUpdate(entropy, bufLen);
   return NS_OK;
 }
 
 static const char* const PROFILE_BEFORE_CHANGE_TOPIC = "profile-before-change";
-static const char* const PROFILE_DO_CHANGE_TOPIC = "profile-do-change";
 
 NS_IMETHODIMP
 nsNSSComponent::Observe(nsISupports* aSubject, const char* aTopic,
                         const char16_t* someData)
 {
   if (nsCRT::strcmp(aTopic, PROFILE_BEFORE_CHANGE_TOPIC) == 0) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("receiving profile change topic\n"));
     DoProfileBeforeChange();
-  }
-  else if (nsCRT::strcmp(aTopic, PROFILE_DO_CHANGE_TOPIC) == 0) {
-    if (someData && NS_LITERAL_STRING("startup").Equals(someData)) {
-      // The application is initializing against a known profile directory for
-      // the first time during process execution.
-      // However, earlier code execution might have already triggered NSS init.
-      // We must ensure that NSS gets shut down prior to any attempt to init
-      // it again. We use the same cleanup functionality used when switching
-      // profiles. The order of function calls must correspond to the order
-      // of notifications sent by Profile Manager (nsProfile).
-      DoProfileBeforeChange();
-    }
-
-    bool needsInit = true;
-
-    {
-      MutexAutoLock lock(mutex);
-
-      if (mNSSInitialized) {
-        // We have already initialized NSS before the profile came up,
-        // no need to do it again
-        needsInit = false;
-      }
-    }
-
-    if (needsInit) {
-      if (NS_FAILED(InitializeNSS())) {
-        MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("Unable to Initialize NSS after profile switch.\n"));
-      }
-    }
-  }
-  else if (nsCRT::strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
+  } else if (nsCRT::strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
 
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent: XPCom shutdown observed\n"));
 
     // Cleanup code that requires services, it's too late in destructor.
 
     nsCOMPtr<nsIEntropyCollector> ec
         = do_GetService(NS_ENTROPYCOLLECTOR_CONTRACTID);
 
@@ -1421,17 +1391,16 @@ nsNSSComponent::RegisterObservers()
   }
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent: adding observers\n"));
   // Using false for the ownsweak parameter means the observer service will
   // keep a strong reference to this component. As a result, this will live at
   // least as long as the observer service.
   observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
   observerService->AddObserver(this, PROFILE_BEFORE_CHANGE_TOPIC, false);
-  observerService->AddObserver(this, PROFILE_DO_CHANGE_TOPIC, false);
 
   return NS_OK;
 }
 
 void
 nsNSSComponent::DoProfileBeforeChange()
 {
   bool needsCleanup = true;
--- a/security/manager/ssl/tests/unit/test_pkcs11_safe_mode.js
+++ b/security/manager/ssl/tests/unit/test_pkcs11_safe_mode.js
@@ -1,26 +1,25 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
-// This test loads a testing PKCS #11 module. After simulating a shutdown and
-// startup, the test verifies that the module is automatically loaded again.
-// After simulating a shutdown and restart in safe mode, the test verifies that
-// the module is not automatically loaded again.
+// In safe mode, PKCS#11 modules should not be loaded. This test tests this by
+// simulating starting in safe mode and then attempting to load a module.
 
 var { XPCOMUtils } = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
 
-// Registers an nsIXULRuntime so the test can control whether or not the
-// platform thinks it's in safe mode.
-function registerXULRuntime() {
+function run_test() {
+  do_get_profile();
+
+  // Simulate starting in safe mode.
   let xulRuntime = {
-    inSafeMode: false,
+    inSafeMode: true,
     logConsoleErrors: true,
     OS: "XPCShell",
     XPCOMABI: "noarch-spidermonkey",
     invalidateCachesOnRestart: function invalidateCachesOnRestart() {
       // Do nothing
     },
     QueryInterface: XPCOMUtils.generateQI([Ci.nsIXULRuntime])
   };
@@ -34,67 +33,25 @@ function registerXULRuntime() {
     }
   };
 
   let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
   const XULRUNTIME_CONTRACTID = "@mozilla.org/xre/runtime;1";
   const XULRUNTIME_CID = Components.ID("{f0f0b230-5525-4127-98dc-7bca39059e70}");
   registrar.registerFactory(XULRUNTIME_CID, "XULRuntime", XULRUNTIME_CONTRACTID,
                             xulRuntimeFactory);
-  return xulRuntime;
-}
 
-// Loads the PKCS11 test module.
-function loadTestModule() {
+  // When starting in safe mode, the test module should fail to load.
   let pkcs11 = Cc["@mozilla.org/security/pkcs11;1"].getService(Ci.nsIPKCS11);
   let libraryName = ctypes.libraryName("pkcs11testmodule");
   let libraryFile = Services.dirsvc.get("CurWorkD", Ci.nsILocalFile);
   libraryFile.append("pkcs11testmodule");
   libraryFile.append(libraryName);
   ok(libraryFile.exists(), "The pkcs11testmodule file should exist");
-  pkcs11.addModule("PKCS11 Test Module", libraryFile.path, 0, 0);
-}
-
-// Tests finding the PKCS11 test module. 'shouldSucceed' is a boolean
-// indicating whether or not the test module is expected to be found.
-function testFindTestModule(shouldSucceed) {
-  let pkcs11ModuleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
-                         .getService(Ci.nsIPKCS11ModuleDB);
+  let exceptionCaught = false;
   try {
-    let module = pkcs11ModuleDB.findModuleByName("PKCS11 Test Module");
-    ok(shouldSucceed, "Success expected: findModuleByName should not throw");
-    ok(module, "module should be non-null");
+    pkcs11.addModule("PKCS11 Test Module", libraryFile.path, 0, 0);
+    ok(false, "addModule should have thrown an exception");
   } catch (e) {
-    ok(!shouldSucceed, "Failure expected: findModuleByName should throw");
+    exceptionCaught = true;
   }
-}
-
-function simulateShutdown() {
-  let psmComponent = Cc["@mozilla.org/psm;1"].getService(Ci.nsIObserver);
-  psmComponent.observe(null, "profile-before-change", null);
-  psmComponent.observe(null, "xpcom-shutdown", null);
+  ok(exceptionCaught, "addModule should have thrown an exception");
 }
-
-function simulateStartup() {
-  let psmComponent = Cc["@mozilla.org/psm;1"].getService(Ci.nsIObserver);
-  psmComponent.observe(null, "profile-do-change", null);
-}
-
-function run_test() {
-  do_get_profile();
-  let xulRuntime = registerXULRuntime();
-  loadTestModule();
-
-  // After having loaded the test module, it should be available.
-  testFindTestModule(true);
-  simulateShutdown();
-  simulateStartup();
-  // After shutting down and starting up, the test module should automatically
-  // be loaded again.
-  testFindTestModule(true);
-
-  simulateShutdown();
-  // Simulate starting in safe mode.
-  xulRuntime.inSafeMode = true;
-  simulateStartup();
-  // When starting in safe mode, the test module should not be loaded.
-  testFindTestModule(false);
-}