Implement Activate-Storage-Access: load
header semantics
This adds support for the `load` response header described in https://github.com/cfredric/storage-access-headers. Specifically, it gives servers a way to ask that the UA load an iframe's content after having already "activated" storage-access permission. (The alternative would be to load the iframe's content, then allow the iframe's script to execute `document.requestStorageAccess()`, then allow the iframe to reload itself if needed.) Design (google-internal): https://docs.google.com/document/d/1BTNveUPets9cmgTVsD2kvc8DI7FUdE8Uhv1u7TEnI3w/edit?resourcekey=0-3XpjuHX7DFhpTlRi5rfdVw&tab=t.0#heading=h.ojcogmkpsrt9 Bug: 344608182 Change-Id: I1db1b43de3793cbfd783f7f2abccfa11ce0c6bcf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5594289 Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: mmenke <mmenke@chromium.org> Auto-Submit: Chris Fredrickson <cfredric@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/main@{#1310650}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
e48226ce51
commit
22763f6077
chrome/browser/storage_access_api
content/browser/renderer_host
net
base
http
url_request
services/network
@ -2800,6 +2800,7 @@ class StorageAccessHeadersBrowserTest : public StorageAccessAPIBrowserTest {
|
||||
std::vector<base::test::FeatureRefAndParams> GetEnabledFeatures() override {
|
||||
return {
|
||||
{net::features::kStorageAccessHeaderRetry, {}},
|
||||
{net::features::kStorageAccessHeaderLoad, {}},
|
||||
};
|
||||
}
|
||||
};
|
||||
@ -2855,6 +2856,38 @@ IN_PROC_BROWSER_TEST_F(StorageAccessHeadersBrowserTest,
|
||||
EXPECT_EQ(retry_path_fetch_count_, 2);
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(StorageAccessHeadersBrowserTest, LoadHeader) {
|
||||
SetBlockThirdPartyCookies(true);
|
||||
|
||||
// Pre-seed with a <A, B> permission grant.
|
||||
NavigateToPageWithFrame(kHostA);
|
||||
NavigateFrameTo(EchoCookiesURL(kHostB));
|
||||
prompt_factory()->set_response_type(
|
||||
permissions::PermissionRequestManager::ACCEPT_ALL);
|
||||
ASSERT_TRUE(storage::test::RequestAndCheckStorageAccessForFrame(GetFrame()));
|
||||
|
||||
// Now attempt to use that permission grant for a cross-site iframe, without
|
||||
// invoking the Storage Access API.
|
||||
NavigateToPageWithFrame(kHostA);
|
||||
NavigateFrameTo(GetURL(kHostB, "/set-header?Activate-Storage-Access: load"));
|
||||
// No need to request storage access, because we already have it.
|
||||
EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(StorageAccessHeadersBrowserTest,
|
||||
LoadHeader_NoopWithoutGrant) {
|
||||
SetBlockThirdPartyCookies(true);
|
||||
|
||||
// Note: we do *not* pre-seed with a <A, B> permission grant.
|
||||
|
||||
// Now attempt to get storage access in a cross-site iframe, without invoking
|
||||
// the Storage Access API.
|
||||
NavigateToPageWithFrame(kHostA);
|
||||
NavigateFrameTo(GetURL(kHostB, "/set-header?Activate-Storage-Access: load"));
|
||||
// Permission was never granted, so we don't have storage access.
|
||||
EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
|
||||
}
|
||||
|
||||
class StorageAccessHeadersWithThirdPartyCookiesBrowserTest
|
||||
: public StorageAccessHeadersBrowserTest {
|
||||
public:
|
||||
|
@ -1149,6 +1149,44 @@ bool MaybeEvictFromBackForwardCacheBySubframeNavigation(
|
||||
return false;
|
||||
}
|
||||
|
||||
bool ShouldLoadWithStorageAccess(
|
||||
const blink::mojom::BeginNavigationParams& begin_params,
|
||||
const blink::mojom::CommonNavigationParams& common_params,
|
||||
const RenderFrameHostImpl* previous_document_rfh,
|
||||
bool did_encounter_cross_origin_redirect,
|
||||
const GURL response_url,
|
||||
const network::mojom::URLResponseHead* response) {
|
||||
// Experimental: Storage Access API Headers
|
||||
// (https://github.com/cfredric/storage-access-headers)
|
||||
//
|
||||
// A server can opt-in to provide storage access to a document by setting the
|
||||
// `Activate-Storage-Access: load` header, provided that the user has already
|
||||
// granted the relevant `storage-access` permission.
|
||||
//
|
||||
// Note: As of today, `about:blank`, `about:srcdoc`, and MHTML-iframe do not
|
||||
// have a response.
|
||||
if (response && response->load_with_storage_access) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Storage Access API: https://privacycg.github.io/storage-access/#navigation
|
||||
//
|
||||
// If a document has storage access, and initiates a navigation in the same
|
||||
// frame toward a document from the same origin, the `has storage access` bit
|
||||
// is inherited.
|
||||
//
|
||||
// This doesn't hold if there is a cross-origin redirect in between.
|
||||
//
|
||||
// Note: `begin_params` and `common_params` are not trusted, so we have to
|
||||
// check the frame token.
|
||||
return begin_params.has_storage_access && common_params.initiator_origin &&
|
||||
common_params.initiator_origin->IsSameOriginWith(response_url) &&
|
||||
begin_params.initiator_frame_token &&
|
||||
begin_params.initiator_frame_token ==
|
||||
previous_document_rfh->GetFrameToken() &&
|
||||
!did_encounter_cross_origin_redirect;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
NavigationRequest::PrerenderActivationNavigationState::
|
||||
@ -1300,14 +1338,6 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated(
|
||||
common_params->request_destination =
|
||||
GetDestinationFromFrameTreeNode(frame_tree_node);
|
||||
|
||||
const bool load_with_storage_access =
|
||||
begin_params->has_storage_access &&
|
||||
common_params->initiator_origin.has_value() &&
|
||||
common_params->initiator_origin->IsSameOriginWith(common_params->url) &&
|
||||
begin_params->initiator_frame_token.has_value() &&
|
||||
begin_params->initiator_frame_token ==
|
||||
frame_tree_node->current_frame_host()->GetFrameToken();
|
||||
|
||||
// TODO(clamy): See if the navigation start time should be measured in the
|
||||
// renderer and sent to the browser instead of being measured here.
|
||||
blink::mojom::CommitNavigationParamsPtr commit_params =
|
||||
@ -1369,7 +1399,7 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated(
|
||||
base::flat_map<::blink::mojom::RuntimeFeature, bool>(),
|
||||
/*fenced_frame_properties=*/std::nullopt,
|
||||
/*not_restored_reasons=*/nullptr,
|
||||
/*load_with_storage_access=*/load_with_storage_access,
|
||||
/*load_with_storage_access=*/false,
|
||||
/*browsing_context_group_info=*/std::nullopt,
|
||||
/*lcpp_hint=*/nullptr, blink::CreateDefaultRendererContentSettings(),
|
||||
/*cookie_deprecation_label=*/std::nullopt,
|
||||
@ -3386,9 +3416,6 @@ void NavigationRequest::OnRequestRedirected(
|
||||
did_receive_early_hints_before_cross_origin_redirect_ |=
|
||||
did_create_early_hints_manager_params_ && !is_same_origin_redirect;
|
||||
|
||||
commit_params_->load_with_storage_access =
|
||||
commit_params_->load_with_storage_access && is_same_origin_redirect;
|
||||
|
||||
commit_params_->redirects.push_back(common_params_->url);
|
||||
common_params_->url = redirect_info.new_url;
|
||||
common_params_->method = redirect_info.new_method;
|
||||
@ -6154,6 +6181,10 @@ void NavigationRequest::CommitNavigation() {
|
||||
computed_fenced_frame_properties->RedactFor(entity);
|
||||
}
|
||||
|
||||
commit_params_->load_with_storage_access = ShouldLoadWithStorageAccess(
|
||||
begin_params(), common_params(), frame_tree_node()->current_frame_host(),
|
||||
did_encounter_cross_origin_redirect(), GetURL(), response());
|
||||
|
||||
auto common_params = common_params_->Clone();
|
||||
auto commit_params = commit_params_.Clone();
|
||||
auto response_head = response_head_.Clone();
|
||||
|
@ -544,6 +544,9 @@ BASE_FEATURE(kPartitionProxyChains,
|
||||
BASE_FEATURE(kStorageAccessHeaderRetry,
|
||||
"StorageAccessHeaderRetry",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT);
|
||||
BASE_FEATURE(kStorageAccessHeaderLoad,
|
||||
"StorageAccessHeaderLoad",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT);
|
||||
|
||||
BASE_FEATURE(kSpdySessionForProxyAdditionalChecks,
|
||||
"SpdySessionForProxyAdditionalChecks",
|
||||
|
@ -515,6 +515,8 @@ NET_EXPORT BASE_DECLARE_FEATURE(kPartitionProxyChains);
|
||||
|
||||
// Enables the `Activate-Storage-Access: retry` semantics.
|
||||
NET_EXPORT BASE_DECLARE_FEATURE(kStorageAccessHeaderRetry);
|
||||
// Enables the `Activate-Storage-Access: load` semantics.
|
||||
NET_EXPORT BASE_DECLARE_FEATURE(kStorageAccessHeaderLoad);
|
||||
|
||||
// Enables more checks when creating a SpdySession for proxy. These checks are
|
||||
// already applied to non-proxy SpdySession creations.
|
||||
|
@ -110,6 +110,8 @@ const char* const kNonUpdatedHeaderPrefixes[] = {
|
||||
"x-webkit-"
|
||||
};
|
||||
|
||||
constexpr char kActivateStorageAccessHeader[] = "activate-storage-access";
|
||||
|
||||
bool ShouldUpdateHeader(std::string_view name) {
|
||||
for (const auto* header : kNonUpdatedHeaders) {
|
||||
if (base::EqualsCaseInsensitiveASCII(name, header))
|
||||
@ -1094,7 +1096,11 @@ bool HttpResponseHeaders::IsRedirect(std::string* location) const {
|
||||
}
|
||||
|
||||
bool HttpResponseHeaders::HasStorageAccessRetryHeader() const {
|
||||
return HasHeaderValue("Activate-Storage-Access", "retry");
|
||||
return HasHeaderValue(kActivateStorageAccessHeader, "retry");
|
||||
}
|
||||
|
||||
bool HttpResponseHeaders::HasStorageAccessLoadHeader() const {
|
||||
return HasHeaderValue(kActivateStorageAccessHeader, "load");
|
||||
}
|
||||
|
||||
// static
|
||||
|
@ -308,6 +308,10 @@ class NET_EXPORT HttpResponseHeaders
|
||||
// header.
|
||||
bool HasStorageAccessRetryHeader() const;
|
||||
|
||||
// Returns true if this response included the `Activate-Storage-Access: load`
|
||||
// header.
|
||||
bool HasStorageAccessLoadHeader() const;
|
||||
|
||||
// Returns true if the HTTP response code passed in corresponds to a
|
||||
// redirect.
|
||||
static bool IsRedirectResponseCode(int response_code);
|
||||
|
@ -18,6 +18,7 @@
|
||||
#include "base/types/pass_key.h"
|
||||
#include "base/values.h"
|
||||
#include "net/base/auth.h"
|
||||
#include "net/base/features.h"
|
||||
#include "net/base/io_buffer.h"
|
||||
#include "net/base/load_flags.h"
|
||||
#include "net/base/load_timing_info.h"
|
||||
@ -1298,6 +1299,15 @@ void URLRequest::set_socket_tag(const SocketTag& socket_tag) {
|
||||
socket_tag_ = socket_tag;
|
||||
}
|
||||
|
||||
bool URLRequest::ShouldSetLoadWithStorageAccess() const {
|
||||
// TODO(https://crbug.com/344608182): this should probably only return true if
|
||||
// the Storage-Access state is "inactive" or "active"/allowed. For now, this
|
||||
// is fine because this is only used to set the renderer's bool, which is
|
||||
// untrusted anyway.
|
||||
return base::FeatureList::IsEnabled(features::kStorageAccessHeaderLoad) &&
|
||||
response_headers() && response_headers()->HasStorageAccessLoadHeader();
|
||||
}
|
||||
|
||||
base::WeakPtr<URLRequest> URLRequest::GetWeakPtr() {
|
||||
return weak_factory_.GetWeakPtr();
|
||||
}
|
||||
|
@ -847,6 +847,10 @@ class NET_EXPORT URLRequest : public base::SupportsUserData {
|
||||
}
|
||||
bool has_storage_access() const { return has_storage_access_; }
|
||||
|
||||
// Returns true if the corresponding `URLResponseHead`'s
|
||||
// `load_with_storage_access` field should be set.
|
||||
bool ShouldSetLoadWithStorageAccess() const;
|
||||
|
||||
static bool DefaultCanUseCookies();
|
||||
|
||||
base::WeakPtr<URLRequest> GetWeakPtr();
|
||||
|
@ -315,4 +315,8 @@ struct URLResponseHead {
|
||||
|
||||
// True if the response used a shared dictionary for decoding its body.
|
||||
bool did_use_shared_dictionary = false;
|
||||
|
||||
// True if the client should load the response content after setting the
|
||||
// environment's `has storage access` bit.
|
||||
bool load_with_storage_access = false;
|
||||
};
|
||||
|
@ -1431,6 +1431,9 @@ mojom::URLResponseHeadPtr URLLoader::BuildResponseHead() const {
|
||||
response->client_address_space =
|
||||
private_network_access_checker_.ClientAddressSpace();
|
||||
|
||||
response->load_with_storage_access =
|
||||
url_request_->ShouldSetLoadWithStorageAccess();
|
||||
|
||||
return response;
|
||||
}
|
||||
|
||||
|
@ -65,6 +65,7 @@
|
||||
#include "net/dns/mock_host_resolver.h"
|
||||
#include "net/http/http_network_session.h"
|
||||
#include "net/http/http_response_info.h"
|
||||
#include "net/http/http_status_code.h"
|
||||
#include "net/http/http_transaction_test_util.h"
|
||||
#include "net/log/net_log_event_type.h"
|
||||
#include "net/log/test_net_log.h"
|
||||
@ -4934,6 +4935,75 @@ TEST_F(URLLoaderTest, AllowAllCookies) {
|
||||
EXPECT_TRUE(url_loader->AllowFullCookies(third_party_url, site_for_cookies));
|
||||
}
|
||||
|
||||
class StorageAccessHeaderURLLoaderTest : public URLLoaderTest {
|
||||
public:
|
||||
StorageAccessHeaderURLLoaderTest() {
|
||||
features_.InitAndEnableFeature(net::features::kStorageAccessHeaderLoad);
|
||||
}
|
||||
|
||||
protected:
|
||||
static constexpr char kStorageAccessRedirectLoadPath[] =
|
||||
"/redirect-load-with-storage-access";
|
||||
|
||||
private:
|
||||
std::unique_ptr<net::test_server::HttpResponse>
|
||||
HandleLoadWithStorageAccessRequest(
|
||||
const net::test_server::HttpRequest& request) {
|
||||
if (!base::StartsWith(request.GetURL().path(),
|
||||
kStorageAccessRedirectLoadPath)) {
|
||||
return nullptr;
|
||||
}
|
||||
auto http_response =
|
||||
std::make_unique<net::test_server::BasicHttpResponse>();
|
||||
http_response->set_content_type("text/plain");
|
||||
http_response->AddCustomHeader("Activate-Storage-Access", "load");
|
||||
http_response->set_code(net::HTTP_PERMANENT_REDIRECT);
|
||||
http_response->AddCustomHeader("Location", "/empty.html");
|
||||
return http_response;
|
||||
}
|
||||
base::test::ScopedFeatureList features_;
|
||||
};
|
||||
|
||||
TEST_F(StorageAccessHeaderURLLoaderTest, StorageAccessHeader_Load) {
|
||||
base::RunLoop delete_run_loop;
|
||||
ResourceRequest request = CreateResourceRequest(
|
||||
"GET", test_server_.GetURL("/set-header?Activate-Storage-Access: load"));
|
||||
|
||||
mojo::PendingRemote<mojom::URLLoader> loader;
|
||||
std::unique_ptr<URLLoader> url_loader;
|
||||
context().mutable_factory_params().process_id = mojom::kBrowserProcessId;
|
||||
url_loader = URLLoaderOptions().MakeURLLoader(
|
||||
context(), DeleteLoaderCallback(&delete_run_loop, &url_loader),
|
||||
loader.InitWithNewPipeAndPassReceiver(), request,
|
||||
client()->CreateRemote());
|
||||
|
||||
client()->RunUntilComplete();
|
||||
delete_run_loop.Run();
|
||||
|
||||
EXPECT_TRUE(client()->response_head()->load_with_storage_access);
|
||||
}
|
||||
|
||||
TEST_F(StorageAccessHeaderURLLoaderTest, StorageAccessHeader_RedirectWithLoad) {
|
||||
base::RunLoop delete_run_loop;
|
||||
ResourceRequest request = CreateResourceRequest(
|
||||
"GET", test_server_.GetURL(kStorageAccessRedirectLoadPath));
|
||||
|
||||
mojo::PendingRemote<mojom::URLLoader> loader;
|
||||
std::unique_ptr<URLLoader> url_loader;
|
||||
context().mutable_factory_params().process_id = mojom::kBrowserProcessId;
|
||||
url_loader = URLLoaderOptions().MakeURLLoader(
|
||||
context(), DeleteLoaderCallback(&delete_run_loop, &url_loader),
|
||||
loader.InitWithNewPipeAndPassReceiver(), request,
|
||||
client()->CreateRemote());
|
||||
|
||||
client()->RunUntilComplete();
|
||||
delete_run_loop.Run();
|
||||
|
||||
// The redirect response included the `load` header, but the final response
|
||||
// did not, so the URLLoader should not propagate it.
|
||||
EXPECT_FALSE(client()->response_head()->load_with_storage_access);
|
||||
}
|
||||
|
||||
class URLLoaderCookieSettingOverridesTest
|
||||
: public URLLoaderTest,
|
||||
public ::testing::WithParamInterface<std::tuple<bool, bool, bool, bool>> {
|
||||
|
Reference in New Issue
Block a user