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

Consider adding import-src #506

Open
shhnjk opened this issue Jul 16, 2021 · 6 comments
Open

Consider adding import-src #506

shhnjk opened this issue Jul 16, 2021 · 6 comments

Comments

@shhnjk
Copy link
Member

shhnjk commented Jul 16, 2021

JavaScript is capable of loading other scripts (e.g. static and dynamic import).
But it can also load modules like JSON, CSS, and (in the future) HTML.

Issues 243 acknowledges that there is an issue with dynamic import, but we aren't making progress there.

We should probably start a discussion about import-src that fallback to script-src, where we can control those import behavior from JS (which I believe bypasses nonce-based CSP).

In addition to allow-list based based blocking mechanism, it'll be interesting to consider keywords that only denies dynamic import (e.g. import-src 'allow-static-import'), or something that allows import with TrustedScriptURL (not sure if this is doable though).

P.S.
I intentionally haven't mentioned importScripts, because that's probably controlled by worker-src.

@mikewest
Copy link
Member

Can you sketch out import-src in a little more detail? What behavior would it influence?

/cc @lweichselbaum @koto

@shhnjk
Copy link
Member Author

shhnjk commented Jul 19, 2021

There are mainly 2 reasons why we'd probably want to implement import-src.

  1. Currently, dynamic import provides a bypass for nonce-only CSP where it can load external script without 'strict-dynamic' (see Any protection against dynamic module import? #243 (comment)).
  2. JSON, CSS, and HTML Modules are another type of imports where a script imports various contents.

I feel like we should probably separate mitigation for import mechanism out of script-src (similar to worker-src), so that more specific mitigations can be applied for imports without affecting other script loading.

The suggestion I made about 'allow-static-imports' is basically to solve the problem #1. For HTML Modules and co, I just have a feeling that it's little weird to have mitigation for HTML loading in script-src.

@mikewest
Copy link
Member

Thanks! I still don't think I understand the distinction you're drawing between imports and plain ol' scripts. What's a policy you'd recommend that folks construct that would apply specific mitigations for import without affecting other script loading? If we could jam keywords for static/dynamic imports into script-src, would that satisfy the need, or is there something different about the way we want to recommend folks handle imports?

@koto
Copy link
Member

koto commented Jul 19, 2021

I don't have answers, only more questions :)

It kinda depends on two things:

  1. what we want to do with non-JS modules. Do we want to guard their loading? If so, how?
  2. what happens with import maps that would allow module specifiers, now forbidden in the web.

A similar syntax is used for importing JS and non-JS modules, and it's probable these would behave and evolve in a similar way in EcmaScript. They already do - e.g. both can already be loaded dynamically, or statically. If we want to put guards on these specific behaviours, it might make sense to start afresh and put them together under a separate directive and a set of keywords instead of complicating the script-src even more. That said, b/w compatibility would be an issue here.

If we don't plan to guard non-JS modules, and only focus on JS ones, script-src is the closest match indeed, so the only problem is practical - would we be able to add more keywords without that house of cards falling down.

If import maps ship, the advantage of a separate directive is that we could separately guard on module specifiers and resulting URLs:

module-src foobar/*; script-src https://foo.bar/ 

If we think there's value in guarding module specifiers and not URLs, and expect import maps to ship, that directive is also a better fit for new keywords e.g. 'allow-dynamic-import'.

@koto
Copy link
Member

koto commented Jul 19, 2021

Alternatively, we could:

  1. keep both static and dynamic JS import under script-src, guarded by their URL, as it behaves now
  2. add dynamic-import-src directive that can disable it completely ('none') or guard them by URL (now), or module specifiers (after import maps). dynamic-import-src 'self' https://my.cdn/* 'module-foo/*' ? In the most advanced configuration, the module loaded dynamically would have to satisfy both script-srcand dynamic-import-src.

Essentially, I think the problem boils down to dynamic import. The only way to guard it now is by CSP script-src URL allowlists which we are discouraging. And since nonces / TT are not coming to dynamic imports anytime soon, there should be something to guard that sink, at the very least prohibiting it completely.

@shhnjk
Copy link
Member Author

shhnjk commented Jul 19, 2021

Thanks! I still don't think I understand the distinction you're drawing between imports and plain ol' scripts. What's a policy you'd recommend that folks construct that would apply specific mitigations for import without affecting other script loading? If we could jam keywords for static/dynamic imports into script-src, would that satisfy the need, or is there something different about the way we want to recommend folks handle imports?

Plain scripts can be mitigated with nonce, but imports can't be (which should require 'strict-dynamic' but it isn't today).
To solve this problem today, Web apps are forced to add 'strict-dynamic' or add an allow-list for imports in script-src, which either weakens CSP (e.g. 'nonce-R4nd0m' 'strict-dynamic' or 'nonce-R4nd0m' https://import-src.app.example) than nonce-only CSP, or make deployment of such policy much harder (i.e. supply 2 headers of CSP, one with nonce and another with allow-list of all endpoints they use in addition to scripts used for imports).

While a keyword like 'allow-static-imports' can be added to script-src, providing additional directive makes deployment a lot easier, and has potential for extension (e.g. if we see dangerous practice from devs with HTML Modules). It's also important to note that all imports can't be guarded with Trusted Types, so script-src is the only mitigation place for those currently.

With that probably recommended (for now) CSP that demonstrates good use of import-src are:

  1. CSP: script-src 'nonce-R4nd0m'; import-src 'allow-static-imports'; (if a web app doesn't use dynamic import).
  2. CSP: script-src 'nonce-R4nd0m'; import-src https://my-static-endpoint.app.example/modules/; (if a web app does use dynamic import).

2. what happens with import maps that would allow module specifiers, now forbidden in the web.

Import maps requires script tags, therefore it can be mitigated by nonce. So, I'm not terribly concerned about that.

Essentially, I think the problem boils down to dynamic import. The only way to guard it now is by CSP script-src URL allowlists which we are discouraging. And since nonces / TT are not coming to dynamic imports anytime soon, there should be something to guard that sink, at the very least prohibiting it completely.

Agreed that the main problem we have right now is dynamic import.

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

3 participants