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

content/common/navigation_params.h

 // Copyright 2014 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_COMMON_NAVIGATION_PARAMS_H_
 #define CONTENT_COMMON_NAVIGATION_PARAMS_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 40 matching lines @@
       FrameMsg_Navigate_Type::Value navigation_type,
       bool allow_download,
       bool should_replace_current_entry,
       base::TimeTicks ui_timestamp,
       FrameMsg_UILoadMetricsReportType::Value report_type,
       const GURL& base_url_for_data_url,
       const GURL& history_url_for_data_url,
       PreviewsState previews_state,
       const base::TimeTicks& navigation_start,
       std::string method,
-      const scoped_refptr<ResourceRequestBodyImpl>& post_data);
+      const scoped_refptr<ResourceRequestBodyImpl>& post_data,
+      bool should_bypass_main_world_CSP);

[alexmos, 2017/02/10 22:59:53]

for style consistency, I'd favor lowercasing the CSP, i.e.,
should_bypass_main_world_csp.

[arthursonzogni, 2017/02/13 16:33:20]

Done.

   CommonNavigationParams(const CommonNavigationParams& other);
   ~CommonNavigationParams();
 
   // The URL to navigate to.
   // PlzNavigate: May be modified when the navigation is ready to commit.
   GURL url;
 
   // The URL to send in the "Referer" header field. Can be empty if there is
   // no referrer.
   Referrer referrer;
@@ skipping 40 matching lines @@
   // navigation_start value in Blink.
   // PlzNavigate: For renderer initiated navigations, this will be set on the
   // renderer side and sent with FrameHostMsg_BeginNavigation.
   base::TimeTicks navigation_start;
 
   // The request method: GET, POST, etc.
   std::string method;
 
   // Body of HTTP POST request.
   scoped_refptr<ResourceRequestBodyImpl> post_data;
+
+  // Whether or not this navigation, including each redirections, should be
+  // checked against the Content-Security-Policy(CSP) of the frames that
+  // surround it. It is actually used to bypass the 'frame-src' and 'child-src'

[alexmos, 2017/02/10 22:59:53]

nit: drop "actually"

[alexmos, 2017/02/10 22:59:53]

"frames that surround it" is a big vague.  Should this be just "parent frame",
if this is for frame-src only?  I'm not familiar with this bit, but is it used
to bypass other directives in Blink?

[arthursonzogni, 2017/02/13 16:33:20]

Done. I misunderstood what "main_world" means, it is simply the opposite of
"isolated_world". This comment is very bad, I will update it.
AFAIK, it is used for 'frame-src' and not for 'form-action'.

It is also used for others CSP, for instance 'style-src', 'script-src', ..., but
I only care about CSP when it can block a navigation.

[alexmos, 2017/02/14 06:57:20]

Interesting, thanks.  It's sad that an evil renderer can just pass this bit to
turn off the enforcement of frame-src in the browser process.  Perhaps down the
road we could add some validation in the browser process, such as whether the
renderer process really contained extension content scripts that could do this.

By the way, it doesn't seem like the isolated world's CSP is actually enforced
today (looking at how DOMWrapperWorld::setIsolatedWorldContentSecurityPolicy
ignores the passed-in policy).  It's only used to flip the bit that allows
bypassing the main world CSP.  So it doesn't seem like we need to worry about
how to enforce frame-src from the isolated world's CSP, for example.

+  // CSPs when the resource that triggers this navigation lives in an isolated
+  // world.
+  bool should_bypass_main_world_CSP;

[alexmos, 2017/02/10 22:59:53]

Do we have tests that things work properly when this is true, with PlzNavigate
enabled?

[arthursonzogni, 2017/02/13 16:33:20]

Yes, I did this to make this test work:
http/tests/security/isolatedWorld/bypass-main-world-csp-iframes.html

I was able to remove the failure expectation from:
third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation

[alexmos, 2017/02/14 06:57:20]

Acknowledged.

 };
 
 // Provided by the renderer ----------------------------------------------------
 //
 // This struct holds parameters sent by the renderer to the browser. It is only
 // used in PlzNavigate (since in the current architecture, the renderer does not
 // inform the browser of navigations until they commit).
 
 // This struct is not used outside of the PlzNavigate project.
 // PlzNavigate: parameters needed to start a navigation on the IO thread,
@@ skipping 221 matching lines @@
   ~NavigationParams();
 
   CommonNavigationParams common_params;
   StartNavigationParams start_params;
   RequestNavigationParams request_params;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_COMMON_NAVIGATION_PARAMS_H_