Bug 1634065 - re-work how PSM services get initialized on the main thread r=kjacobs,necko-reviewers,bbeurdouche

Some PSM services need to be initialized on the main thread. Before this patch,
this was achieved by dispatching a synchronous task to the main thread in the
event that a different thread was attempting to acquire a given service for the
first time. However, with the upcoming removal of the nested event loop in the
XPCOM service instantiation code (see other patches in this bug), this can
cause a deadlock. This patch avoids the deadlock by removing the synchronous
dispatch and ensuring that these services get initialized on the main thread
relatively early, when PSM itself is initialized.

Differential Revision: https://phabricator.services.mozilla.com/D94145

netwerk/base/nsNetUtil.cpp

@@ -2670,13 +2670,9 @@ void net_EnsurePSMInit() {
   nsCOMPtr<nsISupports> psm = do_GetService(PSM_COMPONENT_CONTRACTID, &rv);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 
-  nsCOMPtr<nsISupports> sss = do_GetService(NS_SSSERVICE_CONTRACTID);
-#ifdef MOZ_NEW_CERT_STORAGE
-  nsCOMPtr<nsISupports> cbl = do_GetService(NS_CERTSTORAGE_CONTRACTID);
-#else
+#ifndef MOZ_NEW_CERT_STORAGE
   nsCOMPtr<nsISupports> cbl = do_GetService(NS_CERTBLOCKLIST_CONTRACTID);
 #endif
-  nsCOMPtr<nsISupports> cos = do_GetService(NS_CERTOVERRIDE_CONTRACTID);
 }
 
 bool NS_IsAboutBlank(nsIURI* uri) {

security/manager/ssl/CSTrustDomain.h

@@ -8,12 +8,14 @@
 #define CSTrustDomain_h
 
 #include "mozpkix/pkixtypes.h"
+#include "nsCOMPtr.h"
+#include "nsTArray.h"
+
 #ifdef MOZ_NEW_CERT_STORAGE
 #  include "nsICertStorage.h"
 #else
 #  include "nsICertBlocklist.h"
 #endif
-#include "nsTArray.h"
 
 namespace mozilla {
 namespace psm {

security/manager/ssl/SecretDecoderRing.cpp

@@ -310,25 +310,27 @@ SecretDecoderRing::ChangePassword() {
 NS_IMETHODIMP
 SecretDecoderRing::Logout() {
   PK11_LogoutAll();
-  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
-  return NS_OK;
+  nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(NS_NSSCOMPONENT_CID));
+  if (!nssComponent) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return nssComponent->ClearSSLExternalAndInternalSessionCache();
 }
 
 NS_IMETHODIMP
 SecretDecoderRing::LogoutAndTeardown() {
-  static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
-
   PK11_LogoutAll();
-  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
+  nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(NS_NSSCOMPONENT_CID));
+  if (!nssComponent) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
 
-  nsresult rv;
-  nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, &rv));
+  // LogoutAuthenticatedPK11 also clears the SSL caches.
+  nsresult rv = nssComponent->LogoutAuthenticatedPK11();
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  rv = nssComponent->LogoutAuthenticatedPK11();
-
   // After we just logged out, we need to prune dead connections to make
   // sure that all connections that should be stopped, are stopped. See
   // bug 517584.
@@ -337,5 +339,5 @@ SecretDecoderRing::LogoutAndTeardown() {
     os->NotifyObservers(nullptr, "net:prune-dead-connections", nullptr);
   }
 
-  return rv;
+  return NS_OK;
 }

security/manager/ssl/cert_storage/src/cert_storage.cpp

@@ -1,23 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 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/. */
-#include "cert_storage.h"
-
-nsresult construct_cert_storage(nsISupports* outer, REFNSIID iid,
-                                void** result) {
-  // Forward to the main thread synchronously.
-  nsCOMPtr<nsIThread> mainThread;
-  nsresult rv = NS_GetMainThread(getter_AddRefs(mainThread));
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  mozilla::SyncRunnable::DispatchToThread(
-      mainThread, new mozilla::SyncRunnable(
-                      NS_NewRunnableFunction("psm::Constructor", [&]() {
-                        rv = cert_storage_constructor(outer, iid, result);
-                      })));
-  return rv;
-}

security/manager/ssl/cert_storage/src/cert_storage.h

@@ -8,7 +8,6 @@
 #define _cert_storage_h_
 
 #include "nsISupportsUtils.h"  // for nsresult, etc.
-#include "mozilla/SyncRunnable.h"
 
 // {16e5c837-f877-4e23-9c64-eddf905e30e6}
 #define NS_CERT_STORAGE_CID                          \
@@ -23,7 +22,4 @@ nsresult cert_storage_constructor(nsISupports* outer, REFNSIID iid,
                                   void** result);
 };
 
-nsresult construct_cert_storage(nsISupports* outer, REFNSIID iid,
-                                void** result);
-
-#endif  // _cert_storage_h_
+#endif  // _cert_storage_h_

security/manager/ssl/cert_storage/src/lib.rs

@@ -1149,6 +1149,10 @@ pub extern "C" fn cert_storage_constructor(
         return NS_ERROR_NO_AGGREGATION;
     }
 
+    if !is_main_thread() {
+        return NS_ERROR_NOT_SAME_THREAD;
+    }
+
     match do_construct_cert_storage(outer, iid, result) {
         Ok(_) => NS_OK,
         Err(_) => {

security/manager/ssl/components.conf

@@ -106,7 +106,8 @@ Classes = [
         'cid': '{67ba681d-5485-4fff-952c-2ee337ffdcd6}',
         'contract_ids': ['@mozilla.org/security/certoverride;1'],
         'type': 'nsCertOverrideService',
-        'legacy_constructor': 'mozilla::psm::NSSConstructor<nsCertOverrideService>',
+        'headers': ['/security/manager/ssl/nsCertOverrideService.h'],
+        'init_method': 'Init',
     },
     {
         'cid': '{be65e2b7-fe46-4e0f-88e0-4b385db4d68a}',
@@ -125,7 +126,8 @@ Classes = [
         'cid': '{16955eee-6c48-4152-9309-c42a465138a1}',
         'contract_ids': ['@mozilla.org/ssservice;1'],
         'type': 'nsSiteSecurityService',
-        'legacy_constructor': 'mozilla::psm::NSSConstructor<nsSiteSecurityService>',
+        'headers': ['/security/manager/ssl/nsSiteSecurityService.h'],
+        'init_method': 'Init',
     },
     {
         'cid': '{57972956-5718-42d2-8070-b3fc72212eaf}',
@@ -157,7 +159,7 @@ if defined('MOZ_NEW_CERT_STORAGE'):
             'cid': '{16e5c837-f877-4e23-9c64-eddf905e30e6}',
             'contract_ids': ['@mozilla.org/security/certstorage;1'],
             'headers': ['/security/manager/ssl/cert_storage/src/cert_storage.h'],
-            'legacy_constructor': 'construct_cert_storage',
+            'legacy_constructor': 'cert_storage_constructor',
         },
     ]
 else:

security/manager/ssl/moz.build

@@ -203,9 +203,6 @@ if CONFIG["MOZ_NEW_CERT_STORAGE"]:
     XPIDL_SOURCES += [
         "nsICertStorage.idl",
     ]
-    UNIFIED_SOURCES += [
-        "cert_storage/src/cert_storage.cpp",
-    ]
 else:
     XPIDL_SOURCES += [
         "nsICertBlocklist.idl",

security/manager/ssl/nsCertOverrideService.h

@@ -153,7 +153,7 @@ class nsCertOverrideService final : public nsICertOverrideService,
                           const nsACString& dbKey,
                           const mozilla::MutexAutoLock& aProofOfLock);
 
-  RefPtr<TaskQueue> mWriterTaskQueue;
+  RefPtr<mozilla::TaskQueue> mWriterTaskQueue;
 
   // Only accessed on the main thread
   uint64_t mPendingWriteCount;

security/manager/ssl/nsClientAuthRemember.cpp

@@ -80,8 +80,11 @@ nsClientAuthRememberService::ForgetRememberedDecision(const nsACString& key) {
   mClientAuthRememberList->Remove(PromiseFlatCString(key),
                                   mozilla::DataStorage_Persistent);
 
-  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
-  return NS_OK;
+  nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(NS_NSSCOMPONENT_CID));
+  if (!nssComponent) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return nssComponent->ClearSSLExternalAndInternalSessionCache();
 }
 
 NS_IMETHODIMP
@@ -105,8 +108,11 @@ nsClientAuthRememberService::GetDecisions(
 NS_IMETHODIMP
 nsClientAuthRememberService::ClearRememberedDecisions() {
   mClientAuthRememberList->Clear();
-  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
-  return NS_OK;
+  nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(NS_NSSCOMPONENT_CID));
+  if (!nssComponent) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return nssComponent->ClearSSLExternalAndInternalSessionCache();
 }
 
 NS_IMETHODIMP
@@ -133,8 +139,11 @@ nsClientAuthRememberService::DeleteDecisionsByHost(
       }
     }
   }
-  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
-  return NS_OK;
+  nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(NS_NSSCOMPONENT_CID));
+  if (!nssComponent) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return nssComponent->ClearSSLExternalAndInternalSessionCache();
 }
 
 NS_IMETHODIMP

security/manager/ssl/nsNSSComponent.cpp

