Skip to content

Commit

Permalink
Fenced frames: Local network access.
Browse files Browse the repository at this point in the history
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: 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}
  • Loading branch information
xiaochen-z authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent 7aa1660 commit c02af16
Show file tree
Hide file tree
Showing 24 changed files with 911 additions and 36 deletions.
31 changes: 30 additions & 1 deletion content/browser/renderer_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4827,13 +4827,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
Original file line number Diff line number Diff line change
Expand Up @@ -2985,10 +2985,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
Original file line number Diff line number Diff line change
Expand Up @@ -1508,17 +1508,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 c02af16

Please sign in to comment.