Commit f1adea56 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Chromium LUCI CQ

Base `Sec-Fetch-Site` checks on the per-NetworkContext OriginAccessList.

After https://crrev.com/c/2566090, the per-NetworkContext
OriginAccessList and the per-factory OriginAccessList cover the same
permissions.  This means that there is no need to maintain both - to
prepare for the removal of `factory_bound_access_patterns`, this CL
switches `SetSecFetchSiteHeader` checks to be based on the
per-NetworkContext OriginAccessList.

Bug: 936310
Change-Id: I33a8631f2ec2dea110ed6ad12bec672e6b564a76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587394
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarTakashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846452}
parent 6e2b4ec8
...@@ -72,10 +72,10 @@ SecFetchSiteValue SecFetchSiteHeaderValue(const GURL& target_url, ...@@ -72,10 +72,10 @@ SecFetchSiteValue SecFetchSiteHeaderValue(const GURL& target_url,
return SecFetchSiteValue::kCrossSite; return SecFetchSiteValue::kCrossSite;
} }
void SetSecFetchSiteHeader( void SetSecFetchSiteHeader(net::URLRequest* request,
net::URLRequest* request, const GURL* pending_redirect_url,
const GURL* pending_redirect_url, const mojom::URLLoaderFactoryParams& factory_params,
const mojom::URLLoaderFactoryParams& factory_params) { const cors::OriginAccessList* origin_access_list) {
SecFetchSiteValue header_value; SecFetchSiteValue header_value;
url::Origin initiator = GetTrustworthyInitiator( url::Origin initiator = GetTrustworthyInitiator(
factory_params.request_initiator_origin_lock, request->initiator()); factory_params.request_initiator_origin_lock, request->initiator());
...@@ -86,14 +86,9 @@ void SetSecFetchSiteHeader( ...@@ -86,14 +86,9 @@ void SetSecFetchSiteHeader(
// `Sec-Fetch-Site: cross-site`. Other requests default to `kSameOrigin`, // `Sec-Fetch-Site: cross-site`. Other requests default to `kSameOrigin`,
// and walk through the request's URL chain to calculate the // and walk through the request's URL chain to calculate the
// correct value. // correct value.
if (factory_params.unsafe_non_webby_initiator) { if (factory_params.unsafe_non_webby_initiator && origin_access_list) {
cors::OriginAccessList origin_access_list; if (origin_access_list->CheckAccessState(initiator, request->url()) ==
origin_access_list.SetAllowListForOrigin( cors::OriginAccessList::AccessState::kAllowed) {
factory_params.factory_bound_access_patterns->source_origin,
factory_params.factory_bound_access_patterns->allow_patterns);
if (origin_access_list.CheckAccessState(
factory_params.factory_bound_access_patterns->source_origin,
request->url()) == cors::OriginAccessList::AccessState::kAllowed) {
header_value = SecFetchSiteValue::kNoOrigin; header_value = SecFetchSiteValue::kNoOrigin;
} else { } else {
header_value = SecFetchSiteValue::kCrossSite; header_value = SecFetchSiteValue::kCrossSite;
...@@ -155,7 +150,8 @@ void SetFetchMetadataHeaders( ...@@ -155,7 +150,8 @@ void SetFetchMetadataHeaders(
bool has_user_activation, bool has_user_activation,
network::mojom::RequestDestination dest, network::mojom::RequestDestination dest,
const GURL* pending_redirect_url, const GURL* pending_redirect_url,
const mojom::URLLoaderFactoryParams& factory_params) { const mojom::URLLoaderFactoryParams& factory_params,
const cors::OriginAccessList* origin_access_list) {
DCHECK(request); DCHECK(request);
DCHECK_NE(0u, request->url_chain().size()); DCHECK_NE(0u, request->url_chain().size());
...@@ -165,7 +161,8 @@ void SetFetchMetadataHeaders( ...@@ -165,7 +161,8 @@ void SetFetchMetadataHeaders(
if (!IsUrlPotentiallyTrustworthy(target_url)) if (!IsUrlPotentiallyTrustworthy(target_url))
return; return;
SetSecFetchSiteHeader(request, pending_redirect_url, factory_params); SetSecFetchSiteHeader(request, pending_redirect_url, factory_params,
origin_access_list);
SetSecFetchModeHeader(request, mode); SetSecFetchModeHeader(request, mode);
SetSecFetchUserHeader(request, has_user_activation); SetSecFetchUserHeader(request, has_user_activation);
SetSecFetchDestHeader(request, dest); SetSecFetchDestHeader(request, dest);
......
...@@ -15,6 +15,10 @@ class URLRequest; ...@@ -15,6 +15,10 @@ class URLRequest;
namespace network { namespace network {
namespace cors {
class OriginAccessList;
} // namespace cors
namespace mojom { namespace mojom {
class URLLoaderFactoryParams; class URLLoaderFactoryParams;
} // namespace mojom } // namespace mojom
...@@ -36,7 +40,8 @@ void SetFetchMetadataHeaders( ...@@ -36,7 +40,8 @@ void SetFetchMetadataHeaders(
bool has_user_activation, bool has_user_activation,
network::mojom::RequestDestination dest, network::mojom::RequestDestination dest,
const GURL* pending_redirect_url, const GURL* pending_redirect_url,
const mojom::URLLoaderFactoryParams& factory_params); const mojom::URLLoaderFactoryParams& factory_params,
const cors::OriginAccessList* origin_access_list);
// Removes any sec-ch- or sec-fetch- prefixed request headers on the |request| // Removes any sec-ch- or sec-fetch- prefixed request headers on the |request|
// if the |pending_redirect_url| is not trustworthy and the current url is. // if the |pending_redirect_url| is not trustworthy and the current url is.
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "net/url_request/url_request.h" #include "net/url_request/url_request.h"
#include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_test_util.h" #include "net/url_request/url_request_test_util.h"
#include "services/network/public/cpp/cors/origin_access_list.h"
#include "services/network/public/mojom/cors_origin_pattern.mojom.h" #include "services/network/public/mojom/cors_origin_pattern.mojom.h"
#include "services/network/public/mojom/fetch_api.mojom.h" #include "services/network/public/mojom/fetch_api.mojom.h"
#include "services/network/public/mojom/network_context.mojom.h" #include "services/network/public/mojom/network_context.mojom.h"
...@@ -20,6 +21,7 @@ namespace { ...@@ -20,6 +21,7 @@ namespace {
constexpr char kSecureSite[] = "https://site.tld"; constexpr char kSecureSite[] = "https://site.tld";
constexpr char kInsecureSite[] = "http://othersite.tld"; constexpr char kInsecureSite[] = "http://othersite.tld";
constexpr char kPrivilegedInitiator[] = "https://chrome-extension.example.com";
constexpr char kKnownSecChHeader[] = "Sec-CH-UA"; constexpr char kKnownSecChHeader[] = "Sec-CH-UA";
constexpr char kKnownSecFetchSiteHeader[] = "Sec-Fetch-Site"; constexpr char kKnownSecFetchSiteHeader[] = "Sec-Fetch-Site";
...@@ -41,8 +43,11 @@ class SecHeaderHelpersTest : public PlatformTest { ...@@ -41,8 +43,11 @@ class SecHeaderHelpersTest : public PlatformTest {
: task_environment_(base::test::TaskEnvironment::MainThreadType::IO), : task_environment_(base::test::TaskEnvironment::MainThreadType::IO),
url_request_(context_.CreateRequest(GURL(kSecureSite), url_request_(context_.CreateRequest(GURL(kSecureSite),
net::DEFAULT_PRIORITY, net::DEFAULT_PRIORITY,
/*request_delegate=*/nullptr, /*delegate=*/nullptr,
TRAFFIC_ANNOTATION_FOR_TESTS)) {} TRAFFIC_ANNOTATION_FOR_TESTS)) {
url_request_->set_initiator(
url::Origin::Create(GURL(kPrivilegedInitiator)));
}
net::URLRequest* url_request() const { return url_request_.get(); } net::URLRequest* url_request() const { return url_request_.get(); }
...@@ -164,14 +169,13 @@ TEST_F(SecHeaderHelpersTest, UnprivilegedRequestOnExtension) { ...@@ -164,14 +169,13 @@ TEST_F(SecHeaderHelpersTest, UnprivilegedRequestOnExtension) {
network::mojom::URLLoaderFactoryParams params; network::mojom::URLLoaderFactoryParams params;
params.unsafe_non_webby_initiator = true; params.unsafe_non_webby_initiator = true;
params.factory_bound_access_patterns =
network::mojom::CorsOriginAccessPatterns::New(); cors::OriginAccessList origin_access_list; // empty in this test
params.factory_bound_access_patterns->source_origin =
url::Origin::Create(url); SetFetchMetadataHeaders(current_url_request,
network::mojom::RequestMode::kCors, false,
SetFetchMetadataHeaders( network::mojom::RequestDestination::kIframe, &url,
current_url_request, network::mojom::RequestMode::kCors, false, params, &origin_access_list);
network::mojom::RequestDestination::kIframe, &url, params);
ASSERT_EQ(3, static_cast<int>(current_url_request->extra_request_headers() ASSERT_EQ(3, static_cast<int>(current_url_request->extra_request_headers()
.GetHeaderVector() .GetHeaderVector()
.size())); .size()));
...@@ -198,19 +202,21 @@ TEST_F(SecHeaderHelpersTest, PrivilegedRequestOnExtension) { ...@@ -198,19 +202,21 @@ TEST_F(SecHeaderHelpersTest, PrivilegedRequestOnExtension) {
network::mojom::URLLoaderFactoryParams params; network::mojom::URLLoaderFactoryParams params;
params.unsafe_non_webby_initiator = true; params.unsafe_non_webby_initiator = true;
params.factory_bound_access_patterns =
network::mojom::CorsOriginAccessPatterns::New(); cors::OriginAccessList origin_access_list;
params.factory_bound_access_patterns->source_origin = origin_access_list.AddAllowListEntryForOrigin(
url::Origin::Create(url); url::Origin::Create(GURL(kPrivilegedInitiator)), // source_origin
params.factory_bound_access_patterns->allow_patterns.push_back( url.scheme(), // protocol
mojom::CorsOriginPattern::New( url.host(), // domain
url.scheme(), url.host(), 0, 0, // port
mojom::CorsDomainMatchMode::kDisallowSubdomains, mojom::CorsDomainMatchMode::kDisallowSubdomains,
mojom::CorsPortMatchMode::kAllowAnyPort, mojom::CorsPortMatchMode::kAllowAnyPort,
mojom::CorsOriginAccessMatchPriority::kDefaultPriority)); mojom::CorsOriginAccessMatchPriority::kDefaultPriority);
SetFetchMetadataHeaders(
current_url_request, network::mojom::RequestMode::kCors, true, SetFetchMetadataHeaders(current_url_request,
network::mojom::RequestDestination::kEmbed, &url, params); network::mojom::RequestMode::kCors, true,
network::mojom::RequestDestination::kEmbed, &url,
params, &origin_access_list);
ASSERT_EQ(4, static_cast<int>(current_url_request->extra_request_headers() ASSERT_EQ(4, static_cast<int>(current_url_request->extra_request_headers()
.GetHeaderVector() .GetHeaderVector()
......
...@@ -610,7 +610,7 @@ URLLoader::URLLoader( ...@@ -610,7 +610,7 @@ URLLoader::URLLoader(
SetFetchMetadataHeaders(url_request_.get(), request_mode_, SetFetchMetadataHeaders(url_request_.get(), request_mode_,
has_user_activation_, request_destination_, nullptr, has_user_activation_, request_destination_, nullptr,
*factory_params_); *factory_params_, origin_access_list_);
if (request.update_first_party_url_on_redirect) { if (request.update_first_party_url_on_redirect) {
url_request_->set_first_party_url_policy( url_request_->set_first_party_url_policy(
...@@ -1143,7 +1143,8 @@ void URLLoader::OnReceivedRedirect(net::URLRequest* url_request, ...@@ -1143,7 +1143,8 @@ void URLLoader::OnReceivedRedirect(net::URLRequest* url_request,
MaybeRemoveSecHeaders(url_request_.get(), redirect_info.new_url); MaybeRemoveSecHeaders(url_request_.get(), redirect_info.new_url);
SetFetchMetadataHeaders(url_request_.get(), request_mode_, SetFetchMetadataHeaders(url_request_.get(), request_mode_,
has_user_activation_, request_destination_, has_user_activation_, request_destination_,
&redirect_info.new_url, *factory_params_); &redirect_info.new_url, *factory_params_,
origin_access_list_);
url_loader_client_->OnReceiveRedirect(redirect_info, std::move(response)); url_loader_client_->OnReceiveRedirect(redirect_info, std::move(response));
} }
......
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