CSP: "local schemes" should inherit policy when embedded.

CSP: "local schemes" should inherit policy when embedded.

https://w3c.github.io/webappsec-csp/#initialize-document-csp mandates
that resources with "local schemes" ('data:', 'blob:', 'filesystem:',
'about:') inherit the policy of their embedding context when pulled in
via an '<iframe>'.

I'm pretty sure this worked at some point in the past, but I apparently
didn't put a test on it. It does work in Firefox. Let's match their
behavior and lock it in.

BUG=513860
R=jochen@chromium.org, dcheng@chromium.org

Committed: https://crrev.com/e82aced182b4fe7ac99437943a7c39238796c1d9
Cr-Commit-Position: refs/heads/master@{#435165}

[Mike West, 2016-11-04 14:52:15]

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

[commit-bot: I haz the power, 2016-11-04 14:52:35]

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

[Mike West, 2016-11-04 14:53:13]

Description was changed from

==========
CSP: "local schemes" should inherit policy when embedded.

https://w3c.github.io/webappsec-csp/#initialize-document-csp mandates
that resources with "local schemes" ('data:', 'blob:', 'filesystem:',
'about:') inherit the policy of their embedding context when pulled in
via an '<iframe>'.

I'm pretty sure this worked at some point in the past, but I apparently
didn't put a test on it. It does work in Firefox.

BUG=513860
R=jochen@chromium.org
==========

to

==========
CSP: "local schemes" should inherit policy when embedded.

https://w3c.github.io/webappsec-csp/#initialize-document-csp mandates
that resources with "local schemes" ('data:', 'blob:', 'filesystem:',
'about:') inherit the policy of their embedding context when pulled in
via an '<iframe>'.

I'm pretty sure this worked at some point in the past, but I apparently
didn't put a test on it. It does work in Firefox. Let's match their
behavior and lock it in.

BUG=513860
R=jochen@chromium.org
==========

[Mike West, 2016-11-04 14:53:26]

WDYT, Jochen? This time I'm putting a test on it.

[jochen (gone - plz use gerrit), 2016-11-04 14:55:07]

jochen@chromium.org changed reviewers:
+ dcheng@chromium.org

[jochen (gone - plz use gerrit), 2016-11-04 14:55:07]

Hum, let's ask dcheng@ to also have a look at this

[commit-bot: I haz the power, 2016-11-04 15:42:19]

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

[commit-bot: I haz the power, 2016-11-04 15:42:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2016-11-04 18:11:28]

https://codereview.chromium.org/2472333003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2472333003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:5427: // those URLs as
'about:blank' in Blink.
So I'm not sure what we /should/ do, but some interesting scenarios:

One thing Blink gets wrong is the origin of about:blank: it's supposed to be
inherited from the initiator, not the parent. I do plan on fixing this at some
point; at that point, does it make sense for CSP to inherit from the parent or
the initiator?

Similarly, for blob and filesystem URLs: they could be different origin from the
parent frame, right? So should they be inheriting the CSP from their creator or
their parent?

https://codereview.chromium.org/2472333003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:5431: m_url.protocolIs("blob")
|| m_url.protocolIs("data")) {
I notice filesystem isn't in this list, though it's in the comment.

[Mike West, 2016-11-18 11:02:57]

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

[commit-bot: I haz the power, 2016-11-18 11:03:13]

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

[Mike West, 2016-11-18 11:06:23]

https://codereview.chromium.org/2472333003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2472333003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:5427: // those URLs as
'about:blank' in Blink.
On 2016/11/04 at 18:11:28, dcheng wrote:
> So I'm not sure what we /should/ do, but some interesting scenarios:
> 
> One thing Blink gets wrong is the origin of about:blank: it's supposed to be
inherited from the initiator, not the parent. I do plan on fixing this at some
point; at that point, does it make sense for CSP to inherit from the parent or
the initiator?

As currently specified, I think we'd end up inheriting from the parent. I think
I agree with your implication that we ought instead inherit from the navigation
initiator.

Today, though, we inherit from neither: that's certainly wrong, and I'd like to
fix it, as it's a fairly straightforward bypass.

> Similarly, for blob and filesystem URLs: they could be different origin from
the parent frame, right?

Can they? I thought we blocked navigation to cross-origin blob:/filesystem:
URLs.

> So should they be inheriting the CSP from their creator or their parent?

Same answer as above.

https://codereview.chromium.org/2472333003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:5431: m_url.protocolIs("blob")
|| m_url.protocolIs("data")) {
On 2016/11/04 at 18:11:27, dcheng wrote:
> I notice filesystem isn't in this list, though it's in the comment.

I had `data:` twice, though, so that's got to count for something. :)

[commit-bot: I haz the power, 2016-11-18 11:53:18]

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

[commit-bot: I haz the power, 2016-11-18 11:53:19]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mike West, 2016-11-18 15:04:02]

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

[commit-bot: I haz the power, 2016-11-18 15:04:31]

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-11-18 17:18:41]

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

[commit-bot: I haz the power, 2016-11-18 17:18:42]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-11-21 10:14:24]

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/cascade/cross-origin.html
(right):

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/cascade/cross-origin.html:27:
"  var i = document.createElement('img');" +
A fun hack I've seen is to define the JS as a function and interpolate it like
this:

