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

new Function causes Content Security Policy failure #31

Closed
kohtala opened this issue Mar 27, 2020 · 15 comments
Closed

new Function causes Content Security Policy failure #31

kohtala opened this issue Mar 27, 2020 · 15 comments

Comments

@kohtala
Copy link

kohtala commented Mar 27, 2020

Use of new Function in troika/packages/troika-worker-utils/src/WorkerModules.js (v0.20.0) causes CSP failure:

"Uncaught EvalError: Refused to evaluate a string as JavaScript because
'unsafe-eval' is not an allowed source of script in the following
Content Security Policy directive"

It requires content security directive script-src 'unsafe-eval'.

Would there be a safer alternative for new Function so that there would be no need to allow this unsafe functionality in policy?

@lojjic
Copy link
Collaborator

lojjic commented Mar 27, 2020

It's used for passing functions from the main thread to the worker so they can be executed there - since you can't pass a function directly we have to stringify it and then rehydrate it on the other side. I'm happy to entertain alternative approaches for doing that.

If it helps, the input to these is always a JS function to begin with, so if you're worried about arbitrary strings getting evaluated into executable JS then that would already have happened before it gets to this code anyway. Also, it executes within a web worker so it's somewhat sandboxed.

If there's a way I can annotate just this section of code as safe that your tools would recognize, let me know.

@lojjic
Copy link
Collaborator

lojjic commented Mar 27, 2020

I think I have an alternative implementation for the worker function rehydration that uses importScripts() with a blob url rather than new Function(). I like that better anyway because it improves debugging.

Would that satisfy your CSP requirements?

@kohtala
Copy link
Author

kohtala commented Mar 29, 2020

I'm new to this technology, so there is a high risk anything I say is mistaken.

If I understood correctly, a change to use importScripts() would require script-src 'unsafe-eval' Content-Security-Policy to be changed to a worker-src blob: policy.

I think it would be an improvement allowing to keep most of the policy intact protecting against security bugs we could expect in other code we use with troika.

The safest would be the worker could use an url to the same origin. I guess that would require some sort of orhestration for the correct deployment and knowing the urls. Sounds difficult.

Thanks.

@lojjic
Copy link
Collaborator

lojjic commented Mar 30, 2020

I think you're right about the blob: policy, that matches what I've read. I'll attempt that change since that sounds preferable. Also I'm not sure what part(s) of Troika you're using specifically, but there are also some packages other than troika-worker-modules that currently use new Function() that I'll have to look into separately.

As you pointed out, a separate URL for the worker code would be difficult, to the point of being a non-starter IMO. Many (perhaps most) users of Troika are currently just using troika-3d-text as a dependency-of-a-dependency and they should be able to expect it to just work without having to place additional files in specific locations. I'd considered using a CDN but that poses its own issues and limits flexibility.

@lojjic
Copy link
Collaborator

lojjic commented Mar 30, 2020

Some more food for thought on this... from https://www.w3.org/TR/CSP2/#source-list-guid-matching :

Especially for the default-src and script-src directives, policy authors should be aware that allowing "data:" URLs is equivalent to unsafe-inline and allowing "blob:" or "filesystem:" URLs is equivalent to unsafe-eval.

@kohtala
Copy link
Author

kohtala commented Mar 31, 2020

A CDN would still need a fallback, because there are local uses that do not have access to Internet.

But if you can provide the file(s) you'd place on CDN, perhaps it would make sense having the worker url(s) be configurable. If not configured, it could build the blob: url to work as a default. Those that have CSP requirements can then host the worker.js themselves and configure the url for the path where they placed it. I think that would not be too difficult after all.

I did realize blob: must be equal to 'unsafe-eval'. But if importScripts() moves the policy change from script-src to worker-src, it would anyway be an improvement.

@lojjic
Copy link
Collaborator

lojjic commented Mar 31, 2020

Thanks for the input, I'll look into the possibility of separate files further.

I found Mapbox's approach to this problem; I suspect that won't be an exact solution though, because Troika uses the worker modules as a general multithreading implementation where it can run arbitrary code off-thread when needed, and have those pieces of code talk together in the same worker. So unlike Mapbox there isn't just a single file that holds all worker code. There would need to be a main worker file, and then separate files that it would import, one for each modular piece of code, and make those all reference each other appropriately.

Another possible "fix" may be to fall back to running the code in the main thread, when the worker can't be used. That may be acceptable for many applications.

So that I'm aware, what part(s) of Troika are you using, so I can target this fix if needed? I've been assuming this is for troika-3d-text support but I shouldn't assume that.

@kohtala
Copy link
Author

kohtala commented Apr 2, 2020

Yes, we are using troika-3d-text.

The Mapbox seemed to be dealing with the same problems. Nice find! You use rollup as well and it seems to be a great help in building the bundles.

