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

content/browser/frame_host/navigation_handle_impl.cc

Index: content/browser/frame_host/navigation_handle_impl.cc
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index 04cdc86e69ec469b15543051643823ccb163d31b..4a7bd510e6fc5481d3955664d2ec5ca532207bc4 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -15,6 +15,7 @@
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/ancestor_throttle.h"
 #include "content/browser/frame_host/debug_urls.h"
+#include "content/browser/frame_host/form_submission_throttle.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/mixed_content_navigation_throttle.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
@@ -66,11 +67,12 @@ std::unique_ptr<NavigationHandleImpl> NavigationHandleImpl::Create(
     const base::TimeTicks& navigation_start,
     int pending_nav_entry_id,
     bool started_from_context_menu,
-    bool should_bypass_main_world_csp) {
+    bool should_bypass_main_world_csp,
+    bool is_form_submission) {
   return std::unique_ptr<NavigationHandleImpl>(new NavigationHandleImpl(
       url, redirect_chain, frame_tree_node, is_renderer_initiated, is_same_page,
       navigation_start, pending_nav_entry_id, started_from_context_menu,
-      should_bypass_main_world_csp));
+      should_bypass_main_world_csp, is_form_submission));
 }
 
 NavigationHandleImpl::NavigationHandleImpl(
@@ -82,7 +84,8 @@ NavigationHandleImpl::NavigationHandleImpl(
     const base::TimeTicks& navigation_start,
     int pending_nav_entry_id,
     bool started_from_context_menu,
-    bool should_bypass_main_world_csp)
+    bool should_bypass_main_world_csp,
+    bool is_form_submission)
     : url_(url),
       has_user_gesture_(false),
       transition_(ui::PAGE_TRANSITION_LINK),
@@ -112,6 +115,7 @@ NavigationHandleImpl::NavigationHandleImpl(
       reload_type_(ReloadType::NONE),
       navigation_type_(NAVIGATION_TYPE_UNKNOWN),
       should_bypass_main_world_csp_(should_bypass_main_world_csp),
+      is_form_submission_(is_form_submission),
       weak_factory_(this) {
   DCHECK(!navigation_start.is_null());
   if (redirect_chain_.empty())
@@ -463,6 +467,12 @@ bool NavigationHandleImpl::should_bypass_main_world_csp() const {
   return should_bypass_main_world_csp_;
 }
 
+bool NavigationHandleImpl::is_form_submission() const {
+  DCHECK(IsBrowserSideNavigationEnabled())
+      << "This method is implemented only with PlzNavigate";
+  return is_form_submission_;
+}
+
 const GlobalRequestID& NavigationHandleImpl::GetGlobalRequestID() {
   DCHECK(state_ == WILL_PROCESS_RESPONSE || state_ == DEFERRING_RESPONSE ||
          state_ == READY_TO_COMMIT);
@@ -912,6 +922,11 @@ void NavigationHandleImpl::RegisterNavigationThrottles() {
   if (ancestor_throttle)
     throttles_.push_back(std::move(ancestor_throttle));
 
+  std::unique_ptr<content::NavigationThrottle> form_submission_throttle =
+      content::FormSubmissionThrottle::MaybeCreateThrottleFor(this);
+  if (form_submission_throttle)
+    throttles_.push_back(std::move(form_submission_throttle));

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

This needs to happen before the mixed content check. Can I assume that throttles
execute in LIFO order?

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

I would say the FIFO order, they are executed from the first one
(mixed_content_throttle, then devtools_throttles, ..., then
form_submission_throttle)

You are probably right, but please, can you tell why the mixed_content_checks
should happens after the 'form-action' checks?
What about the 'frame-src' and 'XFO' checks?

[alexmos, 2017/02/28 02:48:46]

Mike would know these reasons much better, but one thing I recalled is this
comment in FrameFetchContext::canRequestInternal:

  // Check for mixed content. We do this second-to-last so that when folks block
  // mixed content with a CSP policy, they don't get a warning. They'll still
  // get a warning in the console about CSP blocking the load.

[arthursonzogni, 2017/03/07 16:25:51]

Done. Mike please confirm that it looks good to you to move the AncestorThrottle
and the FormSubmissionThrottle before the MixedContentThrottle.

+
   throttles_.insert(throttles_.begin(),
                     std::make_move_iterator(throttles_to_register.begin()),
                     std::make_move_iterator(throttles_to_register.end()));