From 52d8660d0fb402e768c535a90006d0980a9d4bf7 Mon Sep 17 00:00:00 2001
From: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri, 29 Mar 2019 07:45:33 -0700
Subject: [PATCH] [ FeaturePolicy ] 'sandbox' vs 'allow' attributes

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 <tkent@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#645769}
---
 .../resources/helper.js                       | 38 +++++++++++++--
 .../sandbox-policies-in-allow-attribute.html  | 46 +++++++++++++++++++
 2 files changed, 80 insertions(+), 4 deletions(-)
 create mode 100644 feature-policy/feature-policy-for-sandbox/sandbox-policies-in-allow-attribute.html

diff --git a/feature-policy/feature-policy-for-sandbox/resources/helper.js b/feature-policy/feature-policy-for-sandbox/resources/helper.js
index 0d82811bcee613..09a8aa7846cd72 100644
--- a/feature-policy/feature-policy-for-sandbox/resources/helper.js
+++ b/feature-policy/feature-policy-for-sandbox/resources/helper.js
@@ -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;
@@ -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;
@@ -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());
 }
diff --git a/feature-policy/feature-policy-for-sandbox/sandbox-policies-in-allow-attribute.html b/feature-policy/feature-policy-for-sandbox/sandbox-policies-in-allow-attribute.html
new file mode 100644
index 00000000000000..a29d43e5fccabe
--- /dev/null
+++ b/feature-policy/feature-policy-for-sandbox/sandbox-policies-in-allow-attribute.html
@@ -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>
\ No newline at end of file