Commit ee10f041 authored by Matt Menke's avatar Matt Menke Committed by Commit Bot

Add logic to use a default IsolationInfo for certain URLLoaderFactories.

Also make IsolationInfo mandatory for requests using a profile's main
NetworkContext.  In the future this will be extended to other APIs that
take NIKs (WebSockets, DNS lookups, etc).

Bug: 1082280
Change-Id: I630f501cfaf57ea01af8af05c37c9bd79bda20ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2180542
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarShivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#768749}
parent 06d79df0
...@@ -313,6 +313,7 @@ class ExtensionWebRequestApiTest : public ExtensionApiTest { ...@@ -313,6 +313,7 @@ class ExtensionWebRequestApiTest : public ExtensionApiTest {
network::mojom::URLLoaderFactoryParamsPtr params = network::mojom::URLLoaderFactoryParamsPtr params =
network::mojom::URLLoaderFactoryParams::New(); network::mojom::URLLoaderFactoryParams::New();
params->process_id = network::mojom::kBrowserProcessId; params->process_id = network::mojom::kBrowserProcessId;
params->automatically_assign_isolation_info = true;
params->is_corb_enabled = false; params->is_corb_enabled = false;
mojo::PendingRemote<network::mojom::URLLoaderFactory> loader_factory; mojo::PendingRemote<network::mojom::URLLoaderFactory> loader_factory;
content::BrowserContext::GetDefaultStoragePartition(profile()) content::BrowserContext::GetDefaultStoragePartition(profile())
......
...@@ -836,6 +836,10 @@ void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal( ...@@ -836,6 +836,10 @@ void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal(
network_context_params->split_auth_cache_by_network_isolation_key = network_context_params->split_auth_cache_by_network_isolation_key =
ShouldSplitAuthCacheByNetworkIsolationKey(); ShouldSplitAuthCacheByNetworkIsolationKey();
// All consumers of the main NetworkContext must provide NetworkIsolationKeys
// / IsolationInfos, so storage can be isolated on a per-site basis.
network_context_params->require_network_isolation_key = true;
} }
base::FilePath ProfileNetworkContextService::GetPartitionPath( base::FilePath ProfileNetworkContextService::GetPartitionPath(
......
...@@ -262,6 +262,7 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceBrowserTest, ...@@ -262,6 +262,7 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceBrowserTest,
network::mojom::URLLoaderFactoryParamsPtr params = network::mojom::URLLoaderFactoryParamsPtr params =
network::mojom::URLLoaderFactoryParams::New(); network::mojom::URLLoaderFactoryParams::New();
params->process_id = network::mojom::kBrowserProcessId; params->process_id = network::mojom::kBrowserProcessId;
params->automatically_assign_isolation_info = true;
params->is_corb_enabled = false; params->is_corb_enabled = false;
params->is_trusted = true; params->is_trusted = true;
mojo::Remote<network::mojom::URLLoaderFactory> loader_factory; mojo::Remote<network::mojom::URLLoaderFactory> loader_factory;
......
...@@ -2428,6 +2428,7 @@ StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal( ...@@ -2428,6 +2428,7 @@ StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal(
network::mojom::URLLoaderFactoryParamsPtr params = network::mojom::URLLoaderFactoryParamsPtr params =
network::mojom::URLLoaderFactoryParams::New(); network::mojom::URLLoaderFactoryParams::New();
params->process_id = network::mojom::kBrowserProcessId; params->process_id = network::mojom::kBrowserProcessId;
params->automatically_assign_isolation_info = true;
params->is_corb_enabled = corb_enabled; params->is_corb_enabled = corb_enabled;
// Corb requests are likely made on behalf of untrusted renderers. // Corb requests are likely made on behalf of untrusted renderers.
if (!corb_enabled) if (!corb_enabled)
......
...@@ -164,6 +164,7 @@ IN_PROC_BROWSER_TEST_F(StoragePartitionImplBrowsertest, NetworkContext) { ...@@ -164,6 +164,7 @@ IN_PROC_BROWSER_TEST_F(StoragePartitionImplBrowsertest, NetworkContext) {
network::mojom::URLLoaderFactoryParamsPtr params = network::mojom::URLLoaderFactoryParamsPtr params =
network::mojom::URLLoaderFactoryParams::New(); network::mojom::URLLoaderFactoryParams::New();
params->process_id = network::mojom::kBrowserProcessId; params->process_id = network::mojom::kBrowserProcessId;
params->automatically_assign_isolation_info = true;
params->is_corb_enabled = false; params->is_corb_enabled = false;
mojo::Remote<network::mojom::URLLoaderFactory> loader_factory; mojo::Remote<network::mojom::URLLoaderFactory> loader_factory;
BrowserContext::GetDefaultStoragePartition( BrowserContext::GetDefaultStoragePartition(
......
...@@ -293,6 +293,7 @@ void URLLoaderFactoryGetter::HandleNetworkFactoryRequestOnUIThread( ...@@ -293,6 +293,7 @@ void URLLoaderFactoryGetter::HandleNetworkFactoryRequestOnUIThread(
// The browser process is considered trusted. // The browser process is considered trusted.
params->is_trusted = true; params->is_trusted = true;
params->process_id = network::mojom::kBrowserProcessId; params->process_id = network::mojom::kBrowserProcessId;
params->automatically_assign_isolation_info = true;
params->is_corb_enabled = is_corb_enabled; params->is_corb_enabled = is_corb_enabled;
params->disable_web_security = params->disable_web_security =
base::CommandLine::ForCurrentProcess()->HasSwitch( base::CommandLine::ForCurrentProcess()->HasSwitch(
......
...@@ -55,6 +55,7 @@ URLRequestContext::URLRequestContext() ...@@ -55,6 +55,7 @@ URLRequestContext::URLRequestContext()
url_requests_(std::make_unique<std::set<const URLRequest*>>()), url_requests_(std::make_unique<std::set<const URLRequest*>>()),
enable_brotli_(false), enable_brotli_(false),
check_cleartext_permitted_(false), check_cleartext_permitted_(false),
require_network_isolation_key_(false),
name_("unknown") { name_("unknown") {
base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
this, "URLRequestContext", base::ThreadTaskRunnerHandle::Get()); this, "URLRequestContext", base::ThreadTaskRunnerHandle::Get());
......
...@@ -284,6 +284,13 @@ class NET_EXPORT URLRequestContext ...@@ -284,6 +284,13 @@ class NET_EXPORT URLRequestContext
// Returns current value of the |check_cleartext_permitted| flag. // Returns current value of the |check_cleartext_permitted| flag.
bool check_cleartext_permitted() const { return check_cleartext_permitted_; } bool check_cleartext_permitted() const { return check_cleartext_permitted_; }
void set_require_network_isolation_key(bool require_network_isolation_key) {
require_network_isolation_key_ = require_network_isolation_key;
}
bool require_network_isolation_key() const {
return require_network_isolation_key_;
}
#if !BUILDFLAG(DISABLE_FTP_SUPPORT) #if !BUILDFLAG(DISABLE_FTP_SUPPORT)
void set_ftp_auth_cache(FtpAuthCache* auth_cache) { void set_ftp_auth_cache(FtpAuthCache* auth_cache) {
ftp_auth_cache_ = auth_cache; ftp_auth_cache_ = auth_cache;
...@@ -345,6 +352,10 @@ class NET_EXPORT URLRequestContext ...@@ -345,6 +352,10 @@ class NET_EXPORT URLRequestContext
// request. Only used on Android. // request. Only used on Android.
bool check_cleartext_permitted_; bool check_cleartext_permitted_;
// Triggers a DCHECK if a NetworkIsolationKey/IsolationInfo is not provided to
// a request when true.
bool require_network_isolation_key_;
// An optional name which can be set to describe this URLRequestContext. // An optional name which can be set to describe this URLRequestContext.
// Used in MemoryDumpProvier to annotate memory usage. The name does not need // Used in MemoryDumpProvier to annotate memory usage. The name does not need
// to be unique. // to be unique.
......
...@@ -118,6 +118,12 @@ CorsURLLoaderFactory::CorsURLLoaderFactory( ...@@ -118,6 +118,12 @@ CorsURLLoaderFactory::CorsURLLoaderFactory(
DCHECK(context_); DCHECK(context_);
DCHECK(origin_access_list_); DCHECK(origin_access_list_);
DCHECK_NE(mojom::kInvalidProcessId, process_id_); DCHECK_NE(mojom::kInvalidProcessId, process_id_);
if (params->automatically_assign_isolation_info) {
DCHECK(params->isolation_info.IsEmpty());
// Only the browser process is currently permitted to use automatically
// assigned IsolationInfo, to prevent cross-site information leaks.
DCHECK_EQ(mojom::kBrowserProcessId, process_id_);
}
factory_bound_origin_access_list_ = std::make_unique<OriginAccessList>(); factory_bound_origin_access_list_ = std::make_unique<OriginAccessList>();
if (params->factory_bound_access_patterns) { if (params->factory_bound_access_patterns) {
factory_bound_origin_access_list_->SetAllowListForOrigin( factory_bound_origin_access_list_->SetAllowListForOrigin(
......
...@@ -2016,6 +2016,9 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext() { ...@@ -2016,6 +2016,9 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext() {
auto result = auto result =
URLRequestContextOwner(std::move(pref_service), builder.Build()); URLRequestContextOwner(std::move(pref_service), builder.Build());
result.url_request_context->set_require_network_isolation_key(
params_->require_network_isolation_key);
// Subscribe the CertVerifier to configuration changes that are exposed via // Subscribe the CertVerifier to configuration changes that are exposed via
// the mojom::SSLConfig, but which are not part of the // the mojom::SSLConfig, but which are not part of the
// net::SSLConfig[Service] interfaces. // net::SSLConfig[Service] interfaces.
......
...@@ -6261,7 +6261,8 @@ class NetworkContextSplitCacheTest : public NetworkContextTest { ...@@ -6261,7 +6261,8 @@ class NetworkContextSplitCacheTest : public NetworkContextTest {
const net::IsolationInfo& isolation_info, const net::IsolationInfo& isolation_info,
bool was_cached, bool was_cached,
bool expect_redirect = false, bool expect_redirect = false,
base::Optional<GURL> new_url = base::nullopt) { base::Optional<GURL> new_url = base::nullopt,
bool automatically_assign_isolation_info = false) {
ResourceRequest request = CreateResourceRequest("GET", url); ResourceRequest request = CreateResourceRequest("GET", url);
request.load_flags |= net::LOAD_SKIP_CACHE_VALIDATION; request.load_flags |= net::LOAD_SKIP_CACHE_VALIDATION;
...@@ -6278,6 +6279,9 @@ class NetworkContextSplitCacheTest : public NetworkContextTest { ...@@ -6278,6 +6279,9 @@ class NetworkContextSplitCacheTest : public NetworkContextTest {
params->is_trusted = true; params->is_trusted = true;
} }
params->automatically_assign_isolation_info =
automatically_assign_isolation_info;
request.site_for_cookies = isolation_info.site_for_cookies(); request.site_for_cookies = isolation_info.site_for_cookies();
network_context_->CreateURLLoaderFactory( network_context_->CreateURLLoaderFactory(
...@@ -6408,6 +6412,29 @@ TEST_F(NetworkContextSplitCacheTest, ...@@ -6408,6 +6412,29 @@ TEST_F(NetworkContextSplitCacheTest,
true /* was_cached */); true /* was_cached */);
} }
TEST_F(NetworkContextSplitCacheTest, AutomaticallyAssignIsolationInfo) {
GURL url = test_server()->GetURL("/resource");
// Load with an automatically assigned IsolationInfo, which should populate
// the cache using the IsolationInfo for |url|'s origin.
LoadAndVerifyCached(url, net::IsolationInfo(), false /* was_cached */,
false /* expect_redirect */, base::nullopt /* new_url */,
true /* automatically_assign_isolation_info */);
// Load again with a different isolation info. The cached entry should not be
// loaded.
url::Origin other_origin = url::Origin::Create(GURL("http://other.test/"));
net::IsolationInfo other_info =
net::IsolationInfo::CreateForInternalRequest(other_origin);
LoadAndVerifyCached(url, other_info, false /* was_cached */);
// Load explicitly using the requested URL's own IsolationInfo, which should
// use the cached entry.
url::Origin origin = url::Origin::Create(GURL(url));
net::IsolationInfo info =
net::IsolationInfo::CreateForInternalRequest(origin);
LoadAndVerifyCached(url, info, true /* was_cached */);
}
TEST_F(NetworkContextTest, EnableTrustTokens) { TEST_F(NetworkContextTest, EnableTrustTokens) {
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(features::kTrustTokens); scoped_feature_list.InitAndEnableFeature(features::kTrustTokens);
......
...@@ -472,6 +472,10 @@ struct NetworkContextParams { ...@@ -472,6 +472,10 @@ struct NetworkContextParams {
// See comments on NetworkContext.SetSplitAuthCacheByNetworkIsolationKey. // See comments on NetworkContext.SetSplitAuthCacheByNetworkIsolationKey.
bool split_auth_cache_by_network_isolation_key = false; bool split_auth_cache_by_network_isolation_key = false;
// If true, all requests that can take a NetworkIsolationKey or IsolationInfo
// must be given a non-empty one.
bool require_network_isolation_key = false;
}; };
struct NetworkConditions { struct NetworkConditions {
...@@ -629,7 +633,9 @@ struct URLLoaderFactoryParams { ...@@ -629,7 +633,9 @@ struct URLLoaderFactoryParams {
CorsOriginAccessPatterns? factory_bound_access_patterns; CorsOriginAccessPatterns? factory_bound_access_patterns;
// Information used restrict access to identity information (like SameSite // Information used restrict access to identity information (like SameSite
// cookies) and to shard network resources, like the cache. // cookies) and to shard network resources, like the cache. If set, takes
// precedence over ResourceRequest::TrustedParams::IsolationInfo field
// of individual requests.
IsolationInfo isolation_info; IsolationInfo isolation_info;
// Whether secure DNS should be disabled for requests. // Whether secure DNS should be disabled for requests.
...@@ -638,7 +644,14 @@ struct URLLoaderFactoryParams { ...@@ -638,7 +644,14 @@ struct URLLoaderFactoryParams {
// True if this is for exclusive use by a trusted consumer. Only trusted // True if this is for exclusive use by a trusted consumer. Only trusted
// consumers can issue requests with ResourceRequest::trusted_params // consumers can issue requests with ResourceRequest::trusted_params
// populated. // populated.
bool is_trusted; bool is_trusted = false;
// If true, a cookie-less IsolationInfo is automatically assigned to
// URLLoaders based on the URL being requested. The IsolaitonInfo will
// have an empty SiteForCookies. If this is true, |isolation_info| must be
// null. ResourceRequest::TrustedParams::IsolationInfo, if non-empty, will
// take precedence over automatically assigning an IsolationInfo.
bool automatically_assign_isolation_info = false;
// An identifier for toplevel frame. This is used for resource accounting // An identifier for toplevel frame. This is used for resource accounting
// for keepalive requests. When this factory is for a dedicated worker, // for keepalive requests. When this factory is for a dedicated worker,
......
...@@ -518,8 +518,16 @@ URLLoader::URLLoader( ...@@ -518,8 +518,16 @@ URLLoader::URLLoader(
DCHECK(url_request_->isolation_info().site_for_cookies().IsEquivalent( DCHECK(url_request_->isolation_info().site_for_cookies().IsEquivalent(
request.site_for_cookies)); request.site_for_cookies));
} }
} else if (factory_params_->automatically_assign_isolation_info) {
url::Origin origin = url::Origin::Create(request.url);
url_request_->set_isolation_info(net::IsolationInfo::Create(
net::IsolationInfo::RedirectMode::kUpdateNothing, origin, origin,
net::SiteForCookies()));
} }
if (url_request_context_->require_network_isolation_key())
DCHECK(!url_request_->isolation_info().IsEmpty());
if (factory_params_->disable_secure_dns) { if (factory_params_->disable_secure_dns) {
url_request_->SetDisableSecureDns(true); url_request_->SetDisableSecureDns(true);
} else if (request.trusted_params) { } else if (request.trusted_params) {
......
...@@ -82,6 +82,8 @@ URLLoaderFactory::URLLoaderFactory( ...@@ -82,6 +82,8 @@ URLLoaderFactory::URLLoaderFactory(
// Only non-navigation IsolationInfos should be bound to URLLoaderFactories. // Only non-navigation IsolationInfos should be bound to URLLoaderFactories.
DCHECK_EQ(net::IsolationInfo::RedirectMode::kUpdateNothing, DCHECK_EQ(net::IsolationInfo::RedirectMode::kUpdateNothing,
params_->isolation_info.redirect_mode()); params_->isolation_info.redirect_mode());
DCHECK(!params_->automatically_assign_isolation_info ||
params_->isolation_info.IsEmpty());
if (!params_->top_frame_id) { if (!params_->top_frame_id) {
params_->top_frame_id = base::UnguessableToken::Create(); params_->top_frame_id = base::UnguessableToken::Create();
......
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