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.

[nasko, 2017/03/16 21:49:46]

nit: 2017

[alexmos, 2017/03/16 23:05:35]

nit: 2017

[arthursonzogni, 2017/03/17 14:58:24]

Done.

[arthursonzogni, 2017/03/17 14:58:24]

Done.

+// 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) {

[nasko, 2017/03/16 21:49:46]

The method ordering in the .cc file should match the header file. And in general
it is ok for the constructor to be up first, even if it is marked private and
lower in the header file.

[arthursonzogni, 2017/03/17 14:58:24]

Done.

+  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->IsAllowedByCsp(CSPDirective::FormAction, url, is_redirect))
+    return NavigationThrottle::CANCEL;

[nasko, 2017/03/16 21:49:46]

I would invert the if statement and return PROCEED, where the default will
return CANCEL.

[arthursonzogni, 2017/03/17 14:58:24]

Done.

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