Bug 1507110 - Allow hijacking localhost only if network.proxy.allow_hijacking_localhost is set r=Gijs,mayhemer,mkaply,jmaher

Differential Revision: https://phabricator.services.mozilla.com/D19325 --HG-- extra : moz-landing-system : lando
2025-02-27 04:38:02 +00:00 · 2019-02-19 21:53:22 +00:00 · 2019-02-19 21:53:22 +00:00 · b7e28eddd2
commit b7e28eddd2
parent a2f9d0f6af
20 changed files with 78 additions and 32 deletions
--- a/browser/base/content/test/siteIdentity/browser_insecureLoginForms.js
+++ b/browser/base/content/test/siteIdentity/browser_insecureLoginForms.js
@ -21,7 +21,7 @@ add_task(async function test_simple() {
    "set": [
      ["security.insecure_password.ui.enabled", true],
      // By default, proxies don't apply to 127.0.0.1. We need them to for this test, though:
-      ["network.proxy.no_proxies_on", ""],
+      ["network.proxy.allow_hijacking_localhost", true],
    ],
  });

--- a/browser/components/BrowserGlue.jsm
+++ b/browser/components/BrowserGlue.jsm
@ -2239,7 +2239,7 @@ BrowserGlue.prototype = {
  _migrateUI: function BG__migrateUI() {
    // Use an increasing number to keep track of the current migration state.
    // Completely unrelated to the current Firefox release number.
-    const UI_VERSION = 79;
+    const UI_VERSION = 80;
    const BROWSER_DOCURL = AppConstants.BROWSER_CHROME_URL;

    let currentUIVersion;
@ -2614,6 +2614,14 @@ BrowserGlue.prototype = {
      Services.prefs.setCharPref("browser.handlers.migrations", "30boxes");
    }

+    if (currentUIVersion < 80) {
+      let hosts = Services.prefs.getCharPref("network.proxy.no_proxies_on");
+      // remove "localhost" and "127.0.0.1" from the no_proxies_on list
+      const kLocalHosts = new Set(["localhost", "127.0.0.1"]);
+      hosts = hosts.split(/[ ,]+/).filter(host => !kLocalHosts.has(host)).join(", ");
+      Services.prefs.setCharPref("network.proxy.no_proxies_on", hosts);
+    }
+
    // Update the migration version.
    Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
  },
--- a/browser/components/originattributes/test/browser/browser_firstPartyIsolation.js
+++ b/browser/components/originattributes/test/browser/browser_firstPartyIsolation.js
@ -287,9 +287,9 @@ add_task(async function ip_address_test() {
  const ipAddr = "127.0.0.1";
  const ipHost = `http://${ipAddr}/browser/browser/components/originattributes/test/browser/`;

-  Services.prefs.setCharPref("network.proxy.no_proxies_on", "");
+  Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true);
  registerCleanupFunction(function() {
-    Services.prefs.clearUserPref("network.proxy.no_proxies_on");
+    Services.prefs.clearUserPref("network.proxy.allow_hijacking_localhost");
  });

  let tab = BrowserTestUtils.addTab(gBrowser, ipHost + "test_firstParty.html");
--- a/browser/modules/test/browser/browser_UsageTelemetry_uniqueOriginsVisitedInPast24Hours.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_uniqueOriginsVisitedInPast24Hours.js
@ -10,7 +10,7 @@ ChromeUtils.defineModuleGetter(this, "URICountListener",

 add_task(async function test_uniqueDomainsVisitedInPast24Hours() {
  // By default, proxies don't apply to 127.0.0.1. We need them to for this test, though:
-  await SpecialPowers.pushPrefEnv({set: [["network.proxy.no_proxies_on", ""]]});
+  await SpecialPowers.pushPrefEnv({set: [["network.proxy.allow_hijacking_localhost", true]]});
  registerCleanupFunction(async () => {
    info("Cleaning up");
    URICountListener.resetUniqueDomainsVisitedInPast24Hours();
--- a/dom/security/test/csp/test_upgrade_insecure_loopback.html
+++ b/dom/security/test/csp/test_upgrade_insecure_loopback.html
@ -57,7 +57,7 @@ SimpleTest.waitForExplicitFinish();
 // By default, proxies don't apply to 127.0.0.1.
 // We need them to for this test (at least on android), though:
 SpecialPowers.pushPrefEnv({set: [
-  ["network.proxy.no_proxies_on", ""]
+  ["network.proxy.allow_hijacking_localhost", true]
 ]}).then(loadTest);

 </script>
--- a/dom/tests/mochitest/dom-level0/test_setting_document.domain_to_shortened_ipaddr.html
+++ b/dom/tests/mochitest/dom-level0/test_setting_document.domain_to_shortened_ipaddr.html
@ -32,7 +32,7 @@ window.addEventListener("message", receiveMessage, false);
 // By default, proxies don't apply to 127.0.0.1.
 // We need them to for this test (at least on android), though:
 SpecialPowers.pushPrefEnv({set: [
-  ["network.proxy.no_proxies_on", ""]
+  ["network.proxy.allow_hijacking_localhost", true]
 ]}).then(function() {
  var iframe = document.getElementById("childframe");
  iframe.src = "http://127.0.0.1:8888/tests/dom/tests/mochitest/dom-level0/child_ip_address.html";
--- a/dom/u2f/tests/browser/browser_appid_localhost.js
+++ b/dom/u2f/tests/browser/browser_appid_localhost.js
@ -20,7 +20,7 @@ function promiseU2FRegister(tab, app_id) {

 add_task(async function () {
  // By default, proxies don't apply to localhost. We need them to for this test, though:
-  await SpecialPowers.pushPrefEnv({set: [["network.proxy.no_proxies_on", ""]]});
+  await SpecialPowers.pushPrefEnv({set: [["network.proxy.allow_hijacking_localhost", true],]});
  // Enable the soft token.
  Services.prefs.setBoolPref("security.webauth.u2f", true);
  Services.prefs.setBoolPref("security.webauth.webauthn_enable_softtoken", true);
--- a/extensions/cookie/test/test_same_base_domain_6.html
+++ b/extensions/cookie/test/test_same_base_domain_6.html
@ -13,7 +13,7 @@
      // By default, proxies don't apply to 127.0.0.1.
      // We need them to for this test (at least on android), though:
      SpecialPowers.pushPrefEnv({set: [
-        ["network.proxy.no_proxies_on", ""]
+        ["network.proxy.allow_hijacking_localhost", true]
      ]}).then(function() {
        setupTest('http://127.0.0.1:8888/tests/extensions/cookie/test/file_loopback_inner.html', 5, 2);
      });
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@ -2370,7 +2370,9 @@ pref("network.proxy.socks_port",            0);
 pref("network.proxy.socks_version",         5);
 pref("network.proxy.socks_remote_dns",      false);
 pref("network.proxy.proxy_over_tls",        true);
-pref("network.proxy.no_proxies_on",         "localhost, 127.0.0.1");
+pref("network.proxy.no_proxies_on",         "");
+// Set true to allow resolving proxy for localhost
+pref("network.proxy.allow_hijacking_localhost", false);
 pref("network.proxy.failover_timeout",      1800); // 30 minutes
 pref("network.online",                      true); //online/offline
 pref("network.cookie.thirdparty.sessionOnly", false);
--- a/netwerk/base/nsProtocolProxyService.cpp
+++ b/netwerk/base/nsProtocolProxyService.cpp
@ -762,6 +762,7 @@ nsProtocolProxyService::nsProtocolProxyService()
      mSOCKSProxyRemoteDNS(false),
      mProxyOverTLS(true),
      mWPADOverDHCPEnabled(false),
+      mAllowHijackingLocalhost(false),
      mPACMan(nullptr),
      mSessionStart(PR_Now()),
      mFailedProxyTimeout(30 * 60)  // 30 minute default
@ -1009,6 +1010,11 @@ void nsProtocolProxyService::PrefsChanged(nsIPrefBranch *prefBranch,
    reloadPAC = reloadPAC || mProxyConfig == PROXYCONFIG_WPAD;
  }

+  if (!pref || !strcmp(pref, PROXY_PREF("allow_hijacking_localhost"))) {
+    proxy_GetBoolPref(prefBranch, PROXY_PREF("allow_hijacking_localhost"),
+                      mAllowHijackingLocalhost);
+  }
+
  if (!pref || !strcmp(pref, PROXY_PREF("failover_timeout")))
    proxy_GetIntPref(prefBranch, PROXY_PREF("failover_timeout"),
                     mFailedProxyTimeout);
@ -1052,8 +1058,6 @@ void nsProtocolProxyService::PrefsChanged(nsIPrefBranch *prefBranch,
 }

 bool nsProtocolProxyService::CanUseProxy(nsIURI *aURI, int32_t defaultPort) {
-  if (mHostFiltersArray.Length() == 0 && !mFilterLocalHosts) return true;
-
  int32_t port;
  nsAutoCString host;

@ -1084,7 +1088,10 @@ bool nsProtocolProxyService::CanUseProxy(nsIURI *aURI, int32_t defaultPort) {

  // Don't use proxy for local hosts (plain hostname, no dots)
  if ((!is_ipaddr && mFilterLocalHosts && !host.Contains('.')) ||
-      host.EqualsLiteral("127.0.0.1") || host.EqualsLiteral("::1")) {
+      (!mAllowHijackingLocalhost &&
+           (host.EqualsLiteral("127.0.0.1") ||
+            host.EqualsLiteral("::1") ||
+            host.EqualsLiteral("localhost")))) {
    LOG(("Not using proxy for this local host [%s]!\n", host.get()));
    return false;  // don't allow proxying
  }
@ -1765,6 +1772,10 @@ void nsProtocolProxyService::LoadHostFilters(const nsACString &aFilters) {
    mHostFiltersArray.Clear();
  }

+  // Reset mFilterLocalHosts - will be set to true if "<local>" is in pref
+  // string
+  mFilterLocalHosts = false;
+
  if (aFilters.IsEmpty()) {
    return;
  }
@ -1773,10 +1784,6 @@ void nsProtocolProxyService::LoadHostFilters(const nsACString &aFilters) {
  // filter  = ( host | domain | ipaddr ["/" mask] ) [":" port]
  // filters = filter *( "," LWS filter)
  //
-  // Reset mFilterLocalHosts - will be set to true if "<local>" is in pref
-  // string
-  mFilterLocalHosts = false;
-
  mozilla::Tokenizer t(aFilters);
  mozilla::Tokenizer::Token token;
  bool eof = false;
--- a/netwerk/base/nsProtocolProxyService.h
+++ b/netwerk/base/nsProtocolProxyService.h
@ -386,6 +386,7 @@ class nsProtocolProxyService final : public nsIProtocolProxyService2,
  bool mSOCKSProxyRemoteDNS;
  bool mProxyOverTLS;
  bool mWPADOverDHCPEnabled;
+  bool mAllowHijackingLocalhost;

  RefPtr<nsPACMan> mPACMan;  // non-null if we are using PAC
  nsCOMPtr<nsISystemProxySettings> mSystemProxySettings;
--- a/netwerk/test/gtest/TestProtocolProxyService.cpp
+++ b/netwerk/test/gtest/TestProtocolProxyService.cpp
@ -30,6 +30,9 @@ TEST(TestProtocolProxyService, LoadHostFilters) {
    spec = "http://[::1]";
    ASSERT_EQ(NS_NewURI(getter_AddRefs(url), spec), NS_OK);
    ASSERT_EQ(pps->CanUseProxy(url, 80), expected);
+    spec = "http://localhost";
+    ASSERT_EQ(NS_NewURI(getter_AddRefs(url), spec), NS_OK);
+    ASSERT_EQ(pps->CanUseProxy(url, 80), expected);
  };

  auto CheckURLs = [&](bool expected) {
@ -86,7 +89,7 @@ TEST(TestProtocolProxyService, LoadHostFilters) {
  printf("Testing empty filter: %s\n", filter.get());
  pps->LoadHostFilters(filter);

-  CheckLoopbackURLs(true);  // only time when loopbacks can be proxied. bug?
+  CheckLoopbackURLs(false);
  CheckLocalDomain(true);
  CheckURLs(true);
  CheckPortDomain(true);
@ -98,9 +101,10 @@ TEST(TestProtocolProxyService, LoadHostFilters) {
      "[abcd::1]/64:123, *.test.com";
  printf("Testing filter: %s\n", filter.get());
  pps->LoadHostFilters(filter);
+
+  CheckLoopbackURLs(false);
  // Check URLs can no longer use filtered proxy
  CheckURLs(false);
-  CheckLoopbackURLs(false);
  CheckLocalDomain(true);
  CheckPortDomain(true);

@ -111,8 +115,9 @@ TEST(TestProtocolProxyService, LoadHostFilters) {
  filter = "<local> blabla.com:10";
  printf("Testing filter: %s\n", filter.get());
  pps->LoadHostFilters(filter);
-  CheckURLs(true);
+
  CheckLoopbackURLs(false);
+  CheckURLs(true);
  CheckLocalDomain(false);
  CheckPortDomain(false);

@ -125,10 +130,33 @@ TEST(TestProtocolProxyService, LoadHostFilters) {
  filter = "<local>";
  printf("Testing filter: %s\n", filter.get());
  pps->LoadHostFilters(filter);
-  CheckURLs(true);
+
  CheckLoopbackURLs(false);
+  CheckURLs(true);
  CheckLocalDomain(false);
  CheckPortDomain(true);
+
+  // Check that allow_hijacking_localhost works with empty filter
+  Preferences::SetBool("network.proxy.allow_hijacking_localhost", true);
+
+  filter = "";
+  printf("Testing filter: %s\n", filter.get());
+  pps->LoadHostFilters(filter);
+
+  CheckLoopbackURLs(true);
+  CheckLocalDomain(true);
+  CheckURLs(true);
+  CheckPortDomain(true);
+
+  // Check that allow_hijacking_localhost works with non-trivial filter
+  filter = "127.0.0.1, [::1], localhost, blabla.com:10";
+  printf("Testing filter: %s\n", filter.get());
+  pps->LoadHostFilters(filter);
+
+  CheckLoopbackURLs(false);
+  CheckLocalDomain(true);
+  CheckURLs(true);
+  CheckPortDomain(false);
 }

 }  // namespace net
--- a/netwerk/test/unit/test_bug770243.js
+++ b/netwerk/test/unit/test_bug770243.js
@ -199,7 +199,7 @@ function run_test()
                         .getService(Ci.nsIPrefBranch);
  prefs.setCharPref("network.proxy.http", "localhost");
  prefs.setIntPref("network.proxy.http_port", httpserv.identity.primaryPort);
-  prefs.setCharPref("network.proxy.no_proxies_on", "");
+  prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true);
  prefs.setIntPref("network.proxy.type", 1);
  prefs.setBoolPref("network.http.rcwn.enabled", false);

--- a/netwerk/test/unit/test_proxy-failover_canceled.js
+++ b/netwerk/test/unit/test_proxy-failover_canceled.js
@ -41,6 +41,7 @@ function run_test()
  var prefs = prefserv.getBranch("network.proxy.");
    prefs.setIntPref("type", 2);
    prefs.setCharPref("no_proxies_on", "nothing");
+    prefs.setBoolPref("allow_hijacking_localhost", true);
  prefs.setCharPref("autoconfig_url", "data:text/plain," +
    "function FindProxyForURL(url, host) {return 'PROXY a_non_existent_domain_x7x6c572v:80; PROXY localhost:" +
    httpServer.identity.primaryPort + "';}"
--- a/netwerk/test/unit/test_proxyconnect.js
+++ b/netwerk/test/unit/test_proxyconnect.js
@ -282,7 +282,7 @@ function createProxy() {
 function test_connectonly() {
  Services.prefs.setCharPref("network.proxy.ssl", "localhost");
  Services.prefs.setIntPref("network.proxy.ssl_port", socketserver_port);
-  Services.prefs.setCharPref("network.proxy.no_proxies_on", "");
+  Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true);
  Services.prefs.setIntPref("network.proxy.type", 1);

  var chan = makeChan();
@ -304,7 +304,7 @@ function test_connectonly_nonhttp() {

  Services.prefs.setCharPref("network.proxy.socks", "localhost")
  Services.prefs.setIntPref("network.proxy.socks_port", socketserver_port)
-  Services.prefs.setCharPref("network.proxy.no_proxies_on", "")
+  Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true);
  Services.prefs.setIntPref("network.proxy.type", 1)

  var chan = makeChan()
@ -336,7 +336,7 @@ function clearPrefs() {
  Services.prefs.clearUserPref("network.proxy.ssl_port");
  Services.prefs.clearUserPref("network.proxy.socks");
  Services.prefs.clearUserPref("network.proxy.socks_port");
-  Services.prefs.clearUserPref("network.proxy.no_proxies_on");
+  Services.prefs.clearUserPref("network.proxy.allow_hijacking_localhost");
  Services.prefs.clearUserPref("network.proxy.type");
 }

--- a/testing/raptor/raptor/playback/mitmproxy.py
+++ b/testing/raptor/raptor/playback/mitmproxy.py
@ -59,7 +59,7 @@ POLICIES_CONTENT_ON = '''{
      "Mode": "manual",
      "HTTPProxy": "%(host)s:8080",
      "SSLProxy": "%(host)s:8080",
-      "Passthrough": "localhost, 127.0.0.1, %(host)s",
+      "Passthrough": "%(host)s",
      "Locked": true
    }
  }
--- a/testing/raptor/raptor/raptor.py
+++ b/testing/raptor/raptor/raptor.py
@ -531,14 +531,13 @@ class RaptorAndroid(Raptor):
        # proxy; we need to set prefs instead; note that the 'host' may be different
        # than '127.0.0.1' so we must set the prefs accordingly
        self.log.info("setting profile prefs to turn on the android app proxy")
-        no_proxies_on = "localhost, 127.0.0.1, %s" % self.config['host']
        proxy_prefs = {}
        proxy_prefs["network.proxy.type"] = 1
        proxy_prefs["network.proxy.http"] = self.config['host']
        proxy_prefs["network.proxy.http_port"] = 8080
        proxy_prefs["network.proxy.ssl"] = self.config['host']
        proxy_prefs["network.proxy.ssl_port"] = 8080
-        proxy_prefs["network.proxy.no_proxies_on"] = no_proxies_on
+        proxy_prefs["network.proxy.no_proxies_on"] = self.config['host']
        self.profile.set_preferences(proxy_prefs)

    def launch_firefox_android_app(self):
--- a/testing/talos/talos/mitmproxy/mitmproxy.py
+++ b/testing/talos/talos/mitmproxy/mitmproxy.py
@ -35,7 +35,7 @@ POLICIES_CONTENT_ON = '''{
      "Mode": "manual",
      "HTTPProxy": "127.0.0.1:8080",
      "SSLProxy": "127.0.0.1:8080",
-      "Passthrough": "localhost, 127.0.0.1",
+      "Passthrough": "",
      "Locked": true
    }
  }
--- a/toolkit/components/extensions/test/xpcshell/test_ext_proxy_config.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_proxy_config.js
@ -32,7 +32,7 @@ add_task(async function test_browser_settings() {
    "network.proxy.socks_port": 0,
    "network.proxy.socks_version": 5,
    "network.proxy.socks_remote_dns": false,
-    "network.proxy.no_proxies_on": "localhost, 127.0.0.1",
+    "network.proxy.no_proxies_on": "",
    "network.proxy.autoconfig_url": "",
    "signon.autologin.proxy": false,
  };
@ -99,7 +99,7 @@ add_task(async function test_browser_settings() {
      proxyDNS: false,
      httpProxyAll: false,
      socksVersion: 5,
-      passthrough: "localhost, 127.0.0.1",
+      passthrough: "",
      http: "",
      ftp: "",
      ssl: "",
--- a/toolkit/components/passwordmgr/test/browser/browser_insecurePasswordConsoleWarning.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_insecurePasswordConsoleWarning.js
@ -10,7 +10,7 @@ const WARNING_PATTERN = [{

 add_task(async function testInsecurePasswordWarning() {
  // By default, proxies don't apply to 127.0.0.1. We need them to for this test, though:
-  await SpecialPowers.pushPrefEnv({set: [["network.proxy.no_proxies_on", ""]]});
+  await SpecialPowers.pushPrefEnv({set: [["network.proxy.allow_hijacking_localhost", true]]});
  let warningPatternHandler;

  function messageHandler(msgObj) {