bug 1514118 - have CertVerifier use any third-party roots rather than going through NSS r=jcj
authorDana Keeler <dkeeler@mozilla.com>
Fri, 01 Feb 2019 22:01:00 +0000
changeset 456523 b828ed311a01c2977f3c78251cb231bc0f7cfbd1
parent 456522 b7597732606e96cd3337c316b5771ae09e9b8ab9
child 456524 e3670e52a459cee02337afc70bbeb3d1dc68821b
push id111656
push userdvarga@mozilla.com
push dateSat, 02 Feb 2019 09:51:54 +0000
treeherdermozilla-inbound@d8cebb3b46cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcj
bugs1514118, 1478148
milestone67.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 1514118 - have CertVerifier use any third-party roots rather than going through NSS r=jcj Before this patch, if the enterprise roots feature were enabled, nsNSSComponent would gather any such roots and temporarily import them into NSS so that CertVerifier could use them during path building and trust querying. This turned out to be problematic in part because doing so would require unlocking the user's key DB if they had a password. This patch implements a scheme whereby nsNSSComponent can give these extra roots directly to CertVerifier, thus bypassing NSS and any need to unlock/modify any DBs. This should also provide a path forward for other improvements such as not repeatedly searching through all certificates on all tokens, which has inefficiencies (see e.g. bug 1478148). Differential Revision: https://phabricator.services.mozilla.com/D18156
security/certverifier/CertVerifier.cpp
security/certverifier/CertVerifier.h
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/manager/ssl/EnterpriseRoots.cpp
security/manager/ssl/EnterpriseRoots.h
security/manager/ssl/SharedCertVerifier.h
security/manager/ssl/nsINSSComponent.idl
security/manager/ssl/nsIX509CertDB.idl
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/nsNSSComponent.h
security/manager/ssl/tests/unit/test_enterprise_roots.js
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -83,29 +83,45 @@ void CertificateTransparencyInfo::Reset(
 CertVerifier::CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
                            mozilla::TimeDuration ocspTimeoutSoft,
                            mozilla::TimeDuration ocspTimeoutHard,
                            uint32_t certShortLifetimeInDays,
                            PinningMode pinningMode, SHA1Mode sha1Mode,
                            BRNameMatchingPolicy::Mode nameMatchingMode,
                            NetscapeStepUpPolicy netscapeStepUpPolicy,
                            CertificateTransparencyMode ctMode,
-                           DistrustedCAPolicy distrustedCAPolicy)
+                           DistrustedCAPolicy distrustedCAPolicy,
+                           const Vector<Vector<uint8_t>>& thirdPartyRoots)
     : mOCSPDownloadConfig(odc),
       mOCSPStrict(osc == ocspStrict),
       mOCSPTimeoutSoft(ocspTimeoutSoft),
       mOCSPTimeoutHard(ocspTimeoutHard),
       mCertShortLifetimeInDays(certShortLifetimeInDays),
       mPinningMode(pinningMode),
       mSHA1Mode(sha1Mode),
       mNameMatchingMode(nameMatchingMode),
       mNetscapeStepUpPolicy(netscapeStepUpPolicy),
       mCTMode(ctMode),
       mDistrustedCAPolicy(distrustedCAPolicy) {
   LoadKnownCTLogs();
+  for (const auto& root : thirdPartyRoots) {
+    Vector<uint8_t> rootCopy;
+    if (rootCopy.append(root.begin(), root.end())) {
+      // Best-effort. If we run out of memory, users might see untrusted issuer
+      // errors, but the browser will probably crash before then.
+      Unused << mThirdPartyRoots.append(std::move(rootCopy));
+    }
+  }
+  for (const auto& root : mThirdPartyRoots) {
+    Input rootInput;
+    if (rootInput.Init(root.begin(), root.length()) == Success) {
+      // Best effort again.
+      Unused << mThirdPartyRootInputs.append(rootInput);
+    }
+  }
 }
 
 CertVerifier::~CertVerifier() {}
 
 Result IsCertChainRootBuiltInRoot(const UniqueCERTCertList& chain,
                                   bool& result) {
   if (!chain || CERT_LIST_EMPTY(chain)) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
@@ -506,17 +522,18 @@ Result CertVerifier::VerifyCert(
     case certificateUsageSSLClient: {
       // XXX: We don't really have a trust bit for SSL client authentication so
       // just use trustEmail as it is the closest alternative.
       NSSCertDBTrustDomain trustDomain(
           trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
           mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
           MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
           SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch,
-          mDistrustedCAPolicy, originAttributes, builtChain, nullptr, nullptr);
+          mDistrustedCAPolicy, originAttributes, mThirdPartyRootInputs,
+          builtChain, nullptr, nullptr);
       rv = BuildCertChain(
           trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity,
           KeyUsage::digitalSignature, KeyPurposeId::id_kp_clientAuth,
           CertPolicyId::anyPolicy, stapledOCSPResponse);
       break;
     }
 
     case certificateUsageSSLServer: {
@@ -578,18 +595,18 @@ Result CertVerifier::VerifyCert(
           pinningTelemetryInfo->Reset();
         }
 
         NSSCertDBTrustDomain trustDomain(
             trustSSL, evOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
             mOCSPTimeoutHard, mCertShortLifetimeInDays, mPinningMode,
             MIN_RSA_BITS, ValidityCheckingMode::CheckForEV,
             sha1ModeConfigurations[i], mNetscapeStepUpPolicy,
-            mDistrustedCAPolicy, originAttributes, builtChain,
-            pinningTelemetryInfo, hostname);
+            mDistrustedCAPolicy, originAttributes, mThirdPartyRootInputs,
+            builtChain, pinningTelemetryInfo, hostname);
         rv = BuildCertChainForOneKeyUsage(
             trustDomain, certDER, time,
             KeyUsage::digitalSignature,  // (EC)DHE
             KeyUsage::keyEncipherment,   // RSA
             KeyUsage::keyAgreement,      // (EC)DH
             KeyPurposeId::id_kp_serverAuth, evPolicy, stapledOCSPResponse,
             ocspStaplingStatus);
         if (rv == Success &&
@@ -660,17 +677,18 @@ Result CertVerifier::VerifyCert(
           }
 
           NSSCertDBTrustDomain trustDomain(
               trustSSL, defaultOCSPFetching, mOCSPCache, pinArg,
               mOCSPTimeoutSoft, mOCSPTimeoutHard, mCertShortLifetimeInDays,
               mPinningMode, keySizeOptions[i],
               ValidityCheckingMode::CheckingOff, sha1ModeConfigurations[j],
               mNetscapeStepUpPolicy, mDistrustedCAPolicy, originAttributes,
-              builtChain, pinningTelemetryInfo, hostname);
+              mThirdPartyRootInputs, builtChain, pinningTelemetryInfo,
+              hostname);
           rv = BuildCertChainForOneKeyUsage(
               trustDomain, certDER, time,
               KeyUsage::digitalSignature,  //(EC)DHE
               KeyUsage::keyEncipherment,   // RSA
               KeyUsage::keyAgreement,      //(EC)DH
               KeyPurposeId::id_kp_serverAuth, CertPolicyId::anyPolicy,
               stapledOCSPResponse, ocspStaplingStatus);
           if (rv != Success && !IsFatalError(rv) &&
@@ -728,30 +746,32 @@ Result CertVerifier::VerifyCert(
     }
 
     case certificateUsageSSLCA: {
       NSSCertDBTrustDomain trustDomain(
           trustSSL, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
           mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
           MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
           SHA1Mode::Allowed, mNetscapeStepUpPolicy, mDistrustedCAPolicy,
-          originAttributes, builtChain, nullptr, nullptr);
+          originAttributes, mThirdPartyRootInputs, builtChain, nullptr,
+          nullptr);
       rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeCA,
                           KeyUsage::keyCertSign, KeyPurposeId::id_kp_serverAuth,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       break;
     }
 
     case certificateUsageEmailSigner: {
       NSSCertDBTrustDomain trustDomain(
           trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
           mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
           MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
           SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch,
-          mDistrustedCAPolicy, originAttributes, builtChain, nullptr, nullptr);
+          mDistrustedCAPolicy, originAttributes, mThirdPartyRootInputs,
+          builtChain, nullptr, nullptr);
       rv = BuildCertChain(
           trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity,
           KeyUsage::digitalSignature, KeyPurposeId::id_kp_emailProtection,
           CertPolicyId::anyPolicy, stapledOCSPResponse);
       if (rv == Result::ERROR_INADEQUATE_KEY_USAGE) {
         rv = BuildCertChain(
             trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity,
             KeyUsage::nonRepudiation, KeyPurposeId::id_kp_emailProtection,
@@ -764,17 +784,18 @@ Result CertVerifier::VerifyCert(
       // TODO: The higher level S/MIME processing should pass in which key
       // usage it is trying to verify for, and base its algorithm choices
       // based on the result of the verification(s).
       NSSCertDBTrustDomain trustDomain(
           trustEmail, defaultOCSPFetching, mOCSPCache, pinArg, mOCSPTimeoutSoft,
           mOCSPTimeoutHard, mCertShortLifetimeInDays, pinningDisabled,
           MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff,
           SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch,
-          mDistrustedCAPolicy, originAttributes, builtChain, nullptr, nullptr);
+          mDistrustedCAPolicy, originAttributes, mThirdPartyRootInputs,
+          builtChain, nullptr, nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::keyEncipherment,  // RSA
                           KeyPurposeId::id_kp_emailProtection,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       if (rv == Result::ERROR_INADEQUATE_KEY_USAGE) {
         rv = BuildCertChain(trustDomain, certDER, time,
                             EndEntityOrCA::MustBeEndEntity,
--- a/security/certverifier/CertVerifier.h
+++ b/security/certverifier/CertVerifier.h
@@ -201,17 +201,18 @@ class CertVerifier {
 
   CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
                mozilla::TimeDuration ocspTimeoutSoft,
                mozilla::TimeDuration ocspTimeoutHard,
                uint32_t certShortLifetimeInDays, PinningMode pinningMode,
                SHA1Mode sha1Mode, BRNameMatchingPolicy::Mode nameMatchingMode,
                NetscapeStepUpPolicy netscapeStepUpPolicy,
                CertificateTransparencyMode ctMode,
-               DistrustedCAPolicy distrustedCAPolicy);
+               DistrustedCAPolicy distrustedCAPolicy,
+               const Vector<Vector<uint8_t>>& thirdPartyRoots);
   ~CertVerifier();
 
   void ClearOCSPCache() { mOCSPCache.Clear(); }
 
   const OcspDownloadConfig mOCSPDownloadConfig;
   const bool mOCSPStrict;
   const mozilla::TimeDuration mOCSPTimeoutSoft;
   const mozilla::TimeDuration mOCSPTimeoutHard;
@@ -220,16 +221,21 @@ class CertVerifier {
   const SHA1Mode mSHA1Mode;
   const BRNameMatchingPolicy::Mode mNameMatchingMode;
   const NetscapeStepUpPolicy mNetscapeStepUpPolicy;
   const CertificateTransparencyMode mCTMode;
   const DistrustedCAPolicy mDistrustedCAPolicy;
 
  private:
   OCSPCache mOCSPCache;
+  // We keep a copy of the bytes of each third party root to own.
+  Vector<Vector<uint8_t>> mThirdPartyRoots;
+  // This is a reusable, precomputed list of Inputs corresponding to each root
+  // in mThirdPartyRoots that wasn't too long to make an Input out of.
+  Vector<mozilla::pkix::Input> mThirdPartyRootInputs;
 
   // We only have a forward declarations of these classes (see above)
   // so we must allocate dynamically.
   UniquePtr<mozilla::ct::MultiLogCTVerifier> mCTVerifier;
   UniquePtr<mozilla::ct::CTDiversityPolicy> mCTDiversityPolicy;
 
   void LoadKnownCTLogs();
   mozilla::pkix::Result VerifyCertificateTransparencyPolicy(
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -54,17 +54,19 @@ NSSCertDBTrustDomain::NSSCertDBTrustDoma
     SECTrustType certDBTrustType, OCSPFetching ocspFetching,
     OCSPCache& ocspCache,
     /*optional but shouldn't be*/ void* pinArg, TimeDuration ocspTimeoutSoft,
     TimeDuration ocspTimeoutHard, uint32_t certShortLifetimeInDays,
     CertVerifier::PinningMode pinningMode, unsigned int minRSABits,
     ValidityCheckingMode validityCheckingMode, CertVerifier::SHA1Mode sha1Mode,
     NetscapeStepUpPolicy netscapeStepUpPolicy,
     DistrustedCAPolicy distrustedCAPolicy,
-    const OriginAttributes& originAttributes, UniqueCERTCertList& builtChain,
+    const OriginAttributes& originAttributes,
+    const Vector<Input>& thirdPartyRootInputs,
+    /*out*/ UniqueCERTCertList& builtChain,
     /*optional*/ PinningTelemetryInfo* pinningTelemetryInfo,
     /*optional*/ const char* hostname)
     : mCertDBTrustType(certDBTrustType),
       mOCSPFetching(ocspFetching),
       mOCSPCache(ocspCache),
       mPinArg(pinArg),
       mOCSPTimeoutSoft(ocspTimeoutSoft),
       mOCSPTimeoutHard(ocspTimeoutHard),
@@ -72,16 +74,17 @@ NSSCertDBTrustDomain::NSSCertDBTrustDoma
       mPinningMode(pinningMode),
       mMinRSABits(minRSABits),
       mValidityCheckingMode(validityCheckingMode),
       mSHA1Mode(sha1Mode),
       mNetscapeStepUpPolicy(netscapeStepUpPolicy),
       mDistrustedCAPolicy(distrustedCAPolicy),
       mSawDistrustedCAByPolicyError(false),
       mOriginAttributes(originAttributes),
+      mThirdPartyRootInputs(thirdPartyRootInputs),
       mBuiltChain(builtChain),
       mPinningTelemetryInfo(pinningTelemetryInfo),
       mHostname(hostname),
       mCertBlocklist(do_GetService(NS_CERTBLOCKLIST_CONTRACTID)),
       mOCSPStaplingStatus(CertVerifier::OCSP_STAPLING_NEVER_CHECKED),
       mSCTListFromCertificate(),
       mSCTListFromOCSPStapling() {}
 
@@ -112,16 +115,22 @@ Result NSSCertDBTrustDomain::FindIssuer(
       }
     } else {
       if (!intermediateCandidates.append(certDER)) {
         return Result::FATAL_ERROR_NO_MEMORY;
       }
     }
   }
 
+  for (const auto& thirdPartyRootInput : mThirdPartyRootInputs) {
+    if (!rootCandidates.append(thirdPartyRootInput)) {
+      return Result::FATAL_ERROR_NO_MEMORY;
+    }
+  }
+
   // Handle imposed name constraints, if any.
   ScopedAutoSECItem nameConstraints;
   Input nameConstraintsInput;
   Input* nameConstraintsInputPtr = nullptr;
   SECStatus srv =
       CERT_GetImposedNameConstraints(&encodedIssuerNameItem, &nameConstraints);
   if (srv == SECSuccess) {
     if (nameConstraintsInput.Init(nameConstraints.data, nameConstraints.len) !=
@@ -199,16 +208,24 @@ Result NSSCertDBTrustDomain::GetCertTrus
 
     if (isCertRevoked) {
       MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
               ("NSSCertDBTrustDomain: certificate is in blocklist"));
       return Result::ERROR_REVOKED_CERTIFICATE;
     }
   }
 
+  // This may be a third-party root.
+  for (const auto& thirdPartyRootInput : mThirdPartyRootInputs) {
+    if (InputsAreEqual(candidateCertDER, thirdPartyRootInput)) {
+      trustLevel = TrustLevel::TrustAnchor;
+      return Success;
+    }
+  }
+
   // XXX: CERT_GetCertTrust seems to be abusing SECStatus as a boolean, where
   // SECSuccess means that there is a trust record and SECFailure means there
   // is not a trust record. I looked at NSS's internal uses of
   // CERT_GetCertTrust, and all that code uses the result as a boolean meaning
   // "We have a trust record."
   CERTCertTrust trust;
   if (CERT_GetCertTrust(candidateCert.get(), &trust) == SECSuccess) {
     uint32_t flags = SEC_GET_TRUST_FLAGS(&trust, mCertDBTrustType);
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -101,17 +101,19 @@ class NSSCertDBTrustDomain : public mozi
       SECTrustType certDBTrustType, OCSPFetching ocspFetching,
       OCSPCache& ocspCache, void* pinArg, mozilla::TimeDuration ocspTimeoutSoft,
       mozilla::TimeDuration ocspTimeoutHard, uint32_t certShortLifetimeInDays,
       CertVerifier::PinningMode pinningMode, unsigned int minRSABits,
       ValidityCheckingMode validityCheckingMode,
       CertVerifier::SHA1Mode sha1Mode,
       NetscapeStepUpPolicy netscapeStepUpPolicy,
       DistrustedCAPolicy distrustedCAPolicy,
-      const OriginAttributes& originAttributes, UniqueCERTCertList& builtChain,
+      const OriginAttributes& originAttributes,
+      const Vector<mozilla::pkix::Input>& thirdPartyRootInputs,
+      /*out*/ UniqueCERTCertList& builtChain,
       /*optional*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr,
       /*optional*/ const char* hostname = nullptr);
 
   virtual Result FindIssuer(mozilla::pkix::Input encodedIssuerName,
                             IssuerChecker& checker,
                             mozilla::pkix::Time time) override;
 
   virtual Result GetCertTrust(
@@ -209,16 +211,17 @@ class NSSCertDBTrustDomain : public mozi
   CertVerifier::PinningMode mPinningMode;
   const unsigned int mMinRSABits;
   ValidityCheckingMode mValidityCheckingMode;
   CertVerifier::SHA1Mode mSHA1Mode;
   NetscapeStepUpPolicy mNetscapeStepUpPolicy;
   DistrustedCAPolicy mDistrustedCAPolicy;
   bool mSawDistrustedCAByPolicyError;
   const OriginAttributes& mOriginAttributes;
+  const Vector<mozilla::pkix::Input>& mThirdPartyRootInputs;  // non-owning
   UniqueCERTCertList& mBuiltChain;  // non-owning
   PinningTelemetryInfo* mPinningTelemetryInfo;
   const char* mHostname;  // non-owning - only used for pinning checks
   nsCOMPtr<nsICertBlocklist> mCertBlocklist;
   CertVerifier::OCSPStaplingStatus mOCSPStaplingStatus;
   // Certificate Transparency data extracted during certificate verification
   UniqueSECItem mSCTListFromCertificate;
   UniqueSECItem mSCTListFromOCSPStapling;
--- a/security/manager/ssl/EnterpriseRoots.cpp
+++ b/security/manager/ssl/EnterpriseRoots.cpp
@@ -67,39 +67,16 @@ static bool CertIsTrustAnchorForTLSServe
             ("certificate is trust anchor for TLS server auth"));
     return true;
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
           ("certificate not trust anchor for TLS server auth"));
   return false;
 }
 
-// It would be convenient to just use nsIX509CertDB in the following code.
-// However, since nsIX509CertDB depends on nsNSSComponent initialization (and
-// since this code runs during that initialization), we can't use it. Instead,
-// we can use NSS APIs directly (as long as we're called late enough in
-// nsNSSComponent initialization such that those APIs are safe to use).
-
-// Helper function to convert a PCCERT_CONTEXT (i.e. a certificate obtained via
-// a Windows API) to a temporary CERTCertificate (i.e. a certificate for use
-// with NSS APIs).
-UniqueCERTCertificate PCCERT_CONTEXTToCERTCertificate(PCCERT_CONTEXT pccert) {
-  MOZ_ASSERT(pccert);
-  if (!pccert) {
-    return nullptr;
-  }
-
-  SECItem derCert = {siBuffer, pccert->pbCertEncoded, pccert->cbCertEncoded};
-  return UniqueCERTCertificate(
-      CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &derCert,
-                              nullptr,  // nickname unnecessary
-                              false,    // not permanent
-                              true));   // copy DER
-}
-
 // Because HCERTSTORE is just a typedef void*, we can't use any of the nice
 // scoped or unique pointer templates. To elaborate, any attempt would
 // instantiate those templates with T = void. When T gets used in the context
 // of T&, this results in void&, which isn't legal.
 class ScopedCertStore final {
  public:
   explicit ScopedCertStore(HCERTSTORE certstore) : certstore(certstore) {}
 
@@ -119,30 +96,26 @@ class ScopedCertStore final {
 //   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
 //     (for HKLM\SOFTWARE\Microsoft\EnterpriseCertificates\Root\Certificates)
 static void GatherEnterpriseRootsForLocation(DWORD locationFlag,
-                                             UniqueCERTCertList& roots) {
+                                             Vector<Vector<uint8_t>>& roots) {
   MOZ_ASSERT(locationFlag == CERT_SYSTEM_STORE_LOCAL_MACHINE ||
                  locationFlag == CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY ||
                  locationFlag == CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE,
              "unexpected locationFlag for GatherEnterpriseRootsForLocation");
   if (!(locationFlag == CERT_SYSTEM_STORE_LOCAL_MACHINE ||
         locationFlag == CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY ||
         locationFlag == CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE)) {
     return;
   }
-  MOZ_ASSERT(roots, "roots unexpectedly NULL?");
-  if (!roots) {
-    return;
-  }
 
   DWORD flags =
       locationFlag | CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG;
   // The certificate store being opened should consist only of certificates
   // added by a user or administrator and not any certificates that are part
   // of Microsoft's root store program.
   // The 3rd parameter to CertOpenStore should be NULL according to
   // https://msdn.microsoft.com/en-us/library/windows/desktop/aa376559%28v=vs.85%29.aspx
@@ -159,51 +132,44 @@ static void GatherEnterpriseRootsForLoca
   while ((certificate = CertFindCertificateInStore(
               enterpriseRootStore.get(), X509_ASN_ENCODING, 0, CERT_FIND_ANY,
               nullptr, certificate))) {
     if (!CertIsTrustAnchorForTLSServerAuth(certificate)) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
               ("skipping cert not trust anchor for TLS server auth"));
       continue;
     }
-    UniqueCERTCertificate nssCertificate(
-        PCCERT_CONTEXTToCERTCertificate(certificate));
-    if (!nssCertificate) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("couldn't decode certificate"));
+    Vector<uint8_t> certBytes;
+    if (!certBytes.append(certificate->pbCertEncoded,
+                          certificate->cbCertEncoded)) {
+      MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
+              ("couldn't copy cert bytes (out of memory?)"));
       continue;
     }
-    if (CERT_AddCertToListTail(roots.get(), nssCertificate.get()) !=
-        SECSuccess) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("couldn't add cert to list"));
+    if (!roots.append(std::move(certBytes))) {
+      MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
+              ("couldn't append cert bytes to roots list (out of memory?)"));
       continue;
     }
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-            ("Imported '%s'", nssCertificate->subjectName));
     numImported++;
