Bug 1418275 Fix timing of STS header processing for webextensions draft
authorShane Caraveo <scaraveo@mozilla.com>
Wed, 10 Jan 2018 13:21:08 -0800
changeset 718825 3b0f4e2383e008f38f6c0898e0f429395668c103
parent 718290 e4de69553e3faf8136eb9bb7f2f741e1b7e6f866
child 745596 656cb96d768e162f8f6f604e6195f3ae74f1bc37
push id95045
push usermixedpuppy@gmail.com
push dateWed, 10 Jan 2018 21:21:32 +0000
bugs1418275
milestone59.0a1
Bug 1418275 Fix timing of STS header processing for webextensions 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>