diff --git a/.circleci/config.yml b/.circleci/config.yml index 04670d7..51f2d61 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -17,55 +17,31 @@ commands: command: npm test jobs: - node-v4: + node-v12: docker: - - image: node:4 + - image: node:12 steps: - test-nodejs - node-v5: + node-v14: docker: - - image: node:5 + - image: node:14 steps: - test-nodejs - node-v6: + node-v16: docker: - - image: node:6 + - image: node:16 steps: - test-nodejs - node-v7: + node-v18: docker: - - image: node:7 - steps: - - test-nodejs - node-v8: - docker: - - image: node:8 - steps: - - test-nodejs - node-v9: - docker: - - image: node:9 - steps: - - test-nodejs - node-v10: - docker: - - image: node:10 - steps: - - test-nodejs - node-v11: - docker: - - image: node:11 + - image: node:18 steps: - test-nodejs workflows: node-multi-build: jobs: - - node-v4 - - node-v5 - - node-v6 - - node-v7 - - node-v8 - - node-v9 - - node-v10 - - node-v11 \ No newline at end of file + - node-v12 + - node-v14 + - node-v16 + - node-v18 diff --git a/package.json b/package.json index 81d63da..f3098bc 100644 --- a/package.json +++ b/package.json @@ -59,8 +59,8 @@ "sinon": "^6.0.0" }, "engines": { - "npm": ">=1.4.28", - "node": ">=4" + "npm": ">=6", + "node": ">=12" }, "files": [ "lib", diff --git a/sign.js b/sign.js index f8a2877..114ad8c 100644 --- a/sign.js +++ b/sign.js @@ -1,20 +1,21 @@ -var timespan = require('./lib/timespan'); -var PS_SUPPORTED = require('./lib/psSupported'); -var jws = require('jws'); -var includes = require('lodash.includes'); -var isBoolean = require('lodash.isboolean'); -var isInteger = require('lodash.isinteger'); -var isNumber = require('lodash.isnumber'); -var isPlainObject = require('lodash.isplainobject'); -var isString = require('lodash.isstring'); -var once = require('lodash.once'); - -var SUPPORTED_ALGS = ['RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512', 'HS256', 'HS384', 'HS512', 'none']; +const timespan = require('./lib/timespan'); +const PS_SUPPORTED = require('./lib/psSupported'); +const jws = require('jws'); +const includes = require('lodash.includes'); +const isBoolean = require('lodash.isboolean'); +const isInteger = require('lodash.isinteger'); +const isNumber = require('lodash.isnumber'); +const isPlainObject = require('lodash.isplainobject'); +const isString = require('lodash.isstring'); +const once = require('lodash.once'); +const { KeyObject } = require('crypto') + +const SUPPORTED_ALGS = ['RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512', 'HS256', 'HS384', 'HS512', 'none']; if (PS_SUPPORTED) { SUPPORTED_ALGS.splice(3, 0, 'PS256', 'PS384', 'PS512'); } -var sign_options_schema = { +const sign_options_schema = { expiresIn: { isValid: function(value) { return isInteger(value) || (isString(value) && value); }, message: '"expiresIn" should be a number of seconds or string representing a timespan' }, notBefore: { isValid: function(value) { return isInteger(value) || (isString(value) && value); }, message: '"notBefore" should be a number of seconds or string representing a timespan' }, audience: { isValid: function(value) { return isString(value) || Array.isArray(value); }, message: '"audience" must be a string or array' }, @@ -29,7 +30,7 @@ var sign_options_schema = { mutatePayload: { isValid: isBoolean, message: '"mutatePayload" must be a boolean' } }; -var registered_claims_schema = { +const registered_claims_schema = { iat: { isValid: isNumber, message: '"iat" should be a number of seconds' }, exp: { isValid: isNumber, message: '"exp" should be a number of seconds' }, nbf: { isValid: isNumber, message: '"nbf" should be a number of seconds' } @@ -41,7 +42,7 @@ function validate(schema, allowUnknown, object, parameterName) { } Object.keys(object) .forEach(function(key) { - var validator = schema[key]; + const validator = schema[key]; if (!validator) { if (!allowUnknown) { throw new Error('"' + key + '" is not allowed in "' + parameterName + '"'); @@ -62,14 +63,14 @@ function validatePayload(payload) { return validate(registered_claims_schema, true, payload, 'payload'); } -var options_to_payload = { +const options_to_payload = { 'audience': 'aud', 'issuer': 'iss', 'subject': 'sub', 'jwtid': 'jti' }; -var options_for_objects = [ +const options_for_objects = [ 'expiresIn', 'notBefore', 'noTimestamp', @@ -87,10 +88,10 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) { options = options || {}; } - var isObjectPayload = typeof payload === 'object' && + const isObjectPayload = typeof payload === 'object' && !Buffer.isBuffer(payload); - var header = Object.assign({ + const header = Object.assign({ alg: options.algorithm || 'HS256', typ: isObjectPayload ? 'JWT' : undefined, kid: options.keyid @@ -107,6 +108,18 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) { return failure(new Error('secretOrPrivateKey must have a value')); } + // Public keys used with HS algorithms is a misconfiguration + if (header.alg.startsWith('HS')) { + if (secretOrPrivateKey instanceof KeyObject) { + if (secretOrPrivateKey.type !== 'secret') { + return failure(new Error((`secretOrPrivateKey KeyObject must be of type secret when using ${header.alg}. Got ${secretOrPrivateKey.type}`))) + } + } else if (typeof secretOrPrivateKey !== 'string' + && ![ArrayBuffer, Buffer, Object.getPrototypeOf(Int8Array), DataView].some((x) => secretOrPrivateKey instanceof x)){ + return failure(new Error((`secretOrPrivateKey must be a KeyObject or valid input for secret for crypto.createSecretKey when using ${header.alg}`))) + } + } + if (typeof payload === 'undefined') { return failure(new Error('payload is required')); } else if (isObjectPayload) { @@ -120,7 +133,7 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) { payload = Object.assign({},payload); } } else { - var invalid_options = options_for_objects.filter(function (opt) { + const invalid_options = options_for_objects.filter(function (opt) { return typeof options[opt] !== 'undefined'; }); @@ -144,7 +157,7 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) { return failure(error); } - var timestamp = payload.iat || Math.floor(Date.now() / 1000); + const timestamp = payload.iat || Math.floor(Date.now() / 1000); if (options.noTimestamp) { delete payload.iat; @@ -177,7 +190,7 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) { } Object.keys(options_to_payload).forEach(function (key) { - var claim = options_to_payload[key]; + const claim = options_to_payload[key]; if (typeof options[key] !== 'undefined') { if (typeof payload[claim] !== 'undefined') { return failure(new Error('Bad "options.' + key + '" option. The payload already has an "' + claim + '" property.')); @@ -186,7 +199,7 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) { } }); - var encoding = options.encoding || 'utf8'; + const encoding = options.encoding || 'utf8'; if (typeof callback === 'function') { callback = callback && once(callback); diff --git a/test/jwt.hs.tests.js b/test/jwt.hs.tests.js index 5c12a73..7982d12 100644 --- a/test/jwt.hs.tests.js +++ b/test/jwt.hs.tests.js @@ -1,14 +1,18 @@ -var jwt = require('../index'); +const jwt = require('../index'); +const crypto = require("crypto"); -var expect = require('chai').expect; -var assert = require('chai').assert; +const {assert, expect} = require('chai'); describe('HS256', function() { describe('when signing a token', function() { - var secret = 'shhhhhh'; + const secret = 'shhhhhh'; + const { + publicKey: pubRsaKey, + privateKey: privRsaKey + } = crypto.generateKeyPairSync('rsa', { modulusLength: 2048 }); - var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: 'HS256' }); + const token = jwt.sign({foo: 'bar'}, secret, {algorithm: 'HS256'}); it('should be syntactically valid', function() { expect(token).to.be.a('string'); @@ -16,9 +20,9 @@ describe('HS256', function() { }); it('should be able to validate without options', function(done) { - var callback = function(err, decoded) { + const callback = function (err, decoded) { assert.ok(decoded.foo); - assert.equal('bar', decoded.foo); + assert.equal(decoded.foo, 'bar'); done(); }; callback.issuer = "shouldn't affect"; @@ -28,7 +32,7 @@ describe('HS256', function() { it('should validate with secret', function(done) { jwt.verify(token, secret, function(err, decoded) { assert.ok(decoded.foo); - assert.equal('bar', decoded.foo); + assert.equal(decoded.foo, 'bar'); done(); }); }); @@ -42,8 +46,8 @@ describe('HS256', function() { }); it('should throw with secret and token not signed', function(done) { - var signed = jwt.sign({ foo: 'bar' }, secret, { algorithm: 'none' }); - var unsigned = signed.split('.')[0] + '.' + signed.split('.')[1] + '.'; + const signed = jwt.sign({foo: 'bar'}, secret, {algorithm: 'none'}); + const unsigned = signed.split('.')[0] + '.' + signed.split('.')[1] + '.'; jwt.verify(unsigned, 'secret', function(err, decoded) { assert.isUndefined(decoded); assert.isNotNull(err); @@ -52,8 +56,8 @@ describe('HS256', function() { }); it('should work with falsy secret and token not signed', function(done) { - var signed = jwt.sign({ foo: 'bar' }, null, { algorithm: 'none' }); - var unsigned = signed.split('.')[0] + '.' + signed.split('.')[1] + '.'; + const signed = jwt.sign({foo: 'bar'}, null, {algorithm: 'none'}); + const unsigned = signed.split('.')[0] + '.' + signed.split('.')[1] + '.'; jwt.verify(unsigned, 'secret', function(err, decoded) { assert.isUndefined(decoded); assert.isNotNull(err); @@ -70,7 +74,7 @@ describe('HS256', function() { }); it('should return an error when the token is expired', function(done) { - var token = jwt.sign({ exp: 1 }, secret, { algorithm: 'HS256' }); + const token = jwt.sign({exp: 1}, secret, {algorithm: 'HS256'}); jwt.verify(token, secret, { algorithm: 'HS256' }, function(err, decoded) { assert.isUndefined(decoded); assert.isNotNull(err); @@ -79,36 +83,86 @@ describe('HS256', function() { }); it('should NOT return an error when the token is expired with "ignoreExpiration"', function(done) { - var token = jwt.sign({ exp: 1, foo: 'bar' }, secret, { algorithm: 'HS256' }); + const token = jwt.sign({exp: 1, foo: 'bar'}, secret, {algorithm: 'HS256'}); jwt.verify(token, secret, { algorithm: 'HS256', ignoreExpiration: true }, function(err, decoded) { assert.ok(decoded.foo); - assert.equal('bar', decoded.foo); + assert.equal(decoded.foo, 'bar'); assert.isNull(err); done(); }); }); it('should default to HS256 algorithm when no options are passed', function() { - var token = jwt.sign({ foo: 'bar' }, secret); - var verifiedToken = jwt.verify(token, secret); + const token = jwt.sign({foo: 'bar'}, secret); + const verifiedToken = jwt.verify(token, secret); assert.ok(verifiedToken.foo); - assert.equal('bar', verifiedToken.foo); + assert.equal(verifiedToken.foo, 'bar'); }); + + describe('when the algorithm is HS256', function () { + it('should throw when a PublicKeyObject is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, pubRsaKey, {algorithm: 'HS256'})).to.throw(Error, 'must be of type secret'); + }) + + it('should throw when a PrivateKeyObject is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, privRsaKey, {algorithm: 'HS256'})).to.throw(Error, 'must be of type secret'); + }) + + it('should throw when a secret is not valid input for crypto.createSecretKey is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, 5, {algorithm: 'HS256'})).to.throw(Error, 'valid input for secret for crypto.createSecretKey'); + }) + }) + + describe('when the algorithm is HS384', function () { + it('should throw when a PublicKeyObject is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, pubRsaKey, {algorithm: 'HS384'})).to.throw(Error, 'must be of type secret'); + }) + + it('should throw when a PrivateKeyObject is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, privRsaKey, {algorithm: 'HS384'})).to.throw(Error, 'must be of type secret'); + }) + + it('should throw when a secret is not valid input for crypto.createSecretKey is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, 5, {algorithm: 'HS384'})).to.throw(Error, 'valid input for secret for crypto.createSecretKey'); + }) + }) + + describe('when the algorithm is HS512', function () { + it('should throw when a PublicKeyObject is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, pubRsaKey, {algorithm: 'HS512'})).to.throw(Error, 'must be of type secret'); + }) + + it('should throw when a PrivateKeyObject is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, privRsaKey, {algorithm: 'HS512'})).to.throw(Error, 'must be of type secret'); + }) + + it('should throw when a secret is not valid input for crypto.createSecretKey is used as secret', function () { + expect(() => jwt.sign({ foo: 'bar' }, 5, {algorithm: 'HS512'})).to.throw(Error, 'valid input for secret for crypto.createSecretKey'); + }) + }) }); describe('should fail verification gracefully with trailing space in the jwt', function() { - var secret = 'shhhhhh'; - var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: 'HS256' }); + const secret = 'shhhhhh'; + const token = jwt.sign({foo: 'bar'}, secret, {algorithm: 'HS256'}); it('should return the "invalid token" error', function(done) { - var malformedToken = token + ' '; // corrupt the token by adding a space + const malformedToken = token + ' '; // corrupt the token by adding a space jwt.verify(malformedToken, secret, { algorithm: 'HS256', ignoreExpiration: true }, function(err) { assert.isNotNull(err); - assert.equal('JsonWebTokenError', err.name); - assert.equal('invalid token', err.message); + assert.equal(err.name, 'JsonWebTokenError'); + assert.equal(err.message, 'invalid token'); done(); }); }); }); + describe('when verifying a token', function () { + it('should throw when a secret is not valid input for crypto.createSecretKey is used as secret', function () { + const secret = 'shhhhhh'; + const token = jwt.sign({foo: 'bar'}, secret, {algorithm: 'HS256'}); + expect(() => jwt.verify(token, 5, {algorithms: 'HS256'})).to.throw(Error, 'valid input for secret for crypto.createSecretKey'); + }) + }) + }); diff --git a/test/jwt.malicious.tests.js b/test/jwt.malicious.tests.js new file mode 100644 index 0000000..bd90ecc --- /dev/null +++ b/test/jwt.malicious.tests.js @@ -0,0 +1,32 @@ +const jwt = require('../index'); +const crypto = require("crypto"); +const {expect} = require('chai'); +const JsonWebTokenError = require("../lib/JsonWebTokenError"); + +describe('when verifying a malicious token', function () { + // attacker has access to the public rsa key, but crafts the token as HS256 + // with kid set to the id of the rsa key, instead of the id of the hmac secret. + // const maliciousToken = jwt.sign( + // {foo: 'bar'}, + // pubRsaKey, + // {algorithm: 'HS256', keyid: 'rsaKeyId'} + // ); + // consumer accepts self signed tokens (HS256) and third party tokens (RS256) + const options = {algorithms: ['RS256', 'HS256']}; + + const { + publicKey: pubRsaKey + } = crypto.generateKeyPairSync('rsa', {modulusLength: 2048}); + + it('should not allow HMAC verification with an RSA key in KeyObject format', function () { + const maliciousToken = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6InJzYUtleUlkIn0.eyJmb28iOiJiYXIiLCJpYXQiOjE2NTk1MTA2MDh9.cOcHI1TXPbxTMlyVTfjArSWskrmezbrG8iR7uJHwtrQ'; + + expect(() => jwt.verify(maliciousToken, pubRsaKey, options)).to.throw(JsonWebTokenError, 'must be of type secret when using HS'); + }) + + it('should not allow HMAC verification with an RSA key in PEM format', function () { + const maliciousToken = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6InJzYUtleUlkIn0.eyJmb28iOiJiYXIiLCJpYXQiOjE2NTk1MTA2MDh9.cOcHI1TXPbxTMlyVTfjArSWskrmezbrG8iR7uJHwtrQ'; + + expect(() => jwt.verify(maliciousToken, pubRsaKey.export({type: 'spki', format: 'pem'}), options)).to.throw(JsonWebTokenError, 'must be a symmetric key'); + }) +}) diff --git a/verify.js b/verify.js index 8687eb5..b3acf8a 100644 --- a/verify.js +++ b/verify.js @@ -1,14 +1,15 @@ -var JsonWebTokenError = require('./lib/JsonWebTokenError'); -var NotBeforeError = require('./lib/NotBeforeError'); -var TokenExpiredError = require('./lib/TokenExpiredError'); -var decode = require('./decode'); -var timespan = require('./lib/timespan'); -var PS_SUPPORTED = require('./lib/psSupported'); -var jws = require('jws'); - -var PUB_KEY_ALGS = ['RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512']; -var RSA_KEY_ALGS = ['RS256', 'RS384', 'RS512']; -var HS_ALGS = ['HS256', 'HS384', 'HS512']; +const JsonWebTokenError = require('./lib/JsonWebTokenError'); +const NotBeforeError = require('./lib/NotBeforeError'); +const TokenExpiredError = require('./lib/TokenExpiredError'); +const decode = require('./decode'); +const timespan = require('./lib/timespan'); +const PS_SUPPORTED = require('./lib/psSupported'); +const jws = require('jws'); +const {KeyObject} = require("crypto"); + +const PUB_KEY_ALGS = ['RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512']; +const RSA_KEY_ALGS = ['RS256', 'RS384', 'RS512']; +const HS_ALGS = ['HS256', 'HS384', 'HS512']; if (PS_SUPPORTED) { PUB_KEY_ALGS.splice(3, 0, 'PS256', 'PS384', 'PS512'); @@ -28,7 +29,7 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { //clone this object since we are going to mutate it. options = Object.assign({}, options); - var done; + let done; if (callback) { done = callback; @@ -47,7 +48,7 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { return done(new JsonWebTokenError('nonce must be a non-empty string')); } - var clockTimestamp = options.clockTimestamp || Math.floor(Date.now() / 1000); + const clockTimestamp = options.clockTimestamp || Math.floor(Date.now() / 1000); if (!jwtString){ return done(new JsonWebTokenError('jwt must be provided')); @@ -57,13 +58,13 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { return done(new JsonWebTokenError('jwt must be a string')); } - var parts = jwtString.split('.'); + const parts = jwtString.split('.'); if (parts.length !== 3){ return done(new JsonWebTokenError('jwt malformed')); } - var decodedToken; + let decodedToken; try { decodedToken = decode(jwtString, { complete: true }); @@ -75,8 +76,8 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { return done(new JsonWebTokenError('invalid token')); } - var header = decodedToken.header; - var getSecret; + const header = decodedToken.header; + let getSecret; if(typeof secretOrPublicKey === 'function') { if(!callback) { @@ -96,7 +97,7 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { return done(new JsonWebTokenError('error in secret or public key callback: ' + err.message)); } - var hasSignature = parts[2].trim() !== ''; + const hasSignature = parts[2].trim() !== ''; if (!hasSignature && secretOrPublicKey){ return done(new JsonWebTokenError('jwt signature is required')); @@ -117,11 +118,26 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { } - if (!~options.algorithms.indexOf(decodedToken.header.alg)) { + if (options.algorithms.indexOf(decodedToken.header.alg) === -1) { return done(new JsonWebTokenError('invalid algorithm')); } - var valid; + if (decodedToken.header.alg.startsWith('HS')) { + if (secretOrPublicKey instanceof KeyObject) { + if (secretOrPublicKey.type !== 'secret') { + return done(new JsonWebTokenError((`secretOrPrivateKey KeyObject must be of type secret when using ${header.alg}. Got ${secretOrPublicKey.type}`))) + } + } else if (typeof secretOrPublicKey !== 'string' + && ![ArrayBuffer, Buffer, Object.getPrototypeOf(Int8Array), DataView].some((x) => secretOrPublicKey instanceof x)){ + return done(new JsonWebTokenError((`secretOrPrivateKey must be a KeyObject or valid input for secret for crypto.createSecretKey when using ${header.alg}`))) + } + + if (/BEGIN (CERTIFICATE|PUBLIC|RSA)/.test(secretOrPublicKey.toString())) { + return done(new JsonWebTokenError('secretOrPublicKey must be a symmetric key. Got an asymmetric key')) + } + } + + let valid; try { valid = jws.verify(jwtString, decodedToken.header.alg, secretOrPublicKey); @@ -133,7 +149,7 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { return done(new JsonWebTokenError('invalid signature')); } - var payload = decodedToken.payload; + const payload = decodedToken.payload; if (typeof payload.nbf !== 'undefined' && !options.ignoreNotBefore) { if (typeof payload.nbf !== 'number') { @@ -154,10 +170,10 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { } if (options.audience) { - var audiences = Array.isArray(options.audience) ? options.audience : [options.audience]; - var target = Array.isArray(payload.aud) ? payload.aud : [payload.aud]; + const audiences = Array.isArray(options.audience) ? options.audience : [options.audience]; + const target = Array.isArray(payload.aud) ? payload.aud : [payload.aud]; - var match = target.some(function (targetAudience) { + const match = target.some(function (targetAudience) { return audiences.some(function (audience) { return audience instanceof RegExp ? audience.test(targetAudience) : audience === targetAudience; }); @@ -169,9 +185,9 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { } if (options.issuer) { - var invalid_issuer = - (typeof options.issuer === 'string' && payload.iss !== options.issuer) || - (Array.isArray(options.issuer) && options.issuer.indexOf(payload.iss) === -1); + const invalid_issuer = + (typeof options.issuer === 'string' && payload.iss !== options.issuer) || + (Array.isArray(options.issuer) && options.issuer.indexOf(payload.iss) === -1); if (invalid_issuer) { return done(new JsonWebTokenError('jwt issuer invalid. expected: ' + options.issuer)); @@ -201,7 +217,7 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { return done(new JsonWebTokenError('iat required when maxAge is specified')); } - var maxAgeTimestamp = timespan(options.maxAge, payload.iat); + const maxAgeTimestamp = timespan(options.maxAge, payload.iat); if (typeof maxAgeTimestamp === 'undefined') { return done(new JsonWebTokenError('"maxAge" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60')); } @@ -211,7 +227,7 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { } if (options.complete === true) { - var signature = decodedToken.signature; + const signature = decodedToken.signature; return done(null, { header: header,