[feat] add CSP nonces to script/style tags

[Karlinator]

Related to #93

Alternative to #2391 (don't merge them both).

This PR adds an option, kit.cspNonce, which, when enabled, causes Kit to generate CSP nonces and embed them in pages when it renders them. It then makes the nonce value available to as request.locals.nonce, from where it can be used in a hook to be added into the Content Security Policy of the page, either using HTTP headers or HTML <meta> tags.

I made this because it felt a lot cleaner than doing a regex text replace, which is the alternative presented in #2391 (in fact there aren't any string searches). It also removes more of the burden from the developer.

I'm not completely confident that this will work in all environments. node:crypto is now an external runtime dependency, which should be fine for all environments actually running in node.

Again, this obviously doesn't work for adapter-static. A different strategy (externalising the init script) is required there.

The nonce is added to all scripts, even those not inline, in order to support strict-dynamic. They are also added to stylesheets because it can't hurt (and even allows style-src: 'none' in certain cases).

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: cf26271

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@sveltejs/kit	Patch
@sveltejs/adapter-cloudflare-workers	Patch
@sveltejs/adapter-netlify	Patch
@sveltejs/adapter-node	Patch
@sveltejs/adapter-vercel	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Karlinator]

Hmm, looking at the tests I may have overestimated support for nodeJS builtins in serverless environments. We might get somewhere using the Web Crypto API instead? I know Cloudflare supports it, though that's only experimental in Node (and only in node 16).

Perhaps this can be injected somehow from the adapters? So adapter-node would use crypto, Cloudflare would use the global webcrypto object etc?

If there is no reasonable way to generate a random value across platforms then we might just have to leave that up to the developer. An alternative then is to take in the nonce as request.locals.nonce and insert it directly. In effect we would be keeping most of the good parts of this approach (no regex) and requiring that the developer provide a random value themselves based on whichever platform they are running on.

[dominikg]

I like this a lot more than the placeholder approach. Is it allowed to reuse nonce values for multiple scripts or should each one have a separate one?

Would it also be posssible to calculate hashes instead of just a random nonce?

Edit: on randomness... is the network io guaranteeing enough entropy for randomBytes(32) ?

[Karlinator]

