From 86d45aba67c138801450b3f82fa58167282bf52d Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Mon, 25 Sep 2017 15:14:09 -0700 Subject: [PATCH] Bug 1401350 fix proxy auth for system requests, r=kmag MozReview-Commit-ID: CAh89djQobI --HG-- extra : rebase_source : c85ff5148d4ac1df4e21622d2d615040ba9e09d7 --- .../components/extensions/ext-webRequest.js | 14 ++- .../test/xpcshell/test_ext_proxy_auth.js | 89 +++++++++++++++---- toolkit/modules/addons/WebRequest.jsm | 19 ++-- 3 files changed, 95 insertions(+), 27 deletions(-) diff --git a/toolkit/components/extensions/ext-webRequest.js b/toolkit/components/extensions/ext-webRequest.js index 0d246a027c54..e156c986a664 100644 --- a/toolkit/components/extensions/ext-webRequest.js +++ b/toolkit/components/extensions/ext-webRequest.js @@ -17,9 +17,17 @@ function WebRequestEventManager(context, eventName) { let name = `webRequest.${eventName}`; let register = (fire, filter, info) => { let listener = data => { + // data.isProxy is only set from the WebRequest AuthRequestor handler, + // in which case we know that this is onAuthRequired. If this is proxy + // authorization, we allow without any additional matching/filtering. + let isProxyAuth = data.isProxy && context.extension.hasPermission("proxy"); + // Prevent listening in on requests originating from system principal to - // prevent tinkering with OCSP, app and addon updates, etc. - if (data.isSystemPrincipal) { + // prevent tinkering with OCSP, app and addon updates, etc. However, + // proxy addons need to be able to provide auth for any request so we + // allow those through. The exception is for proxy extensions handling + // proxy authentication. + if (data.isSystemPrincipal && !isProxyAuth) { return; } @@ -31,7 +39,7 @@ function WebRequestEventManager(context, eventName) { // and the origin that is loading the resource. const origin = data.documentUrl; const own = origin && origin.startsWith(context.extension.getURL()); - if (origin && !own && !hosts.matches(data.documentURI)) { + if (origin && !own && !isProxyAuth && !hosts.matches(data.documentURI)) { return; } diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_proxy_auth.js b/toolkit/components/extensions/test/xpcshell/test_ext_proxy_auth.js index 8e6e98f17abc..3a909ab9a88d 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_proxy_auth.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_proxy_auth.js @@ -1,5 +1,7 @@ "use strict"; +const XMLHttpRequest = Components.Constructor("@mozilla.org/xmlextras/xmlhttprequest;1", "nsIXMLHttpRequest"); + const proxy = createHttpServer(); // accept proxy connections for mozilla.org @@ -8,14 +10,35 @@ proxy.identity.add("http", "mozilla.org", 80); proxy.registerPathHandler("/", (request, response) => { if (request.hasHeader("Proxy-Authorization")) { response.setStatusLine(request.httpVersion, 200, "OK"); - response.write("ok"); + response.setHeader("Content-Type", "text/plain", false); + response.write("ok, got proxy auth"); } else { response.setStatusLine(request.httpVersion, 407, "Proxy authentication required"); + response.setHeader("Content-Type", "text/plain", false); response.setHeader("Proxy-Authenticate", 'Basic realm="foobar"', false); - response.write("ok"); + response.write("auth required"); } }); +function getExtension(background) { + return ExtensionTestUtils.loadExtension({ + manifest: { + permissions: [ + "proxy", + "webRequest", + "webRequestBlocking", + "", + ], + }, + background: `(${background})(${proxy.identity.primaryPort})`, + files: { + "proxy.js": ` + function FindProxyForURL(url, host) { + return "PROXY localhost:${proxy.identity.primaryPort}; DIRECT"; + }`, + }, + }); +} add_task(async function test_webRequest_auth_proxy() { async function background(port) { browser.webRequest.onBeforeRequest.addListener(details => { @@ -48,23 +71,7 @@ add_task(async function test_webRequest_auth_proxy() { browser.test.sendMessage("pac-ready"); } - let handlingExt = ExtensionTestUtils.loadExtension({ - manifest: { - permissions: [ - "proxy", - "webRequest", - "webRequestBlocking", - "", - ], - }, - background: `(${background})(${proxy.identity.primaryPort})`, - files: { - "proxy.js": ` - function FindProxyForURL(url, host) { - return "PROXY localhost:${proxy.identity.primaryPort}; DIRECT"; - }`, - }, - }); + let handlingExt = getExtension(background); await handlingExt.startup(); await handlingExt.awaitMessage("pac-ready"); @@ -75,3 +82,47 @@ add_task(async function test_webRequest_auth_proxy() { await contentPage.close(); await handlingExt.unload(); }); + +add_task(async function test_webRequest_auth_proxy_system() { + async function background(port) { + browser.webRequest.onBeforeRequest.addListener(details => { + browser.test.fail("onBeforeRequest"); + }, {urls: [""]}); + browser.webRequest.onAuthRequired.addListener(details => { + browser.test.sendMessage("onAuthRequired"); + // cancel is silently ignored, if it were not (e.g someone messes up in + // WebRequest.jsm and allows cancel) this test would fail. + return { + cancel: true, + authCredentials: {username: "puser", password: "ppass"}, + }; + }, {urls: [""]}, ["blocking"]); + + await browser.proxy.register("proxy.js"); + browser.test.sendMessage("pac-ready"); + } + + let handlingExt = getExtension(background); + + await handlingExt.startup(); + await handlingExt.awaitMessage("pac-ready"); + + function fetch(url) { + return new Promise((resolve, reject) => { + let xhr = new XMLHttpRequest(); + xhr.mozBackgroundRequest = true; + xhr.open("GET", url); + xhr.onload = () => { resolve(xhr.responseText); }; + xhr.onerror = () => { reject(xhr.status); }; + // use a different contextId to avoid auth cache. + xhr.setOriginAttributes({userContextId: 1}); + xhr.send(); + }); + } + + await Promise.all([ + handlingExt.awaitMessage("onAuthRequired"), + fetch("http://mozilla.org"), + ]); + await handlingExt.unload(); +}); diff --git a/toolkit/modules/addons/WebRequest.jsm b/toolkit/modules/addons/WebRequest.jsm index 23a309196d27..ef88313406ae 100644 --- a/toolkit/modules/addons/WebRequest.jsm +++ b/toolkit/modules/addons/WebRequest.jsm @@ -833,7 +833,9 @@ HttpObserverManager = { try { let result = callback(data); - if (channel.canModify && result && typeof result === "object" && opts.blocking) { + // isProxy is set during onAuth if the auth request is for a proxy. + // We allow handling proxy auth regardless of canModify. + if ((channel.canModify || data.isProxy) && typeof result === "object" && opts.blocking) { handlerResults.push({opts, result}); } } catch (e) { @@ -865,6 +867,16 @@ HttpObserverManager = { } } + if (kind === "authRequired" && result.authCredentials && channel.authPromptCallback) { + channel.authPromptCallback(result.authCredentials); + } + + // We allow proxy auth to cancel or handle authCredentials regardless of + // canModify, but ensure we do nothing else. + if (!channel.canModify) { + continue; + } + if (result.cancel) { channel.suspended = false; channel.cancel(Cr.NS_ERROR_ABORT); @@ -890,11 +902,8 @@ HttpObserverManager = { if (opts.responseHeaders && result.responseHeaders && responseHeaders) { responseHeaders.applyChanges(result.responseHeaders); } - - if (kind === "authRequired" && result.authCredentials && channel.authPromptCallback) { - channel.authPromptCallback(result.authCredentials); - } } + // If a listener did not cancel the request or provide credentials, we // forward the auth request to the base handler. if (kind === "authRequired" && channel.authPromptForward) {