Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify worker-src goals #146

Closed
briansmith opened this issue Nov 12, 2016 · 26 comments
Closed

Clarify worker-src goals #146

briansmith opened this issue Nov 12, 2016 · 26 comments

Comments

@briansmith
Copy link

IIUC, JS6 module import is equivalent to injecting a <script type=module> which should already be handled by script-src.

It seems to me that web workers are very much like JS6 modules. So, why isn't <script src> used to control web workers too? What is additionally threatening about web workers that make them deserving of special treatment compared to other kinds of script? IMO, the threading issue for web workers is not a significant issue w.r.t. security and so I see no reason to treat them differently than JS6 modules.

Off the top of my head, I can't identify a clear problem that would warrant shared workers getting special treatment compared to other scripts either. Do shared workers actually enable any functionality that a script can't accomplish without them? If not, it seems there's no point in treating shared workers differently from JS6 modules either.

OTOH, despite the similarity in naming, Service Workers seem quite different from web workers and shared workers. I believe Service Workers fundamentally do provide powerful functionality that can't be accomplished another way, though I could be convinced otherwise. In fact, when considering the footgun aspect, Service Workers are potentially much more dangerous than regular script. This makes me think that, regardless of whether script-src should affect Service Workers (#31), it makes sense to have a separate directive that allows one to block Service Workers from origins where other scripts are allowed. However, I wouldn't want to block web workers or shared workers or JS6 modules too.