We could probably calculate hashes, but that requires data flow in the other direction instead (or we're back to regexes). I'm not sure why you want hashes for this? SSR like this is basically the poster child for CSP nonces.

Using the same nonce for every resource on a single page request is (as far as I can tell) accepted practice. There shouldn't be any security degradation from that (still impossible to guess, and all nonces after initial page load are disregarded anyway), and it avoids some unpleasant complexity for us since we don't need to know ahead of time how many nonces to generate.

The randomness is a problem in general (see my above comment on platform support). But the current version (which only works on Node) uses Node's crypto API, which is intended for exactly these kinds of things. I even doubled the recommended minimum nonce length from 128 bits to 256 bits just for the sake of it!

[dominikg]

My concern in regards to random was that in a scenario with a high request volume and low entropy on the server, random might become blocking/adding to the response time https://nodejs.org/api/crypto.html#crypto_crypto_randombytes_size_callback

The advantage of hashes is that they can be precalculated on build time (for anything non-dynamic at least) and reused from a manifest file, so less work at runtime and no need for a source of randomness.

[Karlinator]

I see your concern now, thank you for elaborating. I don't know? I haven't measured response times, but randopmBytes() seems to be the common way to generate nonces in NodeJS.

I kind of consider hashing a separate feature to be honest, though I absolutely support it. As discussed in #93 some parts of the init script is unique to the request under SSR so hashing can't be done ahead-of-time for dynamic environments. However, it would be a nice inclusion for static environments or on a per-request basis if entropy is indeed a problem. I imagine hashing within the render and then providing the hash value as request.locals.cspHashes.scripts[0]. Static environments could insert it into a CSP <meta> tag.

Could we require (or encourage) adapters to shim a generateNonce() (and potentially a hashScript() ) function similar to what we do with fetch? adapter-node would map it to randomBytes(32).toString('base64'), adapter-cloudflare-workers would use the WebCrypto API, etc. It's one more thing for adapters to implement, but I think that's pretty reasonable. I would hope all environments have some way of generating a random string, though I can't guarantee that. I guess if it can't then it should just emit an error at build time if the kit.scpNonce option is set.

[Karlinator]

Marked as draft because of aforementioned platform issues.

[dominikg]

crazy idea, feel free to put me on blast for this.

What if you did extract all inline items into virtual files served from memory

<body>
<script>
  alert('inline');
</script> 
</body>

would become

<body>
<script src="some-virtual-script-uri" />
</body>

As the generated html is served to the client in the very next step the client is going to request the virtual files, the memory cache holding them could evict them after first response, hopefully without blowing up the server requirements.

Yes, extra roundtrips occur, and it would most certainly not be cool for stuff like critical inlined css (but that should be static and could be hashed instead).

Also i am very happy to see your efforts here, please don't take my questions or suggestions as a detriment. I want this as much as you, so if nonces turn out to be a good first step into offering baseline usable csp support for kit with SSR, i'm all for it.

[Karlinator]

That would require server state would it not? I like the idea, but I'm not sure it would work well on serverless platforms? Or if running several parallel instances?

I am developing a SvelteKit application for production (living on the edge here, one of the reasons I'm keen on this feature), and in our setup (adapter-node running in containers) I think I could accommodate that solution, but it would require setting up session-based routing which isn't the case right now since SvelteKit is stateless.

A similar-ish crazy idea is to set up a special endpoint /init-script?session=whatever&hydrate=stuff, and then the init script becomes <script src="/init-script?session=whatever"></script>, essentially feeding the information that is now embedded into the script to GET parameters instead, where they would then be embedded into the script. That avoids any server state, though at the cost of performing more dynamic file rendering.

Either way I think we should still want the option of nonces since it enables strict-dynamic. Hashes technically work for this too, but that would require moving some stuff around (you can nonce a script with src, but you can't hash it).

[Karlinator]

I've (not without a little bit of trouble) changed this to use a cspNonceGenerator which it expects to be shimmed by the adapter, and implemented such a shim in adapter-node (in general they should be quite easy to implement for others as well).

The lint CI job evidently doesn't like the way I did this; any suggestions @dominikg @benmccann ? EDIT: I think I got it?

EDIT: also I did confirm that this actually works for adapter-node. Any ideas for dev? Can I shim it in somewhere? Nevermind I found it.

[Karlinator]

I think this might actually be ready now, though I don't know that I have tested the adapters well enough.

I am open to suggestions for naming things better 😄

Third-party adapters will need to update to implement this, but users will only notice that it's missing if they turn on kit.cspNonce, which will generate runtime warning and promptly not do anything, so it should be safe to roll out and let adapters catch up.

[Karlinator]

So to clarify the final state of this PR:

A new config option kit.cspNonce is added. When this is true, SvelteKit will generate nonces for each request and embed it into all relevant script and style tags. The nonce value is exposed as request.locals.nonce.

Kit relies on the adapters to shim a nonce generator, generateCspNonce(), which should return a cryptographically secure (pseudo)random string with at least 128 bits of entropy in a suitable encoding (like base64). This PR implements this into all the official adapters (as well as the dev server). Adapters which don't shim this will cause Kit to emit a runtime warning if kit.cspNonce is set, and will prevent Kit from inserting any nonce value at all (and request.locals.nonce will be undefined).

Implementation varies a little between the adapters. Most of them have access to Node's crypto API, while adapter-cloudflare-workers uses the WebCrypto API which is provided in the global scope there. I did a quick look at the community adapters, and they should all be able to use one of those two (Deno implements the WebCrypto API, Firebase and Architect use Node).

Nonce generation does not run when prerendering. This PR has absolutely no effect on static sites (we'll need hashing or init script externalisation for that).

[Karlinator]

I've made a change to the adapter API proposed in this PR, in order to be a little bit more generic.

Instead of generateCspNonce, Kit now expects a function generateRandomString(bytes: number), which should return a base64-encoded random string with bytes number of bytes. This allows us to choose the level of entropy generated, for any future needs.

[Karlinator]

Testing has revealed that (in addition to not having implemented it correctly in the first place), noncing only works very inconsistently in adapter-cloudflare-workers. It seems to be serving prerendered pages for the most part, except when it is forbidden to by the presence of a load function? At any rate, it's often serving pages without running any of the relevant code running at all. I don't know how many environments behave like this? I also don't know a great solution at the moment.

EDIT: I think requiring the user to disable prerendering is a reasonable tradeoff for this feature (it's not like you can use nonces with static files anyway). I think I'm just going to add a note in the docs that this will only work on pages that are not prerendered. I don't think it should automatically disable prerendering, both because that's a somewhat opaque behaviour and because there might be situations where you'd want a different CSP for different pages.

[Defman]

Instead of having svelte generate the nonce, could we delegate it to the adapter. Ie having svelte render function take a nonce , which would be added to all scripts, styles, and etc. and added to the returned headers by render.

const rendered = await render({
	host: request_url.host,
	path: request_url.pathname,
	query: request_url.searchParams,
	rawBody: await read(request),
	headers: Object.fromEntries(request.headers),
	method: request.method,
        nonce: "rAnd0m",
});

return new Response(rendered.body, {
	status: rendered.status,
	headers: makeHeaders(rendered.headers)
});

[Karlinator]

@Defman I really liked that, so I did it!

The adapter generates a nonce in it's own way and simply supplies it as incoming.nonce. There is some added complexity related to not generating nonces if kit.cspNonce is false (specifically the adapters have to inject that value with esbuild's define property), but this feels cleaner overall, and the API is a little less implicit this way.

At a later date we probably want to offer hashing as well, which has some similar problems (platform-dependent api to access hash functions, for example) with the added complexity that hashes must be computed inside Kit as they cannot be precomputed. A similar non-global-shim solution for that (on the adapter side) would be something like this (example is adapter-cloudflare, so Web Crypto API):

const rendered = await render(
    {
      host: request_url.host,
      path: request_url.pathname,
      query: request_url.searchParams,
      rawBody: await read(request),
      headers: Object.fromEntries(request.headers),
      method: request.method,
      nonce: generateNonces && btoa(crypto.getRandomValues(new Uint32Array(2)))
    },
    {},
    generateHashes && async (content) => btoa(await crypto.subtle.digest('SHA-256', content))
);

[Defman]

I don't think we have any issue supporting prerendered pages, we would just have to supply a nonce at pregeneration and store it when later serving the pregenerated pages. The nonce would stay the same, but since the content of the pages can't change it would not matter.

[Karlinator]

The CSP Spec does not agree:

Nonces override the other restrictions present in the directive in which they’re delivered. It is critical, then, that they remain unguessable, as bypassing a resource’s policy is otherwise trivial.

If a server delivers a nonce-source expression as part of a policy, the server MUST generate a unique value each time it transmits a policy. The generated value SHOULD be at least 128 bits long (before encoding), and SHOULD be generated via a cryptographically secure random number generator in order to ensure that the value is difficult for an attacker to predict.

https://www.w3.org/TR/CSP3/#security-nonces

"Each time it transmits a policy" is very clear, and does not allow storing nonces generated ahead of time. I would strongly urge we don't break with spec over this (though some developers might hack their way into a system that does).

If you are generating the policy ahead of time (say, in a static environment), you should use hashes instead.

There is more discussion around this topic in general at #93, but my intention is to also implement hashing, especially for use in prerendering. We can return the hash lists as part of the ServerResponse or something, and the adapter/hooks can take it from there.

[Defman]

Like render takes a nonce we can now provide a hash function, which takes bytes and spits out a hash. That way the adapter can chose what hash to use. More general we could provide some way a function which is called pr style/js block and returns an object which are added as properties for the block.

const rendered = await render(
    {
      host: request_url.host,
      path: request_url.pathname,
      query: request_url.searchParams,
      rawBody: await read(request),
      headers: Object.fromEntries(request.headers),
      method: request.method,
      hooks: { script: ({}) => { nonce: "rAnd0m" }, style: script: ({}) => { nonce: "rAnd0m" }}
    }
);

And for prerender

let hashes = [];

const rendered = await render(
    {
      host: request_url.host,
      path: request_url.pathname,
      query: request_url.searchParams,
      rawBody: await read(request),
      headers: Object.fromEntries(request.headers),
      method: request.method,
      hooks: { script: async ({content}) => { hashes.push(await crypto.subtle.digest('SHA-256', content));, ... }
    }
);

Just a suggestion, this way we delegate all the work to the adapter. We can ofcs, provide some utilities to make it easier and simpler.

[benmccann]

Oof. I'm terribly sorry we haven't gotten this PR in yet. I just came back to take a look and I'm afraid there are quite a few conflicts now. I hope it won't be too hard to rebase

[netlify]

❌ Deploy Preview for kit-demo failed.

🔨 Explore the source changes: cf26271

🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/61d90fbe6fd1bc000893a009

[Karlinator]

Hmm, the adapter API overhaul (the Builder and stuff) breaks the way I did this. I need to look more closely at it to find a good way to tell the adapter runtime whether it should pass nonces or not.

[Karlinator]

Closing in favour of #3499