Bug 1427248 - Avoid changing certificate trust in nsNSSComponent initialization. r=fkiefer, r=jcj, a=jcristau
authorDavid Keeler <dkeeler@mozilla.com>
Tue, 15 May 2018 13:37:42 -0700
changeset 802195 f7ba2965406d08645df693bfe3ce8b798a512915
parent 802194 f10bc261be65712b1d508bf7b7b0750f2c66d161
child 802196 b63a4acbf1f84cf76e1e821f40f2149bd8bc7a9f
push id111850
push userbmo:tom@mozilla.com
push dateThu, 31 May 2018 16:41:37 +0000
reviewersfkiefer, jcj, jcristau
bugs1427248
milestone60.0.2
Bug 1427248 - Avoid changing certificate trust in nsNSSComponent initialization. r=fkiefer, r=jcj, a=jcristau If a user has set a master password on their NSS DB(s), when we try to change the trust of a certificate, we may have to authenticate to the DB. This involves bringing up a dialog box, executing javascript, spinning the event loop, etc. In some cases (particularly when antivirus software has injected code into Firefox), this can cause the nsNSSComponent to be initialized if it hasn't already been. So, it's a really, really bad idea to attempt to change the trust of a certificate while we're initializing nsNSSComponent, because this results in a recursive component dependency and everything breaks. To get around this, if we need to load 3rd party roots (e.g. enterprise roots or the family safety root), we defer any trust changes to a later event loop tick. In theory this could cause verification failures early in startup. We'll have to see if this is an issue in practice. MozReview-Commit-ID: FvjHP5dTmpP
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/nsNSSComponent.h
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -559,27 +559,16 @@ nsNSSComponent::MaybeImportFamilySafetyR
     return NS_ERROR_FAILURE;
   }
   // Looking for a certificate with the common name 'Microsoft Family Safety'
   UniquePORTString subjectName(CERT_GetCommonName(&nssCertificate->subject));
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
           ("subject name is '%s'", subjectName.get()));
   if (kMicrosoftFamilySafetyCN.Equals(subjectName.get())) {
     wasFamilySafetyRoot = true;
-    CERTCertTrust trust = {
-      CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_USER,
-      0,
-      0
-    };
-    if (ChangeCertTrustWithPossibleAuthentication(nssCertificate, trust,
-                                                  nullptr) != SECSuccess) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-              ("couldn't trust certificate for TLS server auth"));
-      return NS_ERROR_FAILURE;
-    }
     MOZ_ASSERT(!mFamilySafetyRoot);
     mFamilySafetyRoot = Move(nssCertificate);
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("added Family Safety root"));
   }
   return NS_OK;
 }
 
 // Because HCERTSTORE is just a typedef void*, we can't use any of the nice
@@ -660,36 +649,32 @@ nsNSSComponent::UnloadFamilySafetyRoot()
   if (ChangeCertTrustWithPossibleAuthentication(mFamilySafetyRoot, trust,
                                                 nullptr) != SECSuccess) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
             ("couldn't untrust certificate for TLS server auth"));
   }
   mFamilySafetyRoot = nullptr;
 }
 
-#endif // XP_WIN
-
 // The supported values of this pref are:
 // 0: disable detecting Family Safety mode and importing the root
 // 1: only attempt to detect Family Safety mode (don't import the root)
 // 2: detect Family Safety mode and import the root
 const char* kFamilySafetyModePref = "security.family_safety.mode";
 
 // The telemetry gathered by this function is as follows:
 // 0-2: the value of the Family Safety mode pref
 // 3: detecting Family Safety mode failed
 // 4: Family Safety was not enabled
 // 5: Family Safety was enabled
 // 6: failed to import the Family Safety root
 // 7: successfully imported the root
 void
 nsNSSComponent::MaybeEnableFamilySafetyCompatibility()
 {
-#ifdef XP_WIN
-  UnloadFamilySafetyRoot();
   if (!(IsWin8Point1OrLater() && !IsWin10OrLater())) {
     return;
   }
   // Detect but don't import by default.
   uint32_t familySafetyMode = Preferences::GetUint(kFamilySafetyModePref, 1);
   if (familySafetyMode > 2) {
     familySafetyMode = 0;
   }
@@ -713,20 +698,18 @@ nsNSSComponent::MaybeEnableFamilySafetyC
     if (NS_FAILED(rv)) {
       Telemetry::Accumulate(Telemetry::FAMILY_SAFETY, 6);
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
               ("failed to load Family Safety root"));
     } else {
       Telemetry::Accumulate(Telemetry::FAMILY_SAFETY, 7);
     }
   }
