Skip to content

Commit

Permalink
Merge to M115 of "Fenced frames: Local network access."
Browse files Browse the repository at this point in the history
This CL is to merge the Fenced frames local network access support
(crrev.com/c/4595521) back to M115.



Original CL description:

This is a reland of commit c02af16

Previous submit caused test timeout for the two new tests:
1. fenced-frame-subresource-fetch.tentative.https.window.js
2. fenced-frame.tentative.https.window.js

For fenced frames, we obtain messages by writing value to/reading
value from a server. This is significantly slower than similar tests
using iframes, because iframes can simply use `window.postMessage()`.

I've specified long timeout for both tests by `// META: timeout=long`.

Otherwise this CL is the same as the original one.

Original change's description:
> Fenced frames: Local network access.
>
> Fenced frames are only allowed in secure context. So the tests are
> all in secure contexts.
>
> 1. Subresource fetch:
>
> Fenced frame's IP address space is set to `kPublic` in order to make
> it subject to local network access check.
>
> web_tests/external/wpt/fetch/local-network-access/
> fetch.https.window.js is replicated and replaced iframes with fenced
> frames. All test cases are passing with the same behaviors as iframes.
>
> 2. Document fetch:
>
> Fenced frame's document fetch initiator can only be the parent.
> Fenced frames can only be navigated in two ways:
>
> 1. Directly by their parent, and never by another frame at a distance
> via `window.location` or `window.open`; in this case the `ClientSecurityState` needs to come from the parent.
> 2. By themselves; in this case the `ClientSecurityState` also needs to
> come from its embedder/parent.
>
> The ClientSecurityState of its parent is supplied to the
> NavigationURLLoader.
>
> web_tests/external/wpt/fetch/local-network-access/
> iframe.tentative.https.window.js is replicated and replaced iframes
> with fenced frames. All test cases have the same results as the
> iframe test expectations, except one:
>
> treat-as-public-address to local (same-origin): no preflight
> required
>
> - Iframe: the request is made without preflight. The nested iframe is
> loaded successfully.
>
> - Fenced frame: a preflight is made, and gets blocked. See a. below.
>
> I changed the test expectation for this test only. (PASS for iframe,
> but FAIL for fenced frame)
>
> Here are some noteworthy things we observed for document fetch. The
> following only applies to embedder-initiated navigations (i.e., the
> initial navigation of the frame):
> a. Fenced frame's document fetch's preflight request is always sent
> with `Origin: null`. This applies to embedder-initiated navigations
> (i.e., the initial navigation of the frame). I think this affects the
> outcome of Local Network Access check algorithm.
> https://source.chromium.org/chromium/chromium/src/+/main:content/browser/fenced_frame/fenced_frame.cc;l=119-126?q=fencedframe::n&ss=chromium%2Fchromium%2Fsrc
>
> A `null` origin implies
> LocalNetworkAccessChecker::is_potentially_trustworthy_same_origin_
> will always be false.
>
> b. For testing purposes, we tried manually overriding the initiator
> origin with a real origin and found that the preflight request still
> failed. This is because the credentials mode of the navigation is
> `'include'`, which prevents `Access-Control-Allow-Origin: '*'` from
> working, which iframes equivalently suffer from: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/fetch/local-network-access/iframe.tentative.https.window-expected.txt?q=%22FAIL%20%22%20f:third_party%2Fblink%2Fweb_tests%2Fexternal%2Fwpt%2Ffetch%2Flocal-network-access%2Fiframe.tentative.https.window-expected.txt.
>
> Bug: 1420626
> Change-Id(Omit): I74c97369d235e1725c650bfe87f29372992cb56b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4532557
> Reviewed-by: Titouan Rigoudy <titouan@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Reviewed-by: Weizhong Xia <weizhong@google.com>
> Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1154027}

Change-Id(Omit): I5c1f326e991f14125f5c3991046553d6e3200784
(cherry picked from commit 7fbb078)

Bug: 1420626, 1451954
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4595521
Reviewed-by: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Weizhong Xia <weizhong@google.com>
Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1155231}
Change-Id: I5c1f326e991f14125f5c3991046553d6e3200784
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4606102
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#659}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
xiaochen-z authored and Chromium LUCI CQ committed Jun 12, 2023
1 parent c9ef899 commit cd34b0b
Show file tree
Hide file tree
Showing 24 changed files with 913 additions and 36 deletions.
31 changes: 30 additions & 1 deletion content/browser/renderer_host/navigation_request.cc
Expand Up @@ -4940,13 +4940,42 @@ void NavigationRequest::OnStartChecksComplete(
// there is no client security state for top-level navigations, which mainly
// means that Private Network Access checks are skipped for such requests.
//
// For fenced frames, document fetch initiator can only be the parent. Fenced
// frames can only be navigated in two ways:
// 1. Directly by their parent, and never by another frame at a distance via
// `window.location` or `window.open`; in this case the `ClientSecurityState`
// needs to come from the parent.
// 2. By themselves; in this case the `ClientSecurityState` needs to come
// from the initiator. However, currently the support for getting the
// initator's client security state has not been implemented yet. The one from
// its embedder/parent is used instead. Fenced frame always has an outer
// document, `GetParentFrameOrOuterDocument()` will never be nullptr.
//
// NOTE: For embedder initiated fenced frame navigation that is subject to
// private network access checks:
// 1. The preflight request is sent with an opaque origin: "Origin: null".
// See: `FencedFrame::Navigate()`.
// 2. The credentials mode of the preflight request is "include". This
// prevents response header `Access-Control-Allow-Origin: '*'` from working.
// The response header must explicitly specify the origin.
// 3. However, we cannot know the origin because of 1.
// 4. It is also unsafe to respond to the preflight with response header
// `Access-Control-Allow-Origin: 'null'`. See:
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin
//
// This implies there is a limitation for fenced frame that sends a preflight
// request because of private network access. Fenced frame embedder
// initiated private network access always fails.
//
// TODO(https://crbug.com/1291252): Use the client security state of the
// navigation initiator instead.
//
// TODO(https://crbug.com/1129326): Figure out the UX story for main-frame
// navigations, then revisit the exception made in that case.
network::mojom::ClientSecurityStatePtr client_security_state = nullptr;
RenderFrameHostImpl* parent = GetParentFrame();
RenderFrameHostImpl* parent = (frame_tree_node()->IsFencedFrameRoot())
? GetParentFrameOrOuterDocument()
: GetParentFrame();
if (parent) {
client_security_state = parent->BuildClientSecurityState();

Expand Down
7 changes: 6 additions & 1 deletion content/browser/renderer_host/render_frame_host_impl.cc
Expand Up @@ -2990,10 +2990,15 @@ void RenderFrameHostImpl::InitializePolicyContainerHost(

// Note: the full constructor is used, to force developers to make an
// explicit decision when adding new fields to the PolicyContainer.
// The IP address space of fenced frame is set to `kPublic`.
// 1. This makes it subject to local network access checks, restricting
// its ability to access the private network.
// 2. The IP address space of the parent does not leak to the fenced frame.
SetPolicyContainerHost(
base::MakeRefCounted<PolicyContainerHost>(PolicyContainerPolicies(
network::mojom::ReferrerPolicy::kDefault,
network::mojom::IPAddressSpace::kUnknown,
IsFencedFrameRoot() ? network::mojom::IPAddressSpace::kPublic
: network::mojom::IPAddressSpace::kUnknown,
/*is_web_secure_context=*/false,
std::vector<network::mojom::ContentSecurityPolicyPtr>(),
parent_policies.cross_origin_opener_policy,
Expand Down
10 changes: 7 additions & 3 deletions third_party/blink/web_tests/VirtualTestSuites
Expand Up @@ -1544,17 +1544,21 @@
"wpt_internal/fenced_frame",
"http/tests/inspector-protocol/fenced-frame",
"external/wpt/html/anonymous-iframe",
"external/wpt/fenced-frame"
"external/wpt/fenced-frame",
"external/wpt/fetch/local-network-access/fenced-frame-subresource-fetch.tentative.https.window.js",
"external/wpt/fetch/local-network-access/fenced-frame.tentative.https.window.js"
],
"exclusive_tests": [
"http/tests/fenced_frame",
"wpt_internal/fenced_frame",
"external/wpt/fenced-frame",
"http/tests/inspector-protocol/fenced-frame"
"http/tests/inspector-protocol/fenced-frame",
"external/wpt/fetch/local-network-access/fenced-frame-subresource-fetch.tentative.https.window.js",
"external/wpt/fetch/local-network-access/fenced-frame.tentative.https.window.js"
],
"args": ["--enable-features=FencedFrames:implementation_type/mparch,PrivacySandboxAdsAPIsOverride,SharedStorageAPI,NoncedPartitionedCookies,Fledge,InterestGroupStorage,AdInterestGroupAPI,AllowURNsInIframes,BiddingAndScoringDebugReportingAPI,ComputePressure",
"--enable-blink-features=FencedFramesAPIChanges,FencedFramesDefaultMode"],
"expires": "Jul 1, 2023"
"expires": "Dec 31, 2023"
},
{
"prefix": "cors-non-wildcard-request-headers",
Expand Down

0 comments on commit cd34b0b

Please sign in to comment.