This makes me think that script-src should control JS6 modules, web workers, and shared workers. Maybe it should also affect service workers (#31). But, there should be a separate directive for controlling service workers, separate from the directive used for web workers, shared workers, and JS6 modules. And, it seems to me that a directive separate from script-src isn't needed for web workers, shared workers, and JS6 modules.

@briansmith
Copy link
Author

BTW, if we agree with the above conclusion, then deprecating child-src (#145) makes even more sense.

@briansmith
Copy link
Author

Another thing to consider is whether strict-dynamic should have any effect here, since none of the above things (workers, JS6 modules, etc.) are "parser-inserted".

@annevk
Copy link
Member

annevk commented Nov 13, 2016

See #31.

@mikewest
Copy link
Member

@hillbrad can correct my memory, but I believe that we broke child-src out into a distinct directive because both workers and frames create distinct execution contexts which don't inherit a policy from their embedding context, but are instead governed by their own policy (modulo blob:, data:, etc). That seems like a relevant difference which justifies treating them differently than script.

That said, I think I agree with the underlying criticism of both this and #31 that splitting workers away from script-src was a bad decision. https://www.chromestatus.com/metrics/feature/timeline/popularity/258 tracks the occurrence of workers allowed by child-src, but which would be denied by script-src. When I initially landed that, the number was huge; looking at it now, shifting worker-src to chain to script-src might be a change we could make (~0.002% of page views isn't nothing, but it's also not large).

With regard to service workers, I agree that they're uniquely dangerous, but I think they're uniquely dangerous to the origin, and not to the particular page. Within the context of a single page, treating them like any other worker seems reasonable. The server should, however, have the ability to deny its resources the ability to be used as a service worker. CSP currently does this via sandbox checks on responses (https://w3c.github.io/webappsec-csp/#sandbox-response); that might be a few indirections too far, but folks weren't thrilled with suggestions that SW be purely opt-in via MIME type or similar. I'd be thrilled to revisit that discussion, FWIW (@jakearchibald, @jungkees, etc).

@briansmith
Copy link
Author

don't inherit a policy from their embedding context

Wow, that surprises me! I can see why that's the case for Shared Workers and Service Workers, but I agree with https://lists.w3.org/Archives/Public/public-web-security/2011Nov/0027.html regarding regular workers. (Maybe too late.)

I would expect scripts-src by default to apply to JS modules and <link rel=serviceworker>. In particular, if I have script-src: 'none' then no JS code should be able to be loaded from the page.

Also, the principle of least astonishment would lean towards all kinds of workers being restricted to the page's CSP policy, connect-src in particular, by default. (IIUC, Firefox actually did something similar to this until March 2016.) I could see the need to have a way to say "I don't want fetches from {shared, service} workers started from this page to be restricted by this page's connect-src policy," which presumably would be an additional connect-src directive. For example, connect-src: 'worker-separate-policy' could mean that a worker will be restricted to the current page's policy unless the HTTP response for the fetch of the worker itself contains a CSP policy, in which case that CSP policy is used for that worker instead.

@mikewest
Copy link
Member

Wow, that surprises me! I can see why that's the case for Shared Workers and Service Workers, but I agree with https://lists.w3.org/Archives/Public/public-web-security/2011Nov/0027.html regarding regular workers. (Maybe too late.)

I would not be terribly sad if we changed the behavior for dedicated workers (@hillbrad, @dveditz, @devd, @ckerschb, @johnwilander, @teddink etc. WDYT?).

I would expect scripts-src by default to apply to JS modules and . In particular, if I have script-src: 'none' then no JS code should be able to be loaded from the page.

I would not be terribly sad if we moved worker-src on top of script-src. That might take some time, but based on the numbers above, it's potentially doable.

Also, the principle of least astonishment would lean towards all kinds of workers being restricted to the page's CSP policy

You say "the page", but which page do you mean? Isn't it odd for a {Service,Shared} Worker to have different behavior based on which of an origin's pages you visit first? I agree that it's something that developers have to think about, and it would be nice to relieve them of that burden, but I'm not sure that pushing a policy down to the shared resource makes sense on its own.

@briansmith
Copy link
Author

briansmith commented Nov 15, 2016

Isn't it odd for a {Service,Shared} Worker to have different behavior based on which of an origin's pages you visit first?

Yes. It is weird that {Service, Shared} Workers aren't restricted by connect-src, and it would be weird if they were restricted by different connect-src from different pages. This is why I think connect-src: 'worker-separate-policy' is good: It makes it clear in the policy that connect-src doesn't apply to these workers (as long as they define their own policy), while still allowing the workers to, uh, work. There would be a slightly awkward transition period but I think the end result would be worth the awkwardness.

@arturjanc
Copy link

I would expect scripts-src by default to apply to JS modules and <link rel=serviceworker>. In particular, if I have script-src: 'none' then no JS code should be able to be loaded from the page.
I would not be terribly sad if we moved worker-src on top of script-src. That might take some time, but based on the numbers above, it's potentially doable.

I like the idea of having workers subject to script-src, with the following caveats:

  • It's much more common for developers to set script-src than {worker,child}-src which means that it's likely that workers, which previously weren't restricted by CSP, might suddenly be subject to the policy. Mike, does your UMA data cover the case where the document doesn't set default-src or worker-src?
  • We'd need to figure out how this would interact with strict-dynamic. I think workers added programmatically (e.g. via new Worker() or insertScripts()) should just be allowed to execute, but at least ServiceWorkers can be installed in parser-inserted ways (<link rel=serviceworker>) so this way of installing them should likely require a nonce.

@mikewest
Copy link
Member

We discussed this on the webappsec call yesterday: folks seemed generally ok with moving workers to script-src, and treating dedicated workers as inheriting the policy of their creator document. We probably need more discussion for the 'worker-separate-policy' proposal, as I think I did a bad job explaining it (and I'm not the best person to advocate it in the first place).

@arturjanc:

  1. The metric I posted above is triggered here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp?rcl=0&l=981 It counts the times where the worker-src->child-src->default-src chain allows a worker, while the script-src->default-src chain does not.
  2. Your suggestion for 'strict-dynamic' is more or less what I had in mind. In the presence of that keyword, nonced/hashed scripts can load whatever workers they like. Skimming Chrome's source, it looks like we track parser-inserted status for <link> elements, so treating <link rel="serviceworker"> as similar to <script src="whatever"> seems doable.

@mikewest
Copy link
Member

@annevk
Copy link
Member

annevk commented Nov 29, 2016

https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/ and let @travisleithead know about it so you get VIP treatment.

mikewest added a commit that referenced this issue Nov 29, 2016
Based on the discussion in #146, this
patch makes two substantive changes:

1. 'worker-src' now defers to 'script-src', not 'child-src'.
2. Dedicated workers now always inherit their policy from the context that
   created the worker.

Hopefully this makes more sense, and we won't change it again in a month.
MXEBot pushed a commit to mirror/chromium that referenced this issue Nov 30, 2016
Good discussion at w3c/webappsec-csp#146
suggests that we might want to modify `worker-src` before shipping it.
So let's work that out first.

BUG=666664

Review-Url: https://codereview.chromium.org/2503213005
Cr-Commit-Position: refs/heads/master@{#433152}
(cherry picked from commit 8071e0f)

Review URL: https://codereview.chromium.org/2541443003 .

Cr-Commit-Position: refs/branch-heads/2924@{#151}
Cr-Branched-From: 3a87aec-refs/heads/master@{#433059}
@briansmith
Copy link
Author

briansmith commented Nov 30, 2016

@mikewest wrote in https://groups.google.com/a/chromium.org/d/msg/blink-dev/npKDoKVOUAs/vQtbrZ2VBAAJ:

Workers have to be same-origin, for instance, so a developer who wishes to place as little trust in their server as possible might allow worker-src https://example.com/workers/this-specific-worker.js on the specific page that needs it, while pushing general script out to script-src https://super-fast-and-speedy.cdn.net/all-my-script/.

Here's my understanding of Mike's example: The page author wants to execute a worker, which they would host on https://super-fast-and-speedy.cdn.net/all-my-script/ if HTML allowed that. But, HTML doesn't allow that, so it has to use a worker hosted on 'self'.

In the quoted text above, Mike proposes to solve that by having a worker-src that applies to workers. I think we should first try to “just” change HTML to allow easily load of workers from the non-'self' origin. Then worker-src wouldn't be needed for workers and plus we'd be solving a problem that people have to work around.

See also whatwg/html#1243 (comment), where Mike wrote:

There's a pretty clear analog between Worker("data:...") and <script src="data:...">. If we accept one, we should probably accept the other.

Like I said in the first comment in this issue, I think we should continue to work towards a world where Workers and <script> and JS6 modules are treated as uniformly as possible.

I'll comment later on service workers and shared workers, but I wanted to throw this out ASAP to learn why it wouldn't work.

@briansmith
Copy link
Author

Regarding workers, shared workers, and service workers, and whether the one directive or separate directives should apply to them, I think we should approach this issue as follows:

First, let's stop using the names “{,shared, service} workers," because the common part of the name, “worker,” biases us towards treating them all the same. Let's instead use the names A, B, and C, respectively. Using the new names, there's nothing about A, B, and C that look similar and so I'd expect us to have a-src, b-src, and c-src in CSP. Now, let's construct an argument that A, B, and C are so similar that there's no purpose of distinguishing between them as far as CSP is concerned. Whether we succeed or fail at constructing such an argument would help us make a decision about which directives are needed to control A, B, and C.

@briansmith
Copy link
Author

In addition, https://html.spec.whatwg.org/multipage/workers.html#module-worker-example, which describes using a JS6 module as a worker without the same-origin restriction, basically doing what I suggested in making JS6 modules and workers more uniform.

@mikewest mikewest closed this as completed Dec 1, 2016
@mikewest mikewest reopened this Dec 1, 2016
@mikewest
Copy link
Member

mikewest commented Dec 1, 2016

Rereading this thread, I think you are making a few suggestions, @briansmith:

  1. Dedicated workers are the same as scripts, so script-src should control their instantiation, and they should act under the same policy as the page that instantiates them.

    Moreover, worker-src is unnecessary for dedicated workers, as new Worker() does not provide any capability above and beyond <script src="..."> once we ensure that they always have the same policy as the page that instantiates them.

  2. Shared workers are the same as scripts in most relevant ways, should be controlled by script-src and should inherit the policy of the page that instantiates them. In particular, they ought to inherit connect-src (so that the page can't use them to proxy connections?), potentially with an opt-in mechanism to allow them to have their own policy for things like connect-src.

    worker-src is likewise unnecessary for shared workers.

  3. Service workers are unique, providing capabilities beyond dedicated and shared workers. They might need a new directive entirely, but should also inherit the policy of the page that instantiates them with some sort of opt-out.

Is that a more or less accurate summary of your thinking?

Assuming that it is, and that I haven't just set up strawmen: I agree with much of what you're claiming, but I have some concerns with the model you're proposing. Splitting out the discussion on slightly different axes, let's talk about inheritance and control:

Inheritance:

  1. I don't think there's debate on inheriting policy into dedicated workers in the same way that we do for scripts. If folks disagree, please say so, but I'm taking this as settled at this point.

  2. On the other hand, the suggestion that shared workers and service workers ought to inherit policy from the context that creates them seems wrong to me. I think there's a pretty good case for a service worker to have a policy completely distinct from any particular resource on a site. To some extent, the service worker is the network, so far as the page is concerned, and it is the network for every page it controls. Its policy needs to be a superset of the policies of the pages it controls in order to service their requests. It would be unfortunate if, for instance, a site had to enable object-src on every page just because one page needed Flash (as the Service Worker might be instantiated from that page if that's where the user happened to first land on the site). Likewise, shared workers service requests from multiple pages, and have a lifetime that spans the set of all their connections. I think similar arguments apply: the page and the worker have different roles, and I don't think it makes sense to conflate their policies.

Control:

As I suggested above, I consider service workers uniquely dangerous, but they're dangerous to the origin, not the page. If an origin wishes to prevent service workers from being loaded, doing so on a response-by-response basis seems unlikely to be effective. A more pervasive solution is necessary, and since the ship has sailed on opt-in solutions like MIME type requirements, servers ought to take advantage of the opt-out mechanisms provided, like the Service-Worker: script request header, blocking responses accordingly. In other words, while I think it makes sense to allow a particular resource the ability to prevent itself from loading a service worker for general containment reasons, the risk isn't resource-specific.

I think shared workers have similar properties from a page's perspective: they have a distinct policy (assuming you accept my argument above), they have a distinct lifetime, and they're generally used for different purposes as in-page script.

Given that, I think it makes sense to control both via script-src, as they certainly provide scripting capability. I also think it makes sense to offer the ability to control these workers distinctly from other script via a distinct directive that sits on top of script-src (currently worker-src, but I'm not tied to that), as their capability extends beyond what scripts on a page offer.

I'm less dogmatic about new Worker(), but if we call the directive that controls service workers and shared workers worker-src (as we do in the current spec), then it will confuse developers if new Worker() doesn't respect it. Unreasonable bias or not, names have meaning and value. If we call two things A in one place, but B in another, we're creating complexity that we're better off avoiding. (Tangentially, I'd put child-src forward as an example of treating differently-named things as the same. That doesn't seem to have worked out too well. I'd suggest that the inverse might also be true.)

@mikewest
Copy link
Member

mikewest commented Dec 6, 2016

@briansmith: Ping. :)

It would be helpful to get opinions from other interested folks as well: @hillbrad, @dveditz, @devd, @ckerschb, @johnwilander, @teddink.

@hillbrad
Copy link
Contributor

hillbrad commented Dec 7, 2016 via email

@annevk
Copy link
Member

annevk commented Dec 7, 2016

We have cross-origin workers with module workers, but they're CORS-same-origin.

Less privileged workers would be dedicated/shared workers with an opaque origin. Those can be created with a data URL and maybe at some point through a sandbox setting. Would those nevertheless inherit policy?

@hillbrad
Copy link
Contributor

hillbrad commented Dec 7, 2016 via email

@mikewest
Copy link
Member

mikewest commented Dec 8, 2016

There are three scenarios here:

  1. https://example.com executes new Worker("https://not-example.com/"), which loads a Worker whose origin is https://example.com (I think this is the module-worker behavior specified today).
  2. https://example.com executes new Worker("https://not-example.com/"), which loads a Worker whose origin is https://not-example.com (this would be similar to an <iframe>).
  3. https://example.com executes new Worker("https://not-example.com/"), which loads a Worker which sandboxes itself into an opaque origin via some as-yet-to-be-defined mechanism (CSP: sandbox, for instance).

If we're talking about the first of these, it seems reasonable to treat them like scripts, and ensure that they execute with the same policy as the document that created them. If we're talking about the second, then it's not clear to me what we'd want to do: I can see good arguments for treating them like shared/service workers and giving them their own policy, but it's also reasonable to give the loading context some control to ensure confinement properties. For the last, I think inheriting a policy would make sense, but it might be a good scenario for @briansmith's suggestion to give the embedding context an option.

All that said, I think I'd prefer to err on the side of simplicity with the model that's currently in the spec. I don't think the second option discussed above is something anyone is looking into doing, and I'm not sure that there's much value in building it in addition to something like foreign fetch (if for no other reason than the complexity of auditing existing implementations that all implicitly assume that dedicated workers are always same-origin with their owners).

@mikewest
Copy link
Member

@briansmith: ping. :)

@ghost
Copy link

ghost commented Feb 17, 2017

@mikewest Thanks for adding this to Chrome.

I added this to my CSP today using Chrome V58 seems to work fine now.

I would like to ask a question about worker-src though.

I added it as this: worker-src https://example.com/;

But wanted to ask what is better / correct usage?

src-worker 'self'; or src-worker http://example.com/;

Thanks.

@mikewest
Copy link
Member

@briansmith: Ping! I'm planning on just closing this out with what's currently in the spec. If you disagree with that enough to argue about it more, great! If not, also great!

@mikewest
Copy link
Member

Excellent! Glad we agree. cough

Closing this out in 455ffe5.

@briansmith
Copy link
Author

I don't know what to say beyond what I already wrote and I don't anticipate participating in this more.

I am sorry to leave you hanging.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 20, 2017
Based on the discussion in w3c/webappsec-csp#146,
we're deprecating 'child-src' and moving 'worker-src' onto 'script-src'.

Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/npKDoKVOUAs/ogtlIFmLBAAJ

BUG=662930,694525

Review-Url: https://codereview.chromium.org/2533313002
Cr-Commit-Position: refs/heads/master@{#458026}
fred-wang pushed a commit to Igalia/chromium that referenced this issue Mar 20, 2017
Based on the discussion in w3c/webappsec-csp#146,
we're deprecating 'child-src' and moving 'worker-src' onto 'script-src'.

Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/npKDoKVOUAs/ogtlIFmLBAAJ

BUG=662930,694525

Review-Url: https://codereview.chromium.org/2533313002
Cr-Commit-Position: refs/heads/master@{#458026}
fred-wang pushed a commit to Igalia/chromium that referenced this issue Mar 20, 2017
Based on the discussion in w3c/webappsec-csp#146,
we're dropping the distinct policy for dedicated workers; they will now always
inherit the policy of their responsible document (just like every other script
executing in that page's context).

Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/npKDoKVOUAs/ogtlIFmLBAAJ

BUG=662930

Review-Url: https://codereview.chromium.org/2540983003
Cr-Commit-Position: refs/heads/master@{#458039}
@wanderview
Copy link
Member

Sorry to comment on this old issue, but I want to provide some implementer feedback.

Like I said in the first comment in this issue, I think we should continue to work towards a world where Workers and <script> and JS6 modules are treated as uniformly as possible.

Workers are not the same as <script> tags. Workers create a completely new global. Script elements and modules do not do this.

Things like copying CSP from one global to another are weird exceptions which make it harder to get things right in the implementation.

It would be much cleaner if every global got policy information from the load that created the global. So for a remote worker script load it would come from its own CSP header and for local URLs (blob:. etc) it would come from the global that created the URL.

aarongable pushed a commit to chromium/chromium that referenced this issue Dec 7, 2018
To reduce GetDocument() usage.
As w3c/webappsec-csp#146 has already been
closed and the Blink implemetation has been changed,
this UseCounter is no longer needed.

Bug: 878274
Change-Id: Icb55058d369b29992436229a9b971bc6f0b8f2a6
Reviewed-on: https://chromium-review.googlesource.com/c/1192303
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614762}
ryandel8834 added a commit to ryandel8834/WebAppSec-CSP that referenced this issue Aug 13, 2022
Based on the discussion in w3c/webappsec-csp#146, this
patch makes two substantive changes:

1. 'worker-src' now defers to 'script-src', not 'child-src'.
2. Dedicated workers now always inherit their policy from the context that
   created the worker.

Hopefully this makes more sense, and we won't change it again in a month.
ryandel8834 added a commit to ryandel8834/WebAppSec-CSP that referenced this issue Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants