Skip to content

Commit

Permalink
[COOP] Fix noopener not being applied to same-origin-plus-coep cases
Browse files Browse the repository at this point in the history
COOP requires that when a frame opens a popup, if that frame is
cross-origin with its top frame, and its top frame COOP value is
same-origin, that popup should be opened with noopener.
This fixes the case where we have COOP: same-origin plus COEP:
require-corp, in which case COOP.value will be same-origin-plus-coep.

This fix also corrects the sandbox crash reported initially in the
linked bug.
Indeed sandboxed iframes have an opaque origin, and are therefore cross
origin with their top frame. Applying noopener ensures the initial empty
document is not cross origin isolated, which was the root cause of the
crash (before this, the initial empty document had coop:unsafe-none, but
was cross origin isolated)

Bug: 1181673
Fixed: 1181673

Change-Id: Iaef658778ac25da0c84763b6115ff40c105e618a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2712945
Commit-Queue: Pâris Meuleman <pmeuleman@chromium.org>
Auto-Submit: Pâris Meuleman <pmeuleman@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858605}
  • Loading branch information
ParisMeuleman authored and Chromium LUCI CQ committed Mar 1, 2021
1 parent b39c5bb commit 727b4fd
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 45 deletions.
73 changes: 38 additions & 35 deletions content/browser/cross_origin_opener_policy_browsertest.cc
Expand Up @@ -255,44 +255,47 @@ IN_PROC_BROWSER_TEST_P(CrossOriginOpenerPolicyBrowserTest,
IN_PROC_BROWSER_TEST_P(
CrossOriginOpenerPolicyBrowserTest,
NewPopupCOOP_SameOriginPolicyAndCrossOriginIframeSetsNoopener) {
GURL starting_page(
https_server()->GetURL("a.com", "/cross_site_iframe_factory.html?a(b)"));
EXPECT_TRUE(NavigateToURL(shell(), starting_page));
for (auto coop_value : {CoopSameOriginPlusCoep(), CoopSameOrigin()}) {
GURL starting_page(https_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)"));
EXPECT_TRUE(NavigateToURL(shell(), starting_page));

RenderFrameHostImpl* main_frame = current_frame_host();
main_frame->set_cross_origin_opener_policy_for_testing(CoopSameOrigin());
RenderFrameHostImpl* main_frame = current_frame_host();
main_frame->set_cross_origin_opener_policy_for_testing(coop_value);

ShellAddedObserver new_shell_observer;
RenderFrameHostImpl* iframe = main_frame->child_at(0)->current_frame_host();
EXPECT_TRUE(ExecJs(iframe, "window.open('about:blank')"));
ShellAddedObserver new_shell_observer;
RenderFrameHostImpl* iframe = main_frame->child_at(0)->current_frame_host();
EXPECT_TRUE(ExecJs(iframe, "window.open('about:blank')"));

Shell* new_shell = new_shell_observer.GetShell();
RenderFrameHostImpl* popup_frame =
static_cast<WebContentsImpl*>(new_shell->web_contents())
->GetFrameTree()
->root()
->current_frame_host();
Shell* new_shell = new_shell_observer.GetShell();
RenderFrameHostImpl* popup_frame =
static_cast<WebContentsImpl*>(new_shell->web_contents())
->GetFrameTree()
->root()
->current_frame_host();

scoped_refptr<SiteInstance> main_frame_site_instance(
main_frame->GetSiteInstance());
scoped_refptr<SiteInstance> iframe_site_instance(iframe->GetSiteInstance());
scoped_refptr<SiteInstance> popup_site_instance(
popup_frame->GetSiteInstance());

ASSERT_TRUE(main_frame_site_instance);
ASSERT_TRUE(iframe_site_instance);
ASSERT_TRUE(popup_site_instance);
EXPECT_FALSE(main_frame_site_instance->IsRelatedSiteInstance(
popup_site_instance.get()));
EXPECT_FALSE(
iframe_site_instance->IsRelatedSiteInstance(popup_site_instance.get()));

// Check that `window.opener` is not set.
bool success = false;
EXPECT_TRUE(ExecuteScriptAndExtractBool(
new_shell, "window.domAutomationController.send(window.opener == null);",
&success));
EXPECT_TRUE(success) << "window.opener is set";
scoped_refptr<SiteInstance> main_frame_site_instance(
main_frame->GetSiteInstance());
scoped_refptr<SiteInstance> iframe_site_instance(iframe->GetSiteInstance());
scoped_refptr<SiteInstance> popup_site_instance(
popup_frame->GetSiteInstance());

ASSERT_TRUE(main_frame_site_instance);
ASSERT_TRUE(iframe_site_instance);
ASSERT_TRUE(popup_site_instance);
EXPECT_FALSE(main_frame_site_instance->IsRelatedSiteInstance(
popup_site_instance.get()));
EXPECT_FALSE(
iframe_site_instance->IsRelatedSiteInstance(popup_site_instance.get()));

// Check that `window.opener` is not set.
bool success = false;
EXPECT_TRUE(ExecuteScriptAndExtractBool(
new_shell,
"window.domAutomationController.send(window.opener == null);",
&success));
EXPECT_TRUE(success) << "window.opener is set";
}
}

IN_PROC_BROWSER_TEST_P(CrossOriginOpenerPolicyBrowserTest,
Expand Down Expand Up @@ -2410,7 +2413,7 @@ IN_PROC_BROWSER_TEST_P(CrossOriginOpenerPolicyBrowserTest,
{
RenderFrameHostImpl* popup_frame =
static_cast<WebContentsImpl*>(
OpenPopup(iframe, isolated_page, "")->web_contents())
OpenPopup(iframe, isolated_page, "", "", false)->web_contents())
->GetFrameTree()
->root()
->current_frame_host();
Expand Down
34 changes: 24 additions & 10 deletions content/browser/renderer_host/render_frame_host_impl.cc
Expand Up @@ -5353,16 +5353,30 @@ void RenderFrameHostImpl::CreateNewWindow(
} else {
// The documents are cross origin, leave COOP of the popup to the default
// unsafe-none.
// Then set the popup to noopener if the top level COOP is same origin.
if (top_level_opener->cross_origin_opener_policy().value ==
network::mojom::CrossOriginOpenerPolicyValue::kSameOrigin) {
DCHECK(base::FeatureList::IsEnabled(
network::features::kCrossOriginOpenerPolicy));
params->opener_suppressed = true;
// The frame name should not be forwarded to a noopener popup.
// TODO(https://crbug.com/1060691) This should be applied to all
// popups opened with noopener.
params->frame_name.clear();
switch (top_level_opener->cross_origin_opener_policy().value) {
// Those values are explicitly listed here, to force creator of new
// values to make an explicit decision in the future.
// See regression: https://crbug.com/1181673
case network::mojom::CrossOriginOpenerPolicyValue::kUnsafeNone:
case network::mojom::CrossOriginOpenerPolicyValue::kSameOriginAllowPopups:
break;

// See https://html.spec.whatwg.org/#browsing-context-names (step 8)
// ```
// If current's top-level browsing context's active document's
// cross-origin opener policy's value is "same-origin" or
// "same-origin-plus-COEP", then [...] set noopener to true, name to
// "_blank", and windowType to "new with no opener".
// ```
case network::mojom::CrossOriginOpenerPolicyValue::kSameOrigin:
case network::mojom::CrossOriginOpenerPolicyValue::kSameOriginPlusCoep:
DCHECK(base::FeatureList::IsEnabled(
network::features::kCrossOriginOpenerPolicy));
params->opener_suppressed = true;
// The frame name should not be forwarded to a noopener popup.
// TODO(https://crbug.com/1060691) This should be applied to all
// popups opened with noopener.
params->frame_name.clear();
}
}

Expand Down
@@ -0,0 +1,59 @@
<!doctype html>
<title>Sandboxed Cross-Origin-Opener-Policy popup should result in a network error</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src="/common/utils.js"></script> <!-- Use token() to allow running tests in parallel -->
<div id=log>
<script>
[
"allow-popups allow-scripts allow-same-origin",
"allow-popups allow-scripts",
].forEach(sandboxValue => {
async_test(t => {
const frame = document.createElement("iframe");
const channel = new BroadcastChannel(token());
channel.onmessage = t.unreached_func("A COOP popup was created from a sandboxed frame");
t.add_cleanup(() => frame.remove());
frame.sandbox = sandboxValue;
frame.srcdoc = `<script>
const popup = window.open("resources/coop-coep.py?coop=same-origin&coep=&channel=${channel.name}");
<\/script>`;
document.body.append(frame);
addEventListener('load', t.step_func(() => {
// This uses a timeout to give some time for incorrect implementations to broadcast. A
// theoretical testdriver.js API for browsing contexts could be used to speed this up.
t.step_timeout(() => {
t.done()
}, 1500);
}));
}, `<iframe sandbox="${sandboxValue}"> ${document.title}`);
});

// Verify that the popup does not have sandboxing flags set
async_test(t => {
const frame = document.createElement("iframe");
const channel = new BroadcastChannel(token());
channel.onmessage = t.step_func_done();
t.add_cleanup(() => frame.remove());
frame.sandbox = "allow-popups allow-scripts allow-popups-to-escape-sandbox";
frame.srcdoc = `<script>
window.open("resources/coop-coep.py?coop=same-origin&coep=require-corp&channel=${channel.name}");
<\/script>`;
document.body.append(frame);
}, `<iframe sandbox="allow-popups allow-scripts allow-popups-to-escape-sandbox"> ${document.title}`);

async_test(t => {
const frame = document.createElement("iframe");
const channel = new BroadcastChannel(token());
frame.sandbox = "allow-scripts allow-same-origin";
frame.name = `iframe-${channel.name}`;
frame.src = `resources/coop-coep.py?coop=same-origin&coep=require-corp&channel=${channel.name}`;
channel.onmessage = t.step_func( event => {
const payload = event.data;
assert_equals(payload.name, frame.name, "name");
t.done();
});
t.add_cleanup(() => frame.remove());
document.body.append(frame);
}, `Iframe with sandbox and COOP must load.`);
</script>
@@ -0,0 +1,2 @@
Cross-Origin-Opener-Policy: same-origin
Cross-Origin-Embedder-Policy: require-corp

0 comments on commit 727b4fd

Please sign in to comment.