Origin isolation: implement window.originIsolationRestricted

See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/master@{#782672}
Alan-love · Jun 25, 2020 · 4778c35 · 4778c35
1 parent 89d8659
commit 4778c35
Show file tree

Hide file tree

Showing 51 changed files with 348 additions and 18 deletions.
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
@@ -3304,7 +3304,8 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams(
           GURL() /* web_bundle_physical_url */,
           GURL() /* base_url_override_for_web_bundle */,
           node->pending_frame_policy(),
-          std::vector<std::string>() /* force_enabled_origin_trials */);
+          std::vector<std::string>() /* force_enabled_origin_trials */,
+          false /* origin_isolation_restricted */);
 #if defined(OS_ANDROID)
   if (ValidateDataURLAsString(params.data_url_as_string)) {
     commit_params->data_url_as_string = params.data_url_as_string->data();

diff --git a/content/browser/frame_host/navigation_entry_impl.cc b/content/browser/frame_host/navigation_entry_impl.cc
@@ -829,7 +829,8 @@ NavigationEntryImpl::ConstructCommitNavigationParams(
           false, network::mojom::IPAddressSpace::kUnknown,
           GURL() /* web_bundle_physical_url */,
           GURL() /* base_url_override_for_web_bundle */, frame_policy,
-          std::vector<std::string>() /* force_enabled_origin_trials */);
+          std::vector<std::string>() /* force_enabled_origin_trials */,
+          false /* origin_isolation_restricted */);
 #if defined(OS_ANDROID)
   if (NavigationControllerImpl::ValidateDataURLAsString(GetDataURLAsString())) {
     commit_params->data_url_as_string = GetDataURLAsString()->data();

diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
@@ -911,7 +911,8 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated(
           GURL() /* web_bundle_physical_url */,
           GURL() /* base_url_override_for_web_bundle */,
           frame_tree_node->pending_frame_policy(),
-          std::vector<std::string>() /* force_enabled_origin_trials */);
+          std::vector<std::string>() /* force_enabled_origin_trials */,
+          false /* origin_isolation_restricted */);
 
   // CreateRendererInitiated() should only be triggered when the navigation is
   // initiated by a frame in the same process.
@@ -998,7 +999,8 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateForCommit(
           GURL() /* web_bundle_physical_url */,
           GURL() /* base_url_override_for_web_bundle */,
           frame_tree_node->pending_frame_policy(),
-          std::vector<std::string>() /* force_enabled_origin_trials */
+          std::vector<std::string>() /* force_enabled_origin_trials */,
+          false /* origin_isolation_restricted */
       );
   mojom::BeginNavigationParamsPtr begin_params =
       mojom::BeginNavigationParams::New();
@@ -2017,6 +2019,14 @@ void NavigationRequest::DetermineOriginIsolationEndResult(
                     kRequestedViaHeaderButNotIsolated;
       break;
   }
+
+  commit_params_->origin_isolation_restricted =
+      origin_isolation_end_result_ ==
+          OptInOriginIsolationEndResult::kRequestedViaOriginPolicyAndIsolated ||
+      origin_isolation_end_result_ ==
+          OptInOriginIsolationEndResult::kRequestedViaHeaderAndIsolated ||
+      origin_isolation_end_result_ ==
+          OptInOriginIsolationEndResult::kNotRequestedButIsolated;
 }
 
 void NavigationRequest::ProcessOriginIsolationEndResult() {

diff --git a/content/common/navigation_params.mojom b/content/common/navigation_params.mojom
@@ -414,4 +414,7 @@ struct CommitNavigationParams {
 
   // The names of origin trials to be force enabled for this navigation.
   array<string> force_enabled_origin_trials;
+
+  // Whether origin isolation is restricting certain cross-origin web APIs.
+  bool origin_isolation_restricted = false;
 };
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
@@ -1010,6 +1010,9 @@ void FillMiscNavigationParams(
     navigation_params->force_fetch_cache_mode =
         blink::mojom::FetchCacheMode::kDefault;
   }
+
+  navigation_params->origin_isolation_restricted =
+      commit_params.origin_isolation_restricted;
 }
 
 // Fills in the origin policy associated with this response, if any is present.

