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.

Sign up for GitHub

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

Jump to bottom

Fix: make nonce implementation strict CSP compliant #256

Closed
wants to merge 3 commits into from
Closed

Fix: make nonce implementation strict CSP compliant #256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3411d06
fix: nonce should be strict CSP compliant
trijpstra-fourlights Oct 20, 2023
aeffc6f
chore: update nonce docs to reflect new situation
trijpstra-fourlights Oct 20, 2023
f6f9dfb
chore: update nonce tests to reflect new situation
trijpstra-fourlights Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/runtime/composables/nonce.ts
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@trijpstra-fourlights trijpstra-fourlights Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This file was deleted.

23 changes: 2 additions & 21 deletions src/runtime/server/middleware/cspNonceHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { getRouteRules } from '#imports'

export type NonceOptions = {
enabled: boolean;
mode: 'renew' | 'check';
value: undefined | (() => string);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about giving the developper a way to override crypto primitives which are vetted to be safe

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO that's out of scope of this PR

}

Expand All @@ -16,26 +15,8 @@ export default defineEventHandler((event) => {
if (routeRules.security.nonce !== false) {
const nonceConfig: NonceOptions = routeRules.security.nonce

// See if we are checking the nonce against the current value, or if we are renewing the nonce value
let nonce: string | undefined
switch (nonceConfig?.mode) {
case 'check': {
nonce = event.context.nonce ?? getCookie(event, 'nonce')

if (!nonce) {
return sendError(event, createError({ statusCode: 401, statusMessage: 'Nonce is not set' }))
}

break
}
case 'renew':
default: {
nonce = nonceConfig?.value ? nonceConfig.value() : Buffer.from(crypto.randomUUID()).toString('base64')
setCookie(event, 'nonce', nonce, { sameSite: true, secure: true })
event.context.nonce = nonce
break
}
}
const nonce = nonceConfig?.value ? nonceConfig.value() : Buffer.from(crypto.randomUUID()).toString('base64')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave it to just the crypto primitive

event.context.nonce = nonce

// Set actual nonce value in CSP header
csp = csp.replaceAll('{{nonce}}', nonce as string)
Expand Down