PlzNavigate: Enforce 'form-action' CSP on the browser-side.

content/browser/frame_host/form_submission_throttle.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/frame_host/form_submission_throttle.h"
+#include "content/browser/frame_host/navigation_handle_impl.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/navigation_throttle.h"
+#include "content/public/common/browser_side_navigation_policy.h"
+
+namespace content {
+
+// static
+std::unique_ptr<NavigationThrottle>
+FormSubmissionThrottle::MaybeCreateThrottleFor(NavigationHandle* handle) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!IsBrowserSideNavigationEnabled())
+    return nullptr;
+
+  NavigationHandleImpl* handle_impl =
+      static_cast<NavigationHandleImpl*>(handle);
+
+  if (!handle_impl->is_form_submission())
+    return nullptr;
+
+  return std::unique_ptr<NavigationThrottle>(
+      new FormSubmissionThrottle(handle));
+}
+
+FormSubmissionThrottle::~FormSubmissionThrottle() {}
+
+NavigationThrottle::ThrottleCheckResult
+FormSubmissionThrottle::CheckContentSecurityPolicyFormAction(bool is_redirect) {
+  NavigationHandleImpl* handle =
+      static_cast<NavigationHandleImpl*>(navigation_handle());
+
+  // Allow the request when it bypasses the CSP.
+  if (handle->should_bypass_main_world_csp())
+    return NavigationThrottle::PROCEED;
+
+  const GURL& url = handle->GetURL();
+  RenderFrameHostImpl* render_frame =
+      handle->frame_tree_node()->current_frame_host();
+
+  if (!render_frame->csp_context()->Allow(
+          render_frame->content_security_policies(), CSPDirective::FormAction,
+          url, is_redirect)) {
+    return NavigationThrottle::CANCEL;
+  }
+
+  return NavigationThrottle::PROCEED;

[Mike West, 2017/02/22 15:36:07]

It seems valuable to add some unit tests for this method.

[arthursonzogni, 2017/02/22 17:15:23]

Acknowledged. It looks hard since it depends on the frame_tree_node and its
current_frame_host. These objects cannot be easily replaced by stub objects. I
will look around the code source to find some tests that builds such objects.

[arthursonzogni, 2017/02/23 13:28:53]

Tests added!

+}
+
+NavigationThrottle::ThrottleCheckResult
+FormSubmissionThrottle::WillStartRequest() {
+  return CheckContentSecurityPolicyFormAction(false /* is_redirect */);

[Mike West, 2017/02/22 15:36:07]

I might be misunderstanding things, but I thought that this fired at the
beginning of a redirected request... That is, if A redirects to B, I think we'll
see `WillStartRequest` twice (once for A and once for B) and WillRedirectRequest
once (for the redirection). Is that accurate?

If not, great! If so, then we can drop `WillRedirectRequest` from this throttle,
but the `is_redirect` flag here is incorrect.

[arthursonzogni, 2017/02/22 17:15:23]

I confirm that WillStartRequest is not called twice, which is great!

+}
+
+NavigationThrottle::ThrottleCheckResult
+FormSubmissionThrottle::WillRedirectRequest() {
+  return CheckContentSecurityPolicyFormAction(true /* is_redirect */);
+}
+
+FormSubmissionThrottle::FormSubmissionThrottle(NavigationHandle* handle)
+    : NavigationThrottle(handle) {}
+
+}  // namespace content