@@ -43,6 +43,7 @@
 #include "nsIPrompt.h"
 #include "nsIProperties.h"
 #include "nsISerialEventTarget.h"
+#include "nsISiteSecurityService.h"
 #include "nsITimer.h"
 #include "nsITokenPasswordDialogs.h"
 #include "nsIWindowWatcher.h"
@@ -167,7 +168,7 @@ static const uint32_t OCSP_TIMEOUT_MILLISECONDS_SOFT_MAX = 5000;
 static const uint32_t OCSP_TIMEOUT_MILLISECONDS_HARD_DEFAULT = 10000;
 static const uint32_t OCSP_TIMEOUT_MILLISECONDS_HARD_MAX = 20000;
 
-static void GetRevocationBehaviorFromPrefs(
+void nsNSSComponent::GetRevocationBehaviorFromPrefs(
     /*out*/ CertVerifier::OcspDownloadConfig* odc,
     /*out*/ CertVerifier::OcspStrictConfig* osc,
     /*out*/ uint32_t* certShortLifetimeInDays,
@@ -218,7 +219,7 @@ static void GetRevocationBehaviorFromPrefs(
       std::min(hardTimeoutMillis, OCSP_TIMEOUT_MILLISECONDS_HARD_MAX);
   hardTimeout = TimeDuration::FromMilliseconds(hardTimeoutMillis);
 
-  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
+  ClearSSLExternalAndInternalSessionCache();
 }
 
 nsNSSComponent::nsNSSComponent()
@@ -2045,8 +2046,16 @@ nsresult nsNSSComponent::InitializeNSS() {
     return NS_ERROR_UNEXPECTED;
   }
 
-  nsCOMPtr<nsIClientAuthRememberService> cars =
-      do_GetService(NS_CLIENTAUTHREMEMBERSERVICE_CONTRACTID);
+  nsCOMPtr<nsICertOverrideService> certOverrideService(
+      do_GetService(NS_CERTOVERRIDE_CONTRACTID));
+  nsCOMPtr<nsIClientAuthRememberService> clientAuthRememberService(
+      do_GetService(NS_CLIENTAUTHREMEMBERSERVICE_CONTRACTID));
+  nsCOMPtr<nsISiteSecurityService> siteSecurityService(
+      do_GetService(NS_SSSERVICE_CONTRACTID));
+
+#ifdef MOZ_NEW_CERT_STORAGE
+  nsCOMPtr<nsICertStorage> certStorage(do_GetService(NS_CERT_STORAGE_CID));
+#endif
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("NSS Initialization done\n"));
 
@@ -2459,7 +2468,7 @@ nsNSSComponent::Observe(nsISupports* aSubject, const char* aTopic,
       clearSessionCache = false;
     }
     if (clearSessionCache) {
-      ClearSSLExternalAndInternalSessionCacheNative();
+      ClearSSLExternalAndInternalSessionCache();
     }
 
     // Preferences that don't affect certificate verification.
@@ -2501,7 +2510,7 @@ nsresult nsNSSComponent::LogoutAuthenticatedPK11() {
     icos->ClearValidityOverride("all:temporary-certificates"_ns, 0);
   }
 
-  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
+  ClearSSLExternalAndInternalSessionCache();
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
@@ -2611,8 +2620,17 @@ nsNSSComponent::GetDefaultCertVerifier(SharedCertVerifier** result) {
 }
 
 // static
-void nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative() {
+void nsNSSComponent::DoClearSSLExternalAndInternalSessionCache() {
+  SSL_ClearSessionCache();
+  mozilla::net::SSLTokensCache::Clear();
+}
+
+NS_IMETHODIMP
+nsNSSComponent::ClearSSLExternalAndInternalSessionCache() {
   MOZ_ASSERT(XRE_IsParentProcess());
+  if (!XRE_IsParentProcess()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
 
   if (mozilla::net::nsIOService::UseSocketProcess()) {
     if (mozilla::net::gIOService) {
@@ -2623,17 +2641,6 @@ void nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative() {
     }
   }
   DoClearSSLExternalAndInternalSessionCache();
-}
-
-// static
-void nsNSSComponent::DoClearSSLExternalAndInternalSessionCache() {
-  SSL_ClearSessionCache();
-  mozilla::net::SSLTokensCache::Clear();
-}
-
-NS_IMETHODIMP
-nsNSSComponent::ClearSSLExternalAndInternalSessionCache() {
-  ClearSSLExternalAndInternalSessionCacheNative();
   return NS_OK;
 }
 

security/manager/ssl/nsNSSComponent.h

@@ -79,12 +79,10 @@ class nsNSSComponent final : public nsINSSComponent, public nsIObserver {
 
   static nsresult SetEnabledTLSVersions();
 
-  // This function should be only called on parent process.
-  // When socket process is enabled, this function sends an IPC to clear the
-  // SSLTokensCache in socket process. If not,
-  // DoClearSSLExternalAndInternalSessionCache() will be called.
-  static void ClearSSLExternalAndInternalSessionCacheNative();
-  // This function does the actual work of clearing the session cache.
+  // This function does the actual work of clearing the session cache. It is to
+  // be used by the socket process (where there is no nsINSSComponent) and
+  // internally by nsNSSComponent.
+  // NB: NSS must have already been initialized before this is called.
   static void DoClearSSLExternalAndInternalSessionCache();
 
  protected:
@@ -96,6 +94,13 @@ class nsNSSComponent final : public nsINSSComponent, public nsIObserver {
 
   void setValidationOptions(bool isInitialSetting,
                             const mozilla::MutexAutoLock& proofOfLock);
+  void GetRevocationBehaviorFromPrefs(
+      /*out*/ mozilla::psm::CertVerifier::OcspDownloadConfig* odc,
+      /*out*/ mozilla::psm::CertVerifier::OcspStrictConfig* osc,
+      /*out*/ uint32_t* certShortLifetimeInDays,
+      /*out*/ TimeDuration& softTimeout,
+      /*out*/ TimeDuration& hardTimeout,
+      const mozilla::MutexAutoLock& proofOfLock);
   void UpdateCertVerifierWithEnterpriseRoots();
   nsresult RegisterObservers();
 

security/manager/ssl/nsNSSModule.cpp

@@ -20,7 +20,6 @@
 #include "mozilla/ModuleUtils.h"
 #include "mozilla/SyncRunnable.h"
 #include "nsCURILoader.h"
-#include "nsCertOverrideService.h"
 #include "nsCryptoHash.h"
 #include "nsKeyModule.h"
 #include "nsNSSCertificate.h"
@@ -32,7 +31,6 @@
 #include "nsPKCS11Slot.h"
 #include "nsRandomGenerator.h"
 #include "nsSecureBrowserUI.h"
-#include "nsSiteSecurityService.h"
 #include "nsXULAppAPI.h"
 
 #ifdef MOZ_XUL
@@ -97,20 +95,7 @@ static nsresult Constructor(nsISupports* aOuter, REFNSIID aIID,
 
   if (threadRestriction == ThreadRestriction::MainThreadOnly &&
       !NS_IsMainThread()) {
-    nsCOMPtr<nsIThread> mainThread;
-    nsresult rv = NS_GetMainThread(getter_AddRefs(mainThread));
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    // Forward to the main thread synchronously.
-    mozilla::SyncRunnable::DispatchToThread(
-        mainThread,
-        new SyncRunnable(NS_NewRunnableFunction("psm::Constructor", [&]() {
-          rv = Instantiate<InstanceClass, InitMethod>(aIID, aResult);
-        })));
-
-    return rv;
+    return NS_ERROR_NOT_SAME_THREAD;
   }
 
   return Instantiate<InstanceClass, InitMethod>(aIID, aResult);
@@ -140,12 +125,8 @@ IMPL(nsCryptoHMAC, nullptr, ProcessRestriction::AnyProcess)
 IMPL(nsKeyObject, nullptr, ProcessRestriction::AnyProcess)
 IMPL(nsKeyObjectFactory, nullptr, ProcessRestriction::AnyProcess)
 IMPL(ContentSignatureVerifier, nullptr)
-IMPL(nsCertOverrideService, &nsCertOverrideService::Init,
-     ProcessRestriction::ParentProcessOnly, ThreadRestriction::MainThreadOnly)
 IMPL(nsRandomGenerator, nullptr, ProcessRestriction::AnyProcess)
 IMPL(TransportSecurityInfo, nullptr, ProcessRestriction::AnyProcess)
-IMPL(nsSiteSecurityService, &nsSiteSecurityService::Init,
-     ProcessRestriction::AnyProcess, ThreadRestriction::MainThreadOnly)
 #ifndef MOZ_NEW_CERT_STORAGE
 IMPL(CertBlocklist, &CertBlocklist::Init, ProcessRestriction::ParentProcessOnly,
      ThreadRestriction::MainThreadOnly)

security/manager/ssl/nsSiteSecurityService.cpp

@@ -5,7 +5,6 @@
 #include "nsSiteSecurityService.h"
 
 #include "PublicKeyPinningService.h"
-#include "ScopedNSSTypes.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Base64.h"
 #include "mozilla/LinkedList.h"
@@ -20,8 +19,6 @@
 #include "nsISocketProvider.h"
 #include "nsITransportSecurityInfo.h"
 #include "nsIURI.h"
-#include "nsIX509Cert.h"
-#include "nsNSSCertificateDB.h"
 #include "nsNSSComponent.h"
 #include "nsNetUtil.h"
 #include "nsPromiseFlatString.h"