Bug 1418275 Fix timing of STS header processing for webextensions r=mayhemer
authorShane Caraveo <scaraveo@mozilla.com>
Wed, 10 Jan 2018 13:21:08 -0800
changeset 452905 9fe0fbef78edeba45192d050a4e472172ec13fa2
parent 452904 db0a461b3034f88ada61d13e8536eefdfd2361fb
child 452906 89d7853edd6412328c70ffec3f451da3aa9b1f34
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1418275
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1418275 Fix timing of STS header processing for webextensions r=mayhemer STS header checking was happening before http-on-examine-response which prevents an observer from adding the STS headers to enforce STS. This moves the header processing to after the notification occurs. In a webextension, WebRequest.onHeadersReceived can now be used to inject STS and have that recognized by HttpChannel. MozReview-Commit-ID: KYZCSTBnZL7
netwerk/protocol/http/nsHttpChannel.cpp
toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -2295,28 +2295,19 @@ nsHttpChannel::ProcessResponse()
     }
     if (referrer) {
         nsCOMPtr<nsILoadContextInfo> lci = GetLoadContextInfo(this);
         mozilla::net::Predictor::UpdateCacheability(referrer, mURI, httpStatus,
                                                     mRequestHead, mResponseHead,
                                                     lci, mIsTrackingResource);
     }
 
