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

Should Workers inherit CSP directives from the parent context? #336

Closed
bakulf opened this issue Sep 18, 2018 · 16 comments
Closed

Should Workers inherit CSP directives from the parent context? #336

bakulf opened this issue Sep 18, 2018 · 16 comments
Milestone

Comments

@bakulf
Copy link

bakulf commented Sep 18, 2018

There are several tests in the web-platform-tests suite checking that if the worker applies correctly the directives inherited from the parent contexts, but I don't see why this should happen. Can the spec clarify this point better? Currently in Firefox Workers do not inherit directives, exactly like HTMLIFrameELements. Should we fix the tests instead?

@mikewest
Copy link
Member

Dedicated workers should indeed inherit the policy from their owning document (see step 1 of https://w3c.github.io/webappsec-csp/#initialize-global-object-csp). What would you like to see in the spec to clarify this point? I'd be happy to add a note somewhere if it would be helpful.

(cc @dveditz for Firefox; it's probably worth filing a bug against their implementation)

@annevk
Copy link
Member

annevk commented Sep 18, 2018

I don't think we ever quite reached agreement on that they should inherit them, similar to how they're not supposed to inherit service workers (but do in some implementations). I don't think we have a consistent story across features on that at the moment and I'm not sure CSP should get to decide. (Also, Andrea is with Mozilla.)

@mikewest
Copy link
Member

Ok. If I'm misrepresenting the consensus on this, I'm happy to reopen it. Do y'all think we should require Firefox's behavior instead? (I'd like to understand if this is a request to change the specced behavior, or to change the spec to make the specced behavior more clear. :) )

@annevk
Copy link
Member

annevk commented Sep 19, 2018

I think @wanderview argued for Firefox's behavior in the past. He's no longer employed by Mozilla, but can probably best speak to our position on this matter.

@wanderview
Copy link
Member

wanderview commented Sep 19, 2018

My concern was that in multiple places (this spec and others) dedicated workers were being treated as subresources of the owning document.

Workers are really quite different from subresources. They create a javascript global. They are a service worker Client. The fetch and service worker specs explicitly do not treat them as subresources.

So implementing the inheritance means treating the creation of worker globals different than main thread globals. This is another exceptional case that adds code complexity and surface area for bugs.

Also, the inheritance doesn't work for other worker types like SharedWorker and ServiceWorker. Again, another exceptional case that must be handled, this time by developers using the APIs.

What was the motivation behind adding this inheritance to workers, but not iframes? When I read the issue long ago (sorry, I don't have the link now) I got the impression it was mainly because dedicated workers have some similarity to <script> elements. I think the fact workers create a global, and all that comes with that, sets them apart.

Hope that makes sense.

@bakulf
Copy link
Author

bakulf commented Sep 20, 2018

I agree with @wanderview. Personally, I would like to keep the current Firefox's behavior: workers (any type) should not inherit CSP directives from the parent context.

@annevk
Copy link
Member

annevk commented Sep 20, 2018

Okay, so yeah, please consider this as a request from Mozilla to change the standard (and tests) to not special case dedicated workers.

@oliverdunk
Copy link
Member

👋 Would anyone be able to share some thoughts on what the next steps are here?

To share a real world example of when I've hit this:

  • Top level frame has a CSP which prevents requests to a particular origin, as an extra defence against XSS
  • WASM is loaded inside of a worker
  • Worker (running just static WASM, and accepting only fairly strict request payloads) should be able to make the requests the parent page can't

The intuitive way to handle this is to serve the worker with a CSP which allows external requests. But this currently works inconsistently because there is no agreement on which CSP the worker uses.

@antosart
Copy link
Member

I think there is agreement now. Workers must not inherit CSP directives from the parent context, and rather use their own CSPs as delivered by their response headers.

This is clearly specced in html, where the inheritance is covered by the policy container (see whatwg/html#6504). Dedicated workers get their own policy container, and don't inherit it.

Chrome was the last to adhere to the spec, and shipped this in M97. We have tests for this that pass on all vendors.

I actually believe this issue can be closed.

@codehag
Copy link

codehag commented Oct 10, 2022

Hi, I'd like some clarification in relation to a question I asked here: web-platform-tests/wpt#35641

Currently, wpt tests that dynamic import does not inherit the window's csp, but that static imports do. I can't find this in the specification and am not sure where to look. Should static imports inherit the window's csp, and what is the rationale behind that? thanks

@antosart
Copy link
Member

As agreed above, workers should NOT inherit the window's CSP, but rather use their own (delivered in the worker's response CSP headers).

I think what that WPT is testing is that
(1) when a worker of type 'module' specifies static imports, those static imports are checked against the parent window's CSP.
(2) when a worker of type 'module' specifies dynamic imports, those dynamic imports are checked against the worker CSP.

If I understand correctly, this is specified in the html spec, and is not specific to CSP. My reading of the html spec is that static imports are fetched using the same fetch client settings object used to fetch the main worker script (hence the window's environment settings object), while dynamic imports are fetched using the settings object of the worker.

@smaug----
Copy link

smaug---- commented Oct 11, 2022

I can't interpret what #336 (comment) is trying to say. Should workers inherit window's CSP or should not? The first sentence says NOT, but then 'static imports are checked against the parent window's CSP'.

@annevk
Copy link
Member

annevk commented Oct 11, 2022

They should not, but fetching a module graph is a special case of sorts.

The fetch for the initial worker script is always subject to the document's CSP. After all, it cannot be subject to a CSP it itself is delivering. In the case of it being a module script it can have further static imports that are to be fetched before the module script is executed and the worker environment is established. Therefore those are also subject to the document's CSP.

@smaug----
Copy link

It isn't clear to me why those need to be subject to document's CSP.

@codehag
Copy link

codehag commented Oct 11, 2022

My understanding is:

  • We do have the CSP for the worker before we fetch static imports (after we fetch the top level script, before we execute), that happens before execution. The worker's environment settings object is created in https://html.spec.whatwg.org/#worker-processing-model step 9, but it isn't passed to the module graph for loading, instead we pass the outside settings object (in this case the window).
  • Per spec, I think this aligns with the answer above, however it seemed like an oversight given what I had read in this thread.

This is a little weird with service worker interception, because my understanding (right now, still need to read more) is that the client / environment we are using for fetching a static import is now that of the document, not of the worker. However the intercepted global is that of the worker. I am still trying to understand this better. It might be that we are just missing something in step 5 here: https://w3c.github.io/ServiceWorker/#clients-matchall

@annevk
Copy link
Member

annevk commented Oct 11, 2022

Hmm yeah, @domenic @jakearchibald what is the story for service workers and module worker scripts?

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

9 participants