Skip to content

GitLab

T
tangled
Commit 3adb3051 authored by Ian Clelland's avatar Ian Clelland Committed by Commit Bot
Browse files
Options

Correct inherited feature policy when allow attribute is used. 

This fixes an error in the inherited policy calculation where a frame
which was explicitly allowed to use a feature through the Feature-Policy
header would be blocked if the "allow" attribute was also present on
the iframe element.

The behaviour now matches the spec algorithm at
https://wicg.github.io/feature-policy/#define-inherited-policy-in-container

Tests have been added to catch this case.

Change-Id: I7a394202c615be3088fda738d7fc65f16bcadf34
Reviewed-on: https://chromium-review.googlesource.com/c/1293610Reviewed-by: default avatarLuna Lu <loonybear@chromium.org>
Commit-Queue: Ian Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605431}
parent c234788e
Hide whitespace changes
Inline Side-by-side
Showing with 177 additions and 18 deletions
third_party/WebKit/LayoutTests/external/wpt/feature-policy/feature-policy-header-policy-declined.https.sub.html 0 → 100644
View file @ 3adb3051
<!DOCTYPE html>
<body>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src=/feature-policy/resources/featurepolicy.js></script>
<!-- Feature-Policy: fullscreen cross_origin https://www.example.com; -->
<script>
'use strict';
var same_origin = 'https://{{domains[]}}:{{ports[https][0]}}';
var cross_origin = 'https://{{domains[www]}}:{{ports[https][0]}}';
var same_origin_src = '/feature-policy/resources/feature-policy-allowedfeatures.html';
var cross_origin_src = cross_origin + same_origin_src;
var header_policy = 'Feature-Policy: fullscreen \'self\' ' + cross_origin +
' https://www.example.com;';
// Test that fullscreen's allowlist is [same_origin, cross_origin, 'https://www.example.com']
test(function() {
assert_array_equals(
document.policy.getAllowlistForFeature('fullscreen'),
[cross_origin, 'https://www.example.com']);
}, header_policy + ' -- test allowlist is [cross_origin, https://www.example.com]');
// Test that fullscreen is disallowed on same_origin, allowed on some cross_origin subframes.
test_disallowed_feature_for_subframe(
header_policy + ' -- test fullscreen is allowed on same-origin subframe',
'fullscreen',
same_origin_src);
test_allowed_feature_for_subframe(
header_policy + ' -- test fullscreen is allowed on cross-origin ' + cross_origin_src + ' subframe',
'fullscreen',
cross_origin_src);
var cross_origin_src1 = 'https://{{domains[www1]}}:{{ports[https][0]}}' + same_origin_src;
test_disallowed_feature_for_subframe(
header_policy + ' -- test fullscreen is disallowed on cross-origin ' + cross_origin_src1 + ' subframe',
'fullscreen',
cross_origin_src1);
// dynamically update sub frame's container policy
var disallow = "fullscreen 'none';"
test_disallowed_feature_for_subframe(
header_policy + ', iframe.allow = ' + disallow + ' -- test fullscreen is disallowed on same-origin subframe',
'fullscreen',
same_origin_src,
disallow);
test_disallowed_feature_for_subframe(
header_policy + 'iframe.allow = ' + disallow + ' -- test fullscreen is allowed on specific cross-origin subframe',
'fullscreen',
cross_origin_src,
disallow);
var allow = "fullscreen " + cross_origin;
test_disallowed_feature_for_subframe(
header_policy + ', iframe.allow = ' + allow + ' -- test fullscreen is disallowed on same-origin subframe',
'fullscreen',
same_origin_src,
allow);
test_allowed_feature_for_subframe(
header_policy + 'iframe.allow = ' + allow + ' -- test fullscreen is allowed on specific cross-origin subframe',
'fullscreen',
cross_origin_src,
allow);
</script>
</body>
third_party/WebKit/LayoutTests/external/wpt/feature-policy/feature-policy-header-policy-declined.https.sub.html.sub.headers 0 → 100644
View file @ 3adb3051
Feature-Policy: fullscreen https://{{domains[www]}}:{{ports[https][0]}} https://www.example.com;
third_party/blink/common/feature_policy/feature_policy.cc
View file @ 3adb3051
......@@ -191,6 +191,10 @@ std::unique_ptr<FeaturePolicy> FeaturePolicy::CreateFromParentPolicy(
std::unique_ptr<FeaturePolicy> new_policy =
base::WrapUnique(new FeaturePolicy(origin, features));
// For features which are not keys in a container policy, which is the case
// here *until* we call AddContainerPolicy at the end of this method,
// https://wicg.github.io/feature-policy/#define-inherited-policy-in-container
// returns true if |feature| is enabled in |parent_policy| for |origin|.
for (const auto& feature : features) {
if (!parent_policy ||
parent_policy->IsFeatureEnabledForOrigin(feature.first, origin)) {
......@@ -208,11 +212,16 @@ void FeaturePolicy::AddContainerPolicy(
const ParsedFeaturePolicy& container_policy,
const FeaturePolicy* parent_policy) {
DCHECK(parent_policy);
// For features which are keys in a container policy,
// https://wicg.github.io/feature-policy/#define-inherited-policy-in-container
// returns true only if |feature| is enabled in |parent| for either |origin|
// or |parent|'s origin, and the allowlist for |feature| matches |origin|.
//
// Roughly, If a feature is enabled in the parent frame, and the parent
// chooses to delegate it to the child frame, using the iframe attribute, then
// the feature should be enabled in the child frame.
for (const ParsedFeaturePolicyDeclaration& parsed_declaration :
container_policy) {
// If a feature is enabled in the parent frame, and the parent chooses to
// delegate it to the child frame, using the iframe attribute, then the
// feature should be enabled in the child frame.
mojom::FeaturePolicyFeature feature = parsed_declaration.feature;
// Do not allow setting a container policy for a feature which is not in the
// feature list.
......@@ -220,21 +229,13 @@ void FeaturePolicy::AddContainerPolicy(
if (search == inherited_policies_.end())
continue;
bool& inherited_policy = search->second;
// If the parent frame does not enable the feature, then the child frame
// must not.
inherited_policy = false;
if (parent_policy->IsFeatureEnabled(feature)) {
if (parsed_declaration.matches_opaque_src && origin_.opaque()) {
// If the child frame has an opaque origin, and the declared container
// policy indicates that the feature should be enabled, enable it for
// the child frame.
inherited_policy = true;
} else if (AllowlistFromDeclaration(parsed_declaration)
->Contains(origin_)) {
// Otherwise, enbable the feature if the declared container policy
// includes the origin of the child frame.
inherited_policy = true;
}
// If enabled by |parent_policy| for either |origin| or |parent_policy|'s
// origin, then enable in the child iff the declared container policy
// matches |origin|.
if (inherited_policy || parent_policy->IsFeatureEnabled(feature)) {
inherited_policy =
((parsed_declaration.matches_opaque_src && origin_.opaque()) ||
AllowlistFromDeclaration(parsed_declaration)->Contains(origin_));
}
}
}
......
third_party/blink/common/feature_policy/feature_policy_unittest.cc
View file @ 3adb3051
......@@ -1023,6 +1023,98 @@ TEST_F(FeaturePolicyTest, TestCombineFrameAndHeaderPolicies) {
policy4->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_c_));
}
TEST_F(FeaturePolicyTest, TestFeatureDeclinedAtTopLevel) {
// +-----------------------------------------+
// |(1)Origin A |
// |Feature-Policy: default-self 'none' |
// | |
// |<iframe allow="default-self OriginB"> |
// | +-------------------------------------+ |
// | |(2)Origin B | |
// | |No Policy | |
// | +-------------------------------------+ |
// | |
// |<iframe allow="default-self *"> |
// | +-------------------------------------+ |
// | |(3)Origin A | |
// | |No Policy | |
// | +-------------------------------------+ |
// +-----------------------------------------+
// Default-self feature should be disabled in all frames.
std::unique_ptr<FeaturePolicy> policy1 =
CreateFromParentPolicy(nullptr, origin_a_);
policy1->SetHeaderPolicy(
{{{kDefaultSelfFeature, false, false, std::vector<url::Origin>()}}});
ParsedFeaturePolicy frame_policy1 = {
{{kDefaultSelfFeature, false, false, {origin_b_}}}};
std::unique_ptr<FeaturePolicy> policy2 =
CreateFromParentWithFramePolicy(policy1.get(), frame_policy1, origin_b_);
ParsedFeaturePolicy frame_policy2 = {
{{kDefaultSelfFeature, true, false, std::vector<url::Origin>()}}};
std::unique_ptr<FeaturePolicy> policy3 =
CreateFromParentWithFramePolicy(policy1.get(), frame_policy2, origin_a_);
EXPECT_FALSE(
policy1->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_a_));
EXPECT_FALSE(
policy2->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_b_));
EXPECT_FALSE(
policy3->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_a_));
}
TEST_F(FeaturePolicyTest, TestFeatureDelegatedAndAllowed) {
// +-----------------------------------------+
// |(1)Origin A |
// |Feature-Policy: default-self OriginB |
// | |
// |<iframe allow="default-self OriginA"> |
// | +-------------------------------------+ |
// | |(2)Origin B | |
// | |No Policy | |
// | +-------------------------------------+ |
// | |
// |<iframe allow="default-self OriginB"> |
// | +-------------------------------------+ |
// | |(3)Origin B | |
// | |No Policy | |
// | +-------------------------------------+ |
// | |
// |<iframe allow="default-self *"> |
// | +-------------------------------------+ |
// | |(4)Origin B | |
// | |No Policy | |
// | +-------------------------------------+ |
// +-----------------------------------------+
// Default-self feature should be disabled in top-level frame and frame 2, and
// enabled in the remaining frames.
std::unique_ptr<FeaturePolicy> policy1 =
CreateFromParentPolicy(nullptr, origin_a_);
policy1->SetHeaderPolicy({{kDefaultSelfFeature, false, false, {origin_b_}}});
ParsedFeaturePolicy frame_policy1 = {
{{kDefaultSelfFeature, false, false, {origin_a_}}}};
std::unique_ptr<FeaturePolicy> policy2 =
CreateFromParentWithFramePolicy(policy1.get(), frame_policy1, origin_b_);
ParsedFeaturePolicy frame_policy2 = {
{{kDefaultSelfFeature, false, false, {origin_b_}}}};
std::unique_ptr<FeaturePolicy> policy3 =
CreateFromParentWithFramePolicy(policy1.get(), frame_policy2, origin_b_);
ParsedFeaturePolicy frame_policy3 = {
{{kDefaultSelfFeature, true, false, std::vector<url::Origin>()}}};
std::unique_ptr<FeaturePolicy> policy4 =
CreateFromParentWithFramePolicy(policy1.get(), frame_policy3, origin_b_);
EXPECT_FALSE(
policy1->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_a_));
EXPECT_TRUE(
policy1->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_b_));
EXPECT_FALSE(
policy2->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_a_));
EXPECT_FALSE(
policy2->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_b_));
EXPECT_TRUE(
policy3->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_b_));
EXPECT_TRUE(
policy4->IsFeatureEnabledForOrigin(kDefaultSelfFeature, origin_b_));
}
TEST_F(FeaturePolicyTest, TestDefaultSandboxedFramePolicy) {
// +------------------+
// |(1)Origin A |
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment