Skip to content

GitLab

T
tangled
Commit 15242c0c authored by Eric Roman's avatar Eric Roman Committed by Commit Bot
Browse files
Options

Fix the interpretation of the <local> proxy bypass rule. 

The rule should just match simple hostnames, and previously was confused (both in implementation and consumers) into acting like a bypass for localhost.

The compatibility risk of changing its meaning (given this may be serialized in preferences or used by extensions) is low, since a previous change added implicit bypass on localhost. Hence the overall impact of fixing <local> now is just that IPv6 IP addresses are no longer bypassed. That IPv6 literals were being bypassed by the check was unexpected and a bug.

Bug: 902579, 902418
Change-Id: I5937048e3f927c66668e39e4ceb589ef8516e874
Reviewed-on: https://chromium-review.googlesource.com/c/1320849
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607435}
parent dddcbdbd
Hide whitespace changes
Inline Side-by-side
Showing with 149 additions and 159 deletions
chrome/common/extensions/docs/templates/intros/proxy.html
View file @ 15242c0c
...@@ -21,7 +21,7 @@ Proxy settings are defined in a ...@@ -21,7 +21,7 @@ Proxy settings are defined in a
$(ref:proxy.ProxyConfig) object. Depending on $(ref:proxy.ProxyConfig) object. Depending on
Chrome's proxy settings, the settings may contain Chrome's proxy settings, the settings may contain
$(ref:proxy.ProxyRules) or a $(ref:proxy.ProxyRules) or a
$(ref:proxy.PacScript). $(ref:proxy.PacScript).
</p> </p>
<h3 id="proxy_modes">Proxy modes</h3> <h3 id="proxy_modes">Proxy modes</h3>
...@@ -31,41 +31,41 @@ A ProxyConfig object's <code>mode</code> attribute determines the overall ...@@ -31,41 +31,41 @@ A ProxyConfig object's <code>mode</code> attribute determines the overall
behavior of Chrome with regards to proxy usage. It can take the following behavior of Chrome with regards to proxy usage. It can take the following
values: values:
<dl> <dl>
<dt><code>direct</code></dt> <dt><code>direct</code></dt>
<dd>In <code>direct</code> mode all connections are created directly, without <dd>In <code>direct</code> mode all connections are created directly, without
any proxy involved. This mode allows no further parameters in the any proxy involved. This mode allows no further parameters in the
<code>ProxyConfig</code> object.</dd> <code>ProxyConfig</code> object.</dd>
<dt><code>auto_detect</code></dt> <dt><code>auto_detect</code></dt>
<dd>In <code>auto_detect</code> mode the proxy configuration is determined by <dd>In <code>auto_detect</code> mode the proxy configuration is determined by
a PAC script that can be downloaded at a PAC script that can be downloaded at
<a href="http://wpad/wpad.dat">http://wpad/wpad.dat</a>. <a href="http://wpad/wpad.dat">http://wpad/wpad.dat</a>.
This mode allows no further parameters in the <code>ProxyConfig</code> This mode allows no further parameters in the <code>ProxyConfig</code>
object.</dd> object.</dd>
<dt><code>pac_script</code></dt> <dt><code>pac_script</code></dt>
<dd>In <code>pac_script</code> mode the proxy configuration is determined by <dd>In <code>pac_script</code> mode the proxy configuration is determined by
a PAC script that is either retrieved from the URL specified in the a PAC script that is either retrieved from the URL specified in the
$(ref:proxy.PacScript) object or $(ref:proxy.PacScript) object or
taken literally from the <code>data</code> element specified in the taken literally from the <code>data</code> element specified in the
$(ref:proxy.PacScript) object. $(ref:proxy.PacScript) object.
Besides this, this mode allows no further parameters in the Besides this, this mode allows no further parameters in the
<code>ProxyConfig</code> object.</dd> <code>ProxyConfig</code> object.</dd>
<dt><code>fixed_servers</code></dt> <dt><code>fixed_servers</code></dt>
<dd>In <code>fixed_servers</code> mode the proxy configuration is codified in <dd>In <code>fixed_servers</code> mode the proxy configuration is codified in
a $(ref:proxy.ProxyRules) a $(ref:proxy.ProxyRules)
object. Its structure is described in <a href="#proxy_rules">Proxy rules</a>. object. Its structure is described in <a href="#proxy_rules">Proxy rules</a>.
Besides this, the <code>fixed_servers</code> mode allows no further parameters Besides this, the <code>fixed_servers</code> mode allows no further parameters
in the <code>ProxyConfig</code> object.</dd> in the <code>ProxyConfig</code> object.</dd>
<dt><code>system</code></dt> <dt><code>system</code></dt>
<dd>In <code>system</code> mode the proxy configuration is taken from the <dd>In <code>system</code> mode the proxy configuration is taken from the
operating system. This mode allows no further parameters in the operating system. This mode allows no further parameters in the
<code>ProxyConfig</code> object. Note that the <code>system</code> mode is <code>ProxyConfig</code> object. Note that the <code>system</code> mode is
different from setting no proxy configuration. In the latter case, Chrome different from setting no proxy configuration. In the latter case, Chrome
falls back to the system settings only if no command-line options influence falls back to the system settings only if no command-line options influence
the proxy configuration.</dd> the proxy configuration.</dd>
</dl> </dl>
</p> </p>
...@@ -104,11 +104,11 @@ If no <code>port</code> is defined in a ...@@ -104,11 +104,11 @@ If no <code>port</code> is defined in a
$(ref:proxy.ProxyServer) object, the port is $(ref:proxy.ProxyServer) object, the port is
derived from the scheme. The default ports are: derived from the scheme. The default ports are:
<table> <table>
<tr><th>Scheme</th><th>Port</th></tr> <tr><th>Scheme</th><th>Port</th></tr>
<tr><td>http</td><td>80</td></tr> <tr><td>http</td><td>80</td></tr>
<tr><td>https</td><td>443</td></tr> <tr><td>https</td><td>443</td></tr>
<tr><td>socks4</td><td>1080</td></tr> <tr><td>socks4</td><td>1080</td></tr>
<tr><td>socks5</td><td>1080</td></tr> <tr><td>socks5</td><td>1080</td></tr>
</table> </table>
</p> </p>
...@@ -118,52 +118,55 @@ derived from the scheme. The default ports are: ...@@ -118,52 +118,55 @@ derived from the scheme. The default ports are:
Individual servers may be excluded from being proxied with the Individual servers may be excluded from being proxied with the
<code>bypassList</code>. This list may contain the following entries: <code>bypassList</code>. This list may contain the following entries:
<dl> <dl>
<dt><code>[<em>&lt;scheme&gt;</em>://]<em>&lt;host-pattern&gt;</em>[:<em>&lt;port&gt;</em>]</code></dt> <dt><code>[<em>&lt;scheme&gt;</em>://]<em>&lt;host-pattern&gt;</em>[:<em>&lt;port&gt;</em>]</code></dt>
<dd>Match all hostnames that match the pattern <em>&lt;host-pattern&gt;</em>. <dd>Match all hostnames that match the pattern <em>&lt;host-pattern&gt;</em>.
A leading <code>"."</code> is interpreted as a <code>"*."</code>.<br> A leading <code>"."</code> is interpreted as a <code>"*."</code>.<br>
Examples: <code>"foobar.com", "*foobar.com", "*.foobar.com", "*foobar.com:99", Examples: <code>"foobar.com", "*foobar.com", "*.foobar.com", "*foobar.com:99",
"https://x.*.y.com:99"</code>.<br> "https://x.*.y.com:99"</code>.<br>
<table> <table>
<tr> <tr>
<th>Pattern <th>Pattern
<th>Matches <th>Matches
<th>Does not match <th>Does not match
<tr> <tr>
<td><code>".foobar.com"</code> <td><code>".foobar.com"</code>
<td><code>"www.foobar.com"</code> <td><code>"www.foobar.com"</code>
<td><code>"foobar.com"</code> <td><code>"foobar.com"</code>
<tr> <tr>
<td><code>"*.foobar.com"</code> <td><code>"*.foobar.com"</code>
<td><code>"www.foobar.com"</code> <td><code>"www.foobar.com"</code>
<td><code>"foobar.com"</code> <td><code>"foobar.com"</code>
<tr> <tr>
<td><code>"foobar.com"</code> <td><code>"foobar.com"</code>
<td><code>"foobar.com"</code> <td><code>"foobar.com"</code>
<td><code>"www.foobar.com"</code> <td><code>"www.foobar.com"</code>
<tr> <tr>
<td><code>"*foobar.com"</code> <td><code>"*foobar.com"</code>
<td><code>"foobar.com"</code>, <code>"www.foobar.com"</code>, <td><code>"foobar.com"</code>, <code>"www.foobar.com"</code>,
<code>"foofoobar.com"</code> <code>"foofoobar.com"</code>
<td> <td>
</table> </table>
</dd> </dd>
<dt><code>[<em>&lt;scheme&gt;</em>://]<em>&lt;ip-literal&gt;</em>[:<em>&lt;port&gt;</em>]</code></dt> <dt><code>[<em>&lt;scheme&gt;</em>://]<em>&lt;ip-literal&gt;</em>[:<em>&lt;port&gt;</em>]</code></dt>
<dd>Match URLs that are IP address literals.<br> <dd>Match URLs that are IP address literals.<br>
Conceptually this is the similar to the first case, but with special cases Conceptually this is the similar to the first case, but with special cases
to handle IP literal canonicalization. For example, matching to handle IP literal canonicalization. For example, matching
on "[0:0:0::1]" is the same as matching on "[::1]" because on "[0:0:0::1]" is the same as matching on "[::1]" because
the IPv6 canonicalization is done internally.<br> the IPv6 canonicalization is done internally.<br>
Examples: <code>"127.0.1", "[0:0::1]", "[::1]", "http://[::1]:99"</code></dd> Examples: <code>"127.0.1", "[0:0::1]", "[::1]", "http://[::1]:99"</code></dd>
<dt><code><em>&lt;ip-literal&gt;</em>/<em>&lt;prefix-length-in-bits&gt;</em></code></dt> <dt><code><em>&lt;ip-literal&gt;</em>/<em>&lt;prefix-length-in-bits&gt;</em></code></dt>
<dd>Match any URL containing an IP literal within the given range. The IP <dd>Match any URL containing an IP literal within the given range. The IP
range is specified using CIDR notation.<br> range is specified using CIDR notation.<br>
Examples: <code>"192.168.1.1/16", "fefe:13::abc/33"</code></dd> Examples: <code>"192.168.1.1/16", "fefe:13::abc/33"</code></dd>
<dt><code>&lt;local&gt;</code></dt> <dt><code>&lt;local&gt;</code></dt>
<dd>Match local addresses. An address is local if the host is "127.0.0.1", <dd>Matches simple hostnames. A simple hostname is one that contains no dots
"::1", or "localhost".<br> and is not an IP literal. For instance <code>example</code> and <code>localhost</code> are
simple hostnames, whereas <code>example.com</code>, <code>example.</code>, and
<code>[::1]</code> are not.
<br>
Example: <code>"&lt;local&gt;"</code></dd> Example: <code>"&lt;local&gt;"</code></dd>
</dl> </dl>
......
chromeos/network/proxy/proxy_config_service_impl.cc
View file @ 15242c0c
...@@ -262,7 +262,9 @@ void ProxyConfigServiceImpl::DetermineEffectiveConfigFromDefaultNetwork() { ...@@ -262,7 +262,9 @@ void ProxyConfigServiceImpl::DetermineEffectiveConfigFromDefaultNetwork() {
if (effective_config.value().proxy_rules().type != if (effective_config.value().proxy_rules().type !=
net::ProxyConfig::ProxyRules::Type::EMPTY) { net::ProxyConfig::ProxyRules::Type::EMPTY) {
net::ProxyConfig proxy_config = effective_config.value(); net::ProxyConfig proxy_config = effective_config.value();
proxy_config.proxy_rules().bypass_rules.PrependRuleToBypassLocal(); // TODO(https://crbug.com/902418): Is this rule still needed?
proxy_config.proxy_rules()
.bypass_rules.PrependRuleToBypassSimpleHostnames();
effective_config = net::ProxyConfigWithAnnotation( effective_config = net::ProxyConfigWithAnnotation(
proxy_config, effective_config.traffic_annotation()); proxy_config, effective_config.traffic_annotation());
} }
......
components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
View file @ 15242c0c
...@@ -662,7 +662,7 @@ void DataReductionProxyConfig::AddDefaultProxyBypassRules() { ...@@ -662,7 +662,7 @@ void DataReductionProxyConfig::AddDefaultProxyBypassRules() {
// link-local addresses, so it is not necessary to explicitly add them here. // link-local addresses, so it is not necessary to explicitly add them here.
// See ProxyBypassRules::MatchesImplicitRules() for details. // See ProxyBypassRules::MatchesImplicitRules() for details.
configurator_->SetBypassRules( configurator_->SetBypassRules(
// localhost // Hostnames with no dot in them.
"<local>," "<local>,"
// RFC6890 current network (only valid as source address). // RFC6890 current network (only valid as source address).
......
net/proxy_resolution/proxy_bypass_rules.cc
View file @ 15242c0c
...@@ -29,8 +29,12 @@ namespace { ...@@ -29,8 +29,12 @@ namespace {
// ProxyResolverRules::MatchesImplicitRules(). // ProxyResolverRules::MatchesImplicitRules().
const char kSubtractImplicitBypasses[] = "<-loopback>"; const char kSubtractImplicitBypasses[] = "<-loopback>";
// TODO(eroman): Fix - this should be renamed to kBypassSimpleHostnames. // The <local> rule bypasses any hostname that has no dots (and is not
const char kWinLocal[] = "<local>"; // an IP literal). The name is misleading as it has nothing to do with
// localhost/loopback addresses, and would have better been called
// something like "simple hostnames". However this is the name used on
// Windows so is matched here.
const char kBypassSimpleHostnames[] = "<local>";
bool IsIPv4LinkLocal(const IPAddress& addr) { bool IsIPv4LinkLocal(const IPAddress& addr) {
// 169.254.0.0/16 // 169.254.0.0/16
...@@ -100,23 +104,21 @@ class HostnamePatternRule : public ProxyBypassRules::Rule { ...@@ -100,23 +104,21 @@ class HostnamePatternRule : public ProxyBypassRules::Rule {
DISALLOW_COPY_AND_ASSIGN(HostnamePatternRule); DISALLOW_COPY_AND_ASSIGN(HostnamePatternRule);
}; };
// TODO(https://crbug.com/902579): Fix. class BypassSimpleHostnamesRule : public ProxyBypassRules::Rule {
class WinLocalRule : public ProxyBypassRules::Rule {
public: public:
WinLocalRule() = default; BypassSimpleHostnamesRule() = default;
Result Evaluate(const GURL& url) const override { Result Evaluate(const GURL& url) const override {
const std::string& host = url.host(); return ((url.host_piece().find('.') == std::string::npos) &&
if (host == "127.0.0.1" || host == "[::1]") !url.HostIsIPAddress())
return Result::kBypass; ? Result::kBypass
return (host.find('.') == std::string::npos) ? Result::kBypass : Result::kNoMatch;
: Result::kNoMatch;
} }
std::string ToString() const override { return kWinLocal; } std::string ToString() const override { return kBypassSimpleHostnames; }
private: private:
DISALLOW_COPY_AND_ASSIGN(WinLocalRule); DISALLOW_COPY_AND_ASSIGN(BypassSimpleHostnamesRule);
}; };
class SubtractImplicitBypassesRule : public ProxyBypassRules::Rule { class SubtractImplicitBypassesRule : public ProxyBypassRules::Rule {
...@@ -199,8 +201,8 @@ std::unique_ptr<ProxyBypassRules::Rule> ParseRule( ...@@ -199,8 +201,8 @@ std::unique_ptr<ProxyBypassRules::Rule> ParseRule(
// <local> and <-loopback> are special syntax used by WinInet's bypass list // <local> and <-loopback> are special syntax used by WinInet's bypass list
// -- we allow it on all platforms and interpret it the same way. // -- we allow it on all platforms and interpret it the same way.
if (base::LowerCaseEqualsASCII(raw, kWinLocal)) if (base::LowerCaseEqualsASCII(raw, kBypassSimpleHostnames))
return std::make_unique<WinLocalRule>(); return std::make_unique<BypassSimpleHostnamesRule>();
if (base::LowerCaseEqualsASCII(raw, kSubtractImplicitBypasses)) if (base::LowerCaseEqualsASCII(raw, kSubtractImplicitBypasses))
return std::make_unique<SubtractImplicitBypassesRule>(); return std::make_unique<SubtractImplicitBypassesRule>();
...@@ -382,8 +384,8 @@ bool ProxyBypassRules::AddRuleForHostname(const std::string& optional_scheme, ...@@ -382,8 +384,8 @@ bool ProxyBypassRules::AddRuleForHostname(const std::string& optional_scheme,
return true; return true;
} }
void ProxyBypassRules::PrependRuleToBypassLocal() { void ProxyBypassRules::PrependRuleToBypassSimpleHostnames() {
rules_.insert(rules_.begin(), std::make_unique<WinLocalRule>()); rules_.insert(rules_.begin(), std::make_unique<BypassSimpleHostnamesRule>());
} }
bool ProxyBypassRules::AddRuleFromString(const std::string& raw_untrimmed, bool ProxyBypassRules::AddRuleFromString(const std::string& raw_untrimmed,
......
net/proxy_resolution/proxy_bypass_rules.h
View file @ 15242c0c
...@@ -122,14 +122,14 @@ class NET_EXPORT ProxyBypassRules { ...@@ -122,14 +122,14 @@ class NET_EXPORT ProxyBypassRules {
const std::string& hostname_pattern, const std::string& hostname_pattern,
int optional_port); int optional_port);
// Adds a rule to the front of the list that bypasses all "local" hostnames. // Adds a rule to the front of thelist that bypasses hostnames without a dot
// This matches IE's interpretation of the // in them (and is not an IP literal), which can be indicative of intranet
// "Bypass proxy server for local addresses" settings checkbox. Fully // websites.
// qualified domain names or IP addresses are considered non-local,
// regardless of what they map to (except for the loopback addresses).
// //
// TODO(https://crbug.com/902579): Fix. // On Windows this corresponds to the "Bypass proxy server for local
void PrependRuleToBypassLocal(); // addresses" settings checkbox, and on macOS the "Exclude simple hostnames"
// checkbox.
void PrependRuleToBypassSimpleHostnames();
// Adds a rule given by the string |raw|. The format of |raw| can be any of // Adds a rule given by the string |raw|. The format of |raw| can be any of
// the following: // the following:
...@@ -184,11 +184,10 @@ class NET_EXPORT ProxyBypassRules { ...@@ -184,11 +184,10 @@ class NET_EXPORT ProxyBypassRules {
// //
// (6) "<local>" // (6) "<local>"
// //
// Matches the bypass rule in Windows of the same name, which essentially // Matches hostnames without a period in them (and are not IP
// means hostnames without a period in them, as well as "127.0.0.1", // literals).
// "[::1]" and "localhost" (but not other localhost names).
// //
// TODO(https://crbug.com/902579): Fix. // This is equivalent to the same named bypass rule on Windows.
// //
// (7) "<-loopback>" // (7) "<-loopback>"
// //
......
net/proxy_resolution/proxy_bypass_rules_unittest.cc
View file @ 15242c0c
...@@ -323,50 +323,33 @@ TEST(ProxyBypassRulesTest, Equals) { ...@@ -323,50 +323,33 @@ TEST(ProxyBypassRulesTest, Equals) {
EXPECT_FALSE(rules2 == rules1); EXPECT_FALSE(rules2 == rules1);
} }
TEST(ProxyBypassRulesTest, BypassLocalNames) { TEST(ProxyBypassRulesTest, BypassSimpleHostnames) {
const struct { // Test the simple hostnames rule in isolation, by first removing the
const char* url; // implicit rules.
bool expected_is_local;
} tests[] = {
// Single-component hostnames are considered local.
{"http://localhost/x", true},
{"http://www", true},
// IPv4 loopback interface.
{"http://127.0.0.1/x", true},
{"http://127.0.0.1:80/x", true},
// IPv6 loopback interface.
{"http://[::1]:80/x", true},
{"http://[0:0::1]:6233/x", true},
{"http://[0:0:0:0:0:0:0:1]/x", true},
// Non-local URLs.
{"http://foo.com/", false},
{"http://localhost.i/", false},
{"http://www.google.com/", false},
{"http://192.168.0.1/", false},
// Try with different protocols.
{"ftp://127.0.0.1/x", true},
{"ftp://foobar.com/x", false},
// This is a bit of a gray-area, but GURL does not strip trailing dots
// in host-names, so the following are considered non-local.
{"http://www./x", false},
// localhost. is bypassed by the implict rules already.
{"http://localhost./x", true},
};
ProxyBypassRules rules; ProxyBypassRules rules;
rules.ParseFromString("<local>"); rules.ParseFromString("<-loopback>; <local>");
for (size_t i = 0; i < arraysize(tests); ++i) { ASSERT_EQ(2u, rules.rules().size());
SCOPED_TRACE(base::StringPrintf( EXPECT_EQ("<-loopback>", rules.rules()[0]->ToString());
"Test[%d]: %s", static_cast<int>(i), tests[i].url)); EXPECT_EQ("<local>", rules.rules()[1]->ToString());
EXPECT_EQ(tests[i].expected_is_local, rules.Matches(GURL(tests[i].url)));
} EXPECT_TRUE(rules.Matches(GURL("http://example/")));
EXPECT_FALSE(rules.Matches(GURL("http://example./")));
EXPECT_FALSE(rules.Matches(GURL("http://example.com/")));
EXPECT_FALSE(rules.Matches(GURL("http://[dead::beef]/")));
EXPECT_FALSE(rules.Matches(GURL("http://192.168.1.1/")));
// Confusingly, <local> rule is NOT about localhost names. There is however
// overlap on "localhost6?" as it is both a simple hostname and a localhost
// name
ExpectBypassLocalhost(rules, false, {"localhost", "localhost6"});
// Should NOT bypass link-local addresses.
ExpectBypassLinkLocal(rules, false);
// Should not bypass other names either (except for the ones with no dot).
ExpectBypassMisc(rules, false, {"foo", "loopback"});
} }
TEST(ProxyBypassRulesTest, ParseAndMatchCIDR_IPv4) { TEST(ProxyBypassRulesTest, ParseAndMatchCIDR_IPv4) {
......
net/proxy_resolution/proxy_config_service_mac.cc
View file @ 15242c0c
...@@ -153,7 +153,8 @@ void GetCurrentProxyConfig(const NetworkTrafficAnnotationTag traffic_annotation, ...@@ -153,7 +153,8 @@ void GetCurrentProxyConfig(const NetworkTrafficAnnotationTag traffic_annotation,
if (GetBoolFromDictionary(config_dict.get(), if (GetBoolFromDictionary(config_dict.get(),
kSCPropNetProxiesExcludeSimpleHostnames, kSCPropNetProxiesExcludeSimpleHostnames,
false)) { false)) {
proxy_config.proxy_rules().bypass_rules.PrependRuleToBypassLocal(); proxy_config.proxy_rules()
.bypass_rules.PrependRuleToBypassSimpleHostnames();
} }
*config = ProxyConfigWithAnnotation(proxy_config, traffic_annotation); *config = ProxyConfigWithAnnotation(proxy_config, traffic_annotation);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment