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

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index f65c5b666138d903af0dfc162fdaa37d6b022568..5d915ef6ea7a299a7d7b5e97c668c58116586de4 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -6310,6 +6310,11 @@ void RenderFrameImpl::BeginNavigation(const NavigationPolicyInfo& info) {
                     net::LOAD_ONLY_FROM_CACHE | net::LOAD_DISABLE_CACHE);
     load_flags |= net::LOAD_BYPASS_CACHE;
   }
+
+  bool is_form_submission =
+      info.navigationType == blink::WebNavigationTypeFormSubmitted ||
+      info.navigationType == blink::WebNavigationTypeFormSubmitted;

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

This seems like you pasted the same thing twice. Did you mean Resubmitted? We
apparently don't have any tests that cover the difference here: can you add one?

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

Oops, thanks!
Ack for the test. I assume that a form resubmission happens when the current
page is the result of a form submission and it gets reloaded. right? But if the
first navigation was blocked by the 'form-action', it can't be reloaded since it
wasn't loaded. It's probably for this reason that we don't have any test for
this.
Is it what you had in mind? Do you want to test something else?

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

Again, can't speak for Mike, but there's a Blink unittest,
ParameterizedWebFrameTest.ReloadPost, that shows how to get the Resubmitted
navigation type, which indeed just seems to be a reload (or back-forward) of a
page resulting from a form submit.  The original bug presumably would've
prevented the throttle from being created for the resubmission.  So perhaps you
can have a test where the form submission succeeds (not blocked by form-action),
and then reload the resulting page and ensure that the reload has a correct
is_form_submission() bit?

I'm also a bit confused by how a form resubmission is supposed to interact with
CSP form-action, namely which frame's CSP is supposed to be checked for the
resubmission.  Presumably the old frame's CSP is gone after the first form
submit loads, but it doesn't seem like the new doc's (the result of first
submit) CSP should apply for the resubmit.  The Blink check in
FrameLoader::shouldContinueForNavigationPolicy checks for both regular form
submits and resubmissions, but I'm not sure whether a CSP form-action can allow
a form submission but then later block the resubmission (which is what you might
also want in a test for this).

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

I will add a test with an allowed form submission to a page that disallow form
submission. Then I reload the page to see what happens.

I think I will not get the right behavior with my CL and maybe not with the
current architecture too.

+
   BeginNavigationParams begin_navigation_params(
       GetWebURLRequestHeaders(info.urlRequest), load_flags,
       info.urlRequest.hasUserGesture(),
@@ -6317,7 +6322,7 @@ void RenderFrameImpl::BeginNavigation(const NavigationPolicyInfo& info) {
           blink::WebURLRequest::SkipServiceWorker::None,
       GetRequestContextTypeForWebURLRequest(info.urlRequest),
       GetMixedContentContextTypeForWebURLRequest(info.urlRequest),
-      initiator_origin);
+      is_form_submission, initiator_origin);
 
   if (!info.form.isNull()) {
     WebSearchableFormData web_searchable_form_data(info.form);