-    // now owned by roots
-    Unused << nssCertificate.release();
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("imported %u roots", numImported));
 }
 
-static void GatherEnterpriseRootsWindows(UniqueCERTCertList& roots) {
+static void GatherEnterpriseRootsWindows(Vector<Vector<uint8_t>>& roots) {
   GatherEnterpriseRootsForLocation(CERT_SYSTEM_STORE_LOCAL_MACHINE, roots);
   GatherEnterpriseRootsForLocation(CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY,
                                    roots);
   GatherEnterpriseRootsForLocation(CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE,
                                    roots);
 }
 #endif  // XP_WIN
 
 #ifdef XP_MACOSX
-OSStatus GatherEnterpriseRootsOSX(UniqueCERTCertList& roots) {
-  MOZ_ASSERT(roots, "roots unexpectedly NULL?");
-  if (!roots) {
-    return errSecBadReq;
-  }
+OSStatus GatherEnterpriseRootsOSX(Vector<Vector<uint8_t>>& roots) {
   // The following builds a search dictionary corresponding to:
   // { class: "certificate",
   //   match limit: "match all",
   //   policy: "SSL (TLS)",
   //   only include trusted certificates: true }
   // This operates on items that have been added to the keychain and thus gives
   // us all 3rd party certificates that have been trusted for SSL (TLS), which
   // is what we want (thus we don't import built-in root certificates that ship
@@ -235,54 +201,39 @@ OSStatus GatherEnterpriseRootsOSX(Unique
   for (CFIndex i = 0; i < count; i++) {
     const CFTypeRef c = CFArrayGetValueAtIndex(arr.get(), i);
     // Because we asked for certificates, each CFTypeRef in the array is really
     // a SecCertificateRef.
     const SecCertificateRef s = (const SecCertificateRef)c;
     ScopedCFType<CFDataRef> der(SecCertificateCopyData(s));
     const unsigned char* ptr = CFDataGetBytePtr(der.get());
     unsigned int len = CFDataGetLength(der.get());
-    SECItem derItem = {
-        siBuffer,
-        const_cast<unsigned char*>(ptr),
-        len,
-    };
-    UniqueCERTCertificate cert(
-        CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &derItem,
-                                nullptr,  // nickname unnecessary
-                                false,    // not permanent
-                                true));   // copy DER
-    if (!cert) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-              ("couldn't decode 3rd party root certificate"));
+    Vector<uint8_t> certBytes;
+    if (!certBytes.append(ptr, len)) {
+      MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
+              ("couldn't copy cert bytes (out of memory?)"));
       continue;
     }
-    UniquePORTString subjectName(CERT_GetCommonName(&cert->subject));
-    if (CERT_AddCertToListTail(roots.get(), cert.get()) != SECSuccess) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("couldn't add cert to list"));
+    if (!roots.append(std::move(certBytes))) {
+      MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
+              ("couldn't append cert bytes to roots list (out of memory?)"));
       continue;
     }
     numImported++;
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Imported '%s'", subjectName.get()));
-    mozilla::Unused << cert.release();  // owned by roots
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("imported %u roots", numImported));
   return errSecSuccess;
 }
 #endif  // XP_MACOSX
 
