From 76efeff6a79280a58412bbf31e04288663aef613 Mon Sep 17 00:00:00 2001 From: psibean Date: Sat, 6 Apr 2024 15:45:27 +1030 Subject: [PATCH] use createHmac to sign generated csrf tokens BREAKING CHANGE: This change replaces the createHash method with createHmac for hashing tokens. It introduces the getSessionIdentifier configuration option which by default will return req.session.id. The purpose of this function is to return the id of the session associated with the incoming request. The session id will be included in the hmac signature, forcefully tying the generated csrf token with that session. This means by default generated tokens can only be used by the session which they were originally generated for. If you have any kind of session rotation (to mitigate session hijacking), which you should be doing during privilege escaltions and de-escalations (e.g. sign in, sign out) then you will need to generate a new csrf token at the same time. Additionally this change exposes the delimiter and hmacAlgorithm options. --- package-lock.json | 19 +++++++++++++++++ package.json | 1 + src/index.ts | 46 +++++++++++++++++++++++++---------------- src/tests/utils/mock.ts | 3 +++ src/types.ts | 31 +++++++++++++++++++-------- 5 files changed, 73 insertions(+), 27 deletions(-) diff --git a/package-lock.json b/package-lock.json index 56749b9..7462798 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,6 +17,7 @@ "@types/cookie-parser": "^1.4.6", "@types/cookie-signature": "^1.1.2", "@types/express": "^4.17.21", + "@types/express-session": "^1.18.0", "@types/http-errors": "^2.0.4", "@types/mocha": "^10.0.6", "@types/node": "^18.15.8", @@ -1154,6 +1155,15 @@ "@types/range-parser": "*" } }, + "node_modules/@types/express-session": { + "version": "1.18.0", + "resolved": "https://registry.npmjs.org/@types/express-session/-/express-session-1.18.0.tgz", + "integrity": "sha512-27JdDRgor6PoYlURY+Y5kCakqp5ulC0kmf7y+QwaY+hv9jEFuQOThgkjyA53RP3jmKuBsH5GR6qEfFmvb8mwOA==", + "dev": true, + "dependencies": { + "@types/express": "*" + } + }, "node_modules/@types/http-errors": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/@types/http-errors/-/http-errors-2.0.4.tgz", @@ -6453,6 +6463,15 @@ "@types/range-parser": "*" } }, + "@types/express-session": { + "version": "1.18.0", + "resolved": "https://registry.npmjs.org/@types/express-session/-/express-session-1.18.0.tgz", + "integrity": "sha512-27JdDRgor6PoYlURY+Y5kCakqp5ulC0kmf7y+QwaY+hv9jEFuQOThgkjyA53RP3jmKuBsH5GR6qEfFmvb8mwOA==", + "dev": true, + "requires": { + "@types/express": "*" + } + }, "@types/http-errors": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/@types/http-errors/-/http-errors-2.0.4.tgz", diff --git a/package.json b/package.json index b4af756..a98c2c2 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "@types/cookie-parser": "^1.4.6", "@types/cookie-signature": "^1.1.2", "@types/express": "^4.17.21", + "@types/express-session": "^1.18.0", "@types/http-errors": "^2.0.4", "@types/mocha": "^10.0.6", "@types/node": "^18.15.8", diff --git a/src/index.ts b/src/index.ts index 8450fd3..06f0c3b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ /* eslint-disable @typescript-eslint/no-unsafe-argument */ import type { Request, Response } from "express"; -import { createHash, randomBytes } from "crypto"; +import { createHmac, randomBytes } from "crypto"; import createHttpError from "http-errors"; import type { @@ -18,6 +18,7 @@ import type { export function doubleCsrf({ getSecret, + getSessionIdentifier = (req) => req.session.id, cookieName = "__Host-psifi.x-csrf-token", cookieOptions: { sameSite = "lax", @@ -26,7 +27,9 @@ export function doubleCsrf({ httpOnly = true, ...remainingCookieOptions } = {}, + delimiter = "|", size = 64, + hmacAlgorithm = "sha256", ignoredMethods = ["GET", "HEAD", "OPTIONS"], getTokenFromRequest = (req) => req.headers["x-csrf-token"], errorConfig: { @@ -67,8 +70,14 @@ export function doubleCsrf({ // the existing cookie and reuse it if it is valid. If it isn't valid, then either throw or // generate a new token based on validateOnReuse. if (typeof csrfCookie === "string" && !overwrite) { - const [csrfToken, csrfTokenHash] = csrfCookie.split("|"); - if (validateTokenAndHashPair(csrfToken, csrfTokenHash, possibleSecrets)) { + const [csrfToken, csrfTokenHash] = csrfCookie.split(delimiter); + if ( + validateTokenAndHashPair(req, { + incomingToken: csrfToken, + incomingHash: csrfTokenHash, + possibleSecrets, + }) + ) { // If the pair is valid, reuse it return { csrfToken, csrfTokenHash }; } else if (validateOnReuse) { @@ -81,8 +90,9 @@ export function doubleCsrf({ const csrfToken = randomBytes(size).toString("hex"); // the 'newest' or preferred secret is the first one in the array const secret = possibleSecrets[0]; - const csrfTokenHash = createHash("sha256") - .update(`${csrfToken}${secret}`) + + const csrfTokenHash = createHmac(hmacAlgorithm, secret) + .update(`${getSessionIdentifier(req)}${csrfToken}`) .digest("hex"); return { csrfToken, csrfTokenHash }; @@ -106,7 +116,7 @@ export function doubleCsrf({ overwrite, validateOnReuse, }); - const cookieContent = `${csrfToken}|${csrfTokenHash}`; + const cookieContent = `${csrfToken}${delimiter}${csrfTokenHash}`; res.cookie(cookieName, cookieContent, { ...defaultCookieOptions, ...cookieOptions, @@ -120,17 +130,17 @@ export function doubleCsrf({ // given a secret array, iterates over it and checks whether one of the secrets makes the token and hash pair valid const validateTokenAndHashPair: CsrfTokenAndHashPairValidator = ( - token, - hash, - possibleSecrets, + req, + { incomingHash, incomingToken, possibleSecrets }, ) => { - if (typeof token !== "string" || typeof hash !== "string") return false; + if (typeof incomingToken !== "string" || typeof incomingHash !== "string") + return false; for (const secret of possibleSecrets) { - const expectedHash = createHash("sha256") - .update(`${token}${secret}`) + const expectedHash = createHmac("sha256", secret) + .update(`${getSessionIdentifier(req)}${incomingToken}`) .digest("hex"); - if (hash === expectedHash) return true; + if (incomingHash === expectedHash) return true; } return false; @@ -141,7 +151,7 @@ export function doubleCsrf({ const csrfCookie = getCsrfCookieFromRequest(req); if (typeof csrfCookie !== "string") return false; - // cookie has the form {token}|{hash} + // cookie has the form {token}{delimiter}{hash} const [csrfToken, csrfTokenHash] = csrfCookie.split("|"); // csrf token from the request @@ -154,11 +164,11 @@ export function doubleCsrf({ return ( csrfToken === csrfTokenFromRequest && - validateTokenAndHashPair( - csrfTokenFromRequest, - csrfTokenHash, + validateTokenAndHashPair(req, { + incomingToken: csrfTokenFromRequest, + incomingHash: csrfTokenHash, possibleSecrets, - ) + }) ); }; diff --git a/src/tests/utils/mock.ts b/src/tests/utils/mock.ts index 09c6c2a..8476aa7 100644 --- a/src/tests/utils/mock.ts +++ b/src/tests/utils/mock.ts @@ -16,6 +16,9 @@ export const generateMocks = () => { cookies: {}, signedCookies: {}, secret: COOKIE_SECRET, + session: { + id: "f5d7e7d1-a0dd-cf55-c0bb-5aa5aabe441f", + }, } as unknown as Request; // Internally mock the headers as a map. diff --git a/src/types.ts b/src/types.ts index f232c91..9083a0e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -40,9 +40,16 @@ export type RequestMethod = export type CsrfIgnoredMethods = Array; export type CsrfRequestValidator = (req: Request) => boolean; export type CsrfTokenAndHashPairValidator = ( - token: string, - hash: string, - possibleSecrets: Array, + req: Request, + { + incomingHash, + incomingToken, + possibleSecrets, + }: { + incomingHash: string; + incomingToken: string; + possibleSecrets: Array; + }, ) => boolean; export type CsrfCookieSetter = ( res: Response, @@ -88,24 +95,30 @@ export interface DoubleCsrfConfig { */ getSecret: CsrfSecretRetriever; + getSessionIdentifier: (req: Request) => string; /** * The name of the HTTPOnly cookie that will be set on the response. * @default "__Host-psifi.x-csrf-token" */ cookieName: string; - /** - * The size in bytes of the generated token. - * @default 64 - */ - size: number; - /** * The options for HTTPOnly cookie that will be set on the response. * @default { sameSite: "lax", path: "/", secure: true } */ cookieOptions: CookieOptions; + /** + * Used to separate the plain token and the token hash in the cookie value. + */ + delimiter: string; + /** + * The size in bytes of the generated token. + * @default 64 + */ + size: number; + + hmacAlgorithm: string; /** * The methods that will be ignored by the middleware. * @default ["GET", "HEAD", "OPTIONS"]