w3c / webappsec-csp

WebAppSec Content Security Policy
https://w3c.github.io/webappsec-csp/
Other
209 stars 78 forks source link

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

Open arturjanc opened 5 years ago

arturjanc commented 5 years ago

This is a complicated issue about which we've had several discussions discussions over the years (e.g. https://github.com/w3c/webappsec-csp/issues/15, https://github.com/w3c/webappsec-csp/issues/146, and partly https://github.com/w3c/webappsec-csp/issues/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:

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.
  2. 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().)
  3. 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.
  4. 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 https://github.com/w3c/webappsec-csp/issues/146#issuecomment-335828604 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 commented 5 years ago

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

andypaicu commented 5 years ago

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 https://github.com/w3ctag/design-reviews/issues/310 end up helping decide one and for all.

arturjanc commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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.