Bug 1434300 - Update Imminent Distrust status for future Symantec sanctions r?keeler r?fkiefer draft
authorJ.C. Jones <jjones@mozilla.com>
Wed, 21 Feb 2018 07:39:36 -0500
changeset 758019 76dd581c3642c106c8a2826c1414ecf797579560
parent 756941 d0d3693d9beff5477175a441fdb06e281e8b7f17
child 758020 f6c9341fde050d7079a8934636644aaf54bde922
push id99916
push userbmo:jjones@mozilla.com
push dateWed, 21 Feb 2018 19:10:27 +0000
reviewerskeeler, fkiefer
bugs1434300
milestone60.0a1
Bug 1434300 - Update Imminent Distrust status for future Symantec sanctions r?keeler r?fkiefer This patch does a few things: 1) It adds a permament test mechanism for the "imminent distrust" trust status in nsNSSCallbacks: a simple xpcshell test to exercise a clause in the imminent distrust logic in nsNSSCallbacks' IsCertificateDistrustImminent method. 2) This test removes test_symantec_apple_google_unaffected.js as its functionality is rolled into the new test_imminent_distrust.js. 3) It updates the Symantec imminent distrust warning algorithm to remove the validity date exception; this warns of the upcoming distrust for those affected certs in Firefox 63. This patch does not attempt to edit the browser chrome test that checks the console; that is a subsequent patch. MozReview-Commit-ID: 1HyVLfmEOP7
security/certverifier/TrustOverride-TestImminentDistrustData.inc
security/manager/ssl/nsNSSCallbacks.cpp
security/manager/ssl/tests/unit/bad_certs/ee-imminently-distrusted.pem
security/manager/ssl/tests/unit/bad_certs/ee-imminently-distrusted.pem.certspec
security/manager/ssl/tests/unit/bad_certs/moz.build
security/manager/ssl/tests/unit/test_imminent_distrust.js
security/manager/ssl/tests/unit/test_symantec_apple_google.js
security/manager/ssl/tests/unit/test_symantec_apple_google_unaffected.js
security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
security/manager/ssl/tests/unit/xpcshell.ini
security/manager/tools/crtshToDNStruct/crtshToDNStruct.py
new file mode 100644
--- /dev/null
+++ b/security/certverifier/TrustOverride-TestImminentDistrustData.inc
@@ -0,0 +1,22 @@
+// Script from security/manager/tools/crtshToDNStruct/crtshToDNStruct.py
+// Invocation: security/manager/tools/crtshToDNStruct/crtshToDNStruct.py security/manager/ssl/tests/unit/bad_certs/ee-imminently-distrusted.pem
+
+// This file is used by test_imminent_distrust.js and by
+// browser_console_certificate_imminent_distrust.js to ensure that the UI for
+// alerting users to an upcoming CA distrust action continues to function.
+
+// /C=US/CN=Imminently Distrusted End Entity
+// SHA256 Fingerprint: 63:3A:70:8A:67:42:91:95:98:E9:D1:CB:8B:5D:73:80
+//                     BA:6D:AD:25:82:62:52:AD:5E:5E:DC:06:BF:03:1F:D0
+static const uint8_t CAImminentlyDistrustedEndEntityDN[58] = {
+  0x30, 0x38, 0x31, 0x0B, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02,
+  0x55, 0x53, 0x31, 0x29, 0x30, 0x27, 0x06, 0x03, 0x55, 0x04, 0x03, 0x13, 0x20,
+  0x49, 0x6D, 0x6D, 0x69, 0x6E, 0x65, 0x6E, 0x74, 0x6C, 0x79, 0x20, 0x44, 0x69,
+  0x73, 0x74, 0x72, 0x75, 0x73, 0x74, 0x65, 0x64, 0x20, 0x45, 0x6E, 0x64, 0x20,
+  0x45, 0x6E, 0x74, 0x69, 0x74, 0x79,
+};
+
+static const DataAndLength TestImminentDistrustEndEntityDNs[]= {
+  { CAImminentlyDistrustedEndEntityDN,
+    sizeof(CAImminentlyDistrustedEndEntityDN) },
+};
--- a/security/manager/ssl/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/nsNSSCallbacks.cpp
@@ -34,17 +34,17 @@
 #include "nsStringStream.h"
 #include "pkix/pkixtypes.h"
 #include "ssl.h"
 #include "sslproto.h"
 
 #include "TrustOverrideUtils.h"
 #include "TrustOverride-SymantecData.inc"
 #include "TrustOverride-AppleGoogleData.inc"
