Maybe 'frame-ancestors' CSP should be handled in the browser process |
|||||||
Issue descriptionIssue 555418 tracked moving of processing of XFO to the browser process. This issue is now marked as fixed, but there is still a remaining/unaddressed TODO referring to this bug: //content/browser/frame_host/ancestor_throttle.cc: // TODO(mkwst): 'frame-ancestors' is currently handled in Blink. We should // handle it here instead. Until then, don't block the request, and let // Blink handle it. https://crbug.com/555418 This TODO seems to be still applicable - for example, I don't get any real hits when searching for [fF]rame.*[aA]ncestors in browser-side code: https://cs.chromium.org/search/?q=%5BfF%5Drame.*%5BaA%5Dncestors+lang:cpp+file:browser&sq=package:chromium&type=cs OTOH, I hear from nasko@ that arthursonzogni@ did some work for PlzNavigate that involved moving CSP processing to the browser process. Let's use the current, new bug to decide: 1. If we still need and want to move processing of frame-ancestors to the browser. - Maybe the comment above is just a stale/obsolete TODO that we can safely delete? - Maybe it is fine to handle 'frame-ancestors' directive in the renderer: AFAIK CSP directives from headers sent by origin A will be processed in the renderer for origin A. Renderer A should be able to trust the origin of frame's parent (which will be provided by the browser process). 2. How to do the move if needed. - AFAICT frame-ancestors never fired the 'securitypolicyviolation' DOM event, but I am not sure if the browser process is capable today of A) posting CSP violation reports to http endpoints declared in 'report-uri' or 'report-to' directive B) reporting CSP violations to the js console.
,
Aug 25 2017
AFAIK frame-ancestors is still enforced in the renderer process even with PlzNavigate, and Arthur's work was more about frame-src and form-action - Arthur, is that correct? Note that we also have a more generic issue 376522 to track moving CSP to the browser process.
,
Aug 28 2017
1- 'frame-ancestors' is still enforced in the renderer process.
It appeared that we did not need to move it with PlzNavigate. I only moved
the 'frame-src' and 'form-action' directives. The content of 'child-src' and
'default-src' are accessible from the browser too (for CSP fallback.)
2- RenderFrameHostImpl::ReportContentSecurityPolicyViolation() is used to post
CSP violation reports to http endpoints, emit CSP violation event and print
a message in the console. Internally, it reuses blink's implementation (i.e.
it happens on the Renderer side.)
Moving 'frame-ancestors' is harder:
* The CSP headers are parsed on the renderer-side, transmitted and checked on
the browser-side. If we want to check the 'frame-ancestors' directive on the
browser-side, it means that the request are blocked before having a
Renderer. So we need to parse the CSP on the browser-side. I initially wrote
a parser, but we decided to remove it.
See https://codereview.chromium.org/2612793002/#ps80001 (patchset 1).
* Posting CSP violation reports will have to be done on the browser-side too.
,
Aug 28 2017
I agree with Alex and Arthur: moving `frame-ancestors` (and, really, the rest of CSP) up out of Blink would be a valuable change for a variety of reasons. But it's a lot of work and we haven't done it yet. You interested, lukasza@? :) +andypaicu@ for CSP planning.
,
Nov 10 2017
,
Nov 18 2017
,
Feb 18 2018
,
Apr 23 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by lukasza@chromium.org
, Aug 25 2017