From be3e5761f37aa05c7c1ac8ed44499c51ecec8058 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Thu, 20 Apr 2023 09:39:58 +0200 Subject: [PATCH] Merge pull request from GHSA-qrgf-9gpc-vrxw * fix: enforce hmac key * fix: check only if fastify/cookie is enabled * fix: restored working testts * fix: update README.md Co-authored-by: Uzlopak * Update index.d.ts * Update index.test-d.ts * Update index.d.ts * Update index.test-d.ts * fix unit tests --------- Co-authored-by: Uzlopak --- README.md | 1 + index.js | 5 +++ test/user-info.test.js | 88 ++++++++++++++++++++++++++++++++++++++++++ types/index.d.ts | 27 +++++++++++-- types/index.test-d.ts | 23 +++++++++-- 5 files changed, 136 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ec8218b..651f4f0 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,7 @@ You can also pass the [cookie serialization](https://github.com/fastify/fastify- The option `userInfo` is required if `getUserInfo` has been specified in the module option. The provided `userInfo` is hashed inside the csrf token and it is not directly exposed. This option is needed to protect against cookie tossing. +The option `csrfOpts.hmacKey` is required if `getUserInfo` has been specified in the module option in combination with using [@fastify/cookie](https://github.com/fastify/fastify-cookie) as sessionPlugin ### `fastify.csrfProtection(request, reply, next)` diff --git a/index.js b/index.js index 4c12ef0..478ff49 100644 --- a/index.js +++ b/index.js @@ -42,6 +42,11 @@ async function fastifyCsrfProtection (fastify, opts) { if (opts.getUserInfo) { csrfOpts.userInfo = true } + + if (sessionPlugin === '@fastify/cookie' && csrfOpts.userInfo) { + assert(csrfOpts.hmacKey, 'csrfOpts.hmacKey is required') + } + const tokens = new CSRF(csrfOpts) const isCookieSigned = cookieOpts && cookieOpts.signed diff --git a/test/user-info.test.js b/test/user-info.test.js index 58917c8..269ca80 100644 --- a/test/user-info.test.js +++ b/test/user-info.test.js @@ -17,6 +17,9 @@ test('Cookies with User-Info', async t => { await fastify.register(fastifyCsrf, { getUserInfo (req) { return userInfoDB[req.body.username] + }, + csrfOpts: { + hmacKey: 'foo' } }) @@ -73,6 +76,9 @@ test('Session with User-Info', async t => { sessionPlugin: '@fastify/session', getUserInfo (req) { return req.session.username + }, + csrfOpts: { + hmacKey: 'foo' } }) @@ -122,6 +128,9 @@ test('SecureSession with User-Info', async t => { sessionPlugin: '@fastify/secure-session', getUserInfo (req) { return req.session.get('username') + }, + csrfOpts: { + hmacKey: 'foo' } }) @@ -163,3 +172,82 @@ test('SecureSession with User-Info', async t => { t.equal(response2.statusCode, 200) }) + +test('Validate presence of hmac key with User-Info /1', async (t) => { + const fastify = Fastify() + await fastify.register(fastifyCookie) + + await t.rejects(fastify.register(fastifyCsrf, { + getUserInfo (req) { + return req.session.get('username') + } + }), Error('csrfOpts.hmacKey is required')) +}) + +test('Validate presence of hmac key with User-Info /2', async (t) => { + const fastify = Fastify() + await fastify.register(fastifyCookie) + + await t.rejects(fastify.register(fastifyCsrf, { + getUserInfo (req) { + return req.session.get('username') + }, + sessionPlugin: '@fastify/cookie' + }), Error('csrfOpts.hmacKey is required')) +}) + +test('Validate presence of hmac key with User-Info /3', async (t) => { + const fastify = Fastify() + await fastify.register(fastifyCookie) + + await t.rejects(fastify.register(fastifyCsrf, { + getUserInfo (req) { + return req.session.get('username') + }, + csrfOpts: { + hmacKey: undefined + } + }), Error('csrfOpts.hmacKey is required')) +}) + +test('Validate presence of hmac key with User-Info /4', async (t) => { + const fastify = Fastify() + await fastify.register(fastifyCookie) + + await t.rejects(fastify.register(fastifyCsrf, { + getUserInfo (req) { + return req.session.get('username') + }, + sessionPlugin: '@fastify/cookie', + csrfOpts: { + hmacKey: undefined + } + }), Error('csrfOpts.hmacKey is required')) +}) + +test('Validate presence of hmac key with User-Info /5', async (t) => { + const fastify = Fastify() + await fastify.register(fastifySecureSession, { key, cookie: { path: '/', secure: false } }) + + await t.resolves(fastify.register(fastifyCsrf, { + getUserInfo (req) { + return req.session.get('username') + }, + sessionPlugin: '@fastify/secure-session' + })) +}) + +test('Validate presence of hmac key with User-Info /6', async (t) => { + const fastify = Fastify() + await fastify.register(fastifySecureSession, { key, cookie: { path: '/', secure: false } }) + + await t.resolves(fastify.register(fastifyCsrf, { + getUserInfo (req) { + return req.session.get('username') + }, + sessionPlugin: '@fastify/secure-session', + csrfOpts: { + hmacKey: 'foo' + } + })) +}) diff --git a/types/index.d.ts b/types/index.d.ts index 8c3c30c..365ead8 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -26,17 +26,36 @@ declare namespace fastifyCsrfProtection { export type CookieSerializeOptions = FastifyCookieSerializeOptions export type GetTokenFn = (req: FastifyRequest) => string | void; - - export interface FastifyCsrfProtectionOptions { - csrfOpts?: CSRFOptions; + + interface FastifyCsrfProtectionOptionsBase { cookieKey?: string; cookieOpts?: CookieSerializeOptions; sessionKey?: string; getUserInfo?: (req: FastifyRequest) => string; getToken?: GetTokenFn; - sessionPlugin?: '@fastify/cookie' | '@fastify/session' | '@fastify/secure-session'; } + interface FastifyCsrfProtectionOptionsFastifyCookie { + sessionPlugin?: '@fastify/cookie'; + csrfOpts: Omit & Required>; + } + + interface FastifyCsrfProtectionOptionsFastifySession { + sessionPlugin: '@fastify/session'; + csrfOpts?: CSRFOptions; + } + + interface FastifyCsrfProtectionOptionsFastifySecureSession { + sessionPlugin: '@fastify/secure-session'; + csrfOpts?: CSRFOptions; + } + + export type FastifyCsrfProtectionOptions = FastifyCsrfProtectionOptionsBase & ( + FastifyCsrfProtectionOptionsFastifyCookie | + FastifyCsrfProtectionOptionsFastifySession | + FastifyCsrfProtectionOptionsFastifySecureSession + ) + /** * @deprecated Use FastifyCsrfProtectionOptions instead */ diff --git a/types/index.test-d.ts b/types/index.test-d.ts index 467f402..a7d479c 100644 --- a/types/index.test-d.ts +++ b/types/index.test-d.ts @@ -31,13 +31,28 @@ async function run() { } -fastify.register(FastifyCsrfProtection, { csrfOpts: { algorithm: 'sha1' } }) +fastify.register(FastifyCsrfProtection, { csrfOpts: { algorithm: 'sha1', hmacKey: 'hmac' } }) expectError(fastify.register(FastifyCsrfProtection, { csrfOpts: { algorithm: 1 } })) fastify.register(FastifySession) -fastify.register(FastifyCsrfProtection, { getUserInfo(req) { - return req.session.get('username') -}}) +fastify.register(FastifyCsrfProtection, { + csrfOpts: { + hmacKey: '123' + }, + getUserInfo(req) { + return req.session.get('username') + } +}) expectError(fastify.register(FastifyCsrfProtection, { getUserInfo: 'invalid' })) +fastify.register(FastifyCsrfProtection, { csrfOpts: { hmacKey: 'hmac' }, sessionPlugin: '@fastify/cookie' }) +fastify.register(FastifyCsrfProtection, { csrfOpts: { hmacKey: 'hmac' } }) +expectError(fastify.register(FastifyCsrfProtection, { })) +expectError(fastify.register(FastifyCsrfProtection, { csrfOpts: { }})) +expectError(fastify.register(FastifyCsrfProtection, { sessionPlugin: '@fastify/cookie', csrfOpts: { }})) +fastify.register(FastifyCsrfProtection, { csrfOpts: { }, sessionPlugin: '@fastify/session' }) +fastify.register(FastifyCsrfProtection, { csrfOpts: { }, sessionPlugin: '@fastify/secure-session' }) +fastify.register(FastifyCsrfProtection, { sessionPlugin: '@fastify/session' }) +fastify.register(FastifyCsrfProtection, { sessionPlugin: '@fastify/secure-session' }) + expectDeprecated({} as FastifyCsrfOptions)