Skip to content

Commit

Permalink
Feature-Policy: Opener policies
Browse files Browse the repository at this point in the history
This CL introduces the "inherited opener feature policies". This
includes the logic to propagate feature policy states from a browsing
context to the auxiliary browsing contexts.

As the first step (and hidden behind flag) all the feature policies
will be inherited by the auxiliary browsing context. The only exception
is when the original context is sandboxed but allows popups to escape
sandbox.

The inheritance model will be fine tuned in further work. Firstly, not
all features might follow this "sandbox-like" inheritance model. Also
possibly through introducing a new Feature Policy (that replicates
'allow-popups-to-escape-sandbox') and special casing "rel='noopener'"
there will be exit doors for the open contexts to *not* inherit the
policies.

These issues are currently publicly being tracked here:

w3c/webappsec-permissions-policy#264
w3c/webappsec-permissions-policy#252
w3c/webappsec-permissions-policy#259

Bug: 774620
Change-Id: Ic0b5ab8155c2e5d786bc51d3f9c3a601f7e4d8e9
Reviewed-on: https://chromium-review.googlesource.com/c/1384992
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633452}
  • Loading branch information
ehsan-karamad authored and Commit Bot committed Feb 19, 2019
1 parent b0a2bde commit 3940708
Show file tree
Hide file tree
Showing 49 changed files with 385 additions and 47 deletions.
8 changes: 8 additions & 0 deletions content/browser/frame_host/frame_tree_node.cc
Expand Up @@ -660,4 +660,12 @@ void FrameTreeNode::PruneChildFrameNavigationEntries(
}
}

