From 828ad5a33dc0cc0049923b69f43f97463295456e Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 1 Jan 2020 21:23:05 +0100 Subject: [PATCH] fix: force iat past check when maxTokenAge option is used + JWT refactor --- lib/jwt/verify.js | 156 ++++++++++++++++++++-------------------- test/jwt/verify.test.js | 45 ++++++++---- 2 files changed, 110 insertions(+), 91 deletions(-) diff --git a/lib/jwt/verify.js b/lib/jwt/verify.js index d30810874b..22e0d5f545 100644 --- a/lib/jwt/verify.js +++ b/lib/jwt/verify.js @@ -38,81 +38,85 @@ const isStringOrArrayOfStrings = (value, label, required = false) => { const isNotArrayOfStrings = val => !Array.isArray(val) || val.length === 0 || val.some(isNotString) -const validateOptions = (options) => { - isOptionString(options.profile, 'options.profile') - - if (typeof options.complete !== 'boolean') { +const validateOptions = ({ + algorithms, audience, clockTolerance, complete = false, crit, ignoreExp = false, + ignoreIat = false, ignoreNbf = false, issuer, jti, maxAuthAge, maxTokenAge, nonce, now = new Date(), + profile, subject +}) => { + isOptionString(profile, 'options.profile') + + if (typeof complete !== 'boolean') { throw new TypeError('options.complete must be a boolean') } - if (typeof options.ignoreExp !== 'boolean') { + if (typeof ignoreExp !== 'boolean') { throw new TypeError('options.ignoreExp must be a boolean') } - if (typeof options.ignoreNbf !== 'boolean') { + if (typeof ignoreNbf !== 'boolean') { throw new TypeError('options.ignoreNbf must be a boolean') } - if (typeof options.ignoreIat !== 'boolean') { + if (typeof ignoreIat !== 'boolean') { throw new TypeError('options.ignoreIat must be a boolean') } - isOptionString(options.maxTokenAge, 'options.maxTokenAge') - isOptionString(options.subject, 'options.subject') - isOptionString(options.issuer, 'options.issuer') - isOptionString(options.maxAuthAge, 'options.maxAuthAge') - isOptionString(options.jti, 'options.jti') - isOptionString(options.clockTolerance, 'options.clockTolerance') + isOptionString(maxTokenAge, 'options.maxTokenAge') + isOptionString(subject, 'options.subject') + isOptionString(issuer, 'options.issuer') + isOptionString(maxAuthAge, 'options.maxAuthAge') + isOptionString(jti, 'options.jti') + isOptionString(clockTolerance, 'options.clockTolerance') - if (options.audience !== undefined && (isNotString(options.audience) && isNotArrayOfStrings(options.audience))) { + if (audience !== undefined && (isNotString(audience) && isNotArrayOfStrings(audience))) { throw new TypeError('options.audience must be a string or an array of strings') } - if (options.algorithms !== undefined && isNotArrayOfStrings(options.algorithms)) { + if (algorithms !== undefined && isNotArrayOfStrings(algorithms)) { throw new TypeError('options.algorithms must be an array of strings') } - isOptionString(options.nonce, 'options.nonce') + isOptionString(nonce, 'options.nonce') - if (!(options.now instanceof Date) || !options.now.getTime()) { + if (!(now instanceof Date) || !now.getTime()) { throw new TypeError('options.now must be a valid Date object') } - if (options.ignoreIat && options.maxTokenAge !== undefined) { + if (ignoreIat && maxTokenAge !== undefined) { throw new TypeError('options.ignoreIat and options.maxTokenAge cannot used together') } - if (options.crit !== undefined && isNotArrayOfStrings(options.crit)) { + if (crit !== undefined && isNotArrayOfStrings(crit)) { throw new TypeError('options.crit must be an array of strings') } - switch (options.profile) { + switch (profile) { case IDTOKEN: - if (!options.issuer) { + if (!issuer) { throw new TypeError('"issuer" option is required to validate an ID Token') } - if (!options.audience) { + if (!audience) { throw new TypeError('"audience" option is required to validate an ID Token') } break case ATJWT: - if (!options.issuer) { + if (!issuer) { throw new TypeError('"issuer" option is required to validate a JWT Access Token') } - if (!options.audience) { + if (!audience) { throw new TypeError('"audience" option is required to validate a JWT Access Token') } break case LOGOUTTOKEN: - if (!options.issuer) { + if (!issuer) { throw new TypeError('"issuer" option is required to validate a Logout Token') } - if (!options.audience) { + if (!audience) { throw new TypeError('"audience" option is required to validate a Logout Token') } @@ -120,23 +124,42 @@ const validateOptions = (options) => { case undefined: break default: - throw new TypeError(`unsupported options.profile value "${options.profile}"`) + throw new TypeError(`unsupported options.profile value "${profile}"`) + } + + return { + algorithms, + audience, + clockTolerance, + complete, + crit, + ignoreExp, + ignoreIat, + ignoreNbf, + issuer, + jti, + maxAuthAge, + maxTokenAge, + nonce, + now, + profile, + subject } } -const validateTypes = ({ header, payload }, profile) => { +const validateTypes = ({ header, payload }, profile, options) => { isPayloadString(header.alg, '"alg" header parameter', true) - isTimestamp(payload.iat, 'iat', profile === IDTOKEN || profile === LOGOUTTOKEN) + isTimestamp(payload.iat, 'iat', profile === IDTOKEN || profile === LOGOUTTOKEN || !!options.maxTokenAge) isTimestamp(payload.exp, 'exp', profile === IDTOKEN || profile === ATJWT) - isTimestamp(payload.auth_time, 'auth_time') + isTimestamp(payload.auth_time, 'auth_time', !!options.maxAuthAge) isTimestamp(payload.nbf, 'nbf') - isPayloadString(payload.jti, '"jti" claim', profile === LOGOUTTOKEN) + isPayloadString(payload.jti, '"jti" claim', profile === LOGOUTTOKEN || !!options.jti) isPayloadString(payload.acr, '"acr" claim') - isPayloadString(payload.nonce, '"nonce" claim') - isPayloadString(payload.iss, '"iss" claim', profile === IDTOKEN || profile === ATJWT || profile === LOGOUTTOKEN) - isPayloadString(payload.sub, '"sub" claim', profile === IDTOKEN || profile === ATJWT) - isStringOrArrayOfStrings(payload.aud, 'aud', profile === IDTOKEN || profile === ATJWT || profile === LOGOUTTOKEN) + isPayloadString(payload.nonce, '"nonce" claim', !!options.nonce) + isPayloadString(payload.iss, '"iss" claim', profile === IDTOKEN || profile === ATJWT || profile === LOGOUTTOKEN || !!options.issuer) + isPayloadString(payload.sub, '"sub" claim', profile === IDTOKEN || profile === ATJWT || !!options.subject) + isStringOrArrayOfStrings(payload.aud, 'aud', profile === IDTOKEN || profile === ATJWT || profile === LOGOUTTOKEN || !!options.audience) isPayloadString(payload.azp, '"azp" claim', profile === IDTOKEN && Array.isArray(payload.aud) && payload.aud.length > 1) isStringOrArrayOfStrings(payload.amr, 'amr') @@ -198,92 +221,71 @@ module.exports = (token, key, options = {}) => { } const { - algorithms, audience, clockTolerance, complete = false, crit, ignoreExp = false, - ignoreIat = false, ignoreNbf = false, issuer, jti, maxAuthAge, maxTokenAge, nonce, now = new Date(), - subject, profile - } = options - - validateOptions({ - algorithms, - audience, - clockTolerance, - complete, - crit, - ignoreExp, - ignoreIat, - ignoreNbf, - issuer, - jti, - maxAuthAge, - maxTokenAge, - nonce, - now, - profile, - subject - }) + algorithms, audience, clockTolerance, complete, crit, ignoreExp, ignoreIat, ignoreNbf, issuer, + jti, maxAuthAge, maxTokenAge, nonce, now, profile, subject + } = options = validateOptions(options) const unix = epoch(now) const decoded = decode(token, { complete: true }) - validateTypes(decoded, profile) + validateTypes(decoded, profile, options) if (issuer && decoded.payload.iss !== issuer) { - throw new JWTClaimInvalid('issuer mismatch') + throw new JWTClaimInvalid('unexpected "iss" claim value') } if (nonce && decoded.payload.nonce !== nonce) { - throw new JWTClaimInvalid('nonce mismatch') + throw new JWTClaimInvalid('unexpected "nonce" claim value') } if (subject && decoded.payload.sub !== subject) { - throw new JWTClaimInvalid('subject mismatch') + throw new JWTClaimInvalid('unexpected "sub" claim value') } if (jti && decoded.payload.jti !== jti) { - throw new JWTClaimInvalid('jti mismatch') + throw new JWTClaimInvalid('unexpected "jti" claim value') } if (audience && !checkAudiencePresence(decoded.payload.aud, typeof audience === 'string' ? [audience] : audience, profile)) { - throw new JWTClaimInvalid('audience mismatch') + throw new JWTClaimInvalid('unexpected "aud" claim value') } const tolerance = clockTolerance ? secs(clockTolerance) : 0 if (maxAuthAge) { - if (!('auth_time' in decoded.payload)) { - throw new JWTClaimInvalid('missing auth_time') - } - const maxAuthAgeSeconds = secs(maxAuthAge) if (decoded.payload.auth_time + maxAuthAgeSeconds < unix - tolerance) { - throw new JWTClaimInvalid('too much time has elapsed since the last End-User authentication') + throw new JWTClaimInvalid('"auth_time" claim timestamp check failed (too much time has elapsed since the last End-User authentication)') } } if (!ignoreIat && !('exp' in decoded.payload) && 'iat' in decoded.payload && decoded.payload.iat > unix + tolerance) { - throw new JWTClaimInvalid('token issued in the future') + throw new JWTClaimInvalid('"iat" claim timestamp check failed (it should be in the past)') } if (!ignoreNbf && 'nbf' in decoded.payload && decoded.payload.nbf > unix + tolerance) { - throw new JWTClaimInvalid('token is not active yet') + throw new JWTClaimInvalid('"nbf" claim timestamp check failed') } if (!ignoreExp && 'exp' in decoded.payload && decoded.payload.exp <= unix - tolerance) { - throw new JWTClaimInvalid('token is expired') + throw new JWTClaimInvalid('"exp" claim timestamp check failed') } if (maxTokenAge) { - if (!('iat' in decoded.payload)) { - throw new JWTClaimInvalid('missing iat claim') + const age = unix - decoded.payload.iat + const max = secs(maxTokenAge) + + if (age - tolerance > max) { + throw new JWTClaimInvalid('"iat" claim timestamp check failed (too far in the past)') } - if (decoded.payload.iat + secs(maxTokenAge) < unix + tolerance) { - throw new JWTClaimInvalid('maxTokenAge exceeded') + if (age - tolerance > max || age < 0 - tolerance) { + throw new JWTClaimInvalid('"iat" claim timestamp check failed (it should be in the past)') } } if (profile === IDTOKEN && Array.isArray(decoded.payload.aud) && decoded.payload.aud.length > 1 && decoded.payload.azp !== audience) { - throw new JWTClaimInvalid('azp mismatch') + throw new JWTClaimInvalid('unexpected "azp" claim value') } if (profile === ATJWT && decoded.header.typ !== ATJWT) { diff --git a/test/jwt/verify.test.js b/test/jwt/verify.test.js index 08a1cc1cd1..75fc7b9ee2 100644 --- a/test/jwt/verify.test.js +++ b/test/jwt/verify.test.js @@ -150,7 +150,11 @@ Object.entries({ t.throws(() => { const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({ [claim]: 'foo' })}.` JWT.verify(invalid, key, { [option]: 'bar' }) - }, { instanceOf: errors.JWTClaimInvalid, message: `${option} mismatch` }) + }, { instanceOf: errors.JWTClaimInvalid, message: `unexpected "${claim}" claim value` }) + t.throws(() => { + const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({ [claim]: undefined })}.` + JWT.verify(invalid, key, { [option]: 'bar' }) + }, { instanceOf: errors.JWTClaimInvalid, message: `"${claim}" claim is missing` }) }) test(`option.${option} validation success`, t => { @@ -164,11 +168,11 @@ test('option.audience validation fails', t => { t.throws(() => { const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({ aud: 'foo' })}.` JWT.verify(invalid, key, { audience: 'bar' }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'audience mismatch' }) + }, { instanceOf: errors.JWTClaimInvalid, message: 'unexpected "aud" claim value' }) t.throws(() => { const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({ aud: ['foo'] })}.` JWT.verify(invalid, key, { audience: 'bar' }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'audience mismatch' }) + }, { instanceOf: errors.JWTClaimInvalid, message: 'unexpected "aud" claim value' }) }) test('option.audience validation success', t => { @@ -192,7 +196,7 @@ test('option.maxAuthAge requires iat to be in the payload', t => { t.throws(() => { const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({})}.` JWT.verify(invalid, key, { maxAuthAge: '30s' }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'missing auth_time' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"auth_time" claim is missing' }) }) const epoch = 1265328501 @@ -202,7 +206,7 @@ test('option.maxAuthAge checks auth_time', t => { t.throws(() => { const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({ auth_time: epoch - 31 })}.` JWT.verify(invalid, key, { maxAuthAge: '30s', now }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'too much time has elapsed since the last End-User authentication' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"auth_time" claim timestamp check failed (too much time has elapsed since the last End-User authentication)' }) }) test('option.maxAuthAge checks auth_time (with tolerance)', t => { @@ -215,14 +219,14 @@ test('option.maxTokenAge requires iat to be in the payload', t => { t.throws(() => { const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({})}.` JWT.verify(invalid, key, { maxTokenAge: '30s' }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'missing iat claim' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"iat" claim is missing' }) }) test('option.maxTokenAge checks iat elapsed time', t => { t.throws(() => { const invalid = `eyJhbGciOiJub25lIn0.${base64url.JSON.encode({ iat: epoch - 31 })}.` JWT.verify(invalid, key, { maxTokenAge: '30s', now }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'maxTokenAge exceeded' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"iat" claim timestamp check failed (too far in the past)' }) }) test('option.maxTokenAge checks iat (with tolerance)', t => { @@ -253,15 +257,28 @@ test('iat check (failed)', t => { const token = JWT.sign({}, key, { now: new Date((epoch + 1) * 1000) }) t.throws(() => { JWT.verify(token, key, { now }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'token issued in the future' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"iat" claim timestamp check failed (it should be in the past)' }) }) -test('iat check (ignored since exp is also present)', t => { +test('iat future check (ignored since exp is also present)', t => { const token = JWT.sign({}, key, { now: new Date((epoch + 1) * 1000), expiresIn: '2h' }) JWT.verify(token, key, { now }) t.pass() }) +test('iat future check (part of maxTokenAge)', t => { + const token = JWT.sign({}, key, { now: new Date((epoch + 1) * 1000), expiresIn: '2h' }) + t.throws(() => { + JWT.verify(token, key, { now, maxTokenAge: '30s' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"iat" claim timestamp check failed (it should be in the past)' }) +}) + +test('iat future check with tolerance (part of maxTokenAge)', t => { + const token = JWT.sign({}, key, { now: new Date((epoch + 1) * 1000), expiresIn: '2h' }) + JWT.verify(token, key, { now, maxTokenAge: '30s', clockTolerance: '1s' }) + t.pass() +}) + test('iat check (passed because of ignoreIat)', t => { const token = JWT.sign({}, key, { now: new Date((epoch + 1) * 1000) }) JWT.verify(token, key, { now, ignoreIat: true }) @@ -290,14 +307,14 @@ test('exp check (failed equal)', t => { const token = JWT.sign({ exp: epoch }, key, { iat: false }) t.throws(() => { JWT.verify(token, key, { now }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'token is expired' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"exp" claim timestamp check failed' }) }) test('exp check (failed normal)', t => { const token = JWT.sign({ exp: epoch - 1 }, key, { iat: false }) t.throws(() => { JWT.verify(token, key, { now }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'token is expired' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"exp" claim timestamp check failed' }) }) test('exp check (passed because of ignoreExp)', t => { @@ -328,7 +345,7 @@ test('nbf check (failed)', t => { const token = JWT.sign({ nbf: epoch + 10 }, key, { iat: false }) t.throws(() => { JWT.verify(token, key, { now }) - }, { instanceOf: errors.JWTClaimInvalid, message: 'token is not active yet' }) + }, { instanceOf: errors.JWTClaimInvalid, message: '"nbf" claim timestamp check failed' }) }) test('nbf check (passed because of ignoreIat)', t => { @@ -428,7 +445,7 @@ test('must be a supported value', t => { key, { profile: 'id_token', issuer: 'issuer', audience: 'client_id' } ) - }, { instanceOf: errors.JWTClaimInvalid, message: 'azp mismatch' }) + }, { instanceOf: errors.JWTClaimInvalid, message: 'unexpected "azp" claim value' }) }) test('profile=id_token validates full id tokens', t => { @@ -616,7 +633,7 @@ test('must be a supported value', t => { key, { profile: 'at+JWT', issuer: 'issuer', audience: ['RS-alias1'] } ) - }, { instanceOf: errors.JWTClaimInvalid, message: 'audience mismatch' }) + }, { instanceOf: errors.JWTClaimInvalid, message: 'unexpected "aud" claim value' }) JWT.verify( JWT.sign({ client_id: 'client_id' }, key, { expiresIn: '10m', subject: 'subject', issuer: 'issuer', audience: ['RS-alias1', 'RS-alias2'], header: { typ: 'at+JWT' } }), key,