-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Content Security Policy support #93
Comments
I stumbled over nonce-based csp support in sapper before: sveltejs/sapper#1175 May need a different approach in kit depending on the adapter being used (capability to generate nonce for every request?) That being said, what level of csp support are we looking at? |
Hi - it looks like this (mostly) can be solved with no https://gist.github.com/acoyfellow/d8e86979c66ebea25e1643594e38be73 |
This comment has been minimized.
This comment has been minimized.
@acoyfellow Great work so far! I just want to highlight that it's really important that eval is not used in SvelteKit. I've just been looking at an issue in Sapper here where the issue is that we have to allow cc/ @Conduitry (as we were talking about this in the other ticket). |
Noting this, from Twitter:
For server-rendered pages we could just use a |
could still hash inline content: https://content-security-policy.com/hash/ and pass the hash in a policy via meta in head https://content-security-policy.com/examples/meta/ I'm not sure if meta policies are additive though so may not be ideal. |
As far as I can see it's only the init script in the head, right? Nothing else is inline? Or does this depend on build configurations/adapters? In any case meta policies are allowed alongside HTTP policies. However, any source must pass all specified policies, meaning if you wanted to load scripts from a CDN it must be specified both places and the HTTP header must still send See: https://www.w3.org/TR/CSP3/#multiple-policies Putting the init script in a separate file is a much better option imo. |
I think one of the advantages of nonces is that with them and I'm not an expert here, but I think if the page is guaranteed to be static, then a static nonce would also work and not pose a security risk. Alternatively, the pages could be build with a static, known nonce placeholder (ideally a build/deploy time secret) which could then be replaced on each request with a dynamic nonce. We've used this approach in our cloudflare workers app and it seems to be working pretty well so far. |
@pzuraq Nonces must always be unique for each HTTP request. If you send the same one twice then an attacker can inject inline script with that nonce simply by checking the nonce themselves previously. Nonces also don't work for static hosts where you don't have the ability to generate them. Additionally, we have the problem described above that adding the nonce (or a hash) in a It'd be nice if CSPs were just combined additively, but according to the spec they're not. I think it would generally be a good idea to not rely on the adapters and/or runtime modification to avoid Hashing the script would work, as it's all build-time (and therefore works for static as well), but we'd still need Additionally, as discussed in #1776, using nonces and hashes don't work with manifest V3. It'd be a bit of a shame to not support writing browser extensions at all. The only downside to moving the script to an external file is in implementation. I've started looking at it (I'm writing an app that really should have a strict CSP), but I'm having a hard time figuring how to do it. Speaking of, if @benmccann or someone has a tip to how one could modify the page render pipeline to emit a script file I'm all ears. I've found the script itself in |
Right, but that only works if the contents of the page itself are dynamic. If they are not, there is no way to attack, unless I'm missing something. The alternative that allows you to still use But that's unfortunate because the script has to start executing in order to begin loading all of the resources. Maybe it'll execute quickly enough that it wouldn't make a difference? Edit: I guess maybe you could MITM with a static nonce value? |
The point where you're relying on CSP to save you you've already assumed someone is able to inject Regardless, static nonces are against spec and we shouldn't use it to essentially circumvent CSP. I strongly believe that SvelteKit should not by default violate the CSP specification. https://www.w3.org/TR/CSP3/#security-nonces If I have the ability to inject a I'm mostly concerned about the inline snippet which SvelteKit generates in the head of the document which is required to start hydration. If that script was instead loaded from My ambition right now is that a SvelteKit app by default should require only We would be waiting for an additional document load (it's a very small document, but it's at least another round trip, several over HTTP/1), but given that the rest of the scripts are preloaded (and also necessary for hydration if I've understood correctly) I don't think it has any real impact on loading time/performance. |
So I've just realised that I was testing stuff in a repo without any styling. SvelteKit currently also requires |
Is this only required in Dev mode? |
Just came back to say this, looks like it is. When building all the styles are extracted. |
I've run into a new style issue. Transitions rely on inline styles (the kind which is injected in the tag itself). I understand why this is done (it's a very natural place to put it), but sadly it requires These transitions are generated at compile time, so it is possible to get them into a CSP. Hashing is possible, but with the caveats above (as well as requiring EDIT: Though, come to think of it, the transitions are managed by Svelte and not SvelteKit I believe, and should be raised there (sveltejs/svelte#6662) |
I am running into this issue when attempting to use SvelteKit to produce a SPA webextension (which explicitly disallows |
@benmccann I would like to propose to shift this issue from "post-1.0" to "pre-1.0" as security should be a day 1-feature. Therefor it would be great to target support - at least - for CSP with a "strict-dynamic"-setting (where nonce'd scripts can generate inlined script-tags at will) for 1.0 and go from there. The path from there, post-1.0, could be enhanced support for things like Integrations like those above could help mitigate many common errors and attacks for many users by default and make sveltekit a privacy- & security-by-design framework for the years to come. Would love to see steps in this direction! |
You are welcome to send a PR for it to make it pre-1.0 😄 |
I generally would love to dig in, but as I'm not familiar with the build-system of sveltekit at all and there is, as far as I know, no documentation, but plans and implicit knowledge for pre-1.0 steps, two bundlers and quite some rough edges involved, I'll have to miss that shot. In the end I'll take this, sadly, as a "no"... fair enough... Even if I had hoped, my request for rethinking priorities here, would have been considered a bit more seriously. |
Perhaps @acoyfellow has an insight to the work done in their comment? Sapper had nonce support, how difficult would it be to port that and is it even possible? |
Here's where the code is for anyone who'd like to take a stab at it:
|
I've done a really ugly hacky version of a CSP here in case anybody needs a sticky-tape solution until we get around to implementing this. https://github.com/antony/sveltekit-adapter-browser-extension |
I made an adapter based on the static adapter that removes inline scripts for use cases for chrome extensions (this includes manifest v3). Its really more of a band-aid for now but hopefully it helps some of your which have this problem: |
|
fixed |
I'm taking a long overdue look at this and #2394 (thank you @Karlinator). I agree with the sentiment that this should ideally be a pre-1.0 consideration, if we're able to figure out the right API. We need to support inline scripts and styles for a couple of reasons:
(In the case of Chrome extensions clearly we do need to be able to externalise scripts; whether that needs built-in support or can be implemented using the existing trickery is up for debate.) Since prerendering is something supported by all adapters, not just adapter-static, I lean towards using hashes rather than nonces. One possible idea would be to have Kit add CSP headers automatically, if enabled, with the ability to add additional directives. A config like this... // svelte.config.js
export default {
kit: {
csp: {
directives: {
'img-src': ['https://example.com/']
}
}
}
}; ...might result in the following header...
...because Kit computes hashes for the inline init script and any inlined styles, and adds custom directives specified in the config. Pros:
Cons:
Thoughts? Re inline styles added by Svelte itself — I'm hoping that we can transition (geddit?) to use the Web Animations API before too long, so that we no longer need to use |
Well, performance is good. But it's going to be a security flow, isn't it? Some security-based companies would not want to use SvelteKit because of it or will need to write a custom solution for extracting the scripts and styles. Ideally I'd like to see a flag in config, based on which important (for performance) scripts and styles can be either inline or in external files. What do you think? |
@Rich-Harris we're currently using nonces, and it impacts every page load since we have to replace the nonce value. So I think I would also lean toward hashes, since it works with prerendering and will generally be more performant. One additional consideration would be supporting strict-dynamic, which I mentioned above. This would mean hashing not just inline scripts, but all scripts loaded on the page, because when enabled it causes |
@Rich-Harris I like the API you propose for this. I've stated elsewhere that the "generate a nonce and pass it to a hook" approach I took in the PR, leaving the actual injection of the headers up to the application, is a stopgap measure and not my preferred API. I have a couple of points on this specifically:
As for the actual substance of the implementation, I think a one-size-fits-all solution might be hard to come by. The woes of being platform agnostic has already revealed that e.g Chrome extensions need to be able to externalise scripts, and I would suggest that this should somehow be an option in all static contexts—prerendered pages should ideally be able to externalise the init script. Attaching this to the prerender covers almost all the situations where I would want it I think and without requiring some nasty workaround for the sateless runtime (like encoding the script in the url which, yeah, yuck). On hash vs nonce I'm a bit uncertain. Hashing can often come with the security benefit of only manually approved scripts being allowed—blocking some vectors where a script is modified maliciously. Of course, by generating the hash at request time we are essentially bypassing that, making the function more or less identical with nonces, so really I think implementation concerns should decide here. Usually nonces are preferred for dynamic script tags, but there's no specific reason against hashes I think. I originally made the PR with nonces because that seemed easier, and I wasn't really considering prerendering. Generate a string in the adapter, and pass it inwards. Hashing requires passing a hash function from the adapter all the way in through the render, hashing the script string, then passing the hash back out again to the header. But both are doable. I have noticed that in my hooks-based workarounds hashes have been sometimes inconsistent. I never dug deep enough to find out why, but they would sometimes block the script anyway, something I don't see with nonces. It might be related to how hashes are sensitive to even a single whitespace character or trailing newline? (And it's not like my regex-based nonsene was the most stable thing ever). That's one reason I kind of prefer hashes only where I know the hash won't change so I can test and verify. If we want there's also nothing stopping hashes and nonces from living together. We can collect hashes during prerendering, store them somewhere, and serve headers with hashes with prerendered pages. Then when we render dynamically we can use nonces instead. I think conceptually this feels the most "correct", but it may be a waste of effort. @RomanistHere whether host-source directives or hash/nonce (with or wothout @pzuraq Remember that the hash would need to be calculated for every page load too, since the inline init script changes on a per-request basis. Only if the page is prerendered will the hash remain the same. I think var s = document.createElement('script');
s.src = "https://cdn.example.com/some-script-you-need.min.js";
document.body.appendChild(s); I actually don't think it's too big of an ask to support // svelte.config.js
export default {
kit: {
externalScripts: [
{
src: "https://cdn.example.com/some-script-you-need.min.js",
tag: "script",
},
{
href: "https://cdn.example.com/some-other-script-you-need.min.js",
tag: "link",
rel: "modulepreload",
}
]
}
}; |
With nonces, you were correct in your original post above, they do have to be unique for every single request. Right now we solve this by caching the response but then find/replacing the nonce after each response, which is a bit of a pain. However, in those cases we never even call SvelteKit's render function, so it returns pretty quickly overall. Re: appending via a hashed inline script, that would mean that the browser has to first parse and run some JavaScript before it can start loading external scripts, which is not ideal IMO. I think noncing/hashing every external script in |
Another thing to note about nonces is that our setup requires a secret to be added to the deployment environment, which is a bit of a pain. We use this secret to be the nonce placeholder to target with the find/replace after each cache hit. It has to be a secret because if it were a well known value, you could target it for script injection in SSR. Unsure if there would be an easy way to remedy that within SvelteKit itself, maybe the secret could just be generated by the framework on load? It would have to persist between different instances though on some providers, Cloudflare (our provider) shares a cache with multiple workers so the placeholder has to be the same across all workers. |
Thanks for the valuable feedback everyone. I think we're close to squaring all the various circles.
Yeah, it probably makes sense for this to live in Kit rather than adapters having to jump through lots of hoops themselves. Something like this perhaps: // svelte.config.js
export default {
kit: {
prerender: {
externaliseScriptsAndStyles: true
}
}
}; That's something that could happen independently of the CSP work, even though it's motivated by it.
Totally doable — at build time we can hash all the assets, and use the hashes at render time if 'strict-dynamic' is enabled: // svelte.config.js
export default {
kit: {
csp: {
directives: {
'script-src': ['strict-dynamic']
}
}
}
};
Woah! This is very useful (I don't think I'd have been able to figure out how to get autocompletion for Agree re
To clarify, I was only proposing hashing the scripts that SvelteKit generates — not injecting hashes (or nonces, for that matter) in cases like this: <div class="comment">
<p>someone forgot to escape user input <script>alert('lol')</script></p>
</div> That would mean people were responsible for their own dynamic scripts (google analytics, etc), either by putting them in
Yeah, I think this hybrid approach makes sense. Nonces for dynamic, hashes for static. And we could expose the nonce — For posterity, I'm going to recap a discussion I had with @antony earlier since he raised a very reasonable question — rather than having Kit generate CSP headers based on config, what if it just gave you the tools you need to generate them yourself? For example this: <meta http-equiv="Content-Security-Policy" content="default-src 'self' %svelte.cspHashes% "> On the face of it this makes a lot of sense, since it allows Kit to get out of the way, and provides app authors with maximum flexibility. But after discussing it we concluded it probably wasn't the right direction:
So there'd be a lot more hoop-jumping, for both authors and the framework, and you'd lose the benefits of typechecked directives etc. The result would be that way fewer people would end up enabling CSP, unless someone made them do it. All in all it feels like this is one of those places where it makes sense for the framework to step in and say 'I got this'. |
@Rich-Harris Would this be optional, to allow people to choose between using hashes or nonces based on what makes more sense to their setup? I think we would prefer hashes for the reasons I mentioned above, would help us cache our responses more aggressively and not need to rerender just to update the nonce. |
Yeah, perhaps we could have an option like this, where 'auto' is the default // svelte.config.js
export default {
kit: {
csp: {
mode: 'hash' // or 'nonce' or 'auto', meaning 'hash' for prerendered and 'nonce' for dynamic
}
}
}; |
Absolutely! I was just saying the (admittedly minor) security differences between hashes and nonces kind of disappear for us. If you can compromise a nonced init script in Kit then you could do the same for a hashed script (though both would be difficult), when with only compile-time hashes you have a slight advantage. On hashes for external scripts: This is a feature of CSP 3, and annoyingly not yet implemented in Firefox. That's a bit of a bummer. As far as I can tell other browsers support this. This is problematic for using hashes generated at compile time to allow our static scripts, which is otherwise an idea I really like. I'm not sure how much we should let this affect us though, since this is Firefox simply not being in line with spec and is likely to be fixed at some point.
We would need to be careful about this, no? As @pzuraq mentions this can be a target for attacking the SSR, and we don't want this to get through: <div class="comment">
<p>someone forgot to escape user input <script nonce="%svelte.nonce%">alert('lol')</script></p>
</div> I forget exactly how Kit's render pipeline works, but I imagine we can search for this string in, say, only the static Suggestion so far (in
In Do we hash/nonce all scripts even if we're not in |
Wait, I left out |
My understanding was that The problem comes if we want to cache the entire response, so we don't need to enter into SSR in the first place. When caching the response, we lose context so there's no way to tell which parts of the response are "safe" to replace in and which are not. Responses could potentially be cached in parts to avoid this, maybe even the nonce placeholder could be cached as part of the response and generated uniquely for each response 🤔 but that could be platform specific. Cloudflare uses the Cache API to cache responses, and it expects a full Response object, so you'd have to like, store this info in a header or something like that. |
Yeah, that was my assumption too, just wanted to make it explicit. |
|
Thought I'd mention something that just popped up in the Discord that's a bit related. Currently, we have a few places where we need to use dynamic styles in our templates, as things like the background color of certain elements are theme-able. This currently does not work with our CSP, as we do not allow Not sure what the best way to solve this would be, and it's definitely an edge case so it doesn't necessarily have to be in the first pass. I believe |
@pzuraq That seems like it touches on both Kit and Svelte itself, no? Svelte is responsible for generating those inline styles (this is one annoyance of CSP, that it often requires passing a nonce or a hash all the way through an application stack from "the thing that generates CSS" to "the thing that sets HTTP headers"). Anyways, this is controlled by the |
Yeah, agreed re: |
CSP docs are here: https://kit.svelte.dev/docs/configuration#csp |
Sveltekit does not currently support this sveltejs/kit#93 so it has to be done manually.
I don't see anything in the code related to CSP support, so I'm guessing that doesn't exist yet unless I missed it.
Sapper supports it via
%sapper.cspnonce%
: https://sapper.svelte.dev/docs#Content_Security_Policy_CSPThe text was updated successfully, but these errors were encountered: