child-src and frame-src CSP are not applicable when navigating a new window.

child-src and frame-src CSP are not applicable when navigating a new window.

Shift-clicking an anchor/link will open the link in a new window
(similarily ctrl-clicking or middle-clicking will open the link
in a new background tab).  In this scenario child-src and frame-src
Content Security Policies of the parent frame are not applicable
(because we are not navigating the frame containing the anchor,
but instead we are navigating a brand new frame in a new window).

BUG=658701
Committed: https://crrev.com/d7e5f244d54d0a0c8615e4ff216f906851e9fb64
Cr-Commit-Position: refs/heads/master@{#428069}

[Łukasz Anforowicz, 2016-10-26 21:47:20]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 21:47:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 22:57:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 22:57:22]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-10-26 23:20:16]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 23:20:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2016-10-26 23:22:43]

lukasza@chromium.org changed reviewers:
+ japhet@chromium.org, mkwst@chromium.org

[Łukasz Anforowicz, 2016-10-26 23:22:44]

japhet@, can you take a look please?  If things look okay to you, I'll ask
mkwst@ to also do a review.

[Nate Chapin, 2016-10-26 23:24:45]

Fine by me, but mkwst knows CSP much better than I do.

[Łukasz Anforowicz, 2016-10-27 00:03:04]

Mike, can you take a look please?

Note that the issue fixed by this CL has been present for a long time - the
history shift-clicks (i.e. bug 658701) used to work fine because history
anchors/links were targeting "_top".  r426483 stopped paying attention to link
target in case of shift-click/ctrl-click (to make shift-clicks work for remote
frames) and therefore increased the importance of not enforcing CSP in case of
shift-click.  But, even before r426483 CSP would have stopped some legitimate
shift-click navigations.

[commit-bot: I haz the power, 2016-10-27 00:51:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 00:51:44]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-10-27 07:39:43]

LGTM % nits. Thanks for the fix!

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-vs-shift-click.html
(right):

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-vs-shift-click.html:58:
var anchor = iframe.contentWindow.document.getElementById('test-anchor');
How does this work? you've set `frame-src 'none'` above; shouldn't we be
blocking this frame entirely?

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/frame-src-vs-shift-click-target.html
(right):

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/frame-src-vs-shift-click-target.html:5:
'history.length': (history.length),
Nit: You don't need () for these values.

[Łukasz Anforowicz, 2016-10-27 15:28:55]

Thanks for reviewing.  I'll push to CQ later today.

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-vs-shift-click.html
(right):

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/frame-src-vs-shift-click.html:58:
var anchor = iframe.contentWindow.document.getElementById('test-anchor');
On 2016/10/27 07:39:43, Mike West wrote:
> How does this work? you've set `frame-src 'none'` above; shouldn't we be
> blocking this frame entirely?

The subframe is populated via srcdoc attribute (not via src), and so frame-src
directive is not applicable here.

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/frame-src-vs-shift-click-target.html
(right):

https://codereview.chromium.org/2453093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/frame-src-vs-shift-click-target.html:5:
'history.length': (history.length),
On 2016/10/27 07:39:43, Mike West wrote:
> Nit: You don't need () for these values.

Done.

[Łukasz Anforowicz, 2016-10-27 15:29:06]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 15:29:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 17:03:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 17:03:36]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-10-27 17:28:58]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-10-27 17:28:59]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2453093003/#ps40001
(title: "Address a CR feedback nit - get rid of unnecessary parens.")

[commit-bot: I haz the power, 2016-10-27 17:29:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 17:35:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-27 18:00:24]

Description was changed from

==========
child-src and frame-src CSP are not applicable when navigating a new window.

Shift-clicking an anchor/link will open the link in a new window
(similarily ctrl-clicking or middle-clicking will open the link
in a new background tab).  In this scenario child-src and frame-src
Content Security Policies of the parent frame are not applicable
(because we are not navigating the frame containing the anchor,
but instead we are navigating a brand new frame in a new window).

BUG=658701
==========

to

==========
child-src and frame-src CSP are not applicable when navigating a new window.

Shift-clicking an anchor/link will open the link in a new window
(similarily ctrl-clicking or middle-clicking will open the link
in a new background tab).  In this scenario child-src and frame-src
Content Security Policies of the parent frame are not applicable
(because we are not navigating the frame containing the anchor,
but instead we are navigating a brand new frame in a new window).

BUG=658701

Committed: https://crrev.com/d7e5f244d54d0a0c8615e4ff216f906851e9fb64
Cr-Commit-Position: refs/heads/master@{#428069}
==========

[commit-bot: I haz the power, 2016-10-27 18:00:25]

Patchset 3 (id:??) landed as
https://crrev.com/d7e5f244d54d0a0c8615e4ff216f906851e9fb64
Cr-Commit-Position: refs/heads/master@{#428069}