whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.1k stars 2.66k forks source link

CSP integration for javascript: URLs seems to be broken #4651

Open bzbarsky opened 5 years ago

bzbarsky commented 5 years ago

For simplicity, let's start at https://html.spec.whatwg.org/multipage/links.html#following-hyperlinks-2 for a javascript: URL.

Step 12 creates a new request. It's not clear what its client is at this point (see https://github.com/whatwg/fetch/issues/907), but given that various parts of the navigation algorithm set the client to things (e.g. in https://html.spec.whatwg.org/multipage/browsing-the-web.html#process-a-navigate-fetch), I'm going to assume it's null at the moment.

We then call into https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate which in step 13 calls into https://html.spec.whatwg.org/multipage/browsing-the-web.html#javascript-protocol. Step 2 of this calls into https://w3c.github.io/webappsec-csp/#should-block-navigation-request which in step 2 does:

For each policy in navigation request’s client’s global object’s CSP list:

So by that point we should really have a client set up, but we don't seem to.

In terms of what implementations do... For the specific case of <a href>, Chrome doesn't support targeting it, so there is only one sane global to use around. But for location.href sets or modifications of the src attribute of <iframe>, it should be observable which global's CSP gets used here.

domenic commented 5 years ago

OK, so the action here is to write tests involving multi-global situations, where one global has CSP applied and one doesn't, and to try to invoke javascript: URLs. When we figure out which global's CSP policy applies, we can use that to specify a client.

(Unless there is a clearly-sensible answer a priori, that we should get browsers to converge on regardless of what they currently do?)

bzbarsky commented 5 years ago

It's not at all clear to me what the "right" answer is, even from first principles of what the goal of preventing inline script via CSP is. And yes, I agree that we need tests to see what browsers are actually doing here.

bzbarsky commented 5 years ago

OK, I wrote some tests:

(They're not WPTs because I needed a simple way to run them in release browsers). The first two tests test doing javascript: loads in subframes that may or may not have CSP that blocks scripts. The second test has a CSP that blocks inline scripts on the toplevel page. The third and fourth tests are similar but use auxiliary browsing contexts (popups) instead of subframes. These tests do not try to tease out the difference between incumbent and entry objects for location sets. They also do not try to tease out when and whether CSP gets snapshotted and/or the exact timing of the CSP checks (this can matter given that <meta> can modify CSP).

Observed behavior seems to be as follows, as far as I can tell, where source and target correspond to the documents of source and browsingContext in https://html.spec.whatwg.org/multipage/browsing-the-web.html#javascript-protocol

bzbarsky commented 5 years ago

and/or the exact timing of the CSP checks (this can matter given that <meta> can modify CSP).

It also matters because CSP violations get reported, and these reports are observable (possibly even within the page, with reporting API). If the Chrome behavior is decided on, then obviously the order of the two CSP checks needs to be specified, because it will affect the reporting that happens.

@mikewest @ckerschb

bzbarsky commented 5 years ago

@dveditz

bzbarsky commented 5 years ago

Given the state of the spec and implementations, by the way, I'm pretty curious what the WPTs at https://wpt.fyi/results/content-security-policy/navigation are based on, if anything.

bzbarsky commented 5 years ago

So one plausible path to interop and greater security here is:

1) Navigations should snapshot the CSP of their client at navigation start time. That client should be determined in an identical way for javascript: navigations and fetch navigations. 2) javascript: navigations should perform CSP checks against both the snapshot CSP and the CSP of the document in whose global they will be executed. This is basically the Chrome behavior, which is the strictest of the above behaviors, but presumably web-compatible enough, since Chrome is shipping it. 3) javascript: loads from user (or extension?) actions should presumably be exempted from both CSP checks. The extent to which this sort of thing can be standardized is not clear to me.

bzbarsky commented 5 years ago

@mikewest Could you, or whoever is actively doing CSP work in Chrome, please take a look at https://github.com/whatwg/html/issues/4651#issuecomment-495333186 and respond with thoughts?

natechapin commented 5 years ago
  • Chrome 76.0.3800.0 dev: Blocks the execution if either source or target has a CSP that would block execution, which just isn't supported at all by the current spec, since that only does one CSP check. Incidentally, it looks like https://bugs.chromium.org/p/chromium/issues/detail?id=889891 may be fixed now?

Yeah, we had multiple issues open for this, and I fixed it on https://bugs.chromium.org/p/chromium/issues/detail?id=944213. I've duped 889891, thanks for pointing it out.

So one plausible path to interop and greater security here is:

  1. Navigations should snapshot the CSP of their client at navigation start time. That client should be determined in an identical way for javascript: navigations and fetch navigations.
  2. javascript: navigations should perform CSP checks against both the snapshot CSP and the CSP of the document in whose global they will be executed. This is basically the Chrome behavior, which is the strictest of the above behaviors, but presumably web-compatible enough, since Chrome is shipping it.
  3. javascript: loads from user (or extension?) actions should presumably be exempted from both CSP checks. The extent to which this sort of thing can be standardized is not clear to me.

@andypaicu or @otherdaniel would probably know better than I do on CSP-specific stuff.

It's not clear to me how intentional chrome's behavior in (2) was, but since it seems web-compatible and the most strict option, it seems good. One weird aspect, though, is that chromium currently performs the client CSP check before queueing the task to execute the javascript url, but performs the CSP check against the targeted document during the queued task. I have a WIP to do both checks before queueing the task (https://chromium-review.googlesource.com/c/chromium/src/+/1648755). That doesn't fit the spec's flow very well, but it avoids having to snapshot the client's CSP as described in (1).

For (3), chromium currently omits all CSP checks for user-initated and extension-initiated navigations, though there are experiments in progress that might put some CSP restrictions on extensions (https://bugs.chromium.org/p/chromium/issues/detail?id=896041)

bzbarsky commented 5 years ago

but it avoids having to snapshot the client's CSP

The "client's CSP" doesn't depend on the targeted document, right?

We could add more special-casing for javascript:, but ideally it would just behave as much like other navigations as possible. And other navigations really do want to snapshot the client CSP, not least because they may need access to it from a different process.

natechapin commented 5 years ago

but it avoids having to snapshot the client's CSP

The "client's CSP" doesn't depend on the targeted document, right?

We could add more special-casing for javascript:, but ideally it would just behave as much like other navigations as possible. And other navigations really do want to snapshot the client CSP, not least because they may need access to it from a different process.

That's probably right. I wasn't thinking about other navigations genuinely needing it.

bzbarsky commented 5 years ago

Other navigations need it for navigate-to checks on redirects, if I read the spec correctly. Plus whatever happens for frame-src, but that's not actually the client's CSP, though the spec currently (incorrectly) says it is. Maybe the spec is wrong about navigate-to as well....

mbrodesser-Igalia commented 1 month ago

For simplicity, let's start at https://html.spec.whatwg.org/multipage/links.html#following-hyperlinks-2 for a javascript: URL.

Step 12 creates a new request.

The spec seems to have changed since filing this ticket, since step 12 doesn't do that.

It's not clear what its client is at this point (see whatwg/fetch#907), but given that various parts of the navigation algorithm set the client to things (e.g. in https://html.spec.whatwg.org/multipage/browsing-the-web.html#process-a-navigate-fetch), I'm going to assume it's null at the moment.

We then call into https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate which in step 13 calls into https://html.spec.whatwg.org/multipage/browsing-the-web.html#javascript-protocol.

That id doesn't exist anymore in the spec.

Step 2 of this

Presumably, the roughly equivalent part of the current version of the spec is step 5 of https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate-to-a-javascript:-url. In step 4, the request is initialized without a client.

calls into https://w3c.github.io/webappsec-csp/#should-block-navigation-request which in step 2 does:

For each policy in navigation request’s client’s global object’s CSP list:

So by that point we should really have a client set up, but we don't seem to.

So also in the current spec, there's no client, but one is needed.

mbrodesser-Igalia commented 4 weeks ago

Given the state of the spec and implementations, by the way, I'm pretty curious what the WPTs at https://wpt.fyi/results/content-security-policy/navigation are based on, if anything.

Don't know what they're based on, but here's a summary of some (not all) tests there:

1) https://searchfox.org/mozilla-central/rev/466db50bfdddc209f068e281e552a2583d82f958/testing/web-platform/tests/content-security-policy/navigation/to-javascript-url-script-src.html:

Sub-test-nr: 1 2 3 4
CSP (scripts-src 'nonce-abc') in loading document: Y Y Y Y
CSP allows script in target document: N N Y N
CSP forbids script in target document: N N N Y
Navigation to javascript: url in iframe blocked: Y Y Y Y

Passes in all major browsers except Gecko. In the latter it'd pass, if ^1 were fixed.

2) https://searchfox.org/mozilla-central/rev/466db50bfdddc209f068e281e552a2583d82f958/testing/web-platform/tests/content-security-policy/navigation/to-javascript-url-frame-src.html:

CSP frame-src 'none' in loading document: Y
Navigation to javascript: url in iframe blocked: N

Passes in all major browsers.

mbrodesser-Igalia commented 1 week ago

So one plausible path to interop and greater security here is:

1. Navigations should snapshot the CSP of their client at navigation start time.  That client should be determined in an identical way for `javascript:` navigations and fetch navigations.

"fetch navigations" set their request's client from their sourceSnapshotParam's fetch client [^1]. javascript: navigations ^2 are only invoked from step 20 of ^3. ^3 defines sourceSnapshotParams in step 2; those could be passed to ^2 and its client could be propagated as the request's client.

This requires checking WebKit/Safari's current behavior though {see ^4).

2. `javascript:` navigations should perform CSP checks against both the snapshot CSP and the CSP of the document in whose global they will be executed.  This is basically the Chrome behavior, which is the strictest of the above behaviors, but presumably web-compatible enough, since Chrome is shipping it.

3. javascript: loads from user (or extension?) actions should presumably be exempted from both CSP checks.  The extent to which this sort of thing can be standardized is not clear to me.

[^1]: Step 3 of https://html.spec.whatwg.org/multipage/browsing-the-web.html#create-navigation-params-by-fetching

mbrodesser-Igalia commented 1 week ago

This requires checking WebKit/Safari's current behavior though {see 4).

https://github.com/web-platform-tests/wpt/blob/cce6f2d13f/content-security-policy/navigation/to-javascript-parent-initiated-parent-csp.html covers checking the source's CSP for location.href. Safari nowadays blocks execution for that case too (https://wpt.fyi/results/content-security-policy/navigation/to-javascript-parent-initiated-parent-csp.html?label=master&label=experimental&aligned).

mbrodesser-Igalia commented 1 week ago

https://searchfox.org/mozilla-central/search?q=&path=%2Fhtml%2Fbrowsers%2Fbrowsing-the-web%2Fnavigating-across-documents%2F*javascript*html%24&case=false&regexp=false might contain relevant tests too.

mbrodesser-Igalia commented 1 week ago

WebKit (Gnome Web 46) behaves differently for

The former respects the source's CSP, the latter doesn't, for location.href.

CC @annevk

domenic commented 1 week ago

@mbrodesser-Igalia it's so awesome that you're investigating this in detail. I can't say I've been following everything, since I haven't paged in the comments from 2019 myself. But if you have concrete spec change proposals, or need someone to review web platform tests, please let me know, and I'd love to help.

mbrodesser-Igalia commented 1 week ago

One weird aspect, though, is that chromium currently performs the client CSP check before queueing the task to execute the javascript url, but performs the CSP check against the targeted document during the queued task. I have a WIP to do both checks before queueing the task (https://chromium-review.googlesource.com/c/chromium/src/+/1648755). That doesn't fit the spec's flow very well, but it avoids having to snapshot the client's CSP as described in (1).

@natechapin: ~today, in Chrome, at least one of the CSP checks is performed before queuing the task. See ^1 for an example. It uses the navigation event which is not supported by other engines than Chromium ^2, so the behavior of other browsers can't be checked that way.~

Edit: according to the explainer ^3, no navigation events are fired for javascript:url's. The spec seems to reflect that too. While it may be possible for web-developers to queue tasks in a way so that snapshotting a CSP vs not snapshotting it leads to user-observable differences, it seems so complicated to achieve that it doesn't matter whether engines snapshot or not.

@domenic: WDYT?

mbrodesser-Igalia commented 1 week ago

Other navigations need it for navigate-to checks on redirects, if I read the spec correctly. Plus whatever happens for frame-src, but that's not actually the client's CSP, though the spec currently (incorrectly) says it is. Maybe the spec is wrong about navigate-to as well....

Given navigate-to was removed from the spec ^1, that argument is now invalid.

domenic commented 6 days ago

While it may be possible for web-developers to queue tasks in a way so that snapshotting a CSP vs not snapshotting it leads to user-observable differences, it seems so complicated to achieve that it doesn't matter whether engines snapshot or not.

@domenic: WDYT?

I think it still matters? Specifying deterministic behavior (via snapshotting) seems better than specifying nondeterministic behavior (where which type of task web developers queue might cause them to see the old vs. new CSP).