Bug 1912344 - only filter non-trust-anchors if they are known built-in roots when loading third-party certificates r=jschanck

In bug 1874054, we made it so Firefox won't import a third party certificate if
it is already a known built-in root. This was to prevent roots that were
mistakenly identified as intermediates (as in, "inherits trust") from
overriding the trust settings of built-in roots and preventing chains being
built to those roots. Additionally, we were concerned about cases where a
built-in root had been set by the user to be distrusted, in which case
importing that root from the OS would unexpectedly make it trusted again.
Revisiting the first issue, this patch restricts this check to only
certificates identified as non-trust-anchors, so roots will still be imported.
As for the second issue, it turns out that we actually do want this feature to
work this way. This will enable (with some additional work) situations where a
built-in root has a distrust after date but the user wants that root to still
work as before. As for any discrepancies between the user's trust settings in
Firefox vs. their operating system, that's up to them to resolve.

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

security/manager/ssl/EnterpriseRoots.cpp

@@ -192,11 +192,13 @@ static void GatherEnterpriseCertsForLocation(DWORD locationFlag,
       EnterpriseCert enterpriseCert(certificate->pbCertEncoded,
                                     certificate->cbCertEncoded,
                                     location.mIsRoot);
-      if (!enterpriseCert.IsKnownRoot(rootsModule)) {
+      if (enterpriseCert.GetIsRoot() ||
+          !enterpriseCert.IsKnownRoot(rootsModule)) {
         certs.AppendElement(std::move(enterpriseCert));
         numImported++;
       } else {
-        MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("skipping known root cert"));
+        MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+                ("skipping intermediate that is a known root cert"));
       }
     }
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
@@ -450,13 +452,15 @@ OSStatus GatherEnterpriseCertsMacOS(nsTArray<EnterpriseCert>& certs,
         certificateTrustResult == CertificateTrustResult::CanUseAsTrustAnchor;
     EnterpriseCert enterpriseCert(CFDataGetBytePtr(der.get()),
                                   CFDataGetLength(der.get()), isRoot);
-    if (!enterpriseCert.IsKnownRoot(rootsModule)) {
+    if (enterpriseCert.GetIsRoot() ||
+        !enterpriseCert.IsKnownRoot(rootsModule)) {
       certs.AppendElement(std::move(enterpriseCert));
       numImported++;
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
               ("importing as %s", isRoot ? "root" : "intermediate"));
     } else {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("skipping known root cert"));
+      MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+              ("skipping intermediate that is a known root cert"));
     }
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("imported %u certs", numImported));
@@ -481,11 +485,13 @@ void GatherEnterpriseCertsAndroid(nsTArray<EnterpriseCert>& certs,
     EnterpriseCert enterpriseCert(
         reinterpret_cast<uint8_t*>(root->GetElements().Elements()),
         root->Length(), true);
-    if (!enterpriseCert.IsKnownRoot(rootsModule)) {
+    if (enterpriseCert.GetIsRoot() ||
+        !enterpriseCert.IsKnownRoot(rootsModule)) {
       certs.AppendElement(std::move(enterpriseCert));
       numImported++;
     } else {
-      MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("skipping known root cert"));
+      MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+              ("skipping intermediate that is a known root cert"));
     }
   }
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("imported %u certs", numImported));