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

content/browser/frame_host/form_submission_throttle.h

Index: content/browser/frame_host/form_submission_throttle.h
diff --git a/content/browser/frame_host/form_submission_throttle.h b/content/browser/frame_host/form_submission_throttle.h
new file mode 100644
index 0000000000000000000000000000000000000000..d8961092b210ac3f14152e818f16b81013dbcf12
--- /dev/null
+++ b/content/browser/frame_host/form_submission_throttle.h
@@ -0,0 +1,39 @@
+// Copyright 2017 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.
+
+#ifndef CONTENT_BROWSER_FRAME_HOST_FORM_SUBMISSION_THROTTLE_H_
+#define CONTENT_BROWSER_FRAME_HOST_FORM_SUBMISSION_THROTTLE_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "content/public/browser/navigation_throttle.h"
+
+namespace content {
+class NavigationHandle;
+
+// A FormSubmittionThrottle is responsible for enforcing the 'form-action' CSP,

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

nit: s/CSP/CSP directive/

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

Done.

+// blocking requests which violate them.

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

Like frame-src, I'd document that this is currently only enforcing form-action
for PlzNavigate, and Blink is still enforcing this without PlzNavigate. 
Presumably, the Blink check is the allowFormAction check in
FrameLoader::shouldContinueForNavigationPolicy?  Does that need an exclusion for
PlzNavigate mode like the frame-src one?

[Mike West, 2017/03/02 10:45:34]

That kind of explicit carveout makes sense to me, if only because it means this
code would be better tested via Blink's layout tests. Right now I don't think
they're running through the WillStartRequest bits for blocked requests at all.

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

Done.
Yes, but not only, it is checked in a few different other places too.
It doesn't need, we cant check the CSP on both side. Not checking the CSP on the
renderer-side could be a good thing in order to make the current Layout test
testing the browser-side implementation instead of the renderer-side one.

Maybe disabling the renderer-side checks with PlzNavigate for all the request is
the right thing to do.

[Mike West, 2017/03/09 08:20:04]

I think it is, for the same reasons that we did so for Mixed Content.

[arthursonzogni, 2017/03/10 09:35:37]

I tried, every layout tests are working with two exceptions.
1) What if the form url is a javascript url? It this case it is not a navigation
and the request use the PlzNavigate path.
2) What if the form target another window?

In addition, maybe there will be a problem (with and without PlzNavigate) in the
following scenario:
Two windows,
1) [a.com] with "form-action redir.com"
2) [b.com]
redir.com is a redirection to b.com
Then happens a form submission from 1) that target 2) with url=redir.com
Is the request blocked after the redirection? I think the answer is no in both
mode.

+class CONTENT_EXPORT FormSubmissionThrottle : public NavigationThrottle {
+ public:
+  static std::unique_ptr<NavigationThrottle> MaybeCreateThrottleFor(
+      NavigationHandle* handle);
+
+  ~FormSubmissionThrottle() override;
+
+  NavigationThrottle::ThrottleCheckResult WillStartRequest() override;
+  NavigationThrottle::ThrottleCheckResult WillRedirectRequest() override;
+
+ private:
+  explicit FormSubmissionThrottle(NavigationHandle* handle);
+  NavigationThrottle::ThrottleCheckResult CheckContentSecurityPolicyFormAction(
+      bool is_redirect);
+
+  DISALLOW_COPY_AND_ASSIGN(FormSubmissionThrottle);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_FRAME_HOST_FORM_SUBMISSION_THROTTLE_H_