[OutOfBlinkSandbox] Compute sandbox flags in RenderFrameHost

Compute the sandbox policy of a document in the RenderFrameHost.

For now, this is simply used to double check the value computed from the
renderer. If this works correctly, the browser process will use it and
stop depending on the renderer process.

Ideally, this patch must land for M84, in order to get useful reports.

This patch is a step in the following direction:
~~
Stop parsing the HTTP CSP sandbox header in blink. Parse it in the
network service CSP parser. This allows the browser process to compute
the Origin of the new document before loading it.
~~
https://docs.google.com/document/d/1PechV73KKMF8leh7uTlyGkR32kD5qILvu3ZeG8TYGfk/edit?usp=sharing

Bug: 1041376
Change-Id: I26d0e3744bc1b5b2d893fd01b6e6ecea35cf47e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2190680
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771551}

content/browser/frame_host/navigation_request.cc

@@ -2578,6 +2578,11 @@ void NavigationRequest::OnServiceWorkerAccessed(
   GetDelegate()->OnServiceWorkerAccessed(this, scope, allowed);
 }
 
+base::Optional<network::mojom::WebSandboxFlags>
+NavigationRequest::SandboxFlagsToCommit() {
+  return sandbox_flags_to_commit_;
+}
+
 void NavigationRequest::OnRedirectChecksComplete(
     NavigationThrottle::ThrottleCheckResult result) {
   DCHECK(result.action() != NavigationThrottle::DEFER);
@@ -2805,6 +2810,7 @@ void NavigationRequest::CommitErrorPage(
     }
   }
 
+  sandbox_flags_to_commit_ = ComputeSandboxFlagsToCommit();
   ReadyToCommitNavigation(true);
   render_frame_host_->FailedNavigation(this, *common_params_, *commit_params_,
                                        has_stale_copy_in_cache_, net_error_,
@@ -2873,6 +2879,7 @@ void NavigationRequest::CommitNavigation() {
     }
   }
 
+  sandbox_flags_to_commit_ = ComputeSandboxFlagsToCommit();
   CreateCoepReporter(render_frame_host_->GetProcess()->GetStoragePartition());
 
   blink::mojom::ServiceWorkerContainerInfoForClientPtr
@@ -4399,4 +4406,43 @@ NavigationRequest::TakeCookieObservers() {
   return cookie_observers_.TakeReceivers();
 }
 
+network::mojom::WebSandboxFlags
+NavigationRequest::ComputeSandboxFlagsToCommit() {
+  DCHECK(commit_params_);
+
+  network::mojom::WebSandboxFlags out;
+  if (IsErrorPage()) {
+    // An error page allows everything except scripts.
+    // TODO(arthursonzogni): Why stopping at script?
+    out = ~network::mojom::WebSandboxFlags::kScripts &
+          ~network::mojom::WebSandboxFlags::kAutomaticFeatures;
+  } else if (commit_params_->frame_policy) {
+    // This corresponds to the sandbox policy of the frame embedding the
+    // document that were active when the navigation started.
+    out = commit_params_->frame_policy->sandbox_flags;
+  } else {
+    // The document doesn't have a sandbox policy. This case should in theory
+    // contains only the navigations that are:
+    // - main frame.
+    // - browser initiated.
+    // - non-history.
+    // - non-error.
+    //
+    // TODO(arthursonzogni): In practice, a few navigations not complying with
+    // one of the 4 items above are using this path. They must be identified
+    // and removed. A set of DCHECK must be added.
+    out = network::mojom::WebSandboxFlags::kNone;
+  }
+
+  // The response can also restrict the policy further.
+  if (response_head_) {
+    for (const auto& csp :
+         response_head_->parsed_headers->content_security_policy) {
+      out |= ~(csp->sandbox);
+    }
+  }
+
+  return out;
+}
+
 }  // namespace content

content/browser/frame_host/navigation_request.h

@@ -45,6 +45,7 @@
 #include "services/metrics/public/cpp/ukm_source_id.h"
 #include "services/network/public/cpp/origin_policy.h"
 #include "services/network/public/mojom/blocked_by_response_reason.mojom-shared.h"
+#include "services/network/public/mojom/web_sandbox_flags.mojom-shared.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/scoped_java_ref.h"
@@ -614,6 +615,14 @@ class CONTENT_EXPORT NavigationRequest
   std::vector<mojo::PendingReceiver<network::mojom::CookieAccessObserver>>
   TakeCookieObservers() WARN_UNUSED_RESULT;
 
