The XHR and importScripts code in dedicated workers currently set the wrong loadingPrincipal when creating a channel. That means that we end up using the CSP of the parent document when doing network security checks. I.e. we "inherit" the policy of the parent document into workers when XHR and importScripts are used. Patch to fix attached

Just add missing file_main_worker.js

Comment on attachment 8685770 [details] [diff] [review] Patch v2 Review of attachment 8685770 [details] [diff] [review]: ----------------------------------------------------------------- Tests looks great, thanks for fixing. Please just address my nits and r=me. ::: dom/security/test/csp/file_redirects_main.html @@ +20,5 @@ > "style-src": thisSite+page+"?testid=style-src&csp=1", > "worker": thisSite+page+"?testid=worker&csp=1", > "xhr-src": thisSite+page+"?testid=xhr-src&csp=1", > + "from-worker": thisSite+page+"?testid=from-worker&csp=1", > + "from-blob-worker": thisSite+page+"?testid=from-blob-worker&csp=1", nit: since you are already modifying those files, can you also remove the redundant: '&csp=1'. This file is the only file that uses file_redirects_page.sjs, you could then also remove the 'if (query["csp"] == 1) {' from file_redirects_page.sjs - thanks. ::: dom/workers/ScriptLoader.cpp @@ +122,5 @@ > return NS_ERROR_DOM_SYNTAX_ERR; > } > > + if (parentDoc && parentDoc->NodePrincipal() != principal) { > + parentDoc = nullptr; please add a comment, similar to the one you have in nsXMLHttpRequest.cpp, otherwise this is really confusing.

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/e72c6539d6b5 for Mulet timeouts in dom/workers/test/test_csp.html, https://treeherder.mozilla.org/logviewer.html#?job_id=17113139&repo=mozilla-inbound, and desktop/Fennec timeouts in dom/security/test/csp/test_redirects.html and failures in dom/security/test/csp/test_worker_redirect.html, https://treeherder.mozilla.org/logviewer.html#?job_id=17114818&repo=mozilla-inbound

Comment on attachment 8686227 [details] [diff] [review] more fixes Review of attachment 8686227 [details] [diff] [review]: ----------------------------------------------------------------- For the record: We are removing test_worker_redirect.html since it relies on the fact that policies are inherited for workers - which was a false assumption. We are not loosing any testcoverage because we have test_redirects.html. ::: dom/security/test/csp/file_redirects_resource.sjs @@ +139,5 @@ > // script that invokes XHR > if (query["res"] == "xhr") { > response.setHeader("Content-Type", "application/javascript", false); > + var resp = 'var x = new XMLHttpRequest();x.open("GET", "' + thisSite + > + resource+'?redir=other&res=xhr-resp&id=xhr-src-redir", false);\n' + Ah I see, so this was not a redirect but tried to access that URL directly, but the CSP error still fired :-( Writing proper CSP tests is hard - thanks. ::: dom/security/test/csp/test_CSP.html @@ +97,5 @@ > window.examiner = new examiner(); > > window.testResult = function(testname, result, msg) { > + // test already complete.... forget it... remember the first result. > + if (window.tests[testname] != -1) ok - so this you are changing back to what was in the initial test.

This apparently broke mochitests on android: https://treeherder.mozilla.org/logviewer.html#?job_id=17160307&repo=mozilla-inbound Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/51dbf899ae40

And a mulet mochitest, too: https://treeherder.mozilla.org/logviewer.html#?job_id=17160000&repo=mozilla-inbound

Turns out we're not just doing the wrong thing for network policies. We also do the wrong thing for JS policies. This makes us grab the CSP from the result-principal of the channel, rather than grab it from the calling JS.

Comment on attachment 8686752 [details] [diff] [review] Also don't inherit JS policy Review of attachment 8686752 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsContentUtils.cpp @@ -6629,5 @@ > - } > - > - csp.forget(aCSP); > - return true; > -} Thanks for removing that piece of code - that is indeed really scary!! ::: dom/workers/test/test_csp.html^headers^ @@ +1,2 @@ > Cache-Control: no-cache > +Content-Security-Policy: default-src 'self' blob: Thanks for adding blob tests here.

Apparently we had tests for the wrong inheritance-behavior with SWs as well. This kills the test which checks that eval() does not work in a SW if it was registered from a page with a CSP. And then applies CSP to the already-existing test which verifies that eval() works in SWs.

Modify the test to verify that we actually apply the csp on the page correctly.

Roll up of all patches

https://hg.mozilla.org/mozilla-central/rev/12a852867c16 https://hg.mozilla.org/mozilla-central/rev/88025b4a432b

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/dedicated-workers-no-longer-inherit-csp-from-parent-document-unless-embedded/