Implement navigation plumbing for per-frame requestStorageAccess
This CL adds plumbing to (conditionally) convey the initiator's `has_storage_access` state to a new document during navigation. Please see the "Navigations" section of the design doc: https://docs.google.com/document/d/1tMFYW_6Av8x-6ercbnMFUtBqpfvT97Nt8Jffsx1qve0/edit?usp=sharing (Note also the discussion in the Security section.) Bug: 1401089 Change-Id: Ie023b48873df50431759265771b0fc0ec9e227d0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4235906 Commit-Queue: Chris Fredrickson <cfredric@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/main@{#1107789}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
e1ace5e8c2
commit
73ba749d46
content
browser
renderer_host
ipc_utils.ccipc_utils.hnavigation_controller_impl.ccnavigation_entry_impl.ccnavigation_request.ccrender_frame_host_impl.cc
security_exploit_browsertest.ccpublic
renderer
third_party/blink
public
renderer
@ -70,6 +70,32 @@ bool VerifyInitiatorOrigin(int process_id,
|
||||
return true;
|
||||
}
|
||||
|
||||
bool VerifyHasStorageAccess(
|
||||
const RenderFrameHostImpl& current_rfh,
|
||||
blink::LocalFrameToken* initiator_frame_token,
|
||||
const blink::mojom::CommonNavigationParams& common_params) {
|
||||
if (!common_params.has_storage_access) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// The initiator origin must be provided, and must be same-origin with the
|
||||
// request URL.
|
||||
if (!common_params.initiator_origin.has_value() ||
|
||||
!common_params.initiator_origin.value().IsSameOriginWith(
|
||||
common_params.url)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// The initiator's frame token must be provided and must be equal to the
|
||||
// current frame token.
|
||||
if (!initiator_frame_token ||
|
||||
*initiator_frame_token != current_rfh.GetFrameToken()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
bool VerifyDownloadUrlParams(SiteInstance* site_instance,
|
||||
@ -150,7 +176,9 @@ bool VerifyOpenURLParams(RenderFrameHostImpl* current_rfh,
|
||||
}
|
||||
|
||||
bool VerifyBeginNavigationCommonParams(
|
||||
const RenderFrameHostImpl& current_rfh,
|
||||
SiteInstance* site_instance,
|
||||
blink::LocalFrameToken* initiator_frame_token,
|
||||
blink::mojom::CommonNavigationParams* common_params) {
|
||||
DCHECK_CURRENTLY_ON(BrowserThread::UI);
|
||||
DCHECK(site_instance);
|
||||
@ -206,6 +234,14 @@ bool VerifyBeginNavigationCommonParams(
|
||||
if (NavigationTypeUtils::IsSameDocument(common_params->navigation_type))
|
||||
return false;
|
||||
|
||||
// Verify |has_storage_access|. This corresponds to some of the changes to
|
||||
// "create navigation params by fetching" in the Storage Access API spec:
|
||||
// https://privacycg.github.io/storage-access/#navigation
|
||||
if (!VerifyHasStorageAccess(current_rfh, initiator_frame_token,
|
||||
*common_params)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Verification succeeded.
|
||||
return true;
|
||||
}
|
||||
|
@ -59,7 +59,9 @@ bool VerifyOpenURLParams(RenderFrameHostImpl* current_rfh,
|
||||
//
|
||||
// This function has to be called on the UI thread.
|
||||
bool VerifyBeginNavigationCommonParams(
|
||||
const RenderFrameHostImpl& current_rfh,
|
||||
SiteInstance* site_instance,
|
||||
blink::LocalFrameToken* initiator_frame_token,
|
||||
blink::mojom::CommonNavigationParams* common_params);
|
||||
|
||||
// Verify that the initiator frame identified by `initiator_frame_token` and
|
||||
|
@ -3855,7 +3855,8 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams(
|
||||
network::mojom::CSPDisposition::CHECK, std::vector<int>(),
|
||||
params.href_translate,
|
||||
false /* is_history_navigation_in_new_child_frame */,
|
||||
params.input_start, network::mojom::RequestDestination::kEmpty);
|
||||
params.input_start, network::mojom::RequestDestination::kEmpty,
|
||||
/*has_storage_access=*/false);
|
||||
|
||||
blink::mojom::CommitNavigationParamsPtr commit_params =
|
||||
blink::mojom::CommitNavigationParams::New(
|
||||
|
@ -861,7 +861,8 @@ NavigationEntryImpl::ConstructCommonNavigationParams(
|
||||
has_user_gesture(), false /* has_text_fragment_token */,
|
||||
network::mojom::CSPDisposition::CHECK, std::vector<int>(), std::string(),
|
||||
false /* is_history_navigation_in_new_child_frame */, input_start,
|
||||
network::mojom::RequestDestination::kEmpty);
|
||||
network::mojom::RequestDestination::kEmpty,
|
||||
false /* has_storage_access */);
|
||||
}
|
||||
|
||||
blink::mojom::CommitNavigationParamsPtr
|
||||
|
@ -1356,7 +1356,8 @@ NavigationRequest::CreateForSynchronousRendererCommit(
|
||||
std::string() /* href_translate */,
|
||||
false /* is_history_navigation_in_new_child_frame */,
|
||||
base::TimeTicks::Now() /* input_start */,
|
||||
network::mojom::RequestDestination::kEmpty);
|
||||
network::mojom::RequestDestination::kEmpty,
|
||||
/*has_storage_access=*/false);
|
||||
// Note that some params are set to default values (e.g. page_state set to
|
||||
// the default blink::PageState()) even if the DidCommit message that came
|
||||
// from the renderer contained relevant info that can be used to fill the
|
||||
@ -3057,6 +3058,9 @@ void NavigationRequest::OnRequestRedirected(
|
||||
did_receive_early_hints_before_cross_origin_redirect_ |=
|
||||
did_create_early_hints_manager_params_ && !is_same_origin_redirect;
|
||||
|
||||
common_params_->has_storage_access =
|
||||
common_params_->has_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;
|
||||
|
@ -8219,8 +8219,10 @@ void RenderFrameHostImpl::BeginNavigation(
|
||||
|
||||
blink::mojom::CommonNavigationParamsPtr validated_common_params =
|
||||
unvalidated_common_params.Clone();
|
||||
if (!VerifyBeginNavigationCommonParams(GetSiteInstance(),
|
||||
&*validated_common_params)) {
|
||||
if (!VerifyBeginNavigationCommonParams(
|
||||
*this, GetSiteInstance(),
|
||||
base::OptionalToPtr(begin_params->initiator_frame_token),
|
||||
&*validated_common_params)) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
@ -1518,7 +1518,8 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitViaDisabledWebSecurityTest,
|
||||
std::string() /* href_translate */,
|
||||
false /* is_history_navigation_in_new_child_frame */,
|
||||
base::TimeTicks() /* input_start */,
|
||||
network::mojom::RequestDestination::kDocument);
|
||||
network::mojom::RequestDestination::kDocument,
|
||||
false /* has_storage_access */);
|
||||
blink::mojom::BeginNavigationParamsPtr begin_params =
|
||||
blink::mojom::BeginNavigationParams::New(
|
||||
absl::nullopt /* initiator_frame_token */,
|
||||
|
@ -693,7 +693,8 @@ void RenderViewTest::Reload(const GURL& url) {
|
||||
network::mojom::CSPDisposition::CHECK, std::vector<int>(), std::string(),
|
||||
false /* is_history_navigation_in_new_child_frame */,
|
||||
base::TimeTicks() /* input_start */,
|
||||
network::mojom::RequestDestination::kDocument);
|
||||
network::mojom::RequestDestination::kDocument,
|
||||
false /* has_storage_access */);
|
||||
auto commit_params = blink::CreateCommitNavigationParams();
|
||||
TestRenderFrame* frame = static_cast<TestRenderFrame*>(GetMainRenderFrame());
|
||||
FrameLoadWaiter waiter(frame);
|
||||
@ -825,7 +826,8 @@ void RenderViewTest::GoToOffset(int offset,
|
||||
network::mojom::CSPDisposition::CHECK, std::vector<int>(), std::string(),
|
||||
false /* is_history_navigation_in_new_child_frame */,
|
||||
base::TimeTicks() /* input_start */,
|
||||
network::mojom::RequestDestination::kDocument);
|
||||
network::mojom::RequestDestination::kDocument,
|
||||
false /* has_storage_access */);
|
||||
auto commit_params = blink::CreateCommitNavigationParams();
|
||||
commit_params->page_state = state.ToEncodedData();
|
||||
commit_params->nav_entry_id = pending_offset + 1;
|
||||
|
@ -599,7 +599,7 @@ blink::mojom::CommonNavigationParamsPtr MakeCommonNavigationParams(
|
||||
info->should_check_main_world_content_security_policy,
|
||||
initiator_origin_trial_features, info->href_translate.Latin1(),
|
||||
is_history_navigation_in_new_child_frame, info->input_start,
|
||||
request_destination);
|
||||
request_destination, info->has_storage_access);
|
||||
}
|
||||
|
||||
WebFrameLoadType NavigationTypeToLoadType(
|
||||
@ -2777,6 +2777,8 @@ void RenderFrameImpl::CommitNavigationWithParams(
|
||||
navigation_params->frame_load_type = load_type;
|
||||
navigation_params->history_item = item_for_history_navigation;
|
||||
|
||||
navigation_params->has_storage_access = common_params->has_storage_access;
|
||||
|
||||
if (!container_info) {
|
||||
// An empty network provider will always be created since it is expected in
|
||||
// a certain number of places.
|
||||
|
@ -262,6 +262,12 @@ struct CommonNavigationParams {
|
||||
// Indicates the request destination.
|
||||
network.mojom.RequestDestination request_destination =
|
||||
network.mojom.RequestDestination.kEmpty;
|
||||
|
||||
// Indicates whether the target document of this navigation should load with
|
||||
// the `has_storage_access` bit set. When this is sent from the renderer to
|
||||
// the browser, it is a suggestion that the browser will validate - and
|
||||
// possibly override, during the navigation for the next document.
|
||||
bool has_storage_access = false;
|
||||
};
|
||||
|
||||
// Provided by the browser -----------------------------------------------------
|
||||
|
@ -188,6 +188,9 @@ struct BLINK_EXPORT WebNavigationInfo {
|
||||
// alive until we create the NavigationRequest.
|
||||
CrossVariantMojoRemote<mojom::PolicyContainerHostKeepAliveHandleInterfaceBase>
|
||||
initiator_policy_container_keep_alive_handle;
|
||||
|
||||
// The initiator frame's LocalDOMWindow's has_storage_access state.
|
||||
bool has_storage_access = false;
|
||||
};
|
||||
|
||||
// This structure holds all information provided by the embedder that is
|
||||
@ -528,6 +531,9 @@ struct BLINK_EXPORT WebNavigationParams {
|
||||
// <enum_representing_runtime_enabled_feature, enabled/disabled>
|
||||
base::flat_map<::blink::mojom::RuntimeFeatureState, bool>
|
||||
modified_runtime_features;
|
||||
|
||||
// Whether the document should be loaded with the has_storage_access bit set.
|
||||
bool has_storage_access = false;
|
||||
};
|
||||
|
||||
} // namespace blink
|
||||
|
@ -577,6 +577,19 @@ void LocalFrameClientImpl::BeginNavigation(
|
||||
|
||||
navigation_info->impression = impression;
|
||||
|
||||
// Propagate `has_storage_access` to the next document under certain
|
||||
// circumstances. This corresponds to the "snapshotting source snapshot
|
||||
// params" change and some of the "create navigation params by fetching"
|
||||
// changes in the Storage Access API spec:
|
||||
// https://privacycg.github.io/storage-access/#navigation
|
||||
navigation_info->has_storage_access =
|
||||
origin_window && origin_window->HasStorageAccess() &&
|
||||
navigation_info->initiator_frame_token.has_value() &&
|
||||
navigation_info->initiator_frame_token.value() ==
|
||||
web_frame_->GetLocalFrameToken() &&
|
||||
web_frame_->GetSecurityOrigin().IsSameOriginWith(
|
||||
WebSecurityOrigin::Create(navigation_info->url_request.Url()));
|
||||
|
||||
// Can be null.
|
||||
LocalFrame* local_parent_frame = GetLocalParentFrame(web_frame_);
|
||||
|
||||
|
@ -313,6 +313,7 @@ struct SameSizeAsDocumentLoader
|
||||
absl::optional<ViewTransitionState> view_transition_state;
|
||||
absl::optional<FencedFrame::RedactedFencedFrameProperties>
|
||||
fenced_frame_properties;
|
||||
bool has_storage_access;
|
||||
};
|
||||
|
||||
// Asserts size of DocumentLoader, so that whenever a new attribute is added to
|
||||
@ -511,7 +512,8 @@ DocumentLoader::DocumentLoader(
|
||||
extra_data_(std::move(extra_data)),
|
||||
reduced_accept_language_(params_->reduced_accept_language),
|
||||
navigation_delivery_type_(params_->navigation_delivery_type),
|
||||
view_transition_state_(std::move(params_->view_transition_state)) {
|
||||
view_transition_state_(std::move(params_->view_transition_state)),
|
||||
has_storage_access_(params_->has_storage_access) {
|
||||
DCHECK(frame_);
|
||||
DCHECK(params_);
|
||||
|
||||
@ -648,6 +650,7 @@ DocumentLoader::CreateWebNavigationParamsToCloneDocument() {
|
||||
params->has_fenced_frame_reporting = has_fenced_frame_reporting_;
|
||||
params->reduced_accept_language = reduced_accept_language_;
|
||||
params->navigation_delivery_type = navigation_delivery_type_;
|
||||
params->has_storage_access = has_storage_access_;
|
||||
return params;
|
||||
}
|
||||
|
||||
@ -2299,6 +2302,10 @@ void DocumentLoader::InitializeWindow(Document* owner_document) {
|
||||
origin_agent_cluster) {
|
||||
agent->ForceOriginKeyedBecauseOfInheritance();
|
||||
}
|
||||
|
||||
if (has_storage_access_) {
|
||||
frame_->DomWindow()->SetHasStorageAccess();
|
||||
}
|
||||
} else {
|
||||
if (frame_->GetSettings()->GetShouldReuseGlobalForUnownedMainFrame() &&
|
||||
frame_->IsMainFrame()) {
|
||||
|
@ -813,6 +813,10 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
|
||||
|
||||
absl::optional<FencedFrame::RedactedFencedFrameProperties>
|
||||
fenced_frame_properties_;
|
||||
|
||||
// Indicates whether the document should be loaded with its has_storage_access
|
||||
// bit set.
|
||||
const bool has_storage_access_;
|
||||
};
|
||||
|
||||
DECLARE_WEAK_IDENTIFIER_MAP(DocumentLoader);
|
||||
|
@ -563,6 +563,32 @@ TEST_P(DocumentLoaderTest, SameOriginNavigation) {
|
||||
SecurityOrigin::Create(same_origin_url)),
|
||||
local_frame->DomWindow()->GetStorageKey());
|
||||
|
||||
EXPECT_FALSE(local_frame->DomWindow()->HasStorageAccess());
|
||||
|
||||
EXPECT_TRUE(local_frame->Loader()
|
||||
.GetDocumentLoader()
|
||||
->LastNavigationHadTrustedInitiator());
|
||||
}
|
||||
|
||||
TEST_P(DocumentLoaderTest, SameOriginNavigation_WithStorageAccess) {
|
||||
const KURL& requestor_url =
|
||||
KURL(NullURL(), "https://www.example.com/foo.html");
|
||||
WebViewImpl* web_view_impl =
|
||||
web_view_helper_.InitializeAndLoad("https://example.com/foo.html");
|
||||
|
||||
const KURL& same_origin_url =
|
||||
KURL(NullURL(), "https://www.example.com/bar.html");
|
||||
std::unique_ptr<WebNavigationParams> params =
|
||||
WebNavigationParams::CreateWithHTMLBufferForTesting(
|
||||
SharedBuffer::Create(), same_origin_url);
|
||||
params->requestor_origin = WebSecurityOrigin::Create(WebURL(requestor_url));
|
||||
params->has_storage_access = true;
|
||||
LocalFrame* local_frame =
|
||||
To<LocalFrame>(web_view_impl->GetPage()->MainFrame());
|
||||
local_frame->Loader().CommitNavigation(std::move(params), nullptr);
|
||||
|
||||
EXPECT_TRUE(local_frame->DomWindow()->HasStorageAccess());
|
||||
|
||||
EXPECT_TRUE(local_frame->Loader()
|
||||
.GetDocumentLoader()
|
||||
->LastNavigationHadTrustedInitiator());
|
||||
|
Reference in New Issue
Block a user