Bug 1462006 - ignore response headers during stsUpgrade, r?rpl draft
authorShane Caraveo <scaraveo@mozilla.com>
Thu, 31 May 2018 13:28:26 -0400
changeset 802315 d3b6499dab5af19ed34d60b9a922c255117ab6aa
parent 802102 763f30c3421233a45ef9e67a695c5c241a2c8a3a
push id111859
push usermixedpuppy@gmail.com
push dateThu, 31 May 2018 17:29:09 +0000
reviewersrpl
bugs1462006
milestone62.0a1
Bug 1462006 - ignore response headers during stsUpgrade, r?rpl onBeforeRedirect happens prior to a request/response if the request is internally upgraded. In this case response headers are not avialable and we throw an exception. Extensions should have a way to understand the redirect event is an internal sts upgrade and we should ignore responseHeaders during one. MozReview-Commit-ID: FyouZm1jLVS
toolkit/components/extensions/schemas/web_request.json
toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/schemas/web_request.json
+++ b/toolkit/components/extensions/schemas/web_request.json
@@ -721,16 +721,17 @@
               "tabId": {"type": "integer", "description": "The ID of the tab in which the request takes place. Set to -1 if the request isn't related to a tab."},
               "type": {"$ref": "ResourceType", "description": "How the requested resource will be used."},
               "timeStamp": {"type": "number", "description": "The time when this signal is triggered, in milliseconds since the epoch."},
               "ip": {"type": "string", "optional": true, "description": "The server IP address that the request was actually sent to. Note that it may be a literal IPv6 address."},
               "fromCache": {"type": "boolean", "description": "Indicates if this response was fetched from disk cache."},
               "statusCode": {"type": "integer", "description": "Standard HTTP status code returned by the server."},
               "redirectUrl": {"type": "string", "description": "The new URL."},
               "responseHeaders": {"$ref": "HttpHeaders", "optional": true, "description": "The HTTP response headers that were received along with this redirect."},