"data:text/html,<script>(" +
function () {
  var i = ...;
} + 
")();"</scr" + "ipt>";

Which is kind of nice, because you get actual syntax highlighting (and escaping
is easier). Not really important here, but thought I'd mention it!

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-does-not-affect-child-expected.txt
(left):

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-does-not-affect-child-expected.txt:1:
CONSOLE MESSAGE: Blink Test Plugin: initializing
Is this test diff expected? I would have expected this message to still show up,
but perhaps I'm missing something.

[Mike West, 2016-11-29 10:15:58]

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

[commit-bot: I haz the power, 2016-11-29 10:16:18]

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

[Mike West, 2016-11-29 10:16:29]

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/cascade/cross-origin.html
(right):

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/cascade/cross-origin.html:27:
"  var i = document.createElement('img');" +
On 2016/11/21 at 10:14:24, dcheng wrote:
> A fun hack I've seen is to define the JS as a function and interpolate it like
this:
> 
> "data:text/html,<script>(" +
> function () {
>   var i = ...;
> } + 
> ")();"</scr" + "ipt>";
> 
> Which is kind of nice, because you get actual syntax highlighting (and
escaping is easier). Not really important here, but thought I'd mention it!

*sigh* JavaScript is SO WEIRD.

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-does-not-affect-child-expected.txt
(left):

https://codereview.chromium.org/2472333003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/object-src-does-not-affect-child-expected.txt:1:
CONSOLE MESSAGE: Blink Test Plugin: initializing
On 2016/11/21 at 10:14:24, dcheng wrote:
> Is this test diff expected? I would have expected this message to still show
up, but perhaps I'm missing something.

Sorry, I missed this comment, and was about to hop back into this bug to ping
you!

I typo'd the address; `plugins/` should be `/plugins/`. :)

[commit-bot: I haz the power, 2016-11-29 13:21:08]

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

[commit-bot: I haz the power, 2016-11-29 13:21:09]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-11-29 14:39:32]

Description was changed from

==========
CSP: "local schemes" should inherit policy when embedded.

https://w3c.github.io/webappsec-csp/#initialize-document-csp mandates
that resources with "local schemes" ('data:', 'blob:', 'filesystem:',
'about:') inherit the policy of their embedding context when pulled in
via an '<iframe>'.

I'm pretty sure this worked at some point in the past, but I apparently
didn't put a test on it. It does work in Firefox. Let's match their
behavior and lock it in.

BUG=513860
R=jochen@chromium.org
==========

to

==========
CSP: "local schemes" should inherit policy when embedded.

https://w3c.github.io/webappsec-csp/#initialize-document-csp mandates
that resources with "local schemes" ('data:', 'blob:', 'filesystem:',
'about:') inherit the policy of their embedding context when pulled in
via an '<iframe>'.

I'm pretty sure this worked at some point in the past, but I apparently
didn't put a test on it. It does work in Firefox. Let's match their
behavior and lock it in.

BUG=513860
R=jochen@chromium.org, dcheng@chromium.org
==========

[Mike West, 2016-11-29 14:39:32]

mkwst@chromium.org changed required reviewers:
+ dcheng@chromium.org

[Mike West, 2016-11-29 14:40:05]

jochen@: Assuming dcheng@ is happy with the patch, would you be willing to stamp
the //chrome bits? Marking him as required...

[jochen (gone - plz use gerrit), 2016-11-29 14:49:02]

totally, lgtm

[dcheng, 2016-11-29 22:05:12]

lgtm

[Mike West, 2016-11-30 08:19:14]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-11-30 08:19:54]

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-11-30 08:25:05]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480493954099920,
"parent_rev": "4de4e20db92ba90e5ccf5fed7d81f19b2ca84b5e", "commit_rev":
"5f301226eee456d187f7445529a76999f7b9a233"}

[commit-bot: I haz the power, 2016-11-30 08:25:32]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-30 08:28:11]

Description was changed from

==========
CSP: "local schemes" should inherit policy when embedded.

https://w3c.github.io/webappsec-csp/#initialize-document-csp mandates
that resources with "local schemes" ('data:', 'blob:', 'filesystem:',
'about:') inherit the policy of their embedding context when pulled in
via an '<iframe>'.

I'm pretty sure this worked at some point in the past, but I apparently
didn't put a test on it. It does work in Firefox. Let's match their
behavior and lock it in.

BUG=513860
R=jochen@chromium.org, dcheng@chromium.org
==========

to

==========
CSP: "local schemes" should inherit policy when embedded.

https://w3c.github.io/webappsec-csp/#initialize-document-csp mandates
that resources with "local schemes" ('data:', 'blob:', 'filesystem:',
'about:') inherit the policy of their embedding context when pulled in
via an '<iframe>'.

I'm pretty sure this worked at some point in the past, but I apparently
didn't put a test on it. It does work in Firefox. Let's match their
behavior and lock it in.

BUG=513860
R=jochen@chromium.org, dcheng@chromium.org

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

[commit-bot: I haz the power, 2016-11-30 08:28:12]

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