diff --git a/net/base/proxy_chain.cc b/net/base/proxy_chain.cc index edb41b175f9dd..b304ed397b1a1 100644 --- a/net/base/proxy_chain.cc +++ b/net/base/proxy_chain.cc @@ -106,6 +106,9 @@ bool ProxyChain::IsValidInternal() const { if (!proxy_server_list_.has_value()) { return false; } + if (is_direct()) { + return true; + } if (is_single_proxy()) { bool is_valid = proxy_server_list_.value().at(0).is_valid(); if (proxy_server_list_.value().at(0).is_quic()) { @@ -113,15 +116,28 @@ bool ProxyChain::IsValidInternal() const { } return is_valid; } - bool all_https = base::ranges::all_of( - proxy_server_list_.value(), [](const auto& proxy_server) { - return proxy_server.is_valid() && proxy_server.is_https(); - }); - bool all_quic = base::ranges::all_of( - proxy_server_list_.value(), [](const auto& proxy_server) { - return proxy_server.is_valid() && proxy_server.is_quic(); - }); - return all_https || (all_quic && is_for_ip_protection()); + DCHECK(is_multi_proxy()); + + // Verify that the chain is zero or more SCHEME_QUIC servers followed by zero + // or more SCHEME_HTTPS servers. + bool seen_quic = false; + bool seen_https = false; + for (const auto& proxy_server : proxy_server_list_.value()) { + if (proxy_server.is_quic()) { + if (seen_https) { + // SCHEME_QUIC cannot follow SCHEME_HTTPS. + return false; + } + seen_quic = true; + } else if (proxy_server.is_https()) { + seen_https = true; + } else { + return false; + } + } + + // QUIC is only allowed for IP protection. + return !seen_quic || is_for_ip_protection(); } std::ostream& operator<<(std::ostream& os, const ProxyChain& proxy_chain) { diff --git a/net/base/proxy_chain.h b/net/base/proxy_chain.h index e0ec68df5825e..d8695d0fb1243 100644 --- a/net/base/proxy_chain.h +++ b/net/base/proxy_chain.h @@ -67,7 +67,7 @@ class NET_EXPORT ProxyChain { static ProxyChain Direct() { return ProxyChain(std::vector<ProxyServer>()); } // Creates a `ProxyChain` for use by the IP Protection feature. This is used - // for metrics collection and for special handling. If not give, the + // for metrics collection and for special handling. If not given, the // chain_id defaults to 0 which corresponds to an un-identified chain. static ProxyChain ForIpProtection(std::vector<ProxyServer> proxy_server_list, int chain_id = 0) { @@ -179,11 +179,12 @@ class NET_EXPORT ProxyChain { // A negative value indicates this chain is not used for IP protection. int ip_protection_chain_id_ = kNotIpProtectionChainId; - // Returns true if this chain is valid. A chain is considered valid if (1) is - // a single valid proxy server. If single QUIC proxy, it must - // also be an IP protection proxy chain. (2) is multi-proxy and - // all servers are either HTTPS or QUIC. If QUIC servers, it must also - // be an IP protection proxy chain. + // Returns true if this chain is valid. A chain is considered valid if + // (1) it is a single valid proxy server, or + // (2) it is a chain of servers, composed of zero or more SCHEME_QUIC servers + // followed by zero or more SCHEME_HTTPS servers. + // If any SCHEME_QUIC servers are included, then the chain must be for IP + // protection. bool IsValidInternal() const; }; diff --git a/net/base/proxy_chain_unittest.cc b/net/base/proxy_chain_unittest.cc index 4e5dd8ba94f63..c2862cacd4118 100644 --- a/net/base/proxy_chain_unittest.cc +++ b/net/base/proxy_chain_unittest.cc @@ -8,6 +8,7 @@ #include <sstream> #include "base/strings/string_number_conversions.h" +#include "base/test/gtest_util.h" #include "net/base/proxy_server.h" #include "net/base/proxy_string_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -367,25 +368,43 @@ TEST(ProxyChainTest, IsGetToProxyAllowed) { } TEST(ProxyChainTest, IsValid) { - auto direct_chain = ProxyChain::Direct(); - // Single hop proxy of type Direct is valid. - EXPECT_TRUE(direct_chain.IsValid()); + EXPECT_TRUE(ProxyChain::Direct().IsValid()); - auto http_proxy1 = - ProxyUriToProxyServer("foo:444", ProxyServer::SCHEME_HTTPS); - auto http_proxy2 = - ProxyUriToProxyServer("foo:555", ProxyServer::SCHEME_HTTPS); + auto https1 = ProxyUriToProxyServer("foo:444", ProxyServer::SCHEME_HTTPS); + auto https2 = ProxyUriToProxyServer("foo:555", ProxyServer::SCHEME_HTTPS); + auto quic1 = ProxyUriToProxyServer("foo:666", ProxyServer::SCHEME_QUIC); + auto quic2 = ProxyUriToProxyServer("foo:777", ProxyServer::SCHEME_QUIC); + auto socks = ProxyUriToProxyServer("foo:777", ProxyServer::SCHEME_SOCKS5); - // Multi hop proxy with HTTPs type is valid. - EXPECT_TRUE(ProxyChain({http_proxy1, http_proxy2}).IsValid()); + EXPECT_TRUE(ProxyChain({https1}).IsValid()); + EXPECT_FALSE(ProxyChain({quic1}).IsValid()); + EXPECT_TRUE(ProxyChain({https1, https2}).IsValid()); + EXPECT_FALSE(ProxyChain({quic1, https1}).IsValid()); + EXPECT_FALSE(ProxyChain({quic1, quic2, https1, https2}).IsValid()); + EXPECT_FALSE(ProxyChain({https1, quic2}).IsValid()); + EXPECT_FALSE(ProxyChain({https1, https2, quic1, quic2}).IsValid()); + EXPECT_FALSE(ProxyChain({socks, https1}).IsValid()); + EXPECT_FALSE(ProxyChain({socks, https1, https2}).IsValid()); + EXPECT_FALSE(ProxyChain({https1, socks}).IsValid()); + EXPECT_FALSE(ProxyChain({https1, https2, socks}).IsValid()); - auto quic_proxy1 = ProxyUriToProxyServer("foo:666", ProxyServer::SCHEME_QUIC); - auto quic_proxy2 = ProxyUriToProxyServer("foo:777", ProxyServer::SCHEME_QUIC); - auto ip_protection_quic_proxy_chain = - ProxyChain::ForIpProtection({quic_proxy1, quic_proxy2}); - // Multi hop proxy with QUIC and IP Protection is valid. - EXPECT_TRUE(ip_protection_quic_proxy_chain.IsValid()); + // IP protection accepts chains with SCHEME_QUIC, but CHECKs on failure + // instead of just creating an invalid chain. + auto IppChain = [](std::vector<ProxyServer> proxy_servers) { + return ProxyChain::ForIpProtection(std::move(proxy_servers)); + }; + EXPECT_TRUE(IppChain({https1}).IsValid()); + EXPECT_TRUE(IppChain({quic1}).IsValid()); + EXPECT_TRUE(IppChain({https1, https2}).IsValid()); + EXPECT_TRUE(IppChain({quic1, https1}).IsValid()); + EXPECT_TRUE(IppChain({quic1, quic2, https1, https2}).IsValid()); + EXPECT_CHECK_DEATH(IppChain({https1, quic2}).IsValid()); + EXPECT_CHECK_DEATH(IppChain({https1, https2, quic1, quic2}).IsValid()); + EXPECT_CHECK_DEATH(IppChain({socks, https1}).IsValid()); + EXPECT_CHECK_DEATH(IppChain({socks, https1, https2}).IsValid()); + EXPECT_CHECK_DEATH(IppChain({https1, socks}).IsValid()); + EXPECT_CHECK_DEATH(IppChain({https1, https2, socks}).IsValid()); } TEST(ProxyChainTest, Unequal) {