Fix: make nonce implementation strict CSP compliant #256
Conversation
- Remove insecure bypass of nonce generation using modes - Remove setting the nonce in a cookie - Remove `useNonce` composable which exposes the nonce in the client
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
To be fair, you can keep the nonce composable server-side.
It could be useful to expose the nonce to third-party modules when they run server-side, such as NuxtImg as you rightly pointed out.
return useNuxtApp().ssrContext?.event?.context.nonce
and drop the useCookie part
There was a problem hiding this comment.
IMO, this would then be the best solution: https://github.com/trijpstra-fourlights/nuxt-security/pull/1
as it allows the useNonce, even on SPA mode (although it will use the cookie in that case).
Interestingly enough, the <img> element does not have a nonce attribute according to mdn. So it doesn't apply there.
| @@ -5,7 +5,6 @@ import { getRouteRules } from '#imports' | |||
|
|
|||
| export type NonceOptions = { | |||
| enabled: boolean; | |||
| mode: 'renew' | 'check'; | |||
| value: undefined | (() => string); | |||
There was a problem hiding this comment.
I'm not sure about giving the developper a way to override crypto primitives which are vetted to be safe
There was a problem hiding this comment.
IMO that's out of scope of this PR
| break | ||
| } | ||
| } | ||
| const nonce = nonceConfig?.value ? nonceConfig.value() : Buffer.from(crypto.randomUUID()).toString('base64') |
There was a problem hiding this comment.
I'd leave it to just the crypto primitive
|
I'm done with this. Please remove me as a contributor from this project. |
Types of changes
Description
As reported by @vejja in #241 (comment) the current nonce implementation is insecure by design.
While I was initially unconvinced, after going though all rendering modes and trying out various combinations I'm convinced that @vejja 's analysis is right and both the
nonce mode overrideanduseNoncecomposable should be removed.My apologies to @vejja for not exploring your findings more thorough. You were completely in the right.
Checklist: