'strict-dynamic' does not bless transitively loaded Workers |
||||||||||
Issue descriptionPoC: https://arturjanc.com/csp/workers/web-worker-transitive.html 'strict-dynamic' allows the loading of new Web workers via: `new Worker(url)`. However, when a worker transitively loads another worker via `importScripts(anotherUrl)` this will fail, and Chrome will generate a violation. The PoC is as follows: HTML: <head> <meta http-equiv="content-security-policy" content="script-src 'nonce-foo' 'strict-dynamic' 'unsafe-eval';"> </head> <script nonce=foo> document.addEventListener('securitypolicyviolation', function(e) { console.log("Violation: ", e);}); var worker = new Worker("https://arturjanc.com/cgi-bin/worker-transitive.py"); worker.onmessage = function(e) { console.log("Got a message from the first worker: ", e.data);}; worker.postMessage('Sending a message to the first worker'); </script> Worker JS: self.onmessage=function(e){ console.log('Executing first worker: /cgi-bin/worker-transitive.py'); postMessage('Worker (transitive) executed'); importScripts("/cgi-bin/worker-with-csp.py"); } The violation is: Refused to load the script 'https://arturjanc.com/cgi-bin/worker-with-csp.py' because it violates the following Content Security Policy directive: "script-src 'nonce-foo' 'strict-dynamic' 'unsafe-eval'". 'strict-dynamic' is present, so host-based whitelisting is disabled. Note that importScripts seems to work for Service Workers (https://arturjanc.com/csp/workers/service-worker-external-importscripts.html) but just not for regular Workers. This is, unfortunately, a blocker for deploying nonce-based CSP in Google Spreadsheets. Chrome Version : all (tested on 59.0.3071.115 + 61.0.3141.7) OS Version: all
,
Jul 13 2017
,
Jul 14 2017
Hrm. Looks like we're at least trying to do the right thing for `importScripts`: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp?q=importScripts&sq=package:chromium&l=184. I guess things are going wrong when we actually fetch the script. Skimming through, I think we probably just need to set the correct parser state in the options we pass to the WorkerThreadableLoader around https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/workers/WorkerScriptLoader.cpp?rcl=e9e0a9019bcfc63ba03f55403e6628996593fecb&l=83. Would you like to poke at this, Andy? Or shall I?
,
Jul 14 2017
Actually, I want to stop writing docs and email for a minute. I'll take this. ;)
,
Jul 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c976758dc0eca9aee1ea291ec959aedd2a8cb6b commit 6c976758dc0eca9aee1ea291ec959aedd2a8cb6b Author: Mike West <mkwst@chromium.org> Date: Fri Jul 14 20:32:11 2017 CSP: 'importScripts()' should be allowed under 'strict-dynamic' 'importScripts()' is not a parser-inserted script-loading mechanism; it ought to continue loading script in the presence of 'strict-dynamic'. Bug: 742354 Change-Id: Ice327c6f69183e1b27912c808646d16d0030b934 Reviewed-on: https://chromium-review.googlesource.com/571723 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#486868} [add] https://crrev.com/6c976758dc0eca9aee1ea291ec959aedd2a8cb6b/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/script-src/script-src-strict_dynamic_worker-importScripts.https.html [add] https://crrev.com/6c976758dc0eca9aee1ea291ec959aedd2a8cb6b/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/import-scripts.js [modify] https://crrev.com/6c976758dc0eca9aee1ea291ec959aedd2a8cb6b/third_party/WebKit/Source/core/workers/WorkerScriptLoader.cpp
,
Jul 14 2017
Please request for merge to M60 if needed.
,
Jul 17 2017
This blocks deployment of CSP to Google products; it's a very small, targeted change (one line!), didn't blow up over the weekend, and should be safe to merge back to beta. WDYT, friendly release managers?
,
Jul 17 2017
This bug requires manual review: We are only 7 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 17 2017
Confirmed in comment 7 that this merge is extremely safe. Approving to M60.
,
Jul 18 2017
,
Jul 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0a914ca6111a7ce6a87e6c98617d872eaf55ca6 commit f0a914ca6111a7ce6a87e6c98617d872eaf55ca6 Author: Mike West <mkwst@chromium.org> Date: Tue Jul 18 08:59:35 2017 CSP: 'importScripts()' should be allowed under 'strict-dynamic' 'importScripts()' is not a parser-inserted script-loading mechanism; it ought to continue loading script in the presence of 'strict-dynamic'. TBR=mkwst@chromium.org (cherry picked from commit 6c976758dc0eca9aee1ea291ec959aedd2a8cb6b) Bug: 742354 Change-Id: Ice327c6f69183e1b27912c808646d16d0030b934 Reviewed-on: https://chromium-review.googlesource.com/571723 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#486868} Reviewed-on: https://chromium-review.googlesource.com/575129 Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/branch-heads/3112@{#631} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [add] https://crrev.com/f0a914ca6111a7ce6a87e6c98617d872eaf55ca6/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/script-src/script-src-strict_dynamic_worker-importScripts.https.html [add] https://crrev.com/f0a914ca6111a7ce6a87e6c98617d872eaf55ca6/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/import-scripts.js [modify] https://crrev.com/f0a914ca6111a7ce6a87e6c98617d872eaf55ca6/third_party/WebKit/Source/core/workers/WorkerScriptLoader.cpp
,
Jul 18 2017
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by sheriffbot@chromium.org
, Jul 13 2017