diff --git a/third_party/blink/public/web/web_navigation_params.h b/third_party/blink/public/web/web_navigation_params.h
@@ -390,6 +390,9 @@ struct BLINK_EXPORT WebNavigationParams {
 
   // A list of origin trial names to enable for the document being loaded.
   WebVector<WebString> force_enabled_origin_trials;
+
+  // Whether origin isolation is restricting certain cross-origin web APIs.
+  bool origin_isolation_restricted = false;
 };
 
 }  // namespace blink

diff --git a/third_party/blink/renderer/core/frame/local_dom_window.cc b/third_party/blink/renderer/core/frame/local_dom_window.cc
@@ -1770,6 +1770,14 @@ void LocalDOMWindow::SetOriginPolicyIds(const Vector<String>& ids) {
   origin_policy_ids_ = ids;
 }
 
+bool LocalDOMWindow::originIsolationRestricted() const {
+  return origin_isolation_restricted_;
+}
+
+void LocalDOMWindow::SetOriginIsolationRestricted(bool value) {
+  origin_isolation_restricted_ = value;
+}
+
 int LocalDOMWindow::requestIdleCallback(V8IdleRequestCallback* callback,
                                         const IdleRequestOptions* options) {
   if (!GetFrame())

diff --git a/third_party/blink/renderer/core/frame/local_dom_window.h b/third_party/blink/renderer/core/frame/local_dom_window.h
@@ -283,6 +283,10 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
   const Vector<String>& originPolicyIds() const;
   void SetOriginPolicyIds(const Vector<String>&);
 
+  // https://github.com/whatwg/html/pull/5545
+  bool originIsolationRestricted() const;
+  void SetOriginIsolationRestricted(bool);
+
   // Idle callback extensions
   int requestIdleCallback(V8IdleRequestCallback*, const IdleRequestOptions*);
   void cancelIdleCallback(int id);
@@ -445,6 +449,8 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
 
   Vector<String> origin_policy_ids_;
 
+  bool origin_isolation_restricted_ = false;
+
   mutable Member<ApplicationCache> application_cache_;
 
   scoped_refptr<SerializedScriptValue> pending_state_object_;

diff --git a/third_party/blink/renderer/core/frame/window.idl b/third_party/blink/renderer/core/frame/window.idl
@@ -73,8 +73,10 @@
     [Custom, NotEnumerable, CrossOrigin] getter object (DOMString name);
 
     // the user agent
+    // includes https://github.com/whatwg/html/pull/5545 (originIsolationRestricted)
     [Affects=Nothing, LogActivity=GetterOnly] readonly attribute Navigator navigator;
     [LogActivity=GetterOnly, SecureContext=RestrictAppCacheToSecureContexts, RuntimeEnabled=AppCache] readonly attribute ApplicationCache applicationCache;
+    [RuntimeEnabled=OriginIsolationHeader, SecureContext] readonly attribute boolean originIsolationRestricted;
 
     // user prompts
     [Measure, CallWith=ScriptState] void alert();

diff --git a/third_party/blink/renderer/core/loader/document_loader.cc b/third_party/blink/renderer/core/loader/document_loader.cc
@@ -236,7 +236,8 @@ DocumentLoader::DocumentLoader(
       initiator_origin_trial_features_(
           CopyInitiatorOriginTrials(params_->initiator_origin_trial_features)),
       force_enabled_origin_trials_(
-          CopyForceEnabledOriginTrials(params_->force_enabled_origin_trials)) {
+          CopyForceEnabledOriginTrials(params_->force_enabled_origin_trials)),
+      origin_isolation_restricted_(params_->origin_isolation_restricted) {
   DCHECK(frame_);
 
   // TODO(nasko): How should this work with OOPIF?
@@ -1671,6 +1672,9 @@ void DocumentLoader::InstallNewDocument(
     }
   }
 
+  frame_->DomWindow()->SetOriginIsolationRestricted(
+      origin_isolation_restricted_);
+
   WillCommitNavigation();
 
   Document* document = frame_->DomWindow()->InstallNewDocument(init);

diff --git a/third_party/blink/renderer/core/loader/document_loader.h b/third_party/blink/renderer/core/loader/document_loader.h
@@ -566,6 +566,8 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
 
   // Whether the document can be scrolled on load
   bool navigation_scroll_allowed_ = true;
+
+  bool origin_isolation_restricted_ = false;
 };
 
 DECLARE_WEAK_IDENTIFIER_MAP(DocumentLoader);

diff --git a/third_party/blink/web_tests/external/wpt/origin-isolation/about-blank.https.sub.html b/third_party/blink/web_tests/external/wpt/origin-isolation/about-blank.https.sub.html
@@ -11,7 +11,8 @@
   insertIframe,
   setBothDocumentDomains,
   testSameAgentCluster,
-  testDifferentAgentClusters
+  testDifferentAgentClusters,
+  testOriginIsolationRestricted
 } from "./resources/helpers.mjs";
 
 promise_setup(async () => {
@@ -25,6 +26,10 @@
 testDifferentAgentClusters([0, 1], "about:blank to child2");
 testDifferentAgentClusters([1, 0], "child2 to about:blank");
 
+testOriginIsolationRestricted(self, true, "parent");
+testOriginIsolationRestricted(0, true, "about:blank");
+testOriginIsolationRestricted(1, false, "child2");
+
 async function insertAboutBlankIframe() {
   const iframe = document.createElement("iframe");
   document.body.append(iframe);

diff --git a/third_party/blink/web_tests/external/wpt/origin-isolation/getter-data-url.https.html b/third_party/blink/web_tests/external/wpt/origin-isolation/getter-data-url.https.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>window.originIsolationRestricted for a data: URL</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<div id="log"></div>
+
+<script type="module">
+import {
+  waitForIframe,
+  testOriginIsolationRestricted
+} from "./resources/helpers.mjs";
+
+promise_setup(() => {
+  const iframe = document.createElement("iframe");
+
+  // This copies parts of resources/send-origin-isolation-header.py that allow
+  // us to reuse testOriginIsolationRestricted.
+  iframe.src = `data:text/html,<script>
+    window.onmessage = () => {
+      parent.postMessage(self.originIsolationRestricted, "*");
+    };
+    </` + `script>
+  `;
+
+  const waitPromise = waitForIframe(iframe);
+  document.body.append(iframe);
+  return waitPromise;
+});
+
+// The data: URL iframe has an opaque origin, so it definitely should return
+// false. It's pretty unlikely that it would return true anyway, since we can't
+// set the header on the iframe, but we should test it to make sure there isn't
+// some strange main page -> data: URL iframe inheritance going on.
+
+testOriginIsolationRestricted(0, false);
+</script>
diff --git a/third_party/blink/web_tests/external/wpt/origin-isolation/getter-data-url.https.html.headers b/third_party/blink/web_tests/external/wpt/origin-isolation/getter-data-url.https.html.headers
@@ -0,0 +1 @@
+Origin-Isolation: ?1
diff --git a/..._party/blink/web_tests/external/wpt/origin-isolation/getter-removed-iframe.sub.https.html b/..._party/blink/web_tests/external/wpt/origin-isolation/getter-removed-iframe.sub.https.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>window.crossOriginIsolated for a removed frame</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<div id="log"></div>
+
+<script type="module">
+import { navigateIframe } from "./resources/helpers.mjs";
+
+promise_test(async () => {
+  // We cannot use insertIframe because it sets both `document.domain`s. That
+  // shouldn't matter, but Chrome has a bug (https://crbug.com/1095145), so
+  // let's avoid making the test needlessly fail because of that bug.
+  const iframe = document.createElement("iframe");
+  const navigatePromise = navigateIframe(iframe, "{{hosts[][]}}", "?1");
+  document.body.append(iframe);
+  await navigatePromise;
+
+  const frameWindow = iframe.contentWindow;
+
+  assert_equals(frameWindow.originIsolationRestricted, true, "before");
+  iframe.remove();
+  assert_equals(frameWindow.originIsolationRestricted, true, "after");
+}, "Removing the iframe does not change originIsolationRestricted");
+</script>
diff --git a/...link/web_tests/external/wpt/origin-isolation/getter-removed-iframe.sub.https.html.headers b/...link/web_tests/external/wpt/origin-isolation/getter-removed-iframe.sub.https.html.headers
@@ -0,0 +1 @@
+Origin-Isolation: ?1
diff --git a/...arty/blink/web_tests/external/wpt/origin-isolation/getter-sandboxed-iframe.sub.https.html b/...arty/blink/web_tests/external/wpt/origin-isolation/getter-sandboxed-iframe.sub.https.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>window.originIsolationRestricted for a sandboxed frame</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<div id="log"></div>
+
+<script type="module">
+import {
+  navigateIframe,
+  testOriginIsolationRestricted
+} from "./resources/helpers.mjs";
+
+// We do this manually instead of using insertIframe because we want to add a
+// sandbox="" attribute and we don't want to set both document.domains.
+promise_setup(() => {
+  const iframe = document.createElement("iframe");
+  iframe.sandbox = "allow-scripts";
+  const navigatePromise = navigateIframe(iframe, "{{hosts[][]}}", "?1");
+  document.body.append(iframe);
+  return navigatePromise;
+});
+
+// Because sandboxed iframes have an opaque origin, their agent cluster key is
+// always an origin, so there are no additional restrictions imposed by origin
+// isolation. Thus the getter returns false.
+
+testOriginIsolationRestricted(0, false);
+</script>
diff --git a/...nk/web_tests/external/wpt/origin-isolation/getter-sandboxed-iframe.sub.https.html.headers b/...nk/web_tests/external/wpt/origin-isolation/getter-sandboxed-iframe.sub.https.html.headers
@@ -0,0 +1 @@
+Origin-Isolation: ?1
diff --git a/third_party/blink/web_tests/external/wpt/origin-isolation/insecure-http.sub.html b/third_party/blink/web_tests/external/wpt/origin-isolation/insecure-http.sub.html
@@ -16,4 +16,9 @@
 // All isolation requests are ignored, since this is over insecure HTTP.
 // So both end up in the site-keyed agent cluster.
 testSameAgentCluster([self, 0]);
+
+// Has to be promise_test because we used promise_setup().
+promise_test(async () => {
+  assert_false("originIsolationRestricted" in window);
+}, "The getter must not exist");
 </script>
diff --git a/...link/web_tests/external/wpt/origin-isolation/parent-no-child-bad-subdomain.sub.https.html b/...link/web_tests/external/wpt/origin-isolation/parent-no-child-bad-subdomain.sub.https.html
@@ -7,7 +7,11 @@
 <div id="log"></div>
 
 <script type="module">
-import { insertIframe, testSameAgentCluster } from "./resources/helpers.mjs";
+import {
+  insertIframe,
+  testSameAgentCluster,
+  testOriginIsolationRestricted
+} from "./resources/helpers.mjs";
 
 let frameIndex = 0;
 for (const badValue of ["", "?0", "true", "\"?1\"", "1", "?2", "(?1)"]) {
@@ -17,6 +21,7 @@
 
   // Since the header values are bad there should be no isolation
   testSameAgentCluster([self, frameIndex], `"${badValue}"`);
+  testOriginIsolationRestricted(frameIndex, false, `"${badValue}"`);
   ++frameIndex;
 }
 </script>
diff --git a/...rty/blink/web_tests/external/wpt/origin-isolation/parent-no-child-yes-same.sub.https.html b/...rty/blink/web_tests/external/wpt/origin-isolation/parent-no-child-yes-same.sub.https.html
@@ -7,7 +7,11 @@
 <div id="log"></div>
 
 <script type="module">
-import { insertIframe, testSameAgentCluster } from "./resources/helpers.mjs";
+import {
+  insertIframe,
+  testSameAgentCluster,
+  testOriginIsolationRestricted
+} from "./resources/helpers.mjs";
 
 promise_setup(async () => {
   await insertIframe("{{hosts[][]}}", "?1");
@@ -16,4 +20,6 @@
 // Since they're same-origin, and the parent loaded without isolation, the
 // child's request for isolation gets ignored, and both end up site-keyed.
 testSameAgentCluster([self, 0]);
+testOriginIsolationRestricted(self, false, "parent");
+testOriginIsolationRestricted(0, false, "child");
 </script>
diff --git a/...link/web_tests/external/wpt/origin-isolation/parent-no-child-yes-subdomain.sub.https.html b/...link/web_tests/external/wpt/origin-isolation/parent-no-child-yes-subdomain.sub.https.html
@@ -7,7 +7,11 @@
 <div id="log"></div>
 
 <script type="module">
-import { insertIframe, testDifferentAgentClusters } from "./resources/helpers.mjs";
+import {
+  insertIframe,
+  testDifferentAgentClusters,
+  testOriginIsolationRestricted
+} from "./resources/helpers.mjs";
 
 promise_setup(async () => {
   await insertIframe("{{hosts[][www]}}", "?1");
@@ -17,4 +21,6 @@
 // so the parent ends up in the site-keyed agent cluster and the child in the
 // origin-keyed one.
 testDifferentAgentClusters([self, 0]);
+testOriginIsolationRestricted(self, false, "parent");
+testOriginIsolationRestricted(0, true, "child");
 </script>
diff --git a/...ts/external/wpt/origin-isolation/parent-no-child-yes-with-params-subdomain.sub.https.html b/...ts/external/wpt/origin-isolation/parent-no-child-yes-with-params-subdomain.sub.https.html
@@ -7,7 +7,11 @@
 <div id="log"></div>
 
 <script type="module">
-import { insertIframe, testDifferentAgentClusters } from "./resources/helpers.mjs";
+import {
+  insertIframe,
+  testDifferentAgentClusters,
+  testOriginIsolationRestricted
+} from "./resources/helpers.mjs";
 
 promise_setup(async () => {
   await insertIframe("{{hosts[][www]}}", "?1;param1;param2=value2");
@@ -17,4 +21,6 @@
 // so the parent ends up in the site-keyed agent cluster and the child in the
 // origin-keyed one.
 testDifferentAgentClusters([self, 0]);
+testOriginIsolationRestricted(self, false, "parent");
+testOriginIsolationRestricted(0, true, "child");
 </script>
diff --git a/...nal/wpt/origin-isolation/parent-no-child1-no-child2-yes-children-different.sub.https.html b/...nal/wpt/origin-isolation/parent-no-child1-no-child2-yes-children-different.sub.https.html
@@ -11,6 +11,7 @@
   insertIframe,
   testSameAgentCluster,
   testDifferentAgentClusters,
+  testOriginIsolationRestricted
 } from "./resources/helpers.mjs";
 
 promise_setup(async () => {
@@ -30,4 +31,8 @@
 testDifferentAgentClusters([self, 1], "Parent to child2");
 testDifferentAgentClusters([0, 1], "child1 to child2");
 testDifferentAgentClusters([1, 0], "child2 to child1");
+
+testOriginIsolationRestricted(self, false, "parent");
+testOriginIsolationRestricted(0, false, "child1");
+testOriginIsolationRestricted(1, true, "child2");
 </script>