void FrameTreeNode::SetOpenerFeaturePolicyState(
const blink::FeaturePolicy::FeatureState& feature_state) {
DCHECK(IsMainFrame());
if (base::FeatureList::IsEnabled(features::kFeaturePolicyForSandbox)) {
replication_state_.opener_feature_state = feature_state;
}
}

} // namespace content
5 changes: 5 additions & 0 deletions content/browser/frame_host/frame_tree_node.h
Expand Up @@ -411,6 +411,11 @@ class CONTENT_EXPORT FrameTreeNode {
blink::FrameOwnerElementType frame_owner_element_type() const {
return replication_state_.frame_owner_element_type;
}
// Only meaningful to call on a root frame. The value of |feature_state| will
// be nontrivial if there is an opener which is restricted in some of the
// feature policies.
void SetOpenerFeaturePolicyState(
const blink::FeaturePolicy::FeatureState& feature_state);

private:
FRIEND_TEST_ALL_PREFIXES(SitePerProcessFeaturePolicyBrowserTest,
Expand Down
9 changes: 9 additions & 0 deletions content/browser/frame_host/render_frame_host_impl.cc
Expand Up @@ -5592,6 +5592,15 @@ void RenderFrameHostImpl::CreateWebUsbService(

void RenderFrameHostImpl::ResetFeaturePolicy() {
RenderFrameHostImpl* parent_frame_host = GetParent();
if (!parent_frame_host && frame_tree_node_->opener() &&
!frame_tree_node_->current_replication_state()
.opener_feature_state.empty()) {
DCHECK(base::FeatureList::IsEnabled(features::kFeaturePolicyForSandbox));
feature_policy_ = blink::FeaturePolicy::CreateWithOpenerPolicy(
frame_tree_node_->current_replication_state().opener_feature_state,
last_committed_origin_);
return;
}
const blink::FeaturePolicy* parent_policy =
parent_frame_host ? parent_frame_host->feature_policy() : nullptr;
blink::ParsedFeaturePolicy container_policy =
Expand Down
98 changes: 98 additions & 0 deletions content/browser/site_per_process_browsertest.cc
Expand Up @@ -14337,4 +14337,102 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
actual_scroll_delta);
}

class FeaturePolicyPropagationToAuxiliaryBrowsingContextTest
: public SitePerProcessFeaturePolicyJavaScriptBrowserTest,
public testing::WithParamInterface<std::tuple<
const char* /* Whether or not <iframe> is sandbox or can escape it */,
bool /* <iframe> same origin? */,
bool /* opened window same origin? */,
const char* /* window feature in window.open() */>> {
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
SitePerProcessFeaturePolicyJavaScriptBrowserTest::SetUpCommandLine(
command_line);
feature_list_.InitAndEnableFeature(features::kFeaturePolicyForSandbox);
}

private:
base::test::ScopedFeatureList feature_list_;
};

// This test verifies the correct propagation of FeaturePolicy from an *opener*
// to the opened auxiliary browsing context. This test runs for both cross and
// same origin frames.
IN_PROC_BROWSER_TEST_P(FeaturePolicyPropagationToAuxiliaryBrowsingContextTest,
PropagationForDifferentOrigins) {
EXPECT_TRUE(NavigateToURL(
shell(), embedded_test_server()->GetURL(
"a.com", "/feature_policy_window_open_embedder.html")));
const GURL same_origin_child = embedded_test_server()->GetURL(
"a.com", "/feature_policy_window_open_embedded.html");
const GURL cross_origin_child = embedded_test_server()->GetURL(
"b.com", "/feature_policy_window_open_embedded.html");
const GURL cross_origin_window_url =
embedded_test_server()->GetURL("c.com", "/title1.html");
const GURL same_origin_window_url = GURL("about:blank");
const std::string kScriptCheckPolicy =
"document.featurePolicy.allowsFeature('sync-xhr')";
// Test parameters
const char* iframe_type = std::get<0>(GetParam());
const char* iframe_src =
(std::get<1>(GetParam()) ? same_origin_child : cross_origin_child)
.spec()
.c_str();
const char* window_url = (std::get<2>(GetParam()) ? same_origin_window_url
: cross_origin_window_url)
.spec()
.c_str();
const char* window_feature = std::get<3>(GetParam());
SCOPED_TRACE(testing::Message() << " <iframe> Type: " << iframe_type
<< " <iframe> Source: " << iframe_src
<< " window URL: " << window_url
<< " window Feature: " << window_feature);
// TODO(ekaramad): Modify the test expectation once we resolve the following
// issues in feature policies:
// - "rel='noopener'" might end up resetting the feature policies.
// - A new feature to escape feature policies is introduced (similar to the
// sandbox version "allow-popups-to-escape-sandbox".
// For now, only the sandbox-escaping case should be allowed to have a new
// (default) feature policy state.
bool expected_feature_state_in_auxiliary_browsing_context = iframe_type =
"sandboxed-escaping";
ShellAddedObserver shell_added_observer;
ASSERT_TRUE(
ExecJs(shell(), JsReplace("test($1, $2, $3, $4)", iframe_type, iframe_src,
window_url, window_feature)));
auto* new_shell = shell_added_observer.GetShell();
while (new_shell->web_contents()->GetLastCommittedURL() != GURL(window_url)) {
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
run_loop.Run();
}
ASSERT_TRUE(WaitForLoadStop(new_shell->web_contents()));
ASSERT_EQ(new_shell->web_contents()->GetLastCommittedURL(), window_url);
bool renderer_side_feature_state =
EvalJs(new_shell, kScriptCheckPolicy).ExtractBool();
bool browser_side_feature_state =
static_cast<RenderFrameHostImpl*>(
new_shell->web_contents()->GetMainFrame())
->feature_policy()
->IsFeatureEnabled(blink::mojom::FeaturePolicyFeature::kSyncXHR);
ASSERT_EQ(expected_feature_state_in_auxiliary_browsing_context,
browser_side_feature_state);
ASSERT_EQ(expected_feature_state_in_auxiliary_browsing_context,
renderer_side_feature_state);
new_shell->Close();
}

INSTANTIATE_TEST_CASE_P(SitePerProcess,
FeaturePolicyPropagationToAuxiliaryBrowsingContextTest,
testing::Combine(
/* iframe_type */
testing::ValuesIn({"sandboxed", "notsandboxed",
"sandboxed-escaping"}),
/* iframe_same_origin */
testing::Bool(),
/* window_same_origin */
testing::Bool(),
/* window feature */
testing::ValuesIn({"", "noopener"})));
} // namespace content
15 changes: 12 additions & 3 deletions content/browser/web_contents/web_contents_impl.cc
Expand Up @@ -756,10 +756,19 @@ std::unique_ptr<WebContentsImpl> WebContentsImpl::CreateWithOpener(
blink::WebSandboxFlags opener_flags = opener_rfh->active_sandbox_flags();
const blink::WebSandboxFlags inherit_flag =
blink::WebSandboxFlags::kPropagatesToAuxiliaryBrowsingContexts;
if ((opener_flags & inherit_flag) == inherit_flag) {
// TODO(iclelland): Transfer correct container policy from opener as well.
// https://crbug.com/774620
bool sandbox_propagates_to_auxilary_context =
(opener_flags & inherit_flag) == inherit_flag;
if (sandbox_propagates_to_auxilary_context)
new_root->SetPendingFramePolicy({opener_flags, {}});
if (opener_flags == blink::WebSandboxFlags::kNone ||
sandbox_propagates_to_auxilary_context) {
// TODO(ekaramad, iclelland): Do not propagate feature policies from non-
// sandboxed disowned openers (rel=noopener).
// If the current page is not sandboxed, or if the sandbox is to propagate
// to the popups then opener's feature policy will apply to the new popup
// as well.
new_root->SetOpenerFeaturePolicyState(
opener_rfh->feature_policy()->inherited_policies());
}
}

Expand Down
3 changes: 3 additions & 0 deletions content/child/runtime_features.cc
Expand Up @@ -417,6 +417,9 @@ void SetIndividualRuntimeFeatures(
WebRuntimeFeatures::EnableExperimentalProductivityFeatures(true);
}

if (base::FeatureList::IsEnabled(features::kFeaturePolicyForSandbox))
WebRuntimeFeatures::EnableFeaturePolicyForSandbox(true);

if (base::FeatureList::IsEnabled(features::kPageLifecycle))
WebRuntimeFeatures::EnablePageLifecycle(true);

Expand Down
1 change: 1 addition & 0 deletions content/common/frame_messages.h
Expand Up @@ -540,6 +540,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::FrameReplicationState)
IPC_STRUCT_TRAITS_MEMBER(feature_policy_header)
IPC_STRUCT_TRAITS_MEMBER(active_sandbox_flags)
IPC_STRUCT_TRAITS_MEMBER(frame_policy)
IPC_STRUCT_TRAITS_MEMBER(opener_feature_state)
IPC_STRUCT_TRAITS_MEMBER(accumulated_csp_headers)
IPC_STRUCT_TRAITS_MEMBER(scope)
IPC_STRUCT_TRAITS_MEMBER(insecure_request_policy)
Expand Down
4 changes: 4 additions & 0 deletions content/common/frame_replication_state.h
Expand Up @@ -102,6 +102,10 @@ struct CONTENT_EXPORT FrameReplicationState {
// frame.
blink::FramePolicy frame_policy;

// The state of feature policies in the opener browsing context. This field is
// only relevant for a root FrameTreeNode.
blink::FeaturePolicy::FeatureState opener_feature_state;

// Accumulated CSP headers - gathered from http headers, <meta> elements,
// parent frames (in case of about:blank frames).
std::vector<ContentSecurityPolicyHeader> accumulated_csp_headers;
Expand Down
5 changes: 5 additions & 0 deletions content/public/common/content_features.cc
Expand Up @@ -153,6 +153,11 @@ const base::Feature kExpensiveBackgroundTimerThrottling{
const base::Feature kExtendedMouseButtons{"ExtendedMouseButtons",
base::FEATURE_ENABLED_BY_DEFAULT};

// When enabled Feature Policy propagation is similar to sandbox flags and,
// sandbox flags are implemented on top of Feature Policy.
const base::Feature kFeaturePolicyForSandbox{"FeaturePolicyForSandbox",
base::FEATURE_DISABLED_BY_DEFAULT};

// Enables a blink::FontCache optimization that reuses a font to serve different
// size of font.
const base::Feature kFontCacheScaling{"FontCacheScaling",
Expand Down
1 change: 1 addition & 0 deletions content/public/common/content_features.h
Expand Up @@ -45,6 +45,7 @@ CONTENT_EXPORT extern const base::Feature kDesktopCaptureChangeSource;
CONTENT_EXPORT extern const base::Feature kExperimentalProductivityFeatures;
CONTENT_EXPORT extern const base::Feature kExpensiveBackgroundTimerThrottling;
CONTENT_EXPORT extern const base::Feature kExtendedMouseButtons;
CONTENT_EXPORT extern const base::Feature kFeaturePolicyForSandbox;
CONTENT_EXPORT extern const base::Feature kFontCacheScaling;
CONTENT_EXPORT extern const base::Feature kFontSrcLocalMatching;
CONTENT_EXPORT extern const base::Feature
Expand Down
3 changes: 2 additions & 1 deletion content/renderer/render_frame_impl.cc
Expand Up @@ -1326,7 +1326,8 @@ RenderFrameImpl* RenderFrameImpl::CreateMainFrame(
// This conversion is a little sad, as this often comes from a
// WebString...
WebString::FromUTF8(replicated_state.name),
replicated_state.frame_policy.sandbox_flags);
replicated_state.frame_policy.sandbox_flags,
replicated_state.opener_feature_state);
if (has_committed_real_load)
render_frame->frame_->SetCommittedFirstRealLoad();

Expand Down
6 changes: 4 additions & 2 deletions content/renderer/render_frame_proxy.cc
Expand Up @@ -347,7 +347,8 @@ void RenderFrameProxy::SetReplicatedState(const FrameReplicationState& state) {
web_frame_->SetReplicatedInsecureRequestPolicy(state.insecure_request_policy);
web_frame_->SetReplicatedInsecureNavigationsSet(
state.insecure_navigations_set);
web_frame_->SetReplicatedFeaturePolicyHeader(state.feature_policy_header);
web_frame_->SetReplicatedFeaturePolicyHeaderAndOpenerPolicies(
state.feature_policy_header, state.opener_feature_state);
if (state.has_received_user_gesture) {
web_frame_->UpdateUserActivationState(
blink::UserActivationUpdateType::kNotifyActivation);
Expand Down Expand Up @@ -392,7 +393,8 @@ void RenderFrameProxy::OnDidSetFramePolicyHeaders(
blink::WebSandboxFlags active_sandbox_flags,
blink::ParsedFeaturePolicy feature_policy_header) {
web_frame_->SetReplicatedSandboxFlags(active_sandbox_flags);
web_frame_->SetReplicatedFeaturePolicyHeader(feature_policy_header);
web_frame_->SetReplicatedFeaturePolicyHeaderAndOpenerPolicies(
feature_policy_header, blink::FeaturePolicy::FeatureState());
}

bool RenderFrameProxy::OnMessageReceived(const IPC::Message& msg) {
Expand Down
2 changes: 1 addition & 1 deletion content/renderer/render_view_browsertest.cc
Expand Up @@ -783,7 +783,7 @@ TEST_F(RenderViewImplTest, BeginNavigationForWebUI) {
blink::WebView* new_web_view = view()->CreateView(
GetMainFrame(), popup_request, blink::WebWindowFeatures(), "foo",
blink::kWebNavigationPolicyNewForegroundTab, false,
blink::WebSandboxFlags::kNone,
blink::WebSandboxFlags::kNone, blink::FeaturePolicy::FeatureState(),
blink::AllocateSessionStorageNamespaceId());
RenderViewImpl* new_view = RenderViewImpl::FromWebView(new_web_view);
auto popup_navigation_info = std::make_unique<blink::WebNavigationInfo>();
Expand Down
3 changes: 3 additions & 0 deletions content/renderer/render_view_impl.cc
Expand Up @@ -1307,6 +1307,7 @@ WebView* RenderViewImpl::CreateView(
WebNavigationPolicy policy,
bool suppress_opener,
WebSandboxFlags sandbox_flags,
const blink::FeaturePolicy::FeatureState& opener_feature_state,
const blink::SessionStorageNamespaceId& session_storage_namespace_id) {
RenderFrameImpl* creator_frame = RenderFrameImpl::FromWebFrame(creator);
mojom::CreateNewWindowParamsPtr params = mojom::CreateNewWindowParams::New();
Expand Down Expand Up @@ -1420,6 +1421,8 @@ WebView* RenderViewImpl::CreateView(
<< "Session storage namespace must be populated.";
view_params->replicated_frame_state.frame_policy.sandbox_flags =
sandbox_flags;
view_params->replicated_frame_state.opener_feature_state =
opener_feature_state;
view_params->replicated_frame_state.name = frame_name_utf8;
view_params->devtools_main_frame_token = reply->devtools_main_frame_token;
// Even if the main frame has a name, the main frame's unique name is always
Expand Down
21 changes: 12 additions & 9 deletions content/renderer/render_view_impl.h
Expand Up @@ -39,6 +39,7 @@
#include "ipc/ipc_platform_file.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "third_party/blink/public/common/dom_storage/session_storage_namespace_id.h"
#include "third_party/blink/public/common/feature_policy/feature_policy.h"
#include "third_party/blink/public/mojom/renderer_preference_watcher.mojom.h"
#include "third_party/blink/public/mojom/renderer_preferences.mojom.h"
#include "third_party/blink/public/platform/web_input_event.h"
Expand Down Expand Up @@ -221,15 +222,17 @@ class CONTENT_EXPORT RenderViewImpl : public blink::WebViewClient,

// blink::WebViewClient implementation --------------------------------------

blink::WebView* CreateView(blink::WebLocalFrame* creator,
const blink::WebURLRequest& request,
const blink::WebWindowFeatures& features,
const blink::WebString& frame_name,
blink::WebNavigationPolicy policy,
bool suppress_opener,
blink::WebSandboxFlags sandbox_flags,
const blink::SessionStorageNamespaceId&
session_storage_namespace_id) override;
blink::WebView* CreateView(
blink::WebLocalFrame* creator,
const blink::WebURLRequest& request,
const blink::WebWindowFeatures& features,
const blink::WebString& frame_name,
blink::WebNavigationPolicy policy,
bool suppress_opener,
blink::WebSandboxFlags sandbox_flags,
const blink::FeaturePolicy::FeatureState& opener_feature_state,
const blink::SessionStorageNamespaceId& session_storage_namespace_id)
override;
blink::WebPagePopup* CreatePopup(blink::WebLocalFrame* creator) override;
base::StringPiece GetSessionStorageNamespaceId() override;
void PrintPage(blink::WebLocalFrame* frame) override;
Expand Down
2 changes: 2 additions & 0 deletions content/shell/test_runner/web_view_test_proxy.cc
Expand Up @@ -38,6 +38,7 @@ blink::WebView* WebViewTestProxy::CreateView(
blink::WebNavigationPolicy policy,
bool suppress_opener,
blink::WebSandboxFlags sandbox_flags,
const blink::FeaturePolicy::FeatureState& opener_feature_state,
const blink::SessionStorageNamespaceId& session_storage_namespace_id) {
if (GetTestRunner()->shouldDumpNavigationPolicy()) {
delegate()->PrintMessage("Default policy for createView for '" +
Expand All @@ -54,6 +55,7 @@ blink::WebView* WebViewTestProxy::CreateView(
}
return RenderViewImpl::CreateView(creator, request, features, frame_name,
policy, suppress_opener, sandbox_flags,
blink::FeaturePolicy::FeatureState(),
session_storage_namespace_id);
}

Expand Down
1 change: 1 addition & 0 deletions content/shell/test_runner/web_view_test_proxy.h
Expand Up @@ -77,6 +77,7 @@ class TEST_RUNNER_EXPORT WebViewTestProxy : public content::RenderViewImpl {
blink::WebNavigationPolicy policy,
bool suppress_opener,
blink::WebSandboxFlags sandbox_flags,
const blink::FeaturePolicy::FeatureState&,
const blink::SessionStorageNamespaceId&
session_storage_namespace_id) override;
void PrintPage(blink::WebLocalFrame* frame) override;
Expand Down
7 changes: 7 additions & 0 deletions content/test/data/feature_policy_window_open_embedded.html
@@ -0,0 +1,7 @@
<title>Page that can opens a window to a given
origin provided through postMessage.</title>
<script>
window.addEventListener(
"message", (e) => window.open(
e.data.window_url, "foo", e.data.window_feature));
</script>
35 changes: 35 additions & 0 deletions content/test/data/feature_policy_window_open_embedder.html
@@ -0,0 +1,35 @@
<!doctype html>
<!-- Sandbox-ed <iframe> which does not allow escaping -->
<iframe id="frame1" allow="sync-xhr 'none'"
sandbox="allow-scripts allow-popups">
</iframe>
<!-- Sandbox-ed <iframe> which allows escaping -->
<iframe id="frame2" allow="sync-xhr 'none'"
sandbox="allow-scripts allow-popups allow-popups-to-escape-sandbox">
</iframe>
<!-- Not sandbox-ed <iframe> -->
<iframe id="frame3" allow="sync-xhr 'none'"></iframe>
<script>
var frame1 = document.getElementById("frame1"),
frame2 = document.getElementById("frame2"),
frame3 = document.getElementById("frame3");

var frame_map = {
"sandboxed": frame1,
"sandboxed-escaping": frame2,
"notsandboxed": frame3,
};

function test(iframe_type, iframe_src, window_url, window_feature) {
var iframe = frame_map[iframe_type];
iframe.src = iframe_src;
// Then message will trigger |window.open| to the url |window_url|.
iframe.addEventListener(
"load", () => iframe.contentWindow.postMessage(
{
window_url: window_url,
window_feature: window_feature
},
"*"));
}
</script>
10 changes: 10 additions & 0 deletions third_party/blink/common/feature_policy/feature_policy.cc
Expand Up @@ -130,6 +130,16 @@ std::unique_ptr<FeaturePolicy> FeaturePolicy::CreateFromParentPolicy(
GetDefaultFeatureList());
}

// static
std::unique_ptr<FeaturePolicy> FeaturePolicy::CreateWithOpenerPolicy(
const FeatureState& inherited_policies,
const url::Origin& origin) {
std::unique_ptr<FeaturePolicy> new_policy =
base::WrapUnique(new FeaturePolicy(origin, GetDefaultFeatureList()));
new_policy->inherited_policies_ = inherited_policies;
return new_policy;
}

bool FeaturePolicy::IsFeatureEnabled(
mojom::FeaturePolicyFeature feature) const {
mojom::PolicyValueType feature_type = feature_list_.at(feature).second;
Expand Down

0 comments on commit 3940708

Please sign in to comment.