-#endif // XP_WIN
 }
 
-#ifdef XP_WIN
 // Helper function to determine if the OS considers the given certificate to be
 // a trust anchor for TLS server auth certificates. This is to be used in the
 // context of importing what are presumed to be root certificates from the OS.
 // If this function returns true but it turns out that the given certificate is
 // in some way unsuitable to issue certificates, mozilla::pkix will never build
 // a valid chain that includes the certificate, so importing it even if it
 // isn't a valid CA poses no risk.
 static bool
@@ -769,18 +752,19 @@ CertIsTrustAnchorForTLSServerAuth(PCCERT
     return true;
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
           ("certificate not trust anchor for TLS server auth"));
   return false;
 }
 
 void
-nsNSSComponent::UnloadEnterpriseRoots(const MutexAutoLock& /*proof of lock*/)
+nsNSSComponent::UnloadEnterpriseRoots()
 {
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(NS_IsMainThread());
   if (!NS_IsMainThread()) {
     return;
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("UnloadEnterpriseRoots"));
   if (!mEnterpriseRoots) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("no enterprise roots were present"));
     return;
@@ -833,30 +817,27 @@ nsNSSComponent::GetEnterpriseRoots(nsIX5
   nsCOMPtr<nsIX509CertList> enterpriseRootsCertList(
     new nsNSSCertList(Move(enterpriseRootsCopy)));
   if (!enterpriseRootsCertList) {
     return NS_ERROR_FAILURE;
   }
   enterpriseRootsCertList.forget(enterpriseRoots);
   return NS_OK;
 }
-#endif // XP_WIN
 
 static const char* kEnterpriseRootModePref = "security.enterprise_roots.enabled";
 
 void
 nsNSSComponent::MaybeImportEnterpriseRoots()
 {
-#ifdef XP_WIN
   MutexAutoLock lock(mMutex);
   MOZ_ASSERT(NS_IsMainThread());
   if (!NS_IsMainThread()) {
     return;
   }
-  UnloadEnterpriseRoots(lock);
   bool importEnterpriseRoots = Preferences::GetBool(kEnterpriseRootModePref,
                                                     false);
   if (!importEnterpriseRoots) {
     return;
   }
 
   MOZ_ASSERT(!mEnterpriseRoots);
   mEnterpriseRoots.reset(CERT_NewCertList());
@@ -866,20 +847,18 @@ nsNSSComponent::MaybeImportEnterpriseRoo
     return;
   }
 
   ImportEnterpriseRootsForLocation(CERT_SYSTEM_STORE_LOCAL_MACHINE, lock);
   ImportEnterpriseRootsForLocation(CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY,
                                    lock);
   ImportEnterpriseRootsForLocation(CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE,
                                    lock);
-#endif // XP_WIN
 }
 
-#ifdef XP_WIN
 // Loads the enterprise roots at the registry location corresponding to the
 // given location flag.
 // Supported flags are:
 //   CERT_SYSTEM_STORE_LOCAL_MACHINE
 //     (for HKLM\SOFTWARE\Microsoft\SystemCertificates)
 //   CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY
 //     (for HKLM\SOFTWARE\Policies\Microsoft\SystemCertificates\Root\Certificates)
 //   CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE
@@ -912,21 +891,16 @@ nsNSSComponent::ImportEnterpriseRootsFor
   // https://msdn.microsoft.com/en-us/library/windows/desktop/aa376559%28v=vs.85%29.aspx
   ScopedCertStore enterpriseRootStore(CertOpenStore(
     CERT_STORE_PROV_SYSTEM_REGISTRY_W, 0, NULL, flags,
     kWindowsDefaultRootStoreName));
   if (!enterpriseRootStore.get()) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("failed to open enterprise root store"));
     return;
   }
-  CERTCertTrust trust = {
-    CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_USER,
-    0,
-    0
-  };
   PCCERT_CONTEXT certificate = nullptr;
   uint32_t numImported = 0;
   while ((certificate = CertFindCertificateInStore(enterpriseRootStore.get(),
                                                    X509_ASN_ENCODING, 0,
                                                    CERT_FIND_ANY, nullptr,
                                                    certificate))) {
     if (!CertIsTrustAnchorForTLSServerAuth(certificate)) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
@@ -952,28 +926,58 @@ nsNSSComponent::ImportEnterpriseRootsFor
     if (!mEnterpriseRoots) {
       return;
     }
     if (CERT_AddCertToListTail(mEnterpriseRoots.get(), nssCertificate.get())
           != SECSuccess) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("couldn't add cert to list"));
       continue;
     }
