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

Choose a consistent model for workers under nonce-based policies #375

Open
arturjanc opened this issue Dec 4, 2018 · 10 comments
Open

Choose a consistent model for workers under nonce-based policies #375

arturjanc opened this issue Dec 4, 2018 · 10 comments

Comments

@arturjanc
Copy link

This is a complicated issue about which we've had several discussions discussions over the years (e.g. #15, #146, and partly #243).

The crux of the problem is that JS APIs which allow the loading of external scripts (e.g. importScripts() in workers or dynamic module imports) do not provide any way for developers to invoke them with a script nonce. Users of policies which rely on script nonces to convey trust thus either can't use these APIs, or can't use safe "nonce-only" policies. Worse, there is no consistency between APIs when it comes to whether they fail open or fail closed:

  • importScripts() fails closed. Under a nonce-only policy (script-src 'nonce-foo'), authors don't have a way to invoke this API. Note that if the policy uses 'strict-dynamic', importScripts is implicitly allowed to execute as per 'strict-dynamic' should bless programmatically added workers #200 so developers need to use policies with 'strict-dynamic' rather than more restrictive policies based purely on nonces, even if they've done the work to manually propagate nonces to other scripts loaded at runtime.
  • Module imports (import("/foo.js")) fail open. Under a nonce-only policy there is no way to restrict the loading of additional modules via this API, as discussed in Any protection against dynamic module import? #243

In practice, this forces users to add 'strict-dynamic' to their policies, making it less likely that applications in the future will be able rely on safer policies with manual nonce propagation.

There are several ways in which we could handle this:

  1. Modify JS APIs such as importScripts() to accept a nonce parameter and require it to be present for nonce-based policies which don't set strict-dynamic.
  • In general, I feel like this would be a wrong direction to go into, because developers which use these APIs would always need to invoke them with a nonce. So even if the argument to the API contains an injection, the callsite would have the proper nonce, so the injection wouldn't be prevented.
  1. Make both APIs fail open and exempt them from enforcement in a nonce-based policy. This would be like an implicit 'strict-dynamic' scoped to just these APIs:
  • We could tackle this problem with Trusted Types, which are better suited to offering this type of protection (i.e. using TTs would require passing a TrustedResourceUrl to importScripts().)
  1. Make both APIs fail closed. This would require the use of 'strict-dynamic' with nonce-based policies if the application uses either of these APIs.
  2. Add another keyword that would enabled 'strict-dynamic'-like behavior only for these APIs.

None of the options above seems particularly appealing to me. However, what might be a reasonable workaround at least in the context of workers (where the problem with importScripts() manifests) is to treat Web Workers similarly to Shared- and Service Workers and use a policy specified by the response carrying the worker (this was suggested in #146 (comment) and is the current behavior in Firefox). This would at least allow developers to specify a different policy for their workers, i.e. the main application could use a nonce-only policy, and the one sent with the worker could contain 'strict-dynamic'. This definitely doesn't address the underlying problem of inconsistent behavior for these APIs, but would give developers a path to having safer nonce-based policies for their applications. If we do this then we could either ignore the inconsistency outlined above, or pick a simple solution, e.g. (2).

It's also a somewhat compelling reason to treat Web Workers more similarly to Shared- and Service Workers; the current difference in behavior seems fairly confusing to most developers.

@arturjanc
Copy link
Author

@mikewest and @andypaicu, I'd appreciate your thoughts on why this is a terrible thing to do :)

@andypaicu
Copy link
Collaborator

I unfortunately am also not excited about the options. Overall I would probably go for 3. or 4. What would 'strict-dynamic' cover that the new keyword not cover? Is it basically only DOM manipulation APIs? I think I slightly prefer 3 over 4 if that's the only difference.

I don't know about deciding the worker CSP inheritance issue based on trying to make it somewhat help address this issue. I would rather w3ctag/design-reviews#310 end up helping decide one and for all.

@arturjanc
Copy link
Author

The main drawback of (3) is that the platform would not allow developers to use these APIs unless they relax their policies and allow the execution of all non-parser-inserted scripts created at runtime with 'strict-dynamic'. For example, jQuery's html() assignments dynamically create a script which will be blessed by 'strict-dynamic', meaning that injections into this API will be exploitable, whereas they would be blocked by a nonce-only policy.

This seems somewhat undesirable for developers because injections into APIs like html() are very frequent, and injections into the URL passed to importScripts() in a worker seem much less likely.

I guess another option mentioned by @mikewest would be to make importScripts obey worker-src instead of script-src; this wouldn't be backwards compatible, but it might be better. OTOH it wouldn't solve the problem for JS module imports.

@annevk
Copy link
Member

annevk commented Dec 10, 2018

It's a bit unfortunate that OP doesn't split out the last set of paragraphs as 5, as that's what I'd vote for.

@andypaicu
Copy link
Collaborator

I don't think the last paragraph actually solves the problem though. Workers having their only policy does not solve the problem of nonce-based policies not allowing importScripts() to execute. It does allow developers to lax (forget?) the policy for workers but the underlying issue still remains.
Again I would like to solve w3ctag/design-reviews#310 based on something more solid then providing developers a wonky bypass for an issue they encounter. And for what it's worth I'm not at all opposed to doing what the last paragraph says I just don't see it as a solution.

@andypaicu
Copy link
Collaborator

The main drawback of (3) is that the platform would not allow developers to use these APIs unless they relax their policies and allow the execution of all non-parser-inserted scripts created at runtime with 'strict-dynamic'. For example, jQuery's html() assignments dynamically create a script which will be blessed by 'strict-dynamic', meaning that injections into this API will be exploitable, whereas they would be blocked by a nonce-only policy.

Then, based on this, I'd go with (4) or maybe with (0) do nothing and accept this as a limitation of purely nonce based policies.

@arturjanc
Copy link
Author

I think doing nothing on the CSP side is a reasonable option, especially if we can: a) relax the fail-closed model of importScripts() by making workers obey the policy from its response, and b) restrict the fail-open model of module imports with something like Trusted Types. It will be kind of ugly that similar script loading mechanisms have diverging behaviors, but it's something developers can deal with.

What I'd really like to avoid is pushing developers who deploy safe policies which require a nonce for every script load to relax them by requiring 'strict-dynamic' (or 'unsafe-eval') in order to use certain APIs, such as importScripts(). This would make it quite painful to retain the full benefit of CSP in the long term, because developers would need to choose between using new APIs and stronger security.

@arturjanc
Copy link
Author

@dveditz pointed out that these same APIs are a problem for features which require scripts to have a valid integrity attribute (i.e. require-sri-for). This might be moot if require-sri-for is on the chopping block, but it's somewhat interesting ;)

@dveditz
Copy link
Member

dveditz commented Jan 25, 2019

Right -- that would be one argument in favor of some variant of 1) such as passing in a dictionary of optional params so we could have an integrity argument as well as an extensibility point next time something like this comes up.

On the other hand I'm not sure nonces make a lot of sense for imports from a worker. nonces only work if they're dynamic, which means it has to be passed into the worker somehow and the importScript calls are now dynamic and another place something might get injected. They're obviously no protection at all if the worker itself is malicious, but you've already lost at that point.

My preference keeps flipping between options 2 and 3 at which point I throw up my hands and pick 4, horrifying myself. I don't like that importScripts() and import() are different though. We do need to fix that.

@dveditz
Copy link
Member

dveditz commented Jan 25, 2019

I'd like to clarify: if we did #2 (fail open) I'd want that to apply only if a nonce (or 'strict-dynamic') was specified in the policy. If the policy used a domain whitelist we should obviously enforce it. Pretty sure that's what you meant, but if you meant "fail open always" I'd love to hear the arguments in favor.

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

4 participants