-
+#include "TrustOverride-TestImminentDistrustData.inc"
 
 using namespace mozilla;
 using namespace mozilla::pkix;
 using namespace mozilla::psm;
 
 extern LazyLogModule gPIPNSSLog;
 
 static void AccumulateCipherSuite(Telemetry::HistogramID probe,
@@ -1250,39 +1250,34 @@ IsCertificateDistrustImminent(nsIX509Cer
   nsCOMPtr<nsIX509Cert> eeCert;
 
   RefPtr<nsNSSCertList> certList = aCertList->GetCertList();
   nsresult rv = certList->SegmentCertificateChain(rootCert, intCerts, eeCert);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  // We need to verify the age of the end entity
-  nsCOMPtr<nsIX509CertValidity> validity;
-  rv = eeCert->GetValidity(getter_AddRefs(validity));
-  if (NS_FAILED(rv)) {
-    return rv;
+  // Check the test certificate condition first; this is a special certificate
+  // that gets the 'imminent distrust' treatment; this is so that the distrust
+  // UX code does not become stale, as it will need regular use. See Bug 1409257
+  // for context. Please do not remove this when adjusting the rest of the
+  // method.
+  UniqueCERTCertificate nssEECert(eeCert->GetCert());
+  if (!nssEECert) {
+    return NS_ERROR_FAILURE;
+  }
+  aResult = CertDNIsInList(nssEECert.get(), TestImminentDistrustEndEntityDNs);
+  if (aResult) {
+    // Exit early
+    return NS_OK;
   }
 
-  PRTime notBefore;
-  rv = validity->GetNotBefore(&notBefore);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  // PRTime is microseconds since the epoch, whereas JS time is milliseconds.
-  // (new Date("2016-06-01T00:00:00Z")).getTime() * 1000
-  static const PRTime JUNE_1_2016 = 1464739200000000;
-
-  // If the end entity's notBefore date is after 2016-06-01, this algorithm
-  // doesn't apply, so exit false before we do any iterating
-  if (notBefore >= JUNE_1_2016) {
-    aResult = false;
-    return NS_OK;
-  }
+  // Proceed with the Symantec imminent distrust algorithm. This algorithm is
+  // to be removed in Firefox 63, when the validity period check will also be
+  // removed from the code in NSSCertDBTrustDomain.
 
   // We need an owning handle when calling nsIX509Cert::GetCert().
   UniqueCERTCertificate nssRootCert(rootCert->GetCert());
   // If the root is not one of the Symantec roots, exit false
   if (!CertDNIsInList(nssRootCert.get(), RootSymantecDNs)) {
     aResult = false;
     return NS_OK;
   }
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/bad_certs/ee-imminently-distrusted.pem
@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDSzCCAjOgAwIBAgIUa5hfI/44eCaBMFte+oFrCN4wsOAwDQYJKoZIhvcNAQEL
+BQAwEjEQMA4GA1UEAwwHVGVzdCBDQTAiGA8yMDE2MTEyNzAwMDAwMFoYDzIwMTkw
+MjA1MDAwMDAwWjA4MQswCQYDVQQGEwJVUzEpMCcGA1UEAxMgSW1taW5lbnRseSBE
+aXN0cnVzdGVkIEVuZCBFbnRpdHkwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK
+AoIBAQC6iFGoRI4W1kH9braIBjYQPTwT2erkNUq07PVoV2wke8HHJajg2B+9sZwG
+m24ahvJr4q9adWtqZHEIeqVap0WH9xzVJJwCfs1D/B5p0DggKZOrIMNJ5Nu5TMJr
+bA7tFYIP8X6taRqx0wI6iypB7qdw4A8Njf1mCyuwJJKkfbmIYXmQsVeQPdI7xeC4
+SB+oN9OIQ+8nFthVt2Zaqn4CkC86exCABiTMHGyXrZZhW7filhLAdTGjDJHdtMr3
+/K0dJdMJ77kXDqdo4bN7LyJvaeO0ipVhHe4m1iWdq5EITjbLHCQELL8Wiy/l8Y+Z
+FzG4s/5JI/pyUcQx1QOs2hgKNe2NAgMBAAGjbzBtMDcGA1UdEQQwMC6CCWxvY2Fs
+aG9zdIIhaW1taW5lbnRseS1kaXN0cnVzdGVkLmV4YW1wbGUuY29tMDIGCCsGAQUF
+BwEBBCYwJDAiBggrBgEFBQcwAYYWaHR0cDovL2xvY2FsaG9zdDo4ODg4LzANBgkq
+hkiG9w0BAQsFAAOCAQEAZbcXYrr6V3GvIjGsHEvI7X5P5kwuu7XADxBZItcu9eLv
+s3Sa7U3tIlhZkLYzsLuPz2q3ZkG+baFayOJgXPPaleEDrxDpElyPYtD+oTvO5oVv
+2G8UlObIdzuym5FPDpvuiQIcDIZELUOQO9V+fjeK0CZ4luFnox+cIvnB39pLa5Xd
+hHUyWMgsf9cW/T1yjjRAS2YX/HUYGjSH9MNhSriiAABers1fyJkn7fdVTav2pQTp
+5yNdtwrFYkWjw1DG17uj/gtkll3ACw9oztjYTGj/okDI+ViLJqL4QeQb4G4Lpp77
++A8JfHGf/yFprXMMExy8FNN8FLIxdN2lX4WwBS5WHQ==
+-----END CERTIFICATE-----
+
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/bad_certs/ee-imminently-distrusted.pem.certspec
@@ -0,0 +1,4 @@
+issuer:Test CA
+subject:printableString/C=US/CN=Imminently Distrusted End Entity
+extension:subjectAlternativeName:localhost,imminently-distrusted.example.com
+extension:authorityInformationAccess:http://localhost:8888/
--- a/security/manager/ssl/tests/unit/bad_certs/moz.build
+++ b/security/manager/ssl/tests/unit/bad_certs/moz.build
@@ -8,16 +8,17 @@
 #test_certificates = (
 #    'badSubjectAltNames.pem',
 #    'beforeEpoch.pem',
 #    'beforeEpochINT.pem',
 #    'beforeEpochIssuer.pem',
 #    'ca-used-as-end-entity.pem',
 #    'default-ee.pem',
 #    'ee-from-missing-intermediate.pem',
+#    'ee-imminently-distrusted.pem',
 #    'eeIssuedByNonCA.pem',
 #    'eeIssuedByV1Cert.pem',
 #    'emptyIssuerName.pem',
 #    'emptyNameCA.pem',
 #    'ev-test-intermediate.pem',
 #    'ev-test.pem',
 #    'evroot.pem',
 #    'expired-ee.pem',
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_imminent_distrust.js
@@ -0,0 +1,29 @@
+/* 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/. */
+"use strict";
+
+// Tests handling of certificates that are selected to emit a distrust warning
+// to the console.
+
+function shouldBeImminentlyDistrusted(aTransportSecurityInfo) {
+  let isDistrust = aTransportSecurityInfo.securityState &
+                     Ci.nsIWebProgressListener.STATE_CERT_DISTRUST_IMMINENT;
+  Assert.ok(isDistrust, "This host should be imminently distrusted");
+}
+
+function shouldNotBeImminentlyDistrusted(aTransportSecurityInfo) {
+  let isDistrust = aTransportSecurityInfo.securityState &
+                     Ci.nsIWebProgressListener.STATE_CERT_DISTRUST_IMMINENT;
+  Assert.ok(!isDistrust, "This host should not be imminently distrusted");
+}
+
+do_get_profile();
+
+add_tls_server_setup("BadCertServer", "bad_certs");
+
+add_connection_test("imminently-distrusted.example.com",
+                    PRErrorCodeSuccess, null, shouldBeImminentlyDistrusted);
+
+add_connection_test("include-subdomains.pinning.example.com",
+                    PRErrorCodeSuccess, null, shouldNotBeImminentlyDistrusted);
--- a/security/manager/ssl/tests/unit/test_symantec_apple_google.js
+++ b/security/manager/ssl/tests/unit/test_symantec_apple_google.js
@@ -26,15 +26,15 @@ add_tls_server_setup("SymantecSanctionsS
 
 // Whitelisted certs aren't to be distrusted
 add_connection_test("symantec-whitelist-after-cutoff.example.com",
                     PRErrorCodeSuccess, null, shouldNotBeImminentlyDistrusted);
 
 add_connection_test("symantec-whitelist-before-cutoff.example.com",
                     PRErrorCodeSuccess, null, shouldNotBeImminentlyDistrusted);
 
-// Not-whitelisted certs after the cutoff aren't distrusted
+// Not-whitelisted certs after the cutoff are to be distrusted
 add_connection_test("symantec-not-whitelisted-after-cutoff.example.com",
-                    PRErrorCodeSuccess, null, shouldNotBeImminentlyDistrusted);
+                    PRErrorCodeSuccess, null, shouldBeImminentlyDistrusted);
 
 // Not whitelisted certs before the cutoff are to be distrusted
 add_connection_test("symantec-not-whitelisted-before-cutoff.example.com",
                     PRErrorCodeSuccess, null, shouldBeImminentlyDistrusted);
deleted file mode 100644
--- a/security/manager/ssl/tests/unit/test_symantec_apple_google_unaffected.js
+++ /dev/null
@@ -1,22 +0,0 @@
-/* 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/. */
-"use strict";
-
-// Tests handling of certificates issued by Symantec. If such
-// certificates have a notBefore before 1 June 2016, and are not
-// issued by an Apple or Google intermediate, they should emit a
-// warning to the console.
-
-function shouldNotBeImminentlyDistrusted(aTransportSecurityInfo) {
-  let isDistrust = aTransportSecurityInfo.securityState &
-                     Ci.nsIWebProgressListener.STATE_CERT_DISTRUST_IMMINENT;
-  Assert.ok(!isDistrust, "This host should not be imminently distrusted");
-}
-
-do_get_profile();
-
-add_tls_server_setup("OCSPStaplingServer", "ocsp_certs");
-
-add_connection_test("ocsp-stapling-good.example.com",
-                    PRErrorCodeSuccess, null, shouldNotBeImminentlyDistrusted);
--- a/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
@@ -73,16 +73,17 @@ const BadCertHost sBadCertHosts[] =
   { "inadequate-key-size-ee.example.com", "inadequateKeySizeEE" },
   { "badSubjectAltNames.example.com", "badSubjectAltNames" },
   { "ipAddressAsDNSNameInSAN.example.com", "ipAddressAsDNSNameInSAN" },
   { "noValidNames.example.com", "noValidNames" },
   { "bug413909.xn--hxajbheg2az3al.xn--jxalpdlp", "idn-certificate" },
   { "emptyissuername.example.com", "emptyIssuerName" },
   { "ev-test.example.com", "ev-test" },
   { "ee-from-missing-intermediate.example.com", "ee-from-missing-intermediate" },
+  { "imminently-distrusted.example.com", "ee-imminently-distrusted" },
   { "localhost", "unknownissuer" },
   { nullptr, nullptr }
 };
 
 int32_t
 DoSNISocketConfigBySubjectCN(PRFileDesc* aFd, const SECItem* aSrvNameArr,
                              uint32_t aSrvNameArrSize)
 {
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -92,16 +92,18 @@ run-sequentially = hardcoded ports
 skip-if = toolkit == 'android'
 [test_getchain.js]
 [test_hash_algorithms.js]
 [test_hash_algorithms_wrap.js]
 # bug 1124289 - run_test_in_child violates the sandbox on android
 skip-if = toolkit == 'android'
 [test_hmac.js]
 [test_intermediate_basic_usage_constraints.js]
+[test_imminent_distrust.js]
+run-sequentially = hardcoded ports
 [test_js_cert_override_service.js]
 run-sequentially = hardcoded ports
 [test_keysize.js]
 [test_keysize_ev.js]
 run-sequentially = hardcoded ports
 [test_local_cert.js]
 [test_logoutAndTeardown.js]
 run-sequentially = hardcoded ports
@@ -174,16 +176,14 @@ skip-if = toolkit == 'android'
 [test_sts_holepunch.js]
 [test_sts_ipv4_ipv6.js]
 [test_sts_parser.js]
 [test_sts_preload_dynamic.js]
 [test_sts_preloadlist_perwindowpb.js]
 [test_sts_preloadlist_selfdestruct.js]
 [test_symantec_apple_google.js]
 run-sequentially = hardcoded ports
-[test_symantec_apple_google_unaffected.js]
-run-sequentially = hardcoded ports
 [test_validity.js]
 run-sequentially = hardcoded ports
 [test_x509.js]
 
 # The TLS error reporting functionality lives in /toolkit but needs tlsserver
 [test_toolkit_securityreporter.js]
--- a/security/manager/tools/crtshToDNStruct/crtshToDNStruct.py
+++ b/security/manager/tools/crtshToDNStruct/crtshToDNStruct.py
@@ -43,17 +43,17 @@ def nameOIDtoString(oid):
     if oid == NameOID.LOCALITY_NAME:
         return "L"
     if oid == NameOID.ORGANIZATION_NAME:
         return "O"
     if oid == NameOID.ORGANIZATIONAL_UNIT_NAME:
         return "OU"
     raise Exception("Unknown OID: {}".format(oid))
 
-def print_block(pemData):
+def print_block(pemData, crtshId):
     substrate = pem.readPemFromFile(io.StringIO(pemData.decode("utf-8")))
     cert, rest = decoder.decode(substrate, asn1Spec=rfc5280.Certificate())
     der_subject = encoder.encode(cert['tbsCertificate']['subject'])
     octets = hex_string_for_struct(der_subject)
 
     cert = x509.load_pem_x509_certificate(pemData, default_backend())
     common_name = cert.subject.get_attributes_for_oid(NameOID.COMMON_NAME)[0]
     block_name = "CA{}DN".format(re.sub(r'[-:=_. ]', '', common_name.value))
@@ -62,18 +62,19 @@ def print_block(pemData):
 
     dn_parts = ["/{id}={value}".format(id=nameOIDtoString(part.oid),
                                        value=part.value) for part in cert.subject]
     distinguished_name = "".join(dn_parts)
 
     print("// {dn}".format(dn=distinguished_name))
     print("// SHA256 Fingerprint: " + ":".join(fingerprint[:16]))
     print("//                     " + ":".join(fingerprint[16:]))
-    print("// https://crt.sh/?id={crtsh} (crt.sh ID={crtsh})"
-          .format(crtsh=crtshId))
+    if crtshId:
+        print("// https://crt.sh/?id={crtsh} (crt.sh ID={crtsh})"
+              .format(crtsh=crtshId))
     print("static const uint8_t {}[{}] = ".format(block_name, len(octets)) + "{")
 
     while len(octets) > 0:
         print("  " + ", ".join(octets[:13]) + ",")
         octets = octets[13:]
 
     print("};")
     print()
@@ -84,19 +85,22 @@ def print_block(pemData):
 if __name__ == "__main__":
     blocks = []
 
     certshIds = sys.argv[1:]
     print("// Script from security/manager/tools/crtshToDNStruct/crtshToDNStruct.py")
     print("// Invocation: {} {}".format(sys.argv[0], " ".join(certshIds)))
     print()
     for crtshId in certshIds:
-        r = requests.get('https://crt.sh/?d={}'.format(crtshId))
-        r.raise_for_status()
-
-        pemData = r.content
-        blocks.append(print_block(pemData))
+        # Try a local file first, then crt.sh
+        try:
+            with open(crtshId, "rb") as pemFile:
+                blocks.append(print_block(pemFile.read(), None))
+        except FileNotFoundError:
+            r = requests.get('https://crt.sh/?d={}'.format(crtshId))
+            r.raise_for_status()
+            blocks.append(print_block(r.content, crtshId))
 
     print("static const DataAndLength RootDNs[]= {")
     for structName in blocks:
         print("  { " + "{},".format(structName))
         print("    sizeof({})".format(structName) + " },")
     print("};")