-    if (ChangeCertTrustWithPossibleAuthentication(nssCertificate, trust,
-                                                  nullptr) != SECSuccess) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-              ("couldn't trust certificate for TLS server auth"));
-    }
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Imported '%s'", subjectName.get()));
     numImported++;
     // now owned by mEnterpriseRoots
     Unused << nssCertificate.release();
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("imported %u roots", numImported));
 }
+
+NS_IMETHODIMP
+nsNSSComponent::TrustLoaded3rdPartyRoots()
+{
+  MutexAutoLock lock(mMutex);
+
+  CERTCertTrust trust = {
+    CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_USER,
+    0,
+    0
+  };
+  if (mEnterpriseRoots) {
+    for (CERTCertListNode* n = CERT_LIST_HEAD(mEnterpriseRoots.get());
+         !CERT_LIST_END(n, mEnterpriseRoots.get()); n = CERT_LIST_NEXT(n)) {
+      if (!n || !n->cert) {
+        MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+                ("library failure: CERTCertListNode null or lacks cert"));
+        continue;
+      }
+      UniqueCERTCertificate cert(CERT_DupCertificate(n->cert));
+      if (ChangeCertTrustWithPossibleAuthentication(cert, trust, nullptr)
+            != SECSuccess) {
+        MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+                ("couldn't trust enterprise certificate for TLS server auth"));
+      }
+    }
+  }
+  if (mFamilySafetyRoot &&
+      ChangeCertTrustWithPossibleAuthentication(mFamilySafetyRoot, trust,
+                                                nullptr) != SECSuccess) {
+    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+            ("couldn't trust family safety certificate for TLS server auth"));
+  }
+  return NS_OK;
+}
 #endif // XP_WIN
 
 class LoadLoadableRootsTask final : public Runnable
 {
 public:
   explicit LoadLoadableRootsTask(nsNSSComponent* nssComponent)
     : Runnable("LoadLoadableRootsTask")
     , mNSSComponent(nssComponent)
@@ -2045,18 +2049,26 @@ nsNSSComponent::InitializeNSS()
 
   rv = setEnabledTLSVersions();
   if (NS_FAILED(rv)) {
     return NS_ERROR_UNEXPECTED;
   }
 
   DisableMD5();
 
+#ifdef XP_WIN
+  // Note that these functions do not change the trust of any loaded 3rd party
+  // roots. Because we're initializing the nsNSSComponent, and because if the
+  // user has a master password set on the softoken it could cause the
+  // authentication dialog to come up, we could conceivably re-enter
+  // nsNSSComponent initialization, which would be bad. Instead, we schedule an
+  // event to set the trust after the component has been initialized (below).
   MaybeEnableFamilySafetyCompatibility();
   MaybeImportEnterpriseRoots();
+#endif // XP_WIN
 
   ConfigureTLSSessionIdentifiers();
 
   bool requireSafeNegotiation =
     Preferences::GetBool("security.ssl.require_safe_negotiation",
                          REQUIRE_SAFE_NEGOTIATION_DEFAULT);
   SSL_OptionSetDefault(SSL_REQUIRE_SAFE_NEGOTIATION, requireSafeNegotiation);
 
@@ -2139,16 +2151,24 @@ nsNSSComponent::InitializeNSS()
 #endif
     mContentSigningRootHash.Truncate();
     Preferences::GetString("security.content.signature.root_hash",
                            mContentSigningRootHash);
 
     mNSSInitialized = true;
   }
 