+  // The sandbox policy of the document to be loaded. This returns nullopt for
+  // navigations that haven't reached the 'ReadyToCommit' stage yet. In
+  // particular, this returns nullopt for same-document navigations.
+  //
+  // TODO(arthursonzogni): After RenderDocument, this can be computed and stored
+  // directly into the RenderDocumentHost.
+  base::Optional<network::mojom::WebSandboxFlags> SandboxFlagsToCommit();
+
  private:
   friend class NavigationRequestTest;
 
@@ -956,6 +965,10 @@ class CONTENT_EXPORT NavigationRequest
   // NavigationRequest is in.
   NavigationControllerImpl* GetNavigationController();
 
+  // Compute the sandbox policy of the document to be loaded. Called once when
+  // reaching the 'ReadyToCommit' stage.
+  network::mojom::WebSandboxFlags ComputeSandboxFlagsToCommit();
+
   FrameTreeNode* frame_tree_node_;
 
   // Value of |is_for_commit| supplied to the constructor.
@@ -1308,6 +1321,10 @@ class CONTENT_EXPORT NavigationRequest
   // made by this navigation.
   mojo::ReceiverSet<network::mojom::CookieAccessObserver> cookie_observers_;
 
+  // The sandbox flags of the document to be loaded. This is computed at
+  // 'ReadyToCommit' time.
+  base::Optional<network::mojom::WebSandboxFlags> sandbox_flags_to_commit_;
+
   base::WeakPtrFactory<NavigationRequest> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(NavigationRequest);

content/browser/frame_host/render_frame_host_impl.cc

@@ -3706,6 +3706,8 @@ void RenderFrameHostImpl::DidSetFramePolicyHeaders(
 
   // Save a copy of the now-active sandbox flags on this RFHI.
   active_sandbox_flags_ = frame_tree_node()->active_sandbox_flags();
+
+  CheckSandboxFlags();
 }
 
 void RenderFrameHostImpl::EnforceInsecureRequestPolicy(
@@ -5664,6 +5666,8 @@ void RenderFrameHostImpl::CommitNavigation(
     CHECK_EQ(GetSiteInstance(), parent_->GetSiteInstance());
   }
 
+  // TODO(https://crbug.com/888079): Compute the Origin to commit here.
+
   // If this is an attempt to commit a URL in an incompatible process, capture a
   // crash dump to diagnose why it is occurring.
   // TODO(creis): Remove this check after we've gathered enough information to
@@ -7940,6 +7944,11 @@ bool RenderFrameHostImpl::DidCommitNavigationInternal(
     }
   }
 
+  // Keep track of the sandbox policy of the document that has just committed.
+  // It will be compared with the value computed from the renderer. The latter
+  // is expected to be received in DidSetFramePolicyHeaders(..).
+  active_sandbox_flags_control_ = navigation_request->SandboxFlagsToCommit();
+
   // If we still have a PeakGpuMemoryTracker, then the loading it was observing
   // never completed. Cancel it's callback so that we don't report partial
   // loads to UMA.
@@ -8968,4 +8977,27 @@ void RenderFrameHostImpl::OnCookiesAccessed(
     delegate_->OnCookiesAccessed(this, blocked);
 }
 
+void RenderFrameHostImpl::CheckSandboxFlags() {
+  if (is_mhtml_document_)
+    return;
+
+  if (!active_sandbox_flags_control_)
+    return;
+
+  if (active_sandbox_flags_ == *active_sandbox_flags_control_)
+    return;
+
+  base::debug::ScopedCrashKeyString scoped_url(
+      base::debug::AllocateCrashKeyString("url",
+                                          base::debug::CrashKeySize::Size256),
+      GetLastCommittedURL().possibly_invalid_spec());
+  base::debug::ScopedCrashKeyString scoped_sandbox(
+      base::debug::AllocateCrashKeyString("sandbox",
+                                          base::debug::CrashKeySize::Size256),
+      base::StringPrintf("%u, %u", uint32_t(active_sandbox_flags_),
+                         uint32_t(*active_sandbox_flags_control_)));
+  DCHECK(false);
+  base::debug::DumpWithoutCrashing();
+}
+
 }  // namespace content

content/browser/frame_host/render_frame_host_impl.h

@@ -2344,6 +2344,13 @@ class CONTENT_EXPORT RenderFrameHostImpl
       mojo::PendingReceiver<blink::mojom::ReportingObserver>
           reporting_observer_receiver);
 
+  // Check the renderer provided sandbox flags matches with what the browser
+  // process computed on its own. This triggers DCHECK and DumpWithoutCrashing()
+  //
+  // TODO(https://crbug.com/1041376) Remove this when we are confident the value
+  // computed from the browser is always matching.
+  void CheckSandboxFlags();
+
   // The RenderViewHost that this RenderFrameHost is associated with.
   //
   // It is kept alive as long as any RenderFrameHosts or RenderFrameProxyHosts
@@ -2717,6 +2724,16 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // deletion.
   network::mojom::WebSandboxFlags active_sandbox_flags_;
 
+  // Same as |active_sandbox_flags_|, except this is computed:
+  // - outside of the renderer process.
+  // - before loading the document.
+  //
+  // For now, this is simply used to double check this matches the renderer
+  // computation. Later this will be used as the source of truth.
+  //
+  // [OutOfBlinkSandbox](https://crbug.com/1041376)
+  base::Optional<network::mojom::WebSandboxFlags> active_sandbox_flags_control_;
+
   // Tracks the document policy which has been set on this frame.
   std::unique_ptr<blink::DocumentPolicy> document_policy_;
 

content/browser/frame_host/render_frame_host_manager_unittest.cc

@@ -3284,31 +3284,26 @@ TEST_P(RenderFrameHostManagerTest, BeginNavigationIgnoredWhenNotActive) {
 // Tests that sandbox flags received after a navigation away has started do not
 // affect the document being navigated to.
 TEST_P(RenderFrameHostManagerTest, ReceivedFramePolicyAfterNavigationStarted) {
-  const GURL kUrl1("http://www.google.com");
-  const GURL kUrl2("http://www.chromium.org");
-
-  contents()->NavigateAndCommit(kUrl1);
-  TestRenderFrameHost* initial_rfh = main_test_rfh();
-
-  // The RFH should start out with an empty frame policy.
+  // The RFH should start out with an fully permissive sandbox policy.
   EXPECT_EQ(network::mojom::WebSandboxFlags::kNone,
-            initial_rfh->frame_tree_node()->active_sandbox_flags());
+            main_test_rfh()->frame_tree_node()->active_sandbox_flags());
 
-  // Navigate cross-site but don't commit the navigation.
-  auto navigation_to_kUrl2 =
-      NavigationSimulator::CreateBrowserInitiated(kUrl2, contents());
-  navigation_to_kUrl2->ReadyToCommit();
+  // Navigate, but don't commit the navigation.
+  auto navigation = NavigationSimulator::CreateBrowserInitiated(
+      GURL("http://a.com"), contents());
+  navigation->ReadyToCommit();
 
   // Now send the frame policy for the initial page.
-  initial_rfh->SendFramePolicy(network::mojom::WebSandboxFlags::kAll,
-                               {} /* feature_policy_header */,
-                               {} /* document_policy_header */);
-  // Verify that the policy landed in the frame tree.
+  main_test_rfh()->SendFramePolicy(network::mojom::WebSandboxFlags::kAll,
+                                   {} /* feature_policy_header */,
+                                   {} /* document_policy_header */);
+
+  // Check 'SendFramePolicy' updated the active sandbox flags.
   EXPECT_EQ(network::mojom::WebSandboxFlags::kAll,
-            initial_rfh->frame_tree_node()->active_sandbox_flags());
+            main_test_rfh()->frame_tree_node()->active_sandbox_flags());
 
-  // Commit the naviagation; the new frame should have a clear frame policy.
-  navigation_to_kUrl2->Commit();
+  // Commit the navigation. The new frame should have a clear frame policy.
+  navigation->Commit();
   EXPECT_EQ(network::mojom::WebSandboxFlags::kNone,
             main_test_rfh()->frame_tree_node()->active_sandbox_flags());
 }

services/network/public/mojom/content_security_policy.mojom

@@ -98,7 +98,8 @@ struct ContentSecurityPolicy {
   bool treat_as_public_address = false;
 
   // https://www.w3.org/TR/CSP3/#directive-sandbox
-  WebSandboxFlags sandbox; // WebSandboxFlags::kNone.
+  // This uses the convention: kAll means "everything is allowed".
+  WebSandboxFlags sandbox = WebSandboxFlags.kAll;
 
   ContentSecurityPolicyHeader header;