-nsresult GatherEnterpriseRoots(UniqueCERTCertList& result) {
-  UniqueCERTCertList roots(CERT_NewCertList());
-  if (!roots) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+nsresult GatherEnterpriseRoots(Vector<Vector<uint8_t>>& roots) {
+  roots.clear();
 #ifdef XP_WIN
   GatherEnterpriseRootsWindows(roots);
 #endif  // XP_WIN
 #ifdef XP_MACOSX
   OSStatus rv = GatherEnterpriseRootsOSX(roots);
   if (rv != errSecSuccess) {
     return NS_ERROR_FAILURE;
   }
 #endif  // XP_MACOSX
-  result = std::move(roots);
   return NS_OK;
 }
--- a/security/manager/ssl/EnterpriseRoots.h
+++ b/security/manager/ssl/EnterpriseRoots.h
@@ -2,14 +2,14 @@
  *
  * 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/. */
 
 #ifndef EnterpriseRoots_h
 #define EnterpriseRoots_h
 
-#include "ErrorList.h"
-#include "ScopedNSSTypes.h"
+#include "mozilla/Vector.h"
 
-nsresult GatherEnterpriseRoots(mozilla::UniqueCERTCertList& result);
+nsresult GatherEnterpriseRoots(
+    mozilla::Vector<mozilla::Vector<uint8_t>>& roots);
 
 #endif  // EnterpriseRoots_h
--- a/security/manager/ssl/SharedCertVerifier.h
+++ b/security/manager/ssl/SharedCertVerifier.h
@@ -22,19 +22,20 @@ class SharedCertVerifier : public mozill
   SharedCertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
                      mozilla::TimeDuration ocspSoftTimeout,
                      mozilla::TimeDuration ocspHardTimeout,
                      uint32_t certShortLifetimeInDays, PinningMode pinningMode,
                      SHA1Mode sha1Mode,
                      BRNameMatchingPolicy::Mode nameMatchingMode,
                      NetscapeStepUpPolicy netscapeStepUpPolicy,
                      CertificateTransparencyMode ctMode,
-                     DistrustedCAPolicy distrustedCAPolicy)
+                     DistrustedCAPolicy distrustedCAPolicy,
+                     const Vector<Vector<uint8_t>>& thirdPartyRoots)
       : mozilla::psm::CertVerifier(
             odc, osc, ocspSoftTimeout, ocspHardTimeout, certShortLifetimeInDays,
             pinningMode, sha1Mode, nameMatchingMode, netscapeStepUpPolicy,
-            ctMode, distrustedCAPolicy) {}
+            ctMode, distrustedCAPolicy, thirdPartyRoots) {}
 };
 
 }  // namespace psm
 }  // namespace mozilla
 
 #endif  // SharedCertVerifier_h
