929292 - CSP connect-src directive not enforced for Workers

Status: RESOLVED DUPLICATE of bug 1045891

(Core :: DOM: Security, defect, P3)

Description

[Garrett Robinson [:grobinson]]

While CSP 1.0 states that Workers should be constrained by their owner document's CSP, only some CSP directives make sense in the context of web workers. Specifically,

1. script-src: 'unsafe-eval'
2. connect-src

Note that only the 'unsafe-eval' keyword can logically be applied to workers. While importScripts can be used to load additional scripts, it is limited by the web workers spec to same origin loads (equivalent to "script-src 'self'").

Our current implementation of CSP for workers only enforces the checks for 'unsafe-eval'. We should also enforce 'connect-src'.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

No, importScripts can load cross-origin scripts. It works much the same way as <script>. So script-src very much applies.

And even if it was same-origin, it is still very much relevant since the site might allow uploads to the same origin and want to limit scripts only to some separate more secure server, no?

And then there's the fact that workers can load workers. Not sure which rule affects where workers can be loaded from? Is that also script-src?

Comment 2

[Garrett Robinson [:grobinson]]

> No, importScripts can load cross-origin scripts. It works much the same way as <script>. So script-src very much applies.

> And even if it was same-origin, it is still very much relevant since the site might allow uploads to the same origin and want to limit scripts only to some separate more secure server, no?

Both very good points, thanks for the clarification!

> And then there's the fact that workers can load workers. Not sure which rule affects where workers can be loaded from? Is that also script-src?

This is currently being debated in webappsec [0]. Currently loading workers is controlled by script-src. Consensus is leaning towards creating a separate directive to handle this, either "worker-src" or "child-src" (to future-proof in case there are future constructs that have a similiar security model as workers) but it has not yet been decided.

[0] http://lists.w3.org/Archives/Public/public-webappsec/2013Nov/0008.html

Comment 3

[pfrazee]

Potentially a separate issue, but related:

Blob URIs do not seem to work with any 'script-src' CSP (Nightly 28.0a1 (2013-12-01)). On that same server and policy, the worker will load fine if fetched. Removing the script-src policy was the only solution I found that allowed blob URIs.

Chrome does not have this issue.

Test page: http://guipedia.com/tmp/worker-blob-test.html
Check the console to see the error.

Comment 4

[Florian Bender]

(In reply to pfrazee from comment #3)
> Blob URIs do not seem to work with any 'script-src' CSP (Nightly 28.0a1
> (2013-12-01)). On that same server and policy, the worker will load fine if
> fetched. Removing the script-src policy was the only solution I found that
> allowed blob URIs.
> 
> Chrome does not have this issue.
> 
> Test page: http://guipedia.com/tmp/worker-blob-test.html
> Check the console to see the error.

Please file a new bug and set this bug URL as "see also".

Comment 5

[Deian Stefan]

I just ran a few tests and it seems like both connect-src and script-src are enforced. Am I missing something?

Comment 6

[Garrett Robinson [:grobinson]]

(In reply to Deian Stefan from comment #5)
> I just ran a few tests and it seems like both connect-src and script-src are
> enforced.

That's entirely possible, I may have misinterpreted the implementation of CSP for workers when I filed this bug. It would be great if everything actually works as intended! We should add a test to make sure (or we can just add a few more test cases to dom/workers/test_csp.html).

Comment 7

[Garrett Robinson [:grobinson]]

FYI, I started adding new tests to dom/workers/test_csp.html yesterday, but ended up concluding it needs to be rewritten. The test depends on implicit behavior (e.g., keeps re-assigning new workers to the same var, so it's making assumptions about how the worker will be gc'ed), doesn't seem to understand that the execution of workers is async (so the printed msgs are often inaccurate, with some missing and some repeated), and the simple error counter pattern is prone to ending the tests early, potentially before any failures.

We may want to re-target this bug as "rewrite this test, and increase test coverage for CSP in workers."

Comment 8

[Sid Stamm [:geekboy or :sstamm]]

Steve: I think this is mostly just testing that we are spec compliant wrt workers.  Can you figure out at least what we need to test?

Comment 9

[Eric Shepherd [:sheppy]]

This may need a mention in Fx for developers article once it's fixed.

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

Workers are governed by the child-src directive which we implemented within Bug 1045891.