Treat 0.0.0.0/8 as non-public
Thus far, 0.0.0.0/8 has been a loophole in Private Network Access, in that requests to 0.0.0.0/8 are allowed when requests to the private and/or local address space would be blocked. This CL fixes that by treating 0.0.0.0/32 as local address space and 0.0.0.0/8 as private address space. A Finch killswitch allows this to be switched back to public address space in case this change turns out to cause unexpected breakage. Intent: https://groups.google.com/a/chromium.org/g/blink-dev/c/9uymCQNGVgw/m/TxWeILuJAwAJ Bug: 40058874 Change-Id: I6669b9a1563e7153c301d1e44b642ca29bff61b1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5700065 Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Commit-Queue: Emily Stark <estark@chromium.org> Reviewed-by: Titouan Rigoudy <titouan@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/main@{#1334428}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
af03ac086c
commit
2ac34fcefc
chrome/browser/net
content/browser/renderer_host
services/network/public/cpp
@ -39,6 +39,7 @@
|
||||
#include "extensions/common/extension_builder.h"
|
||||
#include "net/dns/mock_host_resolver.h"
|
||||
#include "net/test/embedded_test_server/embedded_test_server.h"
|
||||
#include "services/network/public/cpp/features.h"
|
||||
#include "services/network/public/cpp/private_network_access_check_result.h"
|
||||
#include "services/network/public/cpp/url_loader_completion_status.h"
|
||||
#include "testing/gmock/include/gmock/gmock.h"
|
||||
@ -1437,4 +1438,66 @@ IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessAutoReloadBrowserTest,
|
||||
EXPECT_TRUE(observer.last_navigation_succeeded());
|
||||
}
|
||||
|
||||
// ================
|
||||
// 0.0.0.0 TESTS
|
||||
// ================
|
||||
|
||||
// This test verifies that a 0.0.0.0 subresource is blocked on a nonsecure
|
||||
// public URL.
|
||||
IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessWithFeatureEnabledBrowserTest,
|
||||
NullIPBlockedOnNonsecure) {
|
||||
if constexpr (BUILDFLAG(IS_WIN)) {
|
||||
GTEST_SKIP() << "0.0.0.0 behavior varies across platforms and is "
|
||||
"unreachable on Windows.";
|
||||
}
|
||||
|
||||
std::unique_ptr<net::EmbeddedTestServer> server = NewServer();
|
||||
GURL url = PublicNonSecureURL(*server);
|
||||
EXPECT_TRUE(content::NavigateToURL(web_contents(), url));
|
||||
GURL subresource_url = server->GetURL("0.0.0.0", "/cors-ok.txt");
|
||||
EXPECT_EQ(false, content::EvalJs(web_contents(),
|
||||
content::JsReplace(R"(
|
||||
fetch($1).then(response => true).catch(error => false)
|
||||
)",
|
||||
subresource_url)));
|
||||
}
|
||||
|
||||
class PrivateNetworkAccessWithNullIPKillswitchTest
|
||||
: public PrivateNetworkAccessBrowserTestBase {
|
||||
public:
|
||||
PrivateNetworkAccessWithNullIPKillswitchTest()
|
||||
: PrivateNetworkAccessBrowserTestBase(
|
||||
{
|
||||
blink::features::kPlzDedicatedWorker,
|
||||
features::kBlockInsecurePrivateNetworkRequests,
|
||||
features::kBlockInsecurePrivateNetworkRequestsFromPrivate,
|
||||
features::kBlockInsecurePrivateNetworkRequestsDeprecationTrial,
|
||||
features::kPrivateNetworkAccessSendPreflights,
|
||||
features::kPrivateNetworkAccessForNavigations,
|
||||
features::kPrivateNetworkAccessForWorkers,
|
||||
network::features::kTreatNullIPAsPublicAddressSpace,
|
||||
},
|
||||
{}) {}
|
||||
};
|
||||
|
||||
// This test verifies that 0.0.0.0 subresources are not blocked when the
|
||||
// killswitch feature is enabled.
|
||||
IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessWithNullIPKillswitchTest,
|
||||
NullIPNotBlockedWithKillswitch) {
|
||||
if constexpr (BUILDFLAG(IS_WIN)) {
|
||||
GTEST_SKIP() << "0.0.0.0 behavior varies across platforms and is "
|
||||
"unreachable on Windows.";
|
||||
}
|
||||
|
||||
std::unique_ptr<net::EmbeddedTestServer> server = NewServer();
|
||||
GURL url = PublicNonSecureURL(*server);
|
||||
EXPECT_TRUE(content::NavigateToURL(web_contents(), url));
|
||||
GURL subresource_url = server->GetURL("0.0.0.0", "/cors-ok.txt");
|
||||
EXPECT_EQ(true, content::EvalJs(web_contents(),
|
||||
content::JsReplace(R"(
|
||||
fetch($1).then(response => response.ok)
|
||||
)",
|
||||
subresource_url)));
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
@ -553,6 +553,10 @@ class PrivateNetworkAccessBrowserTestBase : public ContentBrowserTest {
|
||||
return secure_public_server_.Get().GetURL(kPublicHost, path);
|
||||
}
|
||||
|
||||
GURL NullIPURL(const std::string& path) {
|
||||
return insecure_public_server_.Get().GetURL("0.0.0.0", path);
|
||||
}
|
||||
|
||||
private:
|
||||
base::test::ScopedFeatureList feature_list_;
|
||||
|
||||
@ -895,6 +899,54 @@ IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessBrowserTest,
|
||||
security_state->ip_address_space);
|
||||
}
|
||||
|
||||
// Tests that a top-level navigation to 0.0.0.0 is in the kLocal address space.
|
||||
IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessBrowserTest,
|
||||
ClientSecurityStateForNullIP) {
|
||||
if constexpr (BUILDFLAG(IS_WIN)) {
|
||||
GTEST_SKIP() << "0.0.0.0 behavior varies across platforms and is "
|
||||
"unreachable on Windows.";
|
||||
}
|
||||
|
||||
EXPECT_TRUE(NavigateToURL(shell(), NullIPURL(kDefaultPath)));
|
||||
|
||||
const network::mojom::ClientSecurityStatePtr security_state =
|
||||
root_frame_host()->BuildClientSecurityState();
|
||||
ASSERT_FALSE(security_state.is_null());
|
||||
EXPECT_FALSE(security_state->is_web_secure_context);
|
||||
EXPECT_EQ(network::mojom::IPAddressSpace::kLocal,
|
||||
security_state->ip_address_space);
|
||||
}
|
||||
|
||||
class PrivateNetworkAccessBrowserTestNullIPKillswitch
|
||||
: public PrivateNetworkAccessBrowserTestBase {
|
||||
public:
|
||||
PrivateNetworkAccessBrowserTestNullIPKillswitch()
|
||||
: PrivateNetworkAccessBrowserTestBase(
|
||||
{
|
||||
network::features::kTreatNullIPAsPublicAddressSpace,
|
||||
},
|
||||
{}) {}
|
||||
};
|
||||
|
||||
// Tests that a top-level navigation to 0.0.0.0 is in the kPublic address space
|
||||
// when a killswitch is enabled to specifically treat it as public.
|
||||
IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessBrowserTestNullIPKillswitch,
|
||||
ClientSecurityStateForNullIPKillswitch) {
|
||||
if constexpr (BUILDFLAG(IS_WIN)) {
|
||||
GTEST_SKIP() << "0.0.0.0 behavior varies across platforms and is "
|
||||
"unreachable on Windows.";
|
||||
}
|
||||
|
||||
EXPECT_TRUE(NavigateToURL(shell(), NullIPURL(kDefaultPath)));
|
||||
|
||||
const network::mojom::ClientSecurityStatePtr security_state =
|
||||
root_frame_host()->BuildClientSecurityState();
|
||||
ASSERT_FALSE(security_state.is_null());
|
||||
EXPECT_FALSE(security_state->is_web_secure_context);
|
||||
EXPECT_EQ(network::mojom::IPAddressSpace::kPublic,
|
||||
security_state->ip_address_space);
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessBrowserTest,
|
||||
ClientSecurityStateForTreatAsPublicAddress) {
|
||||
EXPECT_TRUE(
|
||||
|
@ -474,4 +474,11 @@ const base::FeatureParam<int> kNetworkContextPrefetchMaxLoaders{
|
||||
&kNetworkContextPrefetch,
|
||||
/*name=*/"max_loaders", /*default_value=*/10};
|
||||
|
||||
// This feature enables treating 0.0.0.0/8 as the public address space instead
|
||||
// of private or local. This is a killswitch for a tightening of a loophole in
|
||||
// Private Network Access. See https://crbug.com/40058874.
|
||||
BASE_FEATURE(kTreatNullIPAsPublicAddressSpace,
|
||||
"TreatNullIPAsPublicAddressSpace",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT);
|
||||
|
||||
} // namespace network::features
|
||||
|
@ -187,6 +187,9 @@ BASE_DECLARE_FEATURE(kNetworkContextPrefetch);
|
||||
COMPONENT_EXPORT(NETWORK_CPP)
|
||||
extern const base::FeatureParam<int> kNetworkContextPrefetchMaxLoaders;
|
||||
|
||||
COMPONENT_EXPORT(NETWORK_CPP)
|
||||
BASE_DECLARE_FEATURE(kTreatNullIPAsPublicAddressSpace);
|
||||
|
||||
} // namespace network::features
|
||||
|
||||
#endif // SERVICES_NETWORK_PUBLIC_CPP_FEATURES_H_
|
||||
|
@ -15,6 +15,7 @@
|
||||
#include "net/base/ip_endpoint.h"
|
||||
#include "net/base/transport_info.h"
|
||||
#include "services/network/public/cpp/content_security_policy/content_security_policy.h"
|
||||
#include "services/network/public/cpp/features.h"
|
||||
#include "services/network/public/cpp/network_switches.h"
|
||||
#include "services/network/public/mojom/ip_address_space.mojom.h"
|
||||
#include "services/network/public/mojom/parsed_headers.mojom.h"
|
||||
@ -246,6 +247,19 @@ const AddressSpaceMap& NonPublicAddressSpaceMap() {
|
||||
Entry(IPAddress(192, 168, 0, 0), 16, IPAddressSpace::kPrivate),
|
||||
// IPv4 Link-local (RFC 3927): 169.254.0.0/16
|
||||
Entry(IPAddress(169, 254, 0, 0), 16, IPAddressSpace::kPrivate),
|
||||
// IPv4 Null IP (RFC 5735): 0.0.0.0/32 is "this host on this network".
|
||||
// Other addresses in 0.0.0.0/8 may refer to "specified hosts on this
|
||||
// network". This is somewhat under-defined for the purposes of assigning
|
||||
// local vs private address space but we assign 0.0.0.0/32 to "local" and
|
||||
// the rest of the block to "private". Note that this mapping can be
|
||||
// overridden by a killswitch feature flag in IPAddressToIPAddressSpace()
|
||||
// since these addresses were previously treated as public. See
|
||||
// https://crbug.com/40058874.
|
||||
//
|
||||
// TODO(https://crbug.com/40058874): decide if we should do the same for
|
||||
// the all-zero IPv6 address.
|
||||
Entry(IPAddress(0, 0, 0, 0), 32, IPAddressSpace::kLocal),
|
||||
Entry(IPAddress(0, 0, 0, 0), 8, IPAddressSpace::kPrivate),
|
||||
}));
|
||||
return *kMap;
|
||||
}
|
||||
@ -253,6 +267,17 @@ const AddressSpaceMap& NonPublicAddressSpaceMap() {
|
||||
} // namespace
|
||||
|
||||
IPAddressSpace IPAddressToIPAddressSpace(const IPAddress& address) {
|
||||
// The null IP block (0.0.0.0/8) was previously treated as public, but this
|
||||
// was a loophole in Private Network Access and thus these addresses are now
|
||||
// mapped to the local/private address space instead. This feature is a
|
||||
// killswitch for this behavior to revert these addresses to the public
|
||||
// address space.
|
||||
if (base::FeatureList::IsEnabled(
|
||||
network::features::kTreatNullIPAsPublicAddressSpace) &&
|
||||
address.IsIPv4() &&
|
||||
IPAddressMatchesPrefix(address, IPAddress(0, 0, 0, 0), 8)) {
|
||||
return IPAddressSpace::kPublic;
|
||||
}
|
||||
return NonPublicAddressSpaceMap().Apply(address).value_or(
|
||||
IPAddressSpace::kPublic);
|
||||
}
|
||||
|
@ -5,9 +5,11 @@
|
||||
#include "services/network/public/cpp/ip_address_space_util.h"
|
||||
|
||||
#include "base/command_line.h"
|
||||
#include "base/test/scoped_feature_list.h"
|
||||
#include "net/base/ip_address.h"
|
||||
#include "net/base/ip_endpoint.h"
|
||||
#include "net/base/transport_info.h"
|
||||
#include "services/network/public/cpp/features.h"
|
||||
#include "services/network/public/cpp/network_switches.h"
|
||||
#include "services/network/public/mojom/ip_address_space.mojom.h"
|
||||
#include "services/network/public/mojom/url_response_head.mojom.h"
|
||||
@ -253,6 +255,29 @@ TEST(IPAddressSpaceTest, IPEndPointToAddressSpaceIPv4MappedIPv6) {
|
||||
IPAddressSpace::kLocal);
|
||||
}
|
||||
|
||||
// Verifies that 0.0.0.0/8 is mapped to non-public address spaces.
|
||||
TEST(IPAddressSpaceTest, IPEndPointToAddressSpaceNullIP) {
|
||||
EXPECT_EQ(IPAddressToIPAddressSpace(IPAddress(0, 0, 0, 0)),
|
||||
IPAddressSpace::kLocal);
|
||||
EXPECT_EQ(IPAddressToIPAddressSpace(IPAddress(0, 0, 0, 4)),
|
||||
IPAddressSpace::kPrivate);
|
||||
EXPECT_EQ(IPAddressToIPAddressSpace(IPAddress(0, 255, 255, 255)),
|
||||
IPAddressSpace::kPrivate);
|
||||
}
|
||||
|
||||
// Verifies that 0.0.0.0/8 is mapped to the public address space if configured
|
||||
// via feature flag.
|
||||
TEST(IPAddressSpaceTest, IPEndPointToAddressSpaceNullIPKillSwitch) {
|
||||
base::test::ScopedFeatureList enable{
|
||||
features::kTreatNullIPAsPublicAddressSpace};
|
||||
EXPECT_EQ(IPAddressToIPAddressSpace(IPAddress(0, 0, 0, 0)),
|
||||
IPAddressSpace::kPublic);
|
||||
EXPECT_EQ(IPAddressToIPAddressSpace(IPAddress(0, 0, 0, 4)),
|
||||
IPAddressSpace::kPublic);
|
||||
EXPECT_EQ(IPAddressToIPAddressSpace(IPAddress(0, 255, 255, 255)),
|
||||
IPAddressSpace::kPublic);
|
||||
}
|
||||
|
||||
// Verifies that the `ip-address-space-overrides` switch can be present and
|
||||
// empty, in which case it is ignored.
|
||||
TEST(IPAddressSpaceTest, IPEndPointToAddressSpaceOverrideEmpty) {
|
||||
|
Reference in New Issue
Block a user