Bug 1920561 - only check certificate transparency for certificates issued by built-in roots r=jschanck,extension-reviewers,rpl

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

security/certverifier/CertVerifier.cpp

@@ -234,7 +234,8 @@ Result CertVerifier::VerifyCertificateTransparencyPolicy(
   if (ctInfo) {
     ctInfo->Reset();
   }
-  if (mCTMode == CertificateTransparencyMode::Disabled) {
+  if (mCTMode == CertificateTransparencyMode::Disabled ||
+      !trustDomain.GetIsBuiltChainRootBuiltInRoot()) {
     return Success;
   }
   if (time > TimeFromEpochInSeconds(kCTExpirationTime / PR_USEC_PER_SEC)) {

security/manager/ssl/tests/unit/test_ct.js

@@ -9,6 +9,7 @@ do_get_profile(); // must be called before getting nsIX509CertDB
 
 registerCleanupFunction(() => {
   Services.prefs.clearUserPref("security.pki.certificate_transparency.mode");
+  Services.prefs.clearUserPref("security.test.built_in_root_hash");
   let cert = constructCertFromFile("test_ct/ct-valid.example.com.pem");
   setCertTrust(cert, ",,");
 });
@@ -16,6 +17,25 @@ registerCleanupFunction(() => {
 function run_test() {
   Services.prefs.setIntPref("security.pki.certificate_transparency.mode", 1);
   add_tls_server_setup("BadCertAndPinningServer", "test_ct");
+
+  // Test that certificate transparency is not checked for certificates issued
+  // by roots that are not built-in.
+  add_ct_test(
+    "ct-unknown-log.example.com",
+    Ci.nsITransportSecurityInfo.CERTIFICATE_TRANSPARENCY_NOT_APPLICABLE
+  );
+
+  add_test(function set_test_root_as_built_in() {
+    // Make the test root appear to be a built-in root, so that certificate
+    // transparency is checked.
+    let rootCert = constructCertFromFile("test_ct/test-ca.pem");
+    Services.prefs.setCharPref(
+      "security.test.built_in_root_hash",
+      rootCert.sha256Fingerprint
+    );
+    run_next_test();
+  });
+
   // These certificates have a validity period of 800 days, which is greater
   // than 180 days. Our policy requires 3 embedded SCTs for certificates with a
   // validity period greater than 180 days.
@@ -29,19 +49,6 @@ function run_test() {
     Ci.nsITransportSecurityInfo.CERTIFICATE_TRANSPARENCY_POLICY_NOT_ENOUGH_SCTS
   );
 
-  // Test that if an end-entity is marked as a trust anchor, CT verification
-  // returns a "not enough SCTs" result.
-  add_test(() => {
-    let cert = constructCertFromFile("test_ct/ct-valid.example.com.pem");
-    setCertTrust(cert, "CTu,,");
-    clearSessionCache();
-    run_next_test();
-  });
-  add_ct_test(
-    "ct-valid.example.com",
-    Ci.nsITransportSecurityInfo.CERTIFICATE_TRANSPARENCY_POLICY_NOT_ENOUGH_SCTS
-  );
-
   // Test that SCTs with timestamps from the future are not valid.
   add_ct_test(
     "ct-future-timestamp.example.com",
@@ -62,5 +69,22 @@ function run_test() {
     Ci.nsITransportSecurityInfo.CERTIFICATE_TRANSPARENCY_POLICY_NOT_ENOUGH_SCTS
   );
 
+  // Test that if an end-entity is marked as a trust anchor, CT verification
+  // returns a "not enough SCTs" result.
+  add_test(() => {
+    let cert = constructCertFromFile("test_ct/ct-valid.example.com.pem");
+    Services.prefs.setCharPref(
+      "security.test.built_in_root_hash",
+      cert.sha256Fingerprint
+    );
+    setCertTrust(cert, "CTu,,");
+    clearSessionCache();
+    run_next_test();
+  });
+  add_ct_test(
+    "ct-valid.example.com",
+    Ci.nsITransportSecurityInfo.CERTIFICATE_TRANSPARENCY_POLICY_NOT_ENOUGH_SCTS
+  );
+
   run_next_test();
 }

security/manager/ssl/tests/unit/test_ct_stapled.js

@@ -9,6 +9,14 @@ do_get_profile(); // must be called before getting nsIX509CertDB
 
 function run_test() {
   Services.prefs.setIntPref("security.pki.certificate_transparency.mode", 1);
+  // Make the test root appear to be a built-in root, so that certificate
+  // transparency is checked.
+  let rootCert = constructCertFromFile("test_ct/test-ca.pem");
+  Services.prefs.setCharPref(
+    "security.test.built_in_root_hash",
+    rootCert.sha256Fingerprint
+  );
+
   add_tls_server_setup("OCSPStaplingServer", "test_ct");
 
   add_ct_test(

toolkit/components/extensions/test/mochitest/test_ext_webrequest_getSecurityInfo.html

@@ -13,17 +13,6 @@
 "use strict";
 
 add_task(async function test_getSecurityInfo() {
-  // Certificate transparency telemetry is disabled by default in non-Nightly
-  // builds. Setting this pref (which enables gathering certificate
-  // transparency telemetry) ensures that there is a consistent environment
-  // when checking certificate transparency results. This is important, because
-  // when this patch goes from nightly to e.g. beta (and beyond), this test
-  // would otherwise fail because the certificate telemetry result is not what
-  // is expected, due to the difference in defaults across channels.
-  await SpecialPowers.pushPrefEnv({
-    set: [["security.pki.certificate_transparency.mode", 1]],
-  });
-
   const extension = ExtensionTestUtils.loadExtension({
     manifest: {
       permissions: [
@@ -79,7 +68,7 @@ add_task(async function test_getSecurityInfo() {
         browser.test.assertDeepEq({
           state: "secure",
           isExtendedValidation: false,
-          certificateTransparencyStatus: "policy_not_enough_scts",
+          certificateTransparencyStatus: "not_applicable",
           hsts: false,
           hpkp: false,
           usedEch: false,
@@ -100,10 +89,6 @@ add_task(async function test_getSecurityInfo() {
   await extension.awaitFinish("success");
 
   await extension.unload();
-
-  // Un-do setting the certificate transparency telemetry collection pref (see
-  // the beginning of this function).
-  await SpecialPowers.popPrefEnv();
 });
 
 add_task(async function test_getSecurityInfo_without_permission() {