-    if (mTransaction && mTransaction->ProxyConnectFailed()) {
-        // Only allow 407 (authentication required) to continue
-        if (httpStatus != 407) {
-            return ProcessFailedProxyConnect(httpStatus);
-        }
-        // If proxy CONNECT response needs to complete, wait to process connection
-        // for Strict-Transport-Security.
-    } else {
-        // Given a successful connection, process any STS or PKP data that's
-        // relevant.
-        DebugOnly<nsresult> rv = ProcessSecurityHeaders();
-        MOZ_ASSERT(NS_SUCCEEDED(rv), "ProcessSTSHeader failed, continuing load.");
+    // Only allow 407 (authentication required) to continue
+    if (mTransaction && mTransaction->ProxyConnectFailed() && httpStatus != 407) {
+        return ProcessFailedProxyConnect(httpStatus);
     }
 
     MOZ_ASSERT(!mCachedContentIsValid || mRaceCacheWithNetwork,
                "We should not be hitting the network if we have valid cached "
                "content unless we are racing the network and cache");
 
     ProcessSSLInformation();
 
@@ -2354,24 +2345,30 @@ nsHttpChannel::ContinueProcessResponse1(
 
     // Check if request was cancelled during http-on-examine-response.
     if (mCanceled) {
         return CallOnStartRequest();
     }
 
     uint32_t httpStatus = mResponseHead->Status();
 
-    // Cookies and Alt-Service should not be handled on proxy failure either.
-    // This would be consolidated with ProcessSecurityHeaders but it should
-    // happen after OnExamineResponse.
+    // STS, Cookies and Alt-Service should not be handled on proxy failure.
+    // If proxy CONNECT response needs to complete, wait to process connection
+    // for Strict-Transport-Security.
     if (!(mTransaction && mTransaction->ProxyConnectFailed()) && (httpStatus != 407)) {
         nsAutoCString cookie;
         if (NS_SUCCEEDED(mResponseHead->GetHeader(nsHttp::Set_Cookie, cookie))) {
             SetCookie(cookie.get());
         }
+
+        // Given a successful connection, process any STS or PKP data that's
+        // relevant.
+        DebugOnly<nsresult> rv = ProcessSecurityHeaders();
+        MOZ_ASSERT(NS_SUCCEEDED(rv), "ProcessSTSHeader failed, continuing load.");
+
         if ((httpStatus < 500) && (httpStatus != 421)) {
             ProcessAltService();
         }
     }
 
     if (mConcurrentCacheAccess && mCachedContentIsPartial && httpStatus != 206) {
         LOG(("  only expecting 206 when doing partial request during "
              "interrupted cache concurrent read"));
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html
@@ -7,49 +7,44 @@
   <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
   <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
   <script type="text/javascript" src="head.js"></script>
   <script type="text/javascript" src="head_webrequest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 <script>
 "use strict";
 
-function getExtension(background) {
-  let manifest = {
-    "permissions": [
-      "tabs",
-      "webRequest",
-      "webRequestBlocking",
-      "<all_urls>",
-    ],
-  };
-  return ExtensionTestUtils.loadExtension({
-    manifest,
-    background,
-  });
-}
-
-// This test makes a request against a server that redirects with a 302.
-add_task(async function test_hsts_request() {
-  const testPath = "example.org/tests/toolkit/components/extensions/test/mochitest";
-
-  let expect;
-  let extension = getExtension(async function() {
-    let urls = ["*://example.org/tests/*"];
+function getExtension() {
+  async function background() {
+    let expect;
+    let urls = ["*://*.example.org/tests/*"];
     browser.webRequest.onBeforeRequest.addListener(details => {
       browser.test.assertEq(expect.shift(), "onBeforeRequest");
     }, {urls}, ["blocking"]);
     browser.webRequest.onBeforeSendHeaders.addListener(details => {
       browser.test.assertEq(expect.shift(), "onBeforeSendHeaders");
     }, {urls}, ["blocking", "requestHeaders"]);
     browser.webRequest.onSendHeaders.addListener(details => {
       browser.test.assertEq(expect.shift(), "onSendHeaders");
     }, {urls}, ["requestHeaders"]);
     browser.webRequest.onHeadersReceived.addListener(details => {
       browser.test.assertEq(expect.shift(), "onHeadersReceived");
+
+      let headers = details.responseHeaders || [];
+      for (let header of headers) {
+        if (header.name.toLowerCase() === "strict-transport-security") {
+          return;
+        }
+      }
+
+      headers.push({
+        name: "Strict-Transport-Security",
+        value: "max-age=31536000000",
+      });
+      return {responseHeaders: headers};
     }, {urls}, ["blocking", "responseHeaders"]);
     browser.webRequest.onBeforeRedirect.addListener(details => {
       browser.test.assertEq(expect.shift(), "onBeforeRedirect");
     }, {urls});
     browser.webRequest.onResponseStarted.addListener(details => {
       browser.test.assertEq(expect.shift(), "onResponseStarted");
     }, {urls});
     browser.webRequest.onCompleted.addListener(details => {
@@ -68,17 +63,37 @@ add_task(async function test_hsts_reques
       browser.tabs.onUpdated.removeListener(onUpdated);
       browser.test.sendMessage("tabs-done", tab.url);
     }
     browser.test.onMessage.addListener((url, expected) => {
       expect = expected;
       browser.tabs.onUpdated.addListener(onUpdated);
       browser.tabs.create({url});
     });
+  }
+
+  let manifest = {
+    "permissions": [
+      "tabs",
+      "webRequest",
+      "webRequestBlocking",
+      "<all_urls>",
+    ],
+  };
+  return ExtensionTestUtils.loadExtension({
+    manifest,
+    background,
   });
+}
+
+// This test makes a request against a server that redirects with a 302.
+add_task(async function test_hsts_request() {
+  const testPath = "example.org/tests/toolkit/components/extensions/test/mochitest";
+
+  let extension = getExtension();
   await extension.startup();
 
   // simple redirect
   let sample = "https://example.org/tests/toolkit/components/extensions/test/mochitest/file_sample.html";
   extension.sendMessage(
     `https://${testPath}/redirect_auto.sjs?redirect_uri=${sample}`,
     ["onBeforeRequest", "onBeforeSendHeaders", "onSendHeaders",
      "onHeadersReceived", "onBeforeRedirect", "onBeforeRequest",
@@ -108,14 +123,45 @@ add_task(async function test_hsts_reques
   is(await extension.awaitMessage("tabs-done"),
      "https://example.org/tests/toolkit/components/extensions/test/mochitest/hsts.sjs",
      "hsts upgraded");
   is(await extension.awaitMessage("onCompleted"),
      "https://example.org/tests/toolkit/components/extensions/test/mochitest/hsts.sjs");
 
   await extension.unload();
 });
+
+// This test makes a priming request and adds the STS header, then tests the upgrade.
+add_task(async function test_hsts_header() {
+  const testPath = "test1.example.org/tests/toolkit/components/extensions/test/mochitest";
+
+  let extension = getExtension();
+  await extension.startup();
+
+  // priming hsts, this time there is no STS header, onHeadersReceived adds it.
+  let completed = extension.awaitMessage("onCompleted");
+  let tabdone = extension.awaitMessage("tabs-done");
+  extension.sendMessage(
+    `https://${testPath}/file_sample.html`,
+    ["onBeforeRequest", "onBeforeSendHeaders", "onSendHeaders",
+     "onHeadersReceived", "onResponseStarted", "onCompleted"]);
+  is(await tabdone, `https://${testPath}/file_sample.html`, "priming request done");
+  is(await completed, `https://${testPath}/file_sample.html`, "priming request done");
+
+  // test upgrade from http to https due to onHeadersReceived adding STS header
+  completed = extension.awaitMessage("onCompleted");
+  tabdone = extension.awaitMessage("tabs-done");
+  extension.sendMessage(
+    `http://${testPath}/file_sample.html`,
+    ["onBeforeRequest", "onBeforeRedirect", "onBeforeRequest",
+     "onBeforeSendHeaders", "onSendHeaders", "onHeadersReceived",
+     "onResponseStarted", "onCompleted"]);
+  is(await tabdone, `https://${testPath}/file_sample.html`, "hsts upgraded");
+  is(await completed, `https://${testPath}/file_sample.html`, "request upgraded");
+
+  await extension.unload();
+});
 </script>
 </head>
 <body>
 
 </body>
 </html>