Skip to content

Commit

Permalink
[ FeaturePolicy ] 'sandbox' vs 'allow' attributes
Browse files Browse the repository at this point in the history
This CL ensures that sandbox features set through 'allow' attribute take
precedence over the 'sandbox' attribute itsef. In line with this, any sandbox
flag which is converted to a container policy will be removed from the set of
flags for the corresponding frame owner. This ensures that the container policy
will not be overwritten by FrameOwner sandbox at the time of receiving the
feature policy headers.

The CL also packs a WPT which verifies the precedence of 'allow' over 'sandbox'.

Bug: 795538, 812381, 926293
Change-Id: Ic1d31dfb17ce4a81b5ead23b789119c04cfacd8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1508925
Reviewed-by: Kent Tamura <[email protected]>
Reviewed-by: Ian Clelland <[email protected]>
Commit-Queue: Ehsan Karamad <[email protected]>
Cr-Commit-Position: refs/heads/master@{#645769}
  • Loading branch information
ehsan-karamad authored and chromium-wpt-export-bot committed Mar 29, 2019
1 parent c5bd0d1 commit 52d8660
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 4 deletions.
38 changes: 34 additions & 4 deletions feature-policy/feature-policy-for-sandbox/resources/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@ const all_features = document.featurePolicy.allowedFeatures();

// 'popups' is nonsensical in this test and it is not possible to test 'scripts'
// within this test model.
const ignore_features = ["popups", "scripts"];
const ignore_features_for_auxilary_context = ["popups", "scripts"];

// Feature-policies that represent specific sandbox flags.
const sandbox_features = [
"forms", "modals", "orientation-lock", "pointer-lock", "popups",
"presentation", "scripts", "top-navigation"];

// TODO(ekaramad): Figure out different inheritance requirements for different
// policies.
// Features which will be tested for propagation to auxiliary contexts.
const features_that_propagate = all_features.filter(
(feature) => !ignore_features.includes(feature));
(feature) => !ignore_features_for_auxilary_context.includes(feature));

var last_feature_message = null;
var on_new_feature_callback = null;
Expand All @@ -29,10 +34,28 @@ function add_iframe(options) {
});
}

// Resolves after |c| animation frames.
function wait_for_raf_count(c) {
let count = c;
let callback = null;
function on_raf() {
if (--count === 0) {
callback();
return;
}
window.requestAnimationFrame(on_raf);
}
return new Promise( r => {
callback = r;
window.requestAnimationFrame(on_raf);
});
}

// Returns a promise which is resolved with the next/already received message
// with feature update for |feature|. The resolved value is the state of the
// feature |feature|.
function feature_update(feature) {
// feature |feature|. If |optional_timeout| is provided, after the given delay
// (in terms of rafs) the promise is resolved with false.
function feature_update(feature, optional_timeout_rafs) {
function reset_for_next_update() {
return new Promise((r) => {
const state = last_feature_message.state;
Expand All @@ -43,6 +66,13 @@ function feature_update(feature) {
if (last_feature_message && last_feature_message.feature === feature)
return reset_for_next_update();

if (optional_timeout_rafs) {
wait_for_raf_count(optional_timeout_rafs).then (() => {
last_feature_message = {state: false};
on_new_feature_callback();
});
}

return new Promise((r) => on_new_feature_callback = r)
.then(() => reset_for_next_update());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<!DOCTYPE html>
<head>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<body>
<div id="iframe-embedder"></div>
<script src="./resources/helper.js"></script>
<script>
'use strict';

const iframe_src = "/feature-policy/feature-policy-for-sandbox/resources/window_opener.html";

promise_test( async () => {
for (const feature of sandbox_features) {
// For the test to work correctly we need "scripts";
const sandbox_flags = feature === "scripts" ? "" : "allow-scripts";
const iframe = await add_iframe(
{src: iframe_src, allow: `${feature} *`, sandbox: sandbox_flags});
iframe.contentWindow.postMessage({type: "feature", feature: feature}, "*");
const iframe_state = await feature_update(feature);
assert_true(iframe_state,
`'${feature}' should not be disabled in <iframe>.'`);
iframe.parentElement.removeChild(iframe);
}
}, "Verify that when a sandbox related feature is enabled in 'allow' then " +
" the feature will be enabled regardless of sandbox attribute's value.");

promise_test( async() => {
for (const feature of sandbox_features) {
const sandbox_flags = `allow-${feature} allow-scripts`;
const iframe = await add_iframe(
{src: iframe_src, allow: `${feature} 'none'`, sandbox: sandbox_flags});
iframe.contentWindow.postMessage({type: "feature", feature: feature}, "*");
// 'scripts' will block running code in the subframe and no update can be
// sent. A timeout determines the feature is disabled.
const timeout = (feature === "scripts") ? 10 : false;
const iframe_state = await feature_update(feature, timeout);
assert_false(iframe_state,
`'${feature}' should be disabled in <iframe>.'`);
iframe.parentElement.removeChild(iframe);
}
}, "Verify that when a sandbox related feature is disabled in 'allow' then " +
" the feature will be disabled regardless of sandbox attribute's value.");
</script>
</body>

0 comments on commit 52d8660

Please sign in to comment.