If there are many bundles, it is not a big problem. Perhaps they could have common naming so that they are easy to copy with one glob (dist/*-worker.js or something) to the static files and all you'd need to configure would be a base path, not every individual file. The defineWorkerModule could take the file name to add to the base bath as one parameter (eg. opentype-worker.js). If base path is defined, it'll combine that with the file name for a url for Worker, or else it would build the blob. This way, if the worker bundles change, users need not change their deployment scripts.

lojjic added a commit that referenced this issue Apr 3, 2020
Functions are now rehydrated using `importScripts(blobURL)` rather than
`new Function()`. This makes it slightly easier to whitelist in CSP
rules (see issue #31), and improves debugging since they now show up
as sources in devtools. Added a `name` parameter to defineWorkerModule
which gets inserted as a comment in the rehydrated source so it's easier
to find where each function comes from.
@lojjic
Copy link
Collaborator

lojjic commented Apr 16, 2020

FYI I've just released version 0.23.0 which contains the limited change from new Function to importScripts. Hopefully this improves your situation slightly.

@kohtala
Copy link
Author

kohtala commented Apr 18, 2020

Thanks! I added on our work list to upgrade, modify the CSP and see it works. We'll report back here what happened once we get to it.

@lojjic
Copy link
Collaborator

lojjic commented May 24, 2020

FYI, as of c754d0b#diff-24c89a991ff540df6b54f39f0115695f (in version 0.26.0) there is now a fallback mode where the code will run in the main thread as normal non-eval'd JS code if it cannot initialize the Worker for some reason. This should allow it to work in very strict CSP scenarios, at the expense of main thread performance.

@kohtala
Copy link
Author

kohtala commented May 25, 2020

Thanks. I really feel our issue is being taken care of. Sorry we've been slow testing the new versions. I poked the issue in hopes we'd get to update and test.

@kohtala
Copy link
Author

kohtala commented Oct 6, 2020

Hi. Finally back on this.

We upgraded to troika-three-text@0.33.1, but some CSP repated problems are still there. I'm not sure if this is intended.

Content-Security-Policy: default-src 'none'; script-src 'self'; worker-src blob: 'unsafe-eval'; style-src 'self' 'unsafe-inline'; img-src 'self' blob: data: https://encrypted-tbn0.gstatic.com; connect-src 'self'; font-src 'self'; manifest-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'none'

We do not get the text and get following error from Chrome 85.0.4183.121.

Refused to load the script 'blob:<URL>' because it violates the following Content Security Policy directive: "script-src 'self'". Note that 'script-src-elem' was not explicitly set, so 'script-src' is used as a fallback.

0792ff4e-027c-429b-81cd-b134faaa1fdd:3 DOMException: Failed to execute 'importScripts' on 'WorkerGlobalScope': The script at 'blob:http://localhost:8080/36231714-51e0-4f4c-a3c5-41a6f45fb2b6' failed to load.
    at t (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:248)
    at r (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:657)
    at blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:594
    at Array.map (<anonymous>)
    at r (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:564)
    at blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:594
    at Array.map (<anonymous>)
    at r (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:564)
    at blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:594
    at Array.map (<anonymous>)
t @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
r @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
(anonymous) @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
r @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
(anonymous) @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
r @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
(anonymous) @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
r @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
(anonymous) @ 0792ff4e-027c-429b-81cd-b134faaa1fdd:3
0792ff4e-027c-429b-81cd-b134faaa1fdd:3 worker module init function failed to rehydrate

0792ff4e-027c-429b-81cd-b134faaa1fdd:3 DOMException: Failed to execute 'importScripts' on 'WorkerGlobalScope': The script at 'blob:http://localhost:8080/53d24bbb-d4db-4552-b3cb-7cb7d8df56a6' failed to load.
    at t (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:248)
    at r (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:657)
    at blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:594
    at Array.map (<anonymous>)
    at r (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:564)
    at blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:928

0792ff4e-027c-429b-81cd-b134faaa1fdd:3 DOMException: Failed to execute 'importScripts' on 'WorkerGlobalScope': The script at 'blob:http://localhost:8080/00fc1ea3-1358-49dc-b160-eb4aaddcbe77' failed to load.
    at t (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:248)
    at r (blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:657)
    at blob:http://localhost:8080/0792ff4e-027c-429b-81cd-b134faaa1fdd:3:928

troika-worker-utils.esm.js:495 Uncaught (in promise) Error: Worker module function was called but `init` did not return a callable function
    at troika-worker-utils.esm.js:495
(anonymous) @ troika-worker-utils.esm.js:495
Promise.then (async)
X @ troika-three-text.esm.js:4438
sync @ troika-three-text.esm.js:5194
(anonymous) @ index.js:248
it @ web.js:80
at @ web.js:80
Gt @ web.js:110

Firefox 81.0 also gives similar errors:

Content Security Policy: Sivuston asetukset estivät resurssin lataamisen osoitteesta blob:http://localhost:8080/2a928b45-344c-449a-9109-fdc49ac18211 ("script-src").
NetworkError: WorkerGlobalScope.importScripts: Failed to load worker script at blob:http://localhost:8080/2a928b45-344c-449a-9109-fdc49ac18211 (nsresult = 0x805e0006) 422c15bf-2f0f-45fd-a556-427c6816ee38:3:282
worker module init function failed to rehydrate 422c15bf-2f0f-45fd-a556-427c6816ee38:3:783

Uncaught (in promise) Error: Worker module function was called but `init` did not return a callable function
    c troika-worker-utils.esm.js:495
    c troika-worker-utils.esm.js:491
    X troika-three-text.esm.js:4438

Adding

script-src blob: 'self'

restores text and errors.

@lojjic
Copy link
Collaborator

lojjic commented Oct 6, 2020

Ah, I see now, importScripts is governed by script-src, not worker-src. The worker-src directive is only for the worker's main file itself, not any additional scripts that worker then imports.

So, by specifying worker-src blob:, you're allowing the main worker to be created, but then your script-src policy prevents that worker from doing anything else. It seems there's no way to allow blob evaluation only within a worker but not the main document. It's an unfortunate situation, for sure.

As a possible workaround, I mentioned on May 24 that if the worker is prevented from being created (e.g. by removing the worker-src blob: allowance), it should fall back to main thread execution. That may have performance impact but could be an option for you.

@kohtala
Copy link
Author

kohtala commented Oct 7, 2020

Yes. I think we don't have any ideas how to improve this any more. Except maybe document the CSP. I'll close.

Thank you!

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

2 participants