PlzNavigate: Enforce 'frame-src' CSP on the browser.

content/browser/frame_host/render_frame_host_impl.h

Index: content/browser/frame_host/render_frame_host_impl.h
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
index b1e92c0f70a504187b644233a322bb69ad4c9764..c4bcb87c958644a74607897c47ad4e5a0440ead9 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -24,13 +24,14 @@
 #include "build/build_config.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/bad_message.h"
+#include "content/browser/frame_host/csp_context_impl.h"
 #include "content/browser/loader/global_routing_id.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_impl.h"
 #include "content/common/accessibility_mode_enums.h"
 #include "content/common/ax_content_node_data.h"
 #include "content/common/content_export.h"
-#include "content/common/content_security_policy/content_security_policy.h"
+#include "content/common/content_security_policy/csp_context.h"
 #include "content/common/download/mhtml_save_status.h"
 #include "content/common/frame.mojom.h"
 #include "content/common/frame_message_enums.h"
@@ -271,6 +272,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // Update this frame's last committed origin.
   void set_last_committed_origin(const url::Origin& origin) {
     last_committed_origin_ = origin;
+    csp_context_->SetSelf(origin);
   }
 
   // Returns the associated WebUI or null if none applies.
@@ -551,6 +553,12 @@ class CONTENT_EXPORT RenderFrameHostImpl
                         bool has_stale_copy_in_cache,
                         int error_code);
 
+  // PlzNavigate
+  // Inform the renderer process that a navigation has been blocked by a content
+  // security policy.
+  void ReportContentSecurityPolicyViolation(
+      const CSPViolationParams& violation_params);
+
   // Sets up the Mojo connection between this instance and its associated render
   // frame if it has not yet been set up.
   void SetUpMojoIfNeeded();
@@ -619,6 +627,19 @@ class CONTENT_EXPORT RenderFrameHostImpl
     return has_focused_editable_element_;
   }
 
+  // Returns the set of Content-Security-Policy policies to enforce on the
+  // browser-side.
+  const std::vector<ContentSecurityPolicy>& content_security_policies() const {

[alexmos, 2017/02/24 06:40:27]

I wonder if content's ContentSecurityPolicy should just hide the fact that there
are multiple policies within itself.  I.e., behave similarly to Blink's
ContentSecurityPolicy which I think stores the vector of m_policies internally. 
Then we won't need to pass this vector around whenever we want to check the CSP,
which would make the code in callers easier to follow.  Perhaps CSPContext could
be part of that too; i.e., it'd be nice to just say
parent->content_security_policy()->Allow(CSPDirective::FrameSrc, url,
is_redirect)), instead of dealing with CSPContexts and vectors of policies.  Or
are there reasons that it has to be organized like this?  (This is fine to think
about for a followup cleanup.)

[arthursonzogni, 2017/02/24 16:13:29]

The name of the classes inside blink confused me.
ContentSecurityPolicy is not a policy but the concept, it store several policies
and does what I do in CSPContext also.
DirectiveSourceList is the the policy :)

I am not sure what is the best thing. I will try to store the policies inside
the CSPContext, please tell me if it looks good to you.

+    return content_security_policies_;
+  }
+
+  void ResetContentSecurityPolicy() { content_security_policies_.clear(); }

[alexmos, 2017/02/24 06:40:27]

Should this be ResetContentSecurityPolicies, since it's resetting a set of
policies?

[arthursonzogni, 2017/02/24 16:13:29]

I copied FrameTreeNode::ResetContentSecurityPolicy, will 
Removed in the latest CL.

+
+  // Returns the context that must be used to check if a RenderFrameHost is
+  // allowed to navigate to an URL according to a set of content-security-policy
+  // policies. RenderFrameHostImpl. Never null.

[alexmos, 2017/02/24 06:40:27]

Is "RenderFrameHostImpl." needed?

[arthursonzogni, 2017/02/24 16:13:29]

Removed in the latest CL.

+  CSPContext* csp_context() { return csp_context_.get(); }
+
  protected:
   friend class RenderFrameHostFactory;
 
@@ -1154,6 +1175,13 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // Tracks the feature policy which has been set on this frame.
   std::unique_ptr<FeaturePolicy> feature_policy_;
 
+  // A set of Content-Security-Policy policies to enforce on the browser-side.
+  std::vector<ContentSecurityPolicy> content_security_policies_;

[alexmos, 2017/02/24 06:40:27]

It might be good to document here why we need this to be a set of policies,
rather than a single policy.  (I guess the explanation is in the comment for
FrameHostMsg_DidAddContentSecurityPolicy?)

[arthursonzogni, 2017/02/24 16:13:29]

Removed in the latest CL.

+
+  // Used to check if a frame is allowed to navigate to an URL according to a
+  // set of content-security-policy policies.
+  std::unique_ptr<CSPContext> csp_context_;
+
   // NOTE: This must be the last member.
   base::WeakPtrFactory<RenderFrameHostImpl> weak_ptr_factory_;