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.

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

[SDK-3808] Optionally sign the session store cookie #419

Merged
merged 8 commits into from
Nov 16, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
41 changes: 35 additions & 6 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,12 @@ interface LogoutOptions {
*/
interface ConfigParams {
/**
* REQUIRED. The secret(s) used to derive an encryption key for the user identity in a session cookie and
* to sign the transient cookies used by the login callback.
* Use a single string key or array of keys for an encrypted session cookie.
* REQUIRED. The secret(s) used to derive an encryption key for the user identity in a stateless session cookie,
* to sign the transient cookies used by the login callback and to sign the custom session store cookies if
* {@Link signSessionStoreCookie} is `true`. Use a single string key or array of keys.
* If an array of secrets is provided, only the first element will be used to sign or encrypt the values, while all
* the elements will be considered when decrypting or verifying the values.
*
* Can use env key SECRET instead.
*/
secret?: string | Array<string>;
Expand Down Expand Up @@ -637,12 +640,38 @@ interface SessionConfigParams {
* Be aware the default implementation is slightly different in this library as
* compared to the default session id generation used in express-session.
*
* **IMPORTANT** If you override this method you must use a suitable
* cryptographically strong random value of sufficient size to prevent collisions
* and reduce the ability to hijack a session by guessing the session ID.
* **IMPORTANT** If you override this method you should be careful to generate
* unique IDs so your sessions do not conflict. Also, to reduce the ability
* to hijack a session by guessing the session ID, you must use a suitable
* cryptographically strong random value of sufficient size or sign the cookie
* by setting {@Link signSessionStoreCookie} to `true`.
*/
genid?: (req: OpenidRequest) => string;

/**
* Sign the session store cookies to reduce the chance of collisions
* and reduce the ability to hijack a session by guessing the session ID.
*
* This is required if you override {@Link genid} and don't use a suitable
* cryptographically strong random value of sufficient size.
*/
signSessionStoreCookie: boolean;

/**
* If you enable {@Link signSessionStoreCookie} your existing sessions will
* be invalidated. You can use this flag to temporarily allow unsigned cookies
* while you sign your user's session cookies. For example:
*
* Set {@Link signSessionStoreCookie} to `true` and {@Link requireSignedSessionStoreCookie} to `false`.
* Wait for your {@Link rollingDuration} (default 1 day) or {@Link absoluteDuration} (default 1 week)
* to pass (which ever comes first). By this time all your sessions cookies will either be signed or
* have expired, then you can remove the {@Link requireSignedSessionStoreCookie} config option which
* will set it to `true`.
*
* Signed session store cookies will be mandatory in the next major release.
*/
requireSignedSessionStoreCookie: boolean;

/**
* If you want your session duration to be rolling, eg reset everytime the
* user is active on your site, set this to a `true`. If you want the session
Expand Down
56 changes: 36 additions & 20 deletions lib/appSession.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
const { strict: assert, AssertionError } = require('assert');
const {
JWK,
JWKS,
JWE,
errors: { JOSEError },
} = require('jose');
const { promisify } = require('util');
const cookie = require('cookie');
const onHeaders = require('on-headers');
const COOKIES = require('./cookies');
const { encryption: deriveKey } = require('./hkdf');
const { getKeyStore, verifyCookie, signCookie } = require('./crypto');
const debug = require('./debug')('appSession');

const epoch = () => (Date.now() / 1000) | 0;
Expand Down Expand Up @@ -47,20 +45,17 @@ function replaceSession(req, session, config) {
}

module.exports = (config) => {
let current;

const alg = 'dir';
const enc = 'A256GCM';
const secrets = Array.isArray(config.secret)
? config.secret
: [config.secret];
const sessionName = config.session.name;
const cookieConfig = config.session.cookie;
const {
genid: generateId,
absoluteDuration,
rolling: rollingEnabled,
rollingDuration,
signSessionStoreCookie,
requireSignedSessionStoreCookie,
} = config.session;

const { transient: emptyTransient, ...emptyCookieOptions } = cookieConfig;
Expand All @@ -74,16 +69,7 @@ module.exports = (config) => {
);
const cookieChunkSize = MAX_COOKIE_SIZE - emptyCookie.length;

let keystore = new JWKS.KeyStore();

secrets.forEach((secretString, i) => {
const key = JWK.asKey(deriveKey(secretString));
if (i === 0) {
current = key;
}
keystore.add(key);
});

let [current, keystore] = getKeyStore(config.secret, true);
if (keystore.size === 1) {
keystore = current;
}
Expand Down Expand Up @@ -190,6 +176,10 @@ module.exports = (config) => {
};
}

getCookie(req) {
return req[COOKIES][sessionName];
}

setCookie(req, res, iat) {
setCookie(req, res, iat);
}
Expand All @@ -200,6 +190,13 @@ module.exports = (config) => {
this._get = promisify(store.get).bind(store);
this._set = promisify(store.set).bind(store);
this._destroy = promisify(store.destroy).bind(store);

let [current, keystore] = getKeyStore(config.secret);
if (keystore.size === 1) {
keystore = current;
}
this._keyStore = keystore;
this._current = current;
}

async get(id) {
Expand Down Expand Up @@ -231,6 +228,21 @@ module.exports = (config) => {
}
}

getCookie(req) {
if (signSessionStoreCookie) {
const verified = verifyCookie(
sessionName,
req[COOKIES][sessionName],
this._keyStore
);
if (requireSignedSessionStoreCookie) {
return verified;
}
return verified || req[COOKIES][sessionName];
}
return req[COOKIES][sessionName];
}

setCookie(
id,
req,
Expand All @@ -247,7 +259,11 @@ module.exports = (config) => {
expires: cookieConfig.transient ? 0 : new Date(exp * 1000),
};
delete cookieOptions.transient;
res.cookie(sessionName, id, cookieOptions);
let value = id;
if (signSessionStoreCookie) {
value = signCookie(sessionName, id, this._current);
}
res.cookie(sessionName, value, cookieOptions);
}
}
}
Expand Down Expand Up @@ -281,7 +297,7 @@ module.exports = (config) => {
if (req[COOKIES].hasOwnProperty(sessionName)) {
// get JWE from unchunked session cookie
debug('reading session from %s cookie', sessionName);
existingSessionValue = req[COOKIES][sessionName];
existingSessionValue = store.getCookie(req);
} else if (req[COOKIES].hasOwnProperty(`${sessionName}.0`)) {
// get JWE from chunked session cookie
// iterate all cookie names
Expand Down
4 changes: 4 additions & 0 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ const paramsSchema = Joi.object({
.maxArity(1)
.optional()
.default(() => defaultSessionIdGenerator),
signSessionStoreCookie: Joi.boolean().optional().default(false),
requireSignedSessionStoreCookie: Joi.boolean()
.optional()
.default(Joi.ref('signSessionStoreCookie')),
cookie: Joi.object({
domain: Joi.string().optional(),
transient: Joi.boolean().optional().default(false),
Expand Down
115 changes: 115 additions & 0 deletions lib/crypto.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
const crypto = require('crypto');
const { JWKS, JWK, JWS } = require('jose');

const BYTE_LENGTH = 32;
const ENCRYPTION_INFO = 'JWE CEK';
const SIGNING_INFO = 'JWS Cookie Signing';
const DIGEST = 'sha256';

let encryption, signing;

/**
*
* Derives appropriate sized keys from the end-user provided secret random string/passphrase using
* HKDF (HMAC-based Extract-and-Expand Key Derivation Function) defined in RFC 8569.
*
* @see https://tools.ietf.org/html/rfc5869
*
*/
if (crypto.hkdfSync) {
// added in v15.0.0
encryption = (secret) =>
Buffer.from(
crypto.hkdfSync(
DIGEST,
secret,
Buffer.alloc(0),
ENCRYPTION_INFO,
BYTE_LENGTH
)
);
signing = (secret) =>
Buffer.from(
crypto.hkdfSync(
DIGEST,
secret,
Buffer.alloc(0),
SIGNING_INFO,
BYTE_LENGTH
)
);
} else {
const hkdf = require('futoin-hkdf');
encryption = (secret) =>
hkdf(secret, BYTE_LENGTH, { info: ENCRYPTION_INFO, hash: DIGEST });
signing = (secret) =>
hkdf(secret, BYTE_LENGTH, { info: SIGNING_INFO, hash: DIGEST });
}

const getKeyStore = (secret, forEncryption) => {
let current;
const secrets = Array.isArray(secret) ? secret : [secret];
let keystore = new JWKS.KeyStore();
secrets.forEach((secretString, i) => {
const key = JWK.asKey(
forEncryption ? encryption(secretString) : signing(secretString)
);
if (i === 0) {
current = key;
}
keystore.add(key);
});
return [current, keystore];
};

const header = { alg: 'HS256', b64: false, crit: ['b64'] };

const getPayload = (cookie, value) => Buffer.from(`${cookie}=${value}`);
const flattenedJWSFromCookie = (cookie, value, signature) => ({
protected: Buffer.from(JSON.stringify(header))
.toString('base64')
.replace(/=/g, '')
.replace(/\+/g, '-')
.replace(/\//g, '_'),
payload: getPayload(cookie, value),
signature,
});
const generateSignature = (cookie, value, key) => {
const payload = getPayload(cookie, value);
return JWS.sign.flattened(payload, key, header).signature;
};
const verifySignature = (cookie, value, signature, keystore) => {
try {
return !!JWS.verify(
flattenedJWSFromCookie(cookie, value, signature),
keystore,
{ algorithm: 'HS256', crit: ['b64'] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Any way to not duplicate these values that are already used in the header constant?

Copy link
Contributor Author

@adamjmcgrath adamjmcgrath Nov 16, 2022

Choose a reason for hiding this comment

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

👍 have moved these into constants

Also, noticed a typo - thanks

);
} catch (err) {
return false;
}
};
const verifyCookie = (cookie, value, keystore) => {
if (!value) {
return undefined;
}
let signature;
[value, signature] = value.split('.');
if (verifySignature(cookie, value, signature, keystore)) {
return value;
}

return undefined;
};

const signCookie = (cookie, value, key) => {
const signature = generateSignature(cookie, value, key);
return `${value}.${signature}`;
};

module.exports.signCookie = signCookie;
module.exports.verifyCookie = verifyCookie;

module.exports.getKeyStore = getKeyStore;
module.exports.encryption = encryption;
module.exports.signing = signing;
45 changes: 0 additions & 45 deletions lib/hkdf.js

This file was deleted.

Loading