Bug 1278041 - skip TLS Feature checks so HPKP can be set. r=mgoodwin, a=sylvestre
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 20 Jun 2016 16:36:36 -0700
changeset 341726 852f817ca2c4a23f0736c4407305341adbc99904
parent 341725 b6b52e21758a5ac9855ee5c2938df5cfc1415cb2
child 341727 af923f4926e7606c93218467f4de253c9a2bc783
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgoodwin, sylvestre
bugs1278041
milestone49.0a2
Bug 1278041 - skip TLS Feature checks so HPKP can be set. r=mgoodwin, a=sylvestre This is safe because TLS Feature checks have already been done when connecting to the site in the first place. MozReview-Commit-ID: HfbcrAv4bCJ
security/manager/ssl/nsSiteSecurityService.cpp
security/manager/ssl/tests/unit/test_ocsp_must_staple.js
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -697,22 +697,30 @@ nsSiteSecurityService::ProcessPKPHeader(
   NS_ENSURE_TRUE(cert, NS_ERROR_FAILURE);
   UniqueCERTCertificate nssCert(cert->GetCert());
   NS_ENSURE_TRUE(nssCert, NS_ERROR_FAILURE);
 
   mozilla::pkix::Time now(mozilla::pkix::Now());
   UniqueCERTCertList certList;
   RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
   NS_ENSURE_TRUE(certVerifier, NS_ERROR_UNEXPECTED);
+  // We don't want this verification to cause any network traffic that would
+  // block execution. Also, since we don't have access to the original stapled
+  // OCSP response, we can't enforce this aspect of the TLS Feature extension.
+  // This is ok, because it will have been enforced when we originally connected
+  // to the site (or it's disabled, in which case we wouldn't want to enforce it
+  // anyway).
+  CertVerifier::Flags flags = CertVerifier::FLAG_LOCAL_ONLY |
+                              CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
   if (certVerifier->VerifySSLServerCert(nssCert, nullptr, // stapled ocsp
                                         now, nullptr, // pinarg
                                         host.get(), // hostname
                                         certList,
                                         false, // don't store intermediates
-                                        CertVerifier::FLAG_LOCAL_ONLY)
+                                        flags)
       != SECSuccess) {
     return NS_ERROR_FAILURE;
   }
 
   CERTCertListNode* rootNode = CERT_LIST_TAIL(certList);
   if (CERT_LIST_END(rootNode, certList)) {
     return NS_ERROR_FAILURE;
   }
--- a/security/manager/ssl/tests/unit/test_ocsp_must_staple.js
+++ b/security/manager/ssl/tests/unit/test_ocsp_must_staple.js
@@ -22,16 +22,39 @@ function add_ocsp_test(aHost, aExpectedR
 }
 
 function add_tests() {
   // ensure that the chain is checked for required features in children:
   // First a case where intermediate and ee both have the extension
   add_ocsp_test("ocsp-stapling-must-staple-ee-with-must-staple-int.example.com",
                 PRErrorCodeSuccess, true);
 
+  add_test(() => {
+    Services.prefs.setIntPref("security.cert_pinning.enforcement_level", 1);
+    Services.prefs.setBoolPref("security.cert_pinning.process_headers_from_non_builtin_roots", true);
+    let uri = Services.io.newURI("https://ocsp-stapling-must-staple-ee-with-must-staple-int.example.com",
+                                 null, null);
+    let keyHash = "VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=";
+    let backupKeyHash = "KHAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAN=";
+    let header = `max-age=1000; pin-sha256="${keyHash}"; pin-sha256="${backupKeyHash}"`;
+    let ssservice = Cc["@mozilla.org/ssservice;1"]
+                      .getService(Ci.nsISiteSecurityService);
+    let sslStatus = new FakeSSLStatus();
+    sslStatus.serverCert = constructCertFromFile("ocsp_certs/must-staple-ee-with-must-staple-int.pem");
+    ssservice.processHeader(Ci.nsISiteSecurityService.HEADER_HPKP, uri, header, sslStatus, 0);
+    ok(ssservice.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
+       "ocsp-stapling-must-staple-ee-with-must-staple-int.example.com should have HPKP set");
+
+    // Clear accumulated state.
+    ssservice.removeState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
+    Services.prefs.clearUserPref("security.cert_pinning.process_headers_from_non_builtin_roots");
+    Services.prefs.clearUserPref("security.cert_pinning.enforcement_level");
+    run_next_test();
+  });
+
   // Next, a case where it's present in the intermediate, not the ee
   add_ocsp_test("ocsp-stapling-plain-ee-with-must-staple-int.example.com",
                 MOZILLA_PKIX_ERROR_REQUIRED_TLS_FEATURE_MISSING, true);
 
   // We disable OCSP stapling in the next two tests so we can perform checks
   // on TLS Features in the chain without needing to support the TLS
   // extension values used.
   // Test an issuer with multiple TLS features in matched in the EE