--- a/security/manager/ssl/nsINSSComponent.idl
+++ b/security/manager/ssl/nsINSSComponent.idl
@@ -41,23 +41,23 @@ interface nsINSSComponent : nsISupports 
   /**
    * Used to determine if the given CERTCertificate is the content signing root
    * certificate.
    */
   [noscript] bool isCertContentSigningRoot(in CERTCertificatePtr cert);
 
   /**
    * If enabled by the preference "security.enterprise_roots.enabled", returns
-   * an nsIX509CertList representing the imported enterprise root certificates
-   * (i.e. root certificates gleaned from the OS certificate store). Returns
-   * null otherwise.
-   * Currently this is only implemented on Windows, so this function returns
-   * null on all other platforms.
+   * an array of arrays of bytes representing the imported enterprise root
+   * certificates (i.e. root certificates gleaned from the OS certificate
+   * store). Returns an empty array otherwise.
+   * Currently this is only implemented on Windows and MacOS X, so this
+   * function returns an empty array on all other platforms.
    */
-  [noscript] nsIX509CertList getEnterpriseRoots();
+  Array<Array<octet> > getEnterpriseRoots();
 
   /**
    * For performance reasons, the builtin roots module is loaded on a background
    * thread. When any code that depends on the builtin roots module runs, it
    * must first wait for the module to be loaded.
    */
   [noscript] void blockUntilLoadableRootsLoaded();
 
--- a/security/manager/ssl/nsIX509CertDB.idl
+++ b/security/manager/ssl/nsIX509CertDB.idl
@@ -333,16 +333,9 @@ interface nsIX509CertDB : nsISupports {
   [must_use]
   nsIX509Cert addCertFromBase64(in ACString base64, in ACString trust);
 
   /*
    * Get all the known certs in the database
    */
   [must_use]
   nsIX509CertList getCerts();
-
-  /*
-   * Get a list of imported enterprise root certificates (currently only
-   * implemented on Windows).
-   */
-  [must_use]
-  nsIX509CertList getEnterpriseRoots();
 };
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -1100,36 +1100,16 @@ nsNSSCertificateDB::GetCerts(nsIX509Cert
   // nsNSSCertList 1) adopts certList, and 2) handles the nullptr case fine.
   // (returns an empty list)
   nssCertList = new nsNSSCertList(std::move(certList));
 
   nssCertList.forget(_retval);
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNSSCertificateDB::GetEnterpriseRoots(nsIX509CertList** enterpriseRoots) {
-  MOZ_ASSERT(NS_IsMainThread());
-  if (!NS_IsMainThread()) {
-    return NS_ERROR_NOT_SAME_THREAD;
-  }
-
-  NS_ENSURE_ARG_POINTER(enterpriseRoots);
-
-#ifdef XP_WIN
-  nsCOMPtr<nsINSSComponent> psm(do_GetService(PSM_COMPONENT_CONTRACTID));
-  if (!psm) {
-    return NS_ERROR_FAILURE;
-  }
-  return psm->GetEnterpriseRoots(enterpriseRoots);
-#else
-  return NS_ERROR_NOT_IMPLEMENTED;
-#endif
-}
-
 nsresult VerifyCertAtTime(nsIX509Cert* aCert,
                           int64_t /*SECCertificateUsage*/ aUsage,
                           uint32_t aFlags, const nsACString& aHostname,
                           mozilla::pkix::Time aTime,
                           nsIX509CertList** aVerifiedChain, bool* aHasEVPolicy,
                           int32_t* /*PRErrorCode*/ _retval) {
   NS_ENSURE_ARG_POINTER(aCert);
   NS_ENSURE_ARG_POINTER(aHasEVPolicy);
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -476,58 +476,19 @@ bool nsNSSComponent::ShouldEnableEnterpr
 }
 
 void nsNSSComponent::UnloadEnterpriseRoots() {
   MOZ_ASSERT(NS_IsMainThread());
   if (!NS_IsMainThread()) {
     return;
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("UnloadEnterpriseRoots"));
-
-  // We can't call ChangeCertTrustWithPossibleAuthentication while holding
-  // mMutex (because it could potentially call back in to nsNSSComponent and
-  // attempt to acquire mMutex), so we move mEnterpriseRoots out of
-  // nsNSSComponent into a local handle. This has the side-effect of clearing
-  // mEnterpriseRoots, which is what we want anyway.
-  UniqueCERTCertList enterpriseRoots;
-  {
-    MutexAutoLock lock(mMutex);
-    if (!mEnterpriseRoots) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-              ("no enterprise roots were present"));
-      return;
-    }
-    enterpriseRoots = std::move(mEnterpriseRoots);
-    MOZ_ASSERT(!mEnterpriseRoots);
-  }
-  MOZ_ASSERT(enterpriseRoots);
-  // It would be intuitive to set the trust to { 0, 0, 0 } here. However, this
-  // doesn't work for temporary certificates because CERT_ChangeCertTrust first
-  // looks up the current trust settings in the permanent cert database, finds
-  // that such trust doesn't exist, considers the current trust to be
-  // { 0, 0, 0 }, and decides that it doesn't need to update the trust since
-  // they're the same. To work around this, we set a non-zero flag to ensure
-  // that the trust will get updated.
-  CERTCertTrust trust = {CERTDB_USER, 0, 0};
-  for (CERTCertListNode* n = CERT_LIST_HEAD(enterpriseRoots.get());
-       !CERT_LIST_END(n, enterpriseRoots.get()); n = CERT_LIST_NEXT(n)) {
-    if (!n) {
-      break;
-    }
-    if (!n->cert) {
-      continue;
-    }
-    UniqueCERTCertificate cert(CERT_DupCertificate(n->cert));
-    if (ChangeCertTrustWithPossibleAuthentication(cert, trust, nullptr) !=
-        SECSuccess) {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-              ("couldn't untrust certificate for TLS server auth"));
-    }
-  }
-  MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("unloaded enterprise roots"));
+  MutexAutoLock lock(mMutex);
+  mEnterpriseRoots.clear();
+  setValidationOptions(false, lock);
 }
 
 static const char* kEnterpriseRootModePref =
     "security.enterprise_roots.enabled";
 
 void nsNSSComponent::MaybeImportEnterpriseRoots() {
   MOZ_ASSERT(NS_IsMainThread());
   if (!NS_IsMainThread()) {
@@ -543,90 +504,42 @@ void nsNSSComponent::MaybeImportEnterpri
     importEnterpriseRoots = true;
   }
   if (importEnterpriseRoots) {
     ImportEnterpriseRoots();
   }
 }
 
 void nsNSSComponent::ImportEnterpriseRoots() {
-  UniqueCERTCertList roots;
-  nsresult rv = GatherEnterpriseRoots(roots);
+  MutexAutoLock lock(mMutex);
+  nsresult rv = GatherEnterpriseRoots(mEnterpriseRoots);
   if (NS_FAILED(rv)) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("failed gathering enterprise roots"));
