Bug 1310955 - Fix nsSiteSecurityService cache retrieval r?keeler r?ckerschb draft
authorKate McKinley <kmckinley@mozilla.com>
Tue, 18 Oct 2016 20:09:15 +0900
changeset 426859 5fcaa9dac88124f3557621cedaaa8bf54b569186
parent 426772 0be815a3f2d5ab6899b8d1cdca940c1bdd4cae70
child 534292 d95bd891e893e033966f0ae65f49949533a9955c
push id32830
push userbmo:kmckinley@mozilla.com
push dateWed, 19 Oct 2016 08:39:58 +0000
reviewerskeeler, ckerschb
bugs1310955
milestone52.0a1
Bug 1310955 - Fix nsSiteSecurityService cache retrieval r?keeler r?ckerschb MozReview-Commit-ID: 55DpKrqcL1x
dom/security/test/hsts/browser.ini
dom/security/test/hsts/browser_hsts-priming_no-duplicates.js
dom/security/test/hsts/head.js
security/manager/ssl/nsSiteSecurityService.cpp
--- a/dom/security/test/hsts/browser.ini
+++ b/dom/security/test/hsts/browser.ini
@@ -9,8 +9,9 @@ support-files =
 
 [browser_hsts-priming_allow_active.js]
 [browser_hsts-priming_block_active.js]
 [browser_hsts-priming_hsts_after_mixed.js]
 [browser_hsts-priming_allow_display.js]
 [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]
new file mode 100644
--- /dev/null
+++ b/dom/security/test/hsts/browser_hsts-priming_no-duplicates.js
@@ -0,0 +1,28 @@
+/*
+ * Description of the test:
+ *   Only one request should be sent per host, even if we run the test more
+ *   than once.
+ */
+'use strict';
+
+//jscs:disable
+add_task(function*() {
+  //jscs:enable
+  Observer.add_observers(Services);
+  registerCleanupFunction(do_cleanup);
+
+  let which = "block_display";
+
+  SetupPrefTestEnvironment(which);
+
+  for (let server of Object.keys(test_servers)) {
+    yield execute_test(server, test_settings[which].mimetype);
+  }
+
+  // run the tests twice to validate the cache is being used
+  for (let server of Object.keys(test_servers)) {
+    yield execute_test(server, test_settings[which].mimetype);
+  }
+
+  SpecialPowers.popPrefEnv();
+});
--- a/dom/security/test/hsts/head.js
+++ b/dom/security/test/hsts/head.js
@@ -140,19 +140,26 @@ var which_test = "";
 
 const Observer = {
   observe: function (subject, topic, data) {
     switch (topic) {
       case 'console-api-log-event':
         return Observer.console_api_log_event(subject, topic, data);
       case 'http-on-examine-response':
         return Observer.http_on_examine_response(subject, topic, data);
+      case 'http-on-modify-request':
+        return Observer.http_on_modify_request(subject, topic, data);
     }
     throw "Can't handle topic "+topic;
   },
+  add_observers: function (services) {
+    services.obs.addObserver(Observer, "console-api-log-event", false);
+    services.obs.addObserver(Observer, "http-on-examine-response", false);
+    services.obs.addObserver(Observer, "http-on-modify-request", false);
+  },
   // When a load is blocked which results in an error event within a page, the
   // test logs to the console.
   console_api_log_event: function (subject, topic, data) {
     var message = subject.wrappedJSObject.arguments[0];
     // when we are blocked, this will match the message we sent to the console,
     // ignore everything else.
     var re = RegExp(/^HSTS_PRIMING: Blocked ([-\w]+).*$/);
     if (!re.test(message)) {
@@ -168,40 +175,56 @@ const Observer = {
       return;
     }
 
     is("blocked", test_settings[which_test].result[curTest.id], "HSTS priming "+
         which_test+":"+curTest.id+" expected "+
         test_settings[which_test].result[curTest.id]+", got blocked");
     test_settings[which_test].finished[curTest.id] = "blocked";
   },
+  get_current_test: function(uri) {
+    for (let item in test_servers) {
+      let re = RegExp('https?://'+test_servers[item].host);
+      if (re.test(uri)) {
+        return test_servers[item];
+      }
+    }
+    return null;
+  },
+  http_on_modify_request: function (subject, topic, data) {
+    let channel = subject.QueryInterface(Ci.nsIHttpChannel);
+    if (channel.requestMethod != 'HEAD') {
+      return;
+    }
+
+    let curTest = this.get_current_test(channel.URI.asciiSpec);
+
+    if (!curTest) {
+      return;
+    }
+
+    ok(!(curTest.id in test_settings[which_test].priming), "Already saw a priming request for " + curTest.id);
+    test_settings[which_test].priming[curTest.id] = true;
+  },
   // When we see a response come back, peek at the response and test it is secure
   // or insecure as needed. Addtionally, watch the response for priming requests.
   http_on_examine_response: function (subject, topic, data) {
-    let curTest = null;
     let channel = subject.QueryInterface(Ci.nsIHttpChannel);
-    for (let item in test_servers) {
-      let re = RegExp('https?://'+test_servers[item].host);
-      if (re.test(channel.URI.asciiSpec)) {
-        curTest = test_servers[item];
-        break;
-      }
-    }
+    let curTest = this.get_current_test(channel.URI.asciiSpec);
 
     if (!curTest) {
       return;
     }
 
     let result = (channel.URI.asciiSpec.startsWith('https:')) ? "secure" : "insecure";
 
     // This is a priming request, go ahead and validate we were supposed to see
     // a response from the server
     if (channel.requestMethod == 'HEAD') {
       is(true, curTest.response, "HSTS priming response found " + curTest.id);
-      test_settings[which_test].priming[curTest.id] = true;
       return;
     }
 
     // This is the response to our query, make sure it matches
     is(result, test_settings[which_test].result[curTest.id],
         "HSTS priming result " + which_test + ":" + curTest.id);
     test_settings[which_test].finished[curTest.id] = result;
   },
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -56,17 +56,18 @@ SiteHSTSState::SiteHSTSState(nsCString& 
   uint32_t hstsIncludeSubdomains = 0; // PR_sscanf doesn't handle bools.
   int32_t matches = PR_sscanf(aStateString.get(), "%lld,%lu,%lu",
                               &mHSTSExpireTime, &hstsState,
                               &hstsIncludeSubdomains);
   bool valid = (matches == 3 &&
                 (hstsIncludeSubdomains == 0 || hstsIncludeSubdomains == 1) &&
                 ((SecurityPropertyState)hstsState == SecurityPropertyUnset ||
                  (SecurityPropertyState)hstsState == SecurityPropertySet ||
-                 (SecurityPropertyState)hstsState == SecurityPropertyKnockout));
+                 (SecurityPropertyState)hstsState == SecurityPropertyKnockout ||
+                 (SecurityPropertyState)hstsState == SecurityPropertyNegative));
   if (valid) {
     mHSTSState = (SecurityPropertyState)hstsState;
     mHSTSIncludeSubdomains = (hstsIncludeSubdomains == 1);
   } else {
     SSSLOG(("%s is not a valid SiteHSTSState", aStateString.get()));
     mHSTSExpireTime = 0;
     mHSTSState = SecurityPropertyUnset;
     mHSTSIncludeSubdomains = false;