+              "stsUpgrade": {"type": "boolean", "optional": true, "description": "Indicates if this redirect is a result of an STS upgrade."},
               "statusLine": {"type": "string", "description": "HTTP status line of the response or the 'HTTP/0.9 200 OK' string for HTTP/0.9 responses (i.e., responses that lack a status line) or an empty string if there are no headers."}
             }
           }
         ],
         "extraParameters": [
           {
             "$ref": "RequestFilter",
             "name": "filter",
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html
@@ -12,23 +12,23 @@
 <script>
 "use strict";
 
 function getExtension() {
   async function background() {
     let expect;
     let urls = ["*://*.example.org/tests/*"];
     browser.webRequest.onBeforeRequest.addListener(details => {
-      browser.test.assertEq(expect.shift(), "onBeforeRequest");
+      browser.test.assertEq(expect.events.shift(), "onBeforeRequest");
     }, {urls}, ["blocking"]);
     browser.webRequest.onBeforeSendHeaders.addListener(details => {
-      browser.test.assertEq(expect.shift(), "onBeforeSendHeaders");
+      browser.test.assertEq(expect.events.shift(), "onBeforeSendHeaders");
     }, {urls}, ["blocking", "requestHeaders"]);
     browser.webRequest.onSendHeaders.addListener(details => {
-      browser.test.assertEq(expect.shift(), "onSendHeaders");
+      browser.test.assertEq(expect.events.shift(), "onSendHeaders");
     }, {urls}, ["requestHeaders"]);
 
     async function testSecurityInfo(details, options) {
       let securityInfo = await browser.webRequest.getSecurityInfo(details.requestId, options);
       browser.test.assertTrue(securityInfo && securityInfo.state == "secure",
                               "security info reflects https");
 
       if (options.certificateChain) {
@@ -40,17 +40,17 @@ function getExtension() {
       if (options.rawDER) {
         for (let cert of securityInfo.certificates) {
           browser.test.assertTrue(cert.rawDER.length > 0, "have rawDER");
         }
       }
     }
 
     browser.webRequest.onHeadersReceived.addListener(async (details) => {
-      browser.test.assertEq(expect.shift(), "onHeadersReceived");
+      browser.test.assertEq(expect.events.shift(), "onHeadersReceived");
 
       // We exepect all requests to have been upgraded at this point.
       browser.test.assertTrue(details.url.startsWith("https"), "connection is https");
       await testSecurityInfo(details, {});
       await testSecurityInfo(details, {certificateChain: true});
       await testSecurityInfo(details, {rawDER: true});
       await testSecurityInfo(details, {certificateChain: true, rawDER: true});
 
@@ -63,23 +63,28 @@ function getExtension() {
 
       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.test.assertEq(expect.events.shift(), "onBeforeRedirect");
+      if (expect.stsUpgrade) {
+        browser.test.assertEq(details.responseHeaders, undefined, "responseHeaders no present during stsUpgrade");
+      } else {
+        browser.test.assertTrue(details.responseHeaders !== undefined, "responseHeaders exist");
+      }
+    }, {urls}, ["responseHeaders"]);
     browser.webRequest.onResponseStarted.addListener(details => {
-      browser.test.assertEq(expect.shift(), "onResponseStarted");
+      browser.test.assertEq(expect.events.shift(), "onResponseStarted");
     }, {urls});
     browser.webRequest.onCompleted.addListener(details => {
-      browser.test.assertEq(expect.shift(), "onCompleted");
+      browser.test.assertEq(expect.events.shift(), "onCompleted");
       browser.test.sendMessage("onCompleted", details.url);
     }, {urls});
     browser.webRequest.onErrorOccurred.addListener(details => {
       browser.test.notifyFail(`onErrorOccurred ${JSON.stringify(details)}`);
     }, {urls});
 
     async function onUpdated(tabId, tabInfo, tab) {
       if (tabInfo.status !== "complete") {
@@ -116,41 +121,50 @@ add_task(async function test_hsts_reques
 
   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",
-     "onBeforeSendHeaders", "onSendHeaders", "onHeadersReceived",
-     "onResponseStarted", "onCompleted"]);
+    {
+      events: ["onBeforeRequest", "onBeforeSendHeaders", "onSendHeaders",
+               "onHeadersReceived", "onBeforeRedirect", "onBeforeRequest",
+               "onBeforeSendHeaders", "onSendHeaders", "onHeadersReceived",
+               "onResponseStarted", "onCompleted"],
+      stsUpgrade: false,
+    });
   // redirect_auto adds a query string
   ok((await extension.awaitMessage("tabs-done")).startsWith(sample), "redirection ok");
   ok((await extension.awaitMessage("onCompleted")).startsWith(sample), "redirection ok");
 
   // priming hsts
   extension.sendMessage(
     `https://${testPath}/hsts.sjs`,
-    ["onBeforeRequest", "onBeforeSendHeaders", "onSendHeaders",
-     "onHeadersReceived", "onResponseStarted", "onCompleted"]);
+    {
+      events: ["onBeforeRequest", "onBeforeSendHeaders", "onSendHeaders",
+               "onHeadersReceived", "onResponseStarted", "onCompleted"],
+      stsUpgrade: false,
+    });
   is(await extension.awaitMessage("tabs-done"),
      "https://example.org/tests/toolkit/components/extensions/test/mochitest/hsts.sjs",
      "hsts primed");
   is(await extension.awaitMessage("onCompleted"),
      "https://example.org/tests/toolkit/components/extensions/test/mochitest/hsts.sjs");
 
   // test upgrade
   extension.sendMessage(
     `http://${testPath}/hsts.sjs`,
-    ["onBeforeRequest", "onBeforeRedirect", "onBeforeRequest",
-     "onBeforeSendHeaders", "onSendHeaders", "onHeadersReceived",
-     "onResponseStarted", "onCompleted"]);
+    {
+      events: ["onBeforeRequest", "onBeforeRedirect", "onBeforeRequest",
+               "onBeforeSendHeaders", "onSendHeaders", "onHeadersReceived",
+               "onResponseStarted", "onCompleted"],
+      stsUpgrade: true,
+    });
   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();
 });
@@ -162,29 +176,35 @@ add_task(async function test_hsts_header
   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"]);
+    {
+      events: ["onBeforeRequest", "onBeforeSendHeaders", "onSendHeaders",
+               "onHeadersReceived", "onResponseStarted", "onCompleted"],
+      stsUpgrade: false,
+    });
   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"]);
+    {
+      events: ["onBeforeRequest", "onBeforeRedirect", "onBeforeRequest",
+               "onBeforeSendHeaders", "onSendHeaders", "onHeadersReceived",
+               "onResponseStarted", "onCompleted"],
+      stsUpgrade: true,
+    });
   is(await tabdone, `https://${testPath}/file_sample.html`, "hsts upgraded");
   is(await completed, `https://${testPath}/file_sample.html`, "request upgraded");
 
   await extension.unload();
 });
 
 add_task(async function test_nonBlocking_securityInfo() {
   let extension = ExtensionTestUtils.loadExtension({
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -174,16 +174,17 @@ class ResponseHeaderChanger extends Head
 
 const MAYBE_CACHED_EVENTS = new Set([
   "onResponseStarted", "onHeadersReceived", "onBeforeRedirect", "onCompleted", "onErrorOccurred",
 ]);
 
 const OPTIONAL_PROPERTIES = [
   "requestHeaders", "responseHeaders", "statusCode", "statusLine", "error", "redirectUrl",
   "requestBody", "scheme", "realm", "isProxy", "challenger", "proxyInfo", "ip", "frameAncestors",
+  "stsUpgrade",
 ];
 
 function serializeRequestData(eventName) {
   let data = {
     requestId: this.requestId,
     url: this.url,
     originUrl: this.originUrl,
     documentUrl: this.documentUrl,
@@ -337,17 +338,17 @@ var ChannelEventSink = {
     let catMan = Cc["@mozilla.org/categorymanager;1"].getService(Ci.nsICategoryManager);
     catMan.deleteCategoryEntry("net-channel-event-sinks", this._contractID, false);
   },
 
   // nsIChannelEventSink implementation
   asyncOnChannelRedirect(oldChannel, newChannel, flags, redirectCallback) {
     runLater(() => redirectCallback.onRedirectVerifyCallback(Cr.NS_OK));
     try {
-      HttpObserverManager.onChannelReplaced(oldChannel, newChannel);
+      HttpObserverManager.onChannelReplaced(oldChannel, newChannel, flags);
     } catch (e) {
       // we don't wanna throw: it would abort the redirection
     }
   },
 
   // nsIFactory implementation
   createInstance(outer, iid) {
     if (outer) {
@@ -751,17 +752,17 @@ HttpObserverManager = {
           };
         }
 
         if (opts.requestHeaders) {
           requestHeaders = requestHeaders || new RequestHeaderChanger(channel);
           data.requestHeaders = requestHeaders.toArray();
         }
 
-        if (opts.responseHeaders) {
+        if (opts.responseHeaders && !data.stsUpgrade) {
           responseHeaders = responseHeaders || new ResponseHeaderChanger(channel);
           data.responseHeaders = responseHeaders.toArray();
         }
 
         if (opts.requestBody && channel.canModify) {
           requestBody = requestBody || WebRequestUpload.createRequestBody(channel.channel);
           data.requestBody = requestBody;
         }
@@ -904,23 +905,24 @@ HttpObserverManager = {
 
     if (!channel.hasAuthRequestor &&
         this.shouldHookListener(this.listeners.authRequired, channel, {isProxy: true})) {
       channel.channel.notificationCallbacks = new AuthRequestor(channel.channel, this);
       channel.hasAuthRequestor = true;
     }
   },
 
-  onChannelReplaced(oldChannel, newChannel) {
+  onChannelReplaced(oldChannel, newChannel, flags) {
     let channel = this.getWrapper(oldChannel);
 
     // We want originalURI, this will provide a moz-ext rather than jar or file
     // uri on redirects.
     if (this.hasRedirects) {
-      this.runChannelListener(channel, "onRedirect", {redirectUrl: newChannel.originalURI.spec});
+      let stsUpgrade = !!(flags & Ci.nsIChannelEventSink.REDIRECT_STS_UPGRADE);
+      this.runChannelListener(channel, "onRedirect", {redirectUrl: newChannel.originalURI.spec, stsUpgrade});
     }
     channel.channel = newChannel;
   },
 };
 
 var onBeforeRequest = {
   allowedOptions: ["blocking", "requestBody"],