-    return;
-  }
-
-  {
-    MutexAutoLock lock(mMutex);
-    mEnterpriseRoots = std::move(roots);
   }
 }
 
-nsresult nsNSSComponent::TrustLoaded3rdPartyRoots() {
-  // We can't call ChangeCertTrustWithPossibleAuthentication while holding
-  // mMutex (because it could potentially call back in to nsNSSComponent and
-  // attempt to acquire mMutex), so we copy mEnterpriseRoots.
-  UniqueCERTCertList enterpriseRoots;
-  {
-    MutexAutoLock lock(mMutex);
-    if (mEnterpriseRoots) {
-      enterpriseRoots = nsNSSCertList::DupCertList(mEnterpriseRoots);
-      if (!enterpriseRoots) {
-        return NS_ERROR_OUT_OF_MEMORY;
-      }
-    }
-  }
-
-  CERTCertTrust trust = {CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_USER, 0,
-                         0};
-  if (enterpriseRoots) {
-    for (CERTCertListNode* n = CERT_LIST_HEAD(enterpriseRoots.get());
-         !CERT_LIST_END(n, enterpriseRoots.get()); n = CERT_LIST_NEXT(n)) {
-      if (!n) {
-        break;
-      }
-      if (!n->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"));
-      }
-    }
-  }
-  return NS_OK;
-}
-
 NS_IMETHODIMP