+#ifdef XP_WIN
+  nsCOMPtr<nsINSSComponent> handle(this);
+  NS_DispatchToCurrentThread(NS_NewRunnableFunction("nsNSSComponent::TrustLoaded3rdPartyRoots",
+  [handle]() {
+    MOZ_ALWAYS_SUCCEEDS(handle->TrustLoaded3rdPartyRoots());
+  }));
+#endif // XP_WIN
+
   RefPtr<LoadLoadableRootsTask> loadLoadableRootsTask(
     new LoadLoadableRootsTask(this));
   return loadLoadableRootsTask->Dispatch();
 }
 
 void
 nsNSSComponent::ShutdownNSS()
 {
@@ -2293,25 +2313,37 @@ nsNSSComponent::Observe(nsISupports* aSu
       setValidationOptions(false);
 #ifdef DEBUG
     } else if (prefName.EqualsLiteral("security.test.built_in_root_hash")) {
       MutexAutoLock lock(mMutex);
       mTestBuiltInRootHash.Truncate();
       Preferences::GetString("security.test.built_in_root_hash",
                              mTestBuiltInRootHash);
 #endif // DEBUG
+#ifdef XP_WIN
     } else if (prefName.Equals(kFamilySafetyModePref)) {
+      // When the pref changes, it is safe to change the trust of 3rd party
+      // roots in the same event tick that they're loaded.
+      UnloadFamilySafetyRoot();
       MaybeEnableFamilySafetyCompatibility();
+      TrustLoaded3rdPartyRoots();
+#endif // XP_WIN
     } else if (prefName.EqualsLiteral("security.content.signature.root_hash")) {
       MutexAutoLock lock(mMutex);
       mContentSigningRootHash.Truncate();
       Preferences::GetString("security.content.signature.root_hash",
                              mContentSigningRootHash);
+#ifdef XP_WIN
     } else if (prefName.Equals(kEnterpriseRootModePref)) {
+      // When the pref changes, it is safe to change the trust of 3rd party
+      // roots in the same event tick that they're loaded.
+      UnloadEnterpriseRoots();
       MaybeImportEnterpriseRoots();
+      TrustLoaded3rdPartyRoots();
+#endif // XP_WIN
     } else {
       clearSessionCache = false;
     }
     if (clearSessionCache)
       SSL_ClearSessionCache();
   }
 
   return NS_OK;
--- a/security/manager/ssl/nsNSSComponent.h
+++ b/security/manager/ssl/nsNSSComponent.h
@@ -70,16 +70,17 @@ public:
 #ifdef DEBUG
   NS_IMETHOD IsCertTestBuiltInRoot(CERTCertificate* cert, bool& result) = 0;
 #endif
 
   NS_IMETHOD IsCertContentSigningRoot(CERTCertificate* cert, bool& result) = 0;
 
 #ifdef XP_WIN
   NS_IMETHOD GetEnterpriseRoots(nsIX509CertList** enterpriseRoots) = 0;
+  NS_IMETHOD TrustLoaded3rdPartyRoots() = 0;
 #endif
 
   NS_IMETHOD BlockUntilLoadableRootsLoaded() = 0;
   NS_IMETHOD CheckForSmartCardChanges() = 0;
 
   // Main thread only
   NS_IMETHOD HasActiveSmartCards(bool& result) = 0;
   NS_IMETHOD HasUserCertsInstalled(bool& result) = 0;
@@ -122,16 +123,17 @@ public:
 #ifdef DEBUG
   NS_IMETHOD IsCertTestBuiltInRoot(CERTCertificate* cert, bool& result) override;
 #endif
 
   NS_IMETHOD IsCertContentSigningRoot(CERTCertificate* cert, bool& result) override;
 
 #ifdef XP_WIN
   NS_IMETHOD GetEnterpriseRoots(nsIX509CertList** enterpriseRoots) override;
+  NS_IMETHOD TrustLoaded3rdPartyRoots() override;
 #endif
 
   NS_IMETHOD BlockUntilLoadableRootsLoaded() override;
   NS_IMETHOD CheckForSmartCardChanges() override;
 
   // Main thread only
   NS_IMETHOD HasActiveSmartCards(bool& result) override;
   NS_IMETHOD HasUserCertsInstalled(bool& result) override;
@@ -167,17 +169,17 @@ private:
 #ifdef XP_WIN
   void ImportEnterpriseRootsForLocation(
     DWORD locationFlag, const mozilla::MutexAutoLock& proofOfLock);
   nsresult MaybeImportFamilySafetyRoot(PCCERT_CONTEXT certificate,
                                        bool& wasFamilySafetyRoot);
   nsresult LoadFamilySafetyRoot();
   void UnloadFamilySafetyRoot();
 
-  void UnloadEnterpriseRoots(const mozilla::MutexAutoLock& proofOfLock);
+  void UnloadEnterpriseRoots();
 #endif // XP_WIN
 
   // mLoadableRootsLoadedMonitor protects mLoadableRootsLoaded.
   mozilla::Monitor mLoadableRootsLoadedMonitor;
   bool mLoadableRootsLoaded;
   nsresult mLoadableRootsLoadedResult;
 
   // mMutex protects all members that are accessed from more than one thread.