I've upstreamed Blink's `worker-src` tests to WPT: https://github.com/w3c/web-platform-tests/commit/45185609658ce4ee3576db5a5f362d9e0274bd6b. They require `SecurityPolicyViolationEvent`, however...

Any news regarding this issue? Currently Firefox says 'frame-src' is deprecated and suggests using 'child-src' instead. Chrome deprecated 'child-src' and will remove it in M60 (around August 2017 - next month). Chrome suggests using 'script-src', but the link actually redirects to the 'worker-src' feature status page. The current situation is less than optimal as it is kind of confusing.

FWIW: I'm waiting on deprecation in Chrome for y'all's implementation. Or Safari's implementation. Or anyone else's. *cough* I believe this reflects what we agreed to in WebAppSec. It would be lovely for y'all to follow through on that to reduce developer confusion. :)

Kate, CSP3 defines worker-src [0], which is supposed to govern workers. Previously workers were governed by child-src though. To make it a little more complicated, there is also frame-src which governs frames. Previously frames were also governed by child-src though. To sum it up, here is what we have to implement within this bug. child-src by itself is deprecatd but will be enforced: * for workers (if worker-src is not explicitly specified) * for frames (if frame-src is not explicitly specified) I added parser tests to make sure we can handle worker-src within the CSPParser. In addition I added a mochitest for workers, to make sure workers, shared workers as well as dedicated workers are governed by worker-src. In addition those tests include child-src 'none' to make sure child-src is discarded in case worker-src is specified. In a similar fashion I added an explicit test for frame-src, to make sure frames are governed by frame-src. Please note that I haven't changed any of the child-src tests in our tree, those should keep working even though in CSP2 frame-src was deprecated in favor of child-src ;-) I was able to update some of the worker-src related web-platform tests, but not all of them, because they rely on Security Policy violation events [1] and we haven't implemented Security Policy violation events yet [2]. Once we have those violation events we can also remove: * testing/web-platform/meta/content-security-policy/worker-src/dedicated-none.sub.html.ini * testing/web-platform/meta/content-security-policy/worker-src/service-none.https.sub.html.ini * testing/web-platform/meta/content-security-policy/worker-src/shared-none.sub.html.ini I verified the following, if a page ships a CSP including child-src, then we log the following message to the console: > Content Security Policy: Directive ‘child-src’ has been deprecated. Please use directive ‘worker-src’ to control workers, or directive ‘frame-src’ to control frames respectively. Finally I manually verified that we log correctly to the console in case when blocking some worker. E.g. you see: > Content Security Policy: The page’s settings blocked the loading of a resource at https://test1.example.com/tests/dom/security/test/csp/file_service_worker_src.js (“worker-src https://example.com”). [0] https://w3c.github.io/webappsec-csp/#directive-worker-src [1] https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/tests/content-security-policy/support/testharness-helper.js#90 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1037335

In the absence of worker-src all workers should actually be governed by script-src. I filed Bug 1402332 to make sure that script-src 'none' actually does not allow to run any scripts in case there is no explicit worker-src directive.

Comment on attachment 8911169 [details] [diff] [review] bug_1302667_implement_worker_src.patch Review of attachment 8911169 [details] [diff] [review]: ----------------------------------------------------------------- lgtm ::: dom/security/nsCSPUtils.cpp @@ +1265,5 @@ > + if (aContentType == nsIContentPolicy::TYPE_INTERNAL_WORKER || > + aContentType == nsIContentPolicy::TYPE_INTERNAL_SHARED_WORKER || > + aContentType == nsIContentPolicy::TYPE_INTERNAL_SERVICE_WORKER) { > + return mRestrictWorkers; > + } whitespace

Dan, as discussed within the 'intent to ship' I think it makes the most sense to implement the following fallback mechanism: * worker-src * child-src * script-src * default-src Even though I agree there is some insanity, it probably makes the most sense to implement it that way. Just discussed things with Mike West and Chrome is also shipping that behavior. Any strong objections (besides the maintenance overhead) to ship it that way?

Chrome is shipping that way? Compatibility is good, and updating the spec to say that is even better. Can we get Mike to commit to that ordering and updating the spec in one of the github issues even if he doesn't have time to update the spec itself? If so then that sounds fine. Frames can't work that way though. frame-src => child-src => default-src must be the order there. If we start having script-src control frames just because you don't have a frame-src or child-src that would be pretty bad. Should make sure our tests will check for that case. Should work fine if our fallback is driven by the load type/fetch, but if our fallbacks are tied to directives it could go the wrong way.

We'll need more tests to flesh out the full workers fallback path, too. You've only got tests for worker-src and child-src here.

(In reply to Daniel Veditz [:dveditz] from comment #14) > Chrome is shipping that way? Compatibility is good, and updating the spec to > say that is even better. Can we get Mike to commit to that ordering and > updating the spec in one of the github issues even if he doesn't have time > to update the spec itself? If so then that sounds fine. I added a comment to the github issue you raised: https://github.com/w3c/webappsec-csp/issues/239 I'll have to update our patches to reflect that change and also have to add more tests for that fallback mechanism. > Frames can't work that way though. frame-src => child-src => default-src > must be the order there. Yes, and I'll add tests for that as well.

Dan, CSP3 defines worker-src [0], which is supposed to govern all kinds of workers. Previously workers were governed by child-src though. To make it a little more complicated, there is also frame-src which governs frames. Previously frames were also governed by child-src though. To sum it up, here is what we have to implement within this bug: child-src by itself is deprecatd but will be enforced * for workers (if worker-src is not explicitly specified) * for frames (if frame-src is not explicitly specified) * additionaly we have to add a fallback from worker-src to child-src to script-src to default-src (Chrome is going to ship the same behavior, see [3]) I added parser tests to make sure we can handle worker-src within the CSPParser. In addition I added a mochitest for workers, to make sure workers, shared workers as well as dedicated workers are governed by worker-src, child-src or script-src (using all the different fallbacks). In a similar fashion I added an explicit test for frame-src, to make sure frames are governed by frame-src (also testing the fallback to child-src). Please note that had to update test_child-src_worker.html because script-src now governs workers again even though child-src did not fall back to script-src in our previous implementation. I was able to update some of the worker-src related web-platform tests, but not all of them, because they rely on Security Policy violation events [1] and we haven't implemented Security Policy violation events yet [2]. Once we have those violation events we can also remove: * testing/web-platform/meta/content-security-policy/worker-src/dedicated-none.sub.html.ini * testing/web-platform/meta/content-security-policy/worker-src/service-none.https.sub.html.ini * testing/web-platform/meta/content-security-policy/worker-src/shared-none.sub.html.ini I verified the following, if a page ships a CSP including child-src, then we log the following message to the console: > Content Security Policy: Directive ‘child-src’ has been deprecated. Please use directive ‘worker-src’ to control workers, or directive ‘frame-src’ to control frames respectively. Finally I manually verified that we log correctly to the console in case when blocking some worker. E.g. you see: > Content Security Policy: The page’s settings blocked the loading of a resource at https://test1.example.com/tests/dom/security/test/csp/file_service_worker_src.js (“worker-src https://example.com”). Ultimately, I think we need web-platform tests for that fallback behavior, so I filed Bug 1409706 to write those tests. [0] https://w3c.github.io/webappsec-csp/#directive-worker-src [1] https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/tests/content-security-policy/support/testharness-helper.js#90 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1037335 [3] https://github.com/w3c/webappsec-csp/issues/239

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fc5b054572a628636d7ef2157c373f4d8daae79

Comment on attachment 8919696 [details] [diff] [review] bug_1302667_implement_worker_src.patch Review of attachment 8919696 [details] [diff] [review]: ----------------------------------------------------------------- r=dveditz (with optional l10n comment/suggestion) ::: dom/locales/en-US/chrome/security/csp.properties @@ +113,5 @@ > # %1$S is the name of the duplicate directive > duplicateDirective = Duplicate %1$S directives detected. All but the first instance will be ignored. > +# LOCALIZATION NOTE (deprecatedChildSrcDirective): > +# %1$S is the value of the deprecated directive. > +deprecatedChildSrcDirective = Directive ‘%1$S’ has been deprecated. Please use directive ‘worker-src’ to control workers, or directive ‘frame-src’ to control frames respectively. We never used the old string in more than one place, but in theory it was generic so we could have. This expanded version _only_ works for 'child-src' given the second half of the message. Why not get rid of the %1$S replacement text and just say that? Thanks for using a new label when you change the text -- the l10n folks appreciate it. Maybe add to the localization note that 'worker-src' and 'frame-src' should not be translated? I hope it's obvious enough as it is, but I've seen people make those kinds of l10n notes for things I thought were obvious.

Comment on attachment 8919696 [details] [diff] [review] bug_1302667_implement_worker_src.patch Review of attachment 8919696 [details] [diff] [review]: ----------------------------------------------------------------- Baku, could you sign off on the webidl changes within this patch?

Comment on attachment 8919696 [details] [diff] [review] bug_1302667_implement_worker_src.patch Review of attachment 8919696 [details] [diff] [review]: ----------------------------------------------------------------- r+ for the webidl part

Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/70ccfda99dbc CSP: Implement 'worker-src'. r=baku,dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/e982481dbf2c CSP: Add Parser test for 'worker-src'. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/21c73f9d8fac CSP: Test 'worker-src'. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec6b8bf8c6c CSP: Test 'frame-src'. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/ff86e185e09d CSP: Update test_child-src_worker.html because child-src falls back to script-src. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/ca6ae38c0432 Update wpt tests for worker-src. r=dveditz,mckinley

Backed out changeset 70ccfda99dbc::ca6ae38c0432 (bug 1302667) for frequently failing mochitest in security/test/csp/test_worker_src.html r=backout a=backout on a CLOSED TREE https://hg.mozilla.org/integration/mozilla-inbound/rev/e06653ee31c4d9853ddfef1c69deb8ea5cc83f74 https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ca6ae38c0432883413655ac245445d2a9d8bfb6f&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=140653862

(In reply to Cristina Coroiu [:ccoroiu] from comment #29) > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > e06653ee31c4d9853ddfef1c69deb8ea5cc83f74 > https://treeherder.mozilla.org/#/jobs?repo=mozilla- > inbound&revision=ca6ae38c0432883413655ac245445d2a9d8bfb6f&filter- > resultStatus=usercancel&filter-resultStatus=runnable&filter- > resultStatus=retry&filter-resultStatus=testfailed&filter- > resultStatus=busted&filter-resultStatus=exception&selectedJob=140653862 Thanks. I l.ooked at it and the tests works locally on mac and linux. I looked through the logs and it seems the tests just requires a little longer timeout. Sometimes we see the second blocked shared worker and sometimes not. Ultimately the test should receive 3 messages of blocked shared workers. Let's see if that causes the problem.

I rearranged the test slightly, works better now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bdf54fca76118b0acdb3e6d8b1c4efe332d34de6

Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/bbb4d0097263 CSP: Implement 'worker-src'. r=baku,dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/2219116fd832 CSP: Add Parser test for 'worker-src'. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/8397d379f6a9 CSP: Test 'worker-src'. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/9aa2babaaa5d CSP: Test 'frame-src'. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/b492ad7ed794 CSP: Update test_child-src_worker.html because child-src falls back to script-src. r=dveditz,mckinley https://hg.mozilla.org/integration/mozilla-inbound/rev/c6a6606f3ea7 Update wpt tests for worker-src. r=dveditz,mckinley

https://hg.mozilla.org/mozilla-central/rev/e982481dbf2c https://hg.mozilla.org/mozilla-central/rev/21c73f9d8fac https://hg.mozilla.org/mozilla-central/rev/8ec6b8bf8c6c https://hg.mozilla.org/mozilla-central/rev/ff86e185e09d https://hg.mozilla.org/mozilla-central/rev/bbb4d0097263 https://hg.mozilla.org/mozilla-central/rev/2219116fd832 https://hg.mozilla.org/mozilla-central/rev/8397d379f6a9 https://hg.mozilla.org/mozilla-central/rev/9aa2babaaa5d https://hg.mozilla.org/mozilla-central/rev/b492ad7ed794 https://hg.mozilla.org/mozilla-central/rev/c6a6606f3ea7

https://hg.mozilla.org/mozilla-central/rev/e982481dbf2c https://hg.mozilla.org/mozilla-central/rev/21c73f9d8fac https://hg.mozilla.org/mozilla-central/rev/8ec6b8bf8c6c https://hg.mozilla.org/mozilla-central/rev/ff86e185e09d https://hg.mozilla.org/mozilla-central/rev/bbb4d0097263 https://hg.mozilla.org/mozilla-central/rev/2219116fd832 https://hg.mozilla.org/mozilla-central/rev/8397d379f6a9 https://hg.mozilla.org/mozilla-central/rev/9aa2babaaa5d https://hg.mozilla.org/mozilla-central/rev/b492ad7ed794 https://hg.mozilla.org/mozilla-central/rev/c6a6606f3ea7

I've checked on MDN, and we've already documented worker-src: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/worker-src I've also added a note to the Fx58 rel notes: https://developer.mozilla.org/en-US/Firefox/Releases/58#HTTP Let me know if this all looks OK, or if there's anything else you think needs adding.

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #35) > I've checked on MDN, and we've already documented worker-src: > > https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security- > Policy/worker-src Thanks for adding; could you please update the fallback to something like: If this directive is absent, the user agent will first look for the child-src directive, then the script-src directive and finally for default-src directive when governing worker execution.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #36) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #35) > > I've checked on MDN, and we've already documented worker-src: > > > > https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security- > > Policy/worker-src > > Thanks for adding; could you please update the fallback to something like: > > If this directive is absent, the user agent will first look for the > child-src directive, then the script-src directive and finally for > default-src directive when governing worker execution. OK, updated.