-nsNSSComponent::GetEnterpriseRoots(nsIX509CertList** enterpriseRoots) {
+nsNSSComponent::GetEnterpriseRoots(
+    nsTArray<nsTArray<uint8_t>>& enterpriseRoots) {
   MutexAutoLock nsNSSComponentLock(mMutex);
   MOZ_ASSERT(NS_IsMainThread());
   if (!NS_IsMainThread()) {
     return NS_ERROR_NOT_SAME_THREAD;
   }
-  NS_ENSURE_ARG_POINTER(enterpriseRoots);
 
-  if (!mEnterpriseRoots) {
-    *enterpriseRoots = nullptr;
-    return NS_OK;
+  enterpriseRoots.Clear();
+  for (const auto& root : mEnterpriseRoots) {
+    nsTArray<uint8_t> rootCopy;
+    if (!rootCopy.AppendElements(root.begin(), root.length())) {
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+    if (!enterpriseRoots.AppendElement(std::move(rootCopy))) {
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
   }
-  UniqueCERTCertList enterpriseRootsCopy(
-      nsNSSCertList::DupCertList(mEnterpriseRoots));
-  if (!enterpriseRootsCopy) {
-    return NS_ERROR_FAILURE;
-  }
-  nsCOMPtr<nsIX509CertList> enterpriseRootsCertList(
-      new nsNSSCertList(std::move(enterpriseRootsCopy)));
-  if (!enterpriseRootsCertList) {
-    return NS_ERROR_FAILURE;
-  }
-  enterpriseRootsCertList.forget(enterpriseRoots);
   return NS_OK;
 }
 
 class LoadLoadableRootsTask final : public Runnable {
  public:
   LoadLoadableRootsTask(nsNSSComponent* nssComponent,
                         bool importEnterpriseRoots, uint32_t familySafetyMode,
                         Vector<nsCString>&& possibleLoadableRootsLocations)
@@ -707,29 +620,24 @@ LoadLoadableRootsTask::Run() {
   // feature, see if it's enabled. If so, we want to import enterprise roots.
   if (mNSSComponent->ShouldEnableEnterpriseRootsForFamilySafety(
           mFamilySafetyMode)) {
     mImportEnterpriseRoots = true;
   }
   if (mImportEnterpriseRoots) {
     mNSSComponent->ImportEnterpriseRoots();
   }
-  nsresult rv = mNSSComponent->TrustLoaded3rdPartyRoots();
-  if (NS_FAILED(rv)) {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Error,
-            ("failed to trust loaded 3rd party roots"));
-  }
-
+  mNSSComponent->UpdateCertVerifierWithEnterpriseRoots();
   {
     MonitorAutoLock rootsLoadedLock(mNSSComponent->mLoadableRootsLoadedMonitor);
     mNSSComponent->mLoadableRootsLoaded = true;
     // Cache the result of LoadLoadableRoots so BlockUntilLoadableRootsLoaded
     // can return it to all callers later.
     mNSSComponent->mLoadableRootsLoadedResult = loadLoadableRootsResult;
-    rv = mNSSComponent->mLoadableRootsLoadedMonitor.NotifyAll();
+    nsresult rv = mNSSComponent->mLoadableRootsLoadedMonitor.NotifyAll();
     if (NS_WARN_IF(NS_FAILED(rv))) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Error,
               ("failed to notify loadable roots loaded monitor"));
     }
   }
 
   // Go back to the main thread to clean up this worker thread.
   return NS_DispatchToMainThread(this);
@@ -1153,16 +1061,22 @@ nsresult CipherSuiteChangeObserver::Obse
   }
   return NS_OK;
 }
 
 }  // namespace
 
 void nsNSSComponent::setValidationOptions(
     bool isInitialSetting, const mozilla::MutexAutoLock& proofOfLock) {
+  // We access prefs so this must be done on the main thread.
+  MOZ_ASSERT(NS_IsMainThread());
+  if (NS_WARN_IF(!NS_IsMainThread())) {
+    return;
+  }
+
   // This preference controls whether we do OCSP fetching and does not affect
   // OCSP stapling.
   // 0 = disabled, 1 = enabled
   int32_t ocspEnabled =
       Preferences::GetInt("security.OCSP.enabled", OCSP_ENABLED_DEFAULT);
 
   bool ocspRequired =
       ocspEnabled && Preferences::GetBool("security.OCSP.require", false);
@@ -1276,20 +1190,40 @@ void nsNSSComponent::setValidationOption
   CertVerifier::OcspDownloadConfig odc;
   CertVerifier::OcspStrictConfig osc;
   uint32_t certShortLifetimeInDays;
   TimeDuration softTimeout;
   TimeDuration hardTimeout;
 
   GetRevocationBehaviorFromPrefs(&odc, &osc, &certShortLifetimeInDays,
                                  softTimeout, hardTimeout, proofOfLock);
+
   mDefaultCertVerifier = new SharedCertVerifier(
       odc, osc, softTimeout, hardTimeout, certShortLifetimeInDays, pinningMode,
       sha1Mode, nameMatchingMode, netscapeStepUpPolicy, ctMode,
-      distrustedCAPolicy);
+      distrustedCAPolicy, mEnterpriseRoots);
+}
+
+void nsNSSComponent::UpdateCertVerifierWithEnterpriseRoots() {
+  MutexAutoLock lock(mMutex);
+  MOZ_ASSERT(mDefaultCertVerifier);
+  if (NS_WARN_IF(!mDefaultCertVerifier)) {
+    return;
+  }
+
+  RefPtr<SharedCertVerifier> oldCertVerifier = mDefaultCertVerifier;
+  mDefaultCertVerifier = new SharedCertVerifier(
+      oldCertVerifier->mOCSPDownloadConfig,
+      oldCertVerifier->mOCSPStrict ? CertVerifier::ocspStrict
+                                   : CertVerifier::ocspRelaxed,
+      oldCertVerifier->mOCSPTimeoutSoft, oldCertVerifier->mOCSPTimeoutHard,
+      oldCertVerifier->mCertShortLifetimeInDays, oldCertVerifier->mPinningMode,
+      oldCertVerifier->mSHA1Mode, oldCertVerifier->mNameMatchingMode,
+      oldCertVerifier->mNetscapeStepUpPolicy, oldCertVerifier->mCTMode,
+      oldCertVerifier->mDistrustedCAPolicy, mEnterpriseRoots);
 }
 
 // Enable the TLS versions given in the prefs, defaulting to TLS 1.0 (min) and
 // TLS 1.2 (max) when the prefs aren't set or set to invalid values.
 nsresult nsNSSComponent::setEnabledTLSVersions() {
   // keep these values in sync with security-prefs.js
   // 1 means TLS 1.0, 2 means TLS 1.1, etc.
   static const uint32_t PSM_DEFAULT_MIN_TLS_VERSION = 1;
@@ -1819,18 +1753,17 @@ nsresult nsNSSComponent::InitializeNSS()
                            mContentSigningRootHash);
 
     mMitmCanaryIssuer.Truncate();
     Preferences::GetString("security.pki.mitm_canary_issuer",
                            mMitmCanaryIssuer);
     mMitmDetecionEnabled =
         Preferences::GetBool("security.pki.mitm_canary_issuer.enabled", true);
 
-    // Set dynamic options from prefs. This has to run after
-    // SSL_ConfigServerSessionIDCache.
+    // Set dynamic options from prefs.
     setValidationOptions(true, lock);
 
     bool importEnterpriseRoots =
         Preferences::GetBool(kEnterpriseRootModePref, false);
     uint32_t familySafetyMode =
         Preferences::GetUint(kFamilySafetyModePref, kFamilySafetyModeDefault);
     Vector<nsCString> possibleLoadableRootsLocations;
     rv = ListPossibleLoadableRootsLocations(possibleLoadableRootsLocations);
@@ -1866,25 +1799,23 @@ void nsNSSComponent::ShutdownNSS() {
   // failed, we won't have dispatched the load loadable roots background task.
   // In that case, we don't want to block on an event that will never happen.
   if (loadLoadableRootsTaskDispatched) {
     Unused << BlockUntilLoadableRootsLoaded();
   }
 
   ::mozilla::psm::UnloadLoadableRoots();
 
-  MutexAutoLock lock(mMutex);
-  mEnterpriseRoots = nullptr;
-
   PK11_SetPasswordFunc((PK11PasswordFunc) nullptr);
 
   Preferences::RemoveObserver(this, "security.");
 
   // Release the default CertVerifier. This will cause any held NSS resources
   // to be released.
+  MutexAutoLock lock(mMutex);
   mDefaultCertVerifier = nullptr;
   // We don't actually shut down NSS - XPCOM does, after all threads have been
   // joined and the component manager has been shut down (and so there shouldn't
   // be any XPCOM objects holding NSS resources).
 }
 
 nsresult nsNSSComponent::Init() {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
@@ -1996,17 +1927,18 @@ nsNSSComponent::Observe(nsISupports* aSu
       Preferences::GetString("security.content.signature.root_hash",
                              mContentSigningRootHash);
     } else if (prefName.Equals(kEnterpriseRootModePref) ||
                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.
       UnloadEnterpriseRoots();
       MaybeImportEnterpriseRoots();
-      TrustLoaded3rdPartyRoots();
+      MutexAutoLock lock(mMutex);
+      setValidationOptions(false, lock);
     } else if (prefName.EqualsLiteral("security.pki.mitm_canary_issuer")) {
       MutexAutoLock lock(mMutex);
       mMitmCanaryIssuer.Truncate();
       Preferences::GetString("security.pki.mitm_canary_issuer",
                              mMitmCanaryIssuer);
     } else if (prefName.EqualsLiteral(
                    "security.pki.mitm_canary_issuer.enabled")) {
       MutexAutoLock lock(mMutex);
--- a/security/manager/ssl/nsNSSComponent.h
+++ b/security/manager/ssl/nsNSSComponent.h
@@ -74,27 +74,26 @@ class nsNSSComponent final : public nsIN
   virtual ~nsNSSComponent();
 
  private:
   nsresult InitializeNSS();
   void ShutdownNSS();
 
   void setValidationOptions(bool isInitialSetting,
                             const mozilla::MutexAutoLock& proofOfLock);
+  void UpdateCertVerifierWithEnterpriseRoots();
   nsresult setEnabledTLSVersions();
   nsresult RegisterObservers();
 
   void MaybeImportEnterpriseRoots();
   void ImportEnterpriseRoots();
   void UnloadEnterpriseRoots();
 
   bool ShouldEnableEnterpriseRootsForFamilySafety(uint32_t familySafetyMode);
 
-  nsresult TrustLoaded3rdPartyRoots();
-
   // mLoadableRootsLoadedMonitor protects mLoadableRootsLoaded.
   mozilla::Monitor mLoadableRootsLoadedMonitor;
   bool mLoadableRootsLoaded;
   nsresult mLoadableRootsLoadedResult;
 
   // mMutex protects all members that are accessed from more than one thread.
   mozilla::Mutex mMutex;
 
@@ -102,17 +101,17 @@ class nsNSSComponent final : public nsIN
 
 #ifdef DEBUG
   nsString mTestBuiltInRootHash;
 #endif
   nsString mContentSigningRootHash;
   RefPtr<mozilla::psm::SharedCertVerifier> mDefaultCertVerifier;
   nsString mMitmCanaryIssuer;
   bool mMitmDetecionEnabled;
-  mozilla::UniqueCERTCertList mEnterpriseRoots;
+  mozilla::Vector<mozilla::Vector<uint8_t>> mEnterpriseRoots;
 
   // The following members are accessed only on the main thread:
   static int mInstanceCount;
   // If InitializeNSS succeeds, then we have dispatched an event to load the
   // loadable roots module on a background thread. We must wait for it to
   // complete before attempting to unload the module again in ShutdownNSS. If we
   // never dispatched the event, then we can't wait for it to complete (because
   // it will never complete) so we use this boolean to keep track of if we
--- a/security/manager/ssl/tests/unit/test_enterprise_roots.js
+++ b/security/manager/ssl/tests/unit/test_enterprise_roots.js
@@ -6,51 +6,62 @@
 "use strict";
 
 // Tests enterprise root certificate support. When configured to do so, the
 // platform will attempt to find and import enterprise root certificates. This
 // feature is specific to Windows.
 
 do_get_profile(); // must be called before getting nsIX509CertDB
 
-function check_no_enterprise_roots_imported(certDB, dbKey = undefined) {
-  let enterpriseRoots = certDB.getEnterpriseRoots();
-  equal(enterpriseRoots, null, "should not have imported any enterprise roots");
+async function check_no_enterprise_roots_imported(nssComponent, certDB, dbKey = undefined) {
+  let enterpriseRoots = nssComponent.getEnterpriseRoots();
+  notEqual(enterpriseRoots, null, "enterprise roots list should not be null");
+  equal(enterpriseRoots.length, 0, "should not have imported any enterprise roots");
   if (dbKey) {
     let cert = certDB.findCertByDBKey(dbKey);
     // If the garbage-collector hasn't run, there may be reachable copies of
     // imported enterprise root certificates. If so, they shouldn't be trusted
     // to issue TLS server auth certificates.
     if (cert) {
-      ok(!certDB.isCertTrusted(cert, Ci.nsIX509Cert.CA_CERT,
-                               Ci.nsIX509CertDB.TRUSTED_SSL),
-         "previously-imported enterprise root shouldn't be trusted to issue " +
-         "TLS server auth certificates");
+      await asyncTestCertificateUsages(certDB, cert, []);
     }
   }
 }
 
-function check_some_enterprise_roots_imported(certDB) {
-  let enterpriseRoots = certDB.getEnterpriseRoots();
-  notEqual(enterpriseRoots, null, "should have imported some enterprise roots");
+function der_array_to_string(derArray) {
+  let derString = "";
+  for (let b of derArray) {
+    derString += String.fromCharCode(b);
+  }
+  return derString;
+}
+
+async function check_some_enterprise_roots_imported(nssComponent, certDB) {
+  let enterpriseRoots = nssComponent.getEnterpriseRoots();
+  notEqual(enterpriseRoots, null, "enterprise roots list should not be null");
+  notEqual(enterpriseRoots.length, 0, "should have imported some enterprise roots");
   let foundNonBuiltIn = false;
   let savedDBKey = null;
-  for (let cert of enterpriseRoots.getEnumerator()) {
+  for (let certDer of enterpriseRoots) {
+    let cert = certDB.constructX509(der_array_to_string(certDer));
+    notEqual(cert, null, "should be able to decode cert from DER");
     if (!cert.isBuiltInRoot && !savedDBKey) {
       foundNonBuiltIn = true;
       savedDBKey = cert.dbKey;
       info("saving dbKey from " + cert.commonName);
+      await asyncTestCertificateUsages(certDB, cert, [certificateUsageSSLCA]);
+      break;
     }
   }
   ok(foundNonBuiltIn, "should have found non-built-in root");
   return savedDBKey;
 }
 
-function run_test() {
-  let certDB = Cc["@mozilla.org/security/x509certdb;1"]
-                 .getService(Ci.nsIX509CertDB);
+add_task(async function run_test() {
+  let nssComponent = Cc["@mozilla.org/psm;1"].getService(Ci.nsINSSComponent);
+  let certDB = Cc["@mozilla.org/security/x509certdb;1"].getService(Ci.nsIX509CertDB);
   Services.prefs.setBoolPref("security.enterprise_roots.enabled", false);
-  check_no_enterprise_roots_imported(certDB);
+  await check_no_enterprise_roots_imported(nssComponent, certDB);
   Services.prefs.setBoolPref("security.enterprise_roots.enabled", true);
-  let savedDBKey = check_some_enterprise_roots_imported(certDB);
+  let savedDBKey = await check_some_enterprise_roots_imported(nssComponent, certDB);
   Services.prefs.setBoolPref("security.enterprise_roots.enabled", false);
-  check_no_enterprise_roots_imported(certDB, savedDBKey);
-}
+  await check_no_enterprise_roots_imported(nssComponent, certDB, savedDBKey);
+});