Bug 1322044 - Only mark a subdomain cached when includeSubDomains is true r?keeler,ckerschb draft
authorKate McKinley <kmckinley@mozilla.com>
Wed, 21 Dec 2016 15:03:54 -0800
changeset 480257 b927a1675568ee5f7eef23f6b0ff6fbde8f4652c
parent 480256 8616263a821746f5cca4f32b09d894228070710f
child 544913 a835bc4b8acc7c4f5a852eacfd2e2ce70a19d7a0
push id44502
push userbmo:kmckinley@mozilla.com
push dateWed, 08 Feb 2017 03:18:18 +0000
reviewerskeeler, ckerschb
bugs1322044
milestone54.0a1
Bug 1322044 - Only mark a subdomain cached when includeSubDomains is true r?keeler,ckerschb MozReview-Commit-ID: 3lFkuLauyGg
dom/security/test/hsts/browser.ini
dom/security/test/hsts/browser_hsts-priming_include-subdomains.js
dom/security/test/hsts/file_testserver.sjs
security/manager/ssl/nsSiteSecurityService.cpp
--- a/dom/security/test/hsts/browser.ini
+++ b/dom/security/test/hsts/browser.ini
@@ -15,8 +15,9 @@ support-files =
 [browser_hsts-priming_block_display.js]
 [browser_hsts-priming_block_active_css.js]
 [browser_hsts-priming_block_active_with_redir_same.js]
 [browser_hsts-priming_no-duplicates.js]
 [browser_hsts-priming_cache-timeout.js]
 [browser_hsts-priming_timeout.js]
 [browser_hsts-priming_no-non-standard-ports.js]
 [browser_hsts-priming_no-ip-address.js]
+[browser_hsts-priming_include-subdomains.js]
new file mode 100644
--- /dev/null
+++ b/dom/security/test/hsts/browser_hsts-priming_include-subdomains.js
@@ -0,0 +1,35 @@
+/*
+ * Description of the test:
+ *   If the top-level domain sends the STS header but does not have
+ *   includeSubdomains, HSTS priming requests should still be sent to
+ *   subdomains.
+ */
+'use strict';
+
+//jscs:disable
+add_task(function*() {
+  //jscs:enable
+  Observer.add_observers(Services);
+  registerCleanupFunction(do_cleanup);
+
+  // add the top-level server
+  test_servers['top-level'] = {
+    host: 'example.com',
+    response: true,
+    id: 'top-level',
+  };
+  test_settings.block_active.result['top-level'] = 'secure';
+
+  let which = "block_active";
+
+  SetupPrefTestEnvironment(which);
+
+  yield execute_test("top-level", test_settings[which].mimetype);
+
+  yield execute_test("prime-hsts", test_settings[which].mimetype);
+
+  ok("prime-hsts" in test_settings[which].priming,
+    "HSTS priming on a subdomain when top-level does not includeSubDomains");
+
+  SpecialPowers.popPrefEnv();
+});
--- a/dom/security/test/hsts/file_testserver.sjs
+++ b/dom/security/test/hsts/file_testserver.sjs
@@ -51,17 +51,17 @@ function handleRequest(request, response
         response.finish();
         return;
       }
 
       // if we have a priming header, check for required behavior
       // and set header appropriately
       if (request.hasHeader('Upgrade-Insecure-Requests')) {
         var expected = query.get('primer');
-        if (expected == 'prime-hsts') {
+        if (expected == 'prime-hsts' || expected == 'top-level') {
           // set it for 5 minutes
           response.setHeader("Strict-Transport-Security", "max-age="+(60*5), false);
         } else if (expected == 'reject-upgrade') {
           response.setHeader("Strict-Transport-Security", "max-age=0", false);
         }
         response.write('xyzzy');
         response.finish();
         return;
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -1135,24 +1135,30 @@ nsSiteSecurityService::HostHasHSTSEntry(
   nsCString value = mSiteStateStorage->Get(storageKey, storageType);
   RefPtr<SiteHSTSState> siteState = new SiteHSTSState(aHost, value);
   if (siteState->mHSTSState != SecurityPropertyUnset) {
     SSSLOG(("Found HSTS entry for %s", aHost.get()));
     bool expired = siteState->IsExpired(nsISiteSecurityService::HEADER_HSTS);
     if (!expired) {
       SSSLOG(("Entry for %s is not expired", aHost.get()));
       if (aCached) {
-        *aCached = true;
+        // Only set cached if this includes subdomains
+        *aCached = aRequireIncludeSubdomains ? siteState->mHSTSIncludeSubdomains
+                                             : true;
       }
       if (siteState->mHSTSState == SecurityPropertySet) {
         *aResult = aRequireIncludeSubdomains ? siteState->mHSTSIncludeSubdomains
                                              : true;
         return true;
       } else if (siteState->mHSTSState == SecurityPropertyNegative) {
         *aResult = false;
+        if (aCached) {
+          // if it's negative, it is always cached
+          *aCached = true;
+        }
         return true;
       }
     }
 
     if (expired) {
       SSSLOG(("Entry %s is expired - checking for preload state", aHost.get()));
       // If the entry is expired and is not in either the static or dynamic
       // preload lists, we can remove it.
@@ -1178,19 +1184,27 @@ nsSiteSecurityService::HostHasHSTSEntry(
   RefPtr<SiteHSTSState> dynamicState = new SiteHSTSState(aHost, value);
   if (dynamicState->mHSTSState != SecurityPropertyUnset) {
     SSSLOG(("Found dynamic preload entry for %s", aHost.get()));
     bool expired = dynamicState->IsExpired(nsISiteSecurityService::HEADER_HSTS);
     if (!expired) {
       if (dynamicState->mHSTSState == SecurityPropertySet) {
         *aResult = aRequireIncludeSubdomains ? dynamicState->mHSTSIncludeSubdomains
                                              : true;
+        if (aCached) {
+          // Only set cached if this includes subdomains
+          *aCached = *aResult;
+        }
         return true;
       } else if (dynamicState->mHSTSState == SecurityPropertyNegative) {
         *aResult = false;
+        if (aCached) {
+          // if it's negative, it is always cached
+          *aCached = true;
+        }
         return true;
       }
     } else {
       // if a dynamic preload has expired and is not in the static preload
       // list, we can remove it.
       if (!GetPreloadListEntry(aHost.get())) {
         mPreloadStateStorage->Remove(storageKey,
                                      mozilla::DataStorage_Persistent);