Skip to content

Commit

Permalink
fix!: Prevent accidental use of insecure key sizes & misconfiguration…
Browse files Browse the repository at this point in the history 
… of secrets (#852)

* fix!: Disable use of weak RSA key sizes for asymmetric algorithms

Added checks to prevent invalid secrets from being used with the HS*** algorithms when signing and verifying
Added checks to prevent the use of insecure asymmetric key sizes except when explicitly overriden via options
Prevented Buffers containing malicious objects from being used as key material.

BREAKING CHANGE: Requires node 12.x or later to allow use of `KeyObject`
  • Loading branch information
@david-renaud-okta
david-renaud-okta committed Nov 29, 2022
1 parent 8345030 commit ecdf6cc
Show file tree
Hide file tree
Showing 10 changed files with 267 additions and 138 deletions.
48 changes: 12 additions & 36 deletions .circleci/config.yml
View file
Expand Up @@ -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
- node-v12
- node-v14
- node-v16
- node-v18
47 changes: 27 additions & 20 deletions README.md
View file
@@ -1,7 +1,7 @@
# jsonwebtoken

| **Build** | **Dependency** |
|-----------|---------------|
| **Build** | **Dependency** |
|-----------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------|
| [![Build Status](https://secure.travis-ci.org/auth0/node-jsonwebtoken.svg?branch=master)](http://travis-ci.org/auth0/node-jsonwebtoken) | [![Dependency Status](https://david-dm.org/auth0/node-jsonwebtoken.svg)](https://david-dm.org/auth0/node-jsonwebtoken) |


Expand Down Expand Up @@ -32,8 +32,9 @@ $ npm install jsonwebtoken
> If `payload` is not a buffer or a string, it will be coerced into a string using `JSON.stringify`.
`secretOrPrivateKey` is a string, buffer, or object containing either the secret for HMAC algorithms or the PEM
`secretOrPrivateKey` is a string (utf-8 encoded), buffer, object, or KeyObject containing either the secret for HMAC algorithms or the PEM
encoded private key for RSA and ECDSA. In case of a private key with passphrase an object `{ key, passphrase }` can be used (based on [crypto documentation](https://nodejs.org/api/crypto.html#crypto_sign_sign_private_key_output_format)), in this case be sure you pass the `algorithm` option.
When signing with RSA algorithms the minimum modulus length is 2048 except when the allowInsecureKeySizes option is set to true. Private keys below this size will be rejected with an error.

`options`:

Expand All @@ -50,6 +51,7 @@ encoded private key for RSA and ECDSA. In case of a private key with passphrase
* `header`
* `keyid`
* `mutatePayload`: if true, the sign function will modify the payload object directly. This is useful if you need a raw reference to the payload after claims have been applied to it but before it has been encoded into a token.
* `allowInsecureKeySizes`: if true allows private keys with a modulus below 2048 to be used for RSA



Expand Down Expand Up @@ -129,15 +131,20 @@ jwt.sign({
`token` is the JsonWebToken string

`secretOrPublicKey` is a string or buffer containing either the secret for HMAC algorithms, or the PEM
`secretOrPublicKey` is a string (utf-8 encoded), buffer, or KeyObject containing either the secret for HMAC algorithms, or the PEM
encoded public key for RSA and ECDSA.
If `jwt.verify` is called asynchronous, `secretOrPublicKey` can be a function that should fetch the secret or public key. See below for a detailed example

As mentioned in [this comment](https://github.com/auth0/node-jsonwebtoken/issues/208#issuecomment-231861138), there are other libraries that expect base64 encoded secrets (random bytes encoded using base64), if that is your case you can pass `Buffer.from(secret, 'base64')`, by doing this the secret will be decoded using base64 and the token verification will use the original random bytes.

`options`

* `algorithms`: List of strings with the names of the allowed algorithms. For instance, `["HS256", "HS384"]`.
* `algorithms`: List of strings with the names of the allowed algorithms. For instance, `["HS256", "HS384"]`.
> If not specified a defaults will be used based on the type of key provided
> * secret - ['HS256', 'HS384', 'HS512']
> * rsa - ['RS256', 'RS384', 'RS512']
> * ec - ['ES256', 'ES384', 'ES512']
> * default - ['RS256', 'RS384', 'RS512']
* `audience`: if you want to check audience (`aud`), provide a value here. The audience can be checked against a string, a regular expression or a list of strings and/or regular expressions.
> Eg: `"urn:foo"`, `/urn:f[o]{2}/`, `[/urn:f[o]{2}/, "urn:bar"]`
* `complete`: return an object with the decoded `{ payload, header, signature }` instead of only the usual content of the payload.
Expand Down Expand Up @@ -347,21 +354,21 @@ jwt.verify(token, 'shhhhh', function(err, decoded) {

Array of supported algorithms. The following algorithms are currently supported.

alg Parameter Value | Digital Signature or MAC Algorithm
----------------|----------------------------
HS256 | HMAC using SHA-256 hash algorithm
HS384 | HMAC using SHA-384 hash algorithm
HS512 | HMAC using SHA-512 hash algorithm
RS256 | RSASSA-PKCS1-v1_5 using SHA-256 hash algorithm
RS384 | RSASSA-PKCS1-v1_5 using SHA-384 hash algorithm
RS512 | RSASSA-PKCS1-v1_5 using SHA-512 hash algorithm
PS256 | RSASSA-PSS using SHA-256 hash algorithm (only node ^6.12.0 OR >=8.0.0)
PS384 | RSASSA-PSS using SHA-384 hash algorithm (only node ^6.12.0 OR >=8.0.0)
PS512 | RSASSA-PSS using SHA-512 hash algorithm (only node ^6.12.0 OR >=8.0.0)
ES256 | ECDSA using P-256 curve and SHA-256 hash algorithm
ES384 | ECDSA using P-384 curve and SHA-384 hash algorithm
ES512 | ECDSA using P-521 curve and SHA-512 hash algorithm
none | No digital signature or MAC value included
| alg Parameter Value | Digital Signature or MAC Algorithm |
|---------------------|------------------------------------------------------------------------|
| HS256 | HMAC using SHA-256 hash algorithm |
| HS384 | HMAC using SHA-384 hash algorithm |
| HS512 | HMAC using SHA-512 hash algorithm |
| RS256 | RSASSA-PKCS1-v1_5 using SHA-256 hash algorithm |
| RS384 | RSASSA-PKCS1-v1_5 using SHA-384 hash algorithm |
| RS512 | RSASSA-PKCS1-v1_5 using SHA-512 hash algorithm |
| PS256 | RSASSA-PSS using SHA-256 hash algorithm (only node ^6.12.0 OR >=8.0.0) |
| PS384 | RSASSA-PSS using SHA-384 hash algorithm (only node ^6.12.0 OR >=8.0.0) |
| PS512 | RSASSA-PSS using SHA-512 hash algorithm (only node ^6.12.0 OR >=8.0.0) |
| ES256 | ECDSA using P-256 curve and SHA-256 hash algorithm |
| ES384 | ECDSA using P-384 curve and SHA-384 hash algorithm |
| ES512 | ECDSA using P-521 curve and SHA-512 hash algorithm |
| none | No digital signature or MAC value included |

## Refreshing JWTs

Expand Down
14 changes: 4 additions & 10 deletions package.json
View file
Expand Up @@ -37,15 +37,9 @@
},
"dependencies": {
"jws": "^3.2.2",
"lodash.includes": "^4.3.0",
"lodash.isboolean": "^3.0.3",
"lodash.isinteger": "^4.0.4",
"lodash.isnumber": "^3.0.3",
"lodash.isplainobject": "^4.0.6",
"lodash.isstring": "^4.0.1",
"lodash.once": "^4.0.0",
"lodash": "^4.17.21",
"ms": "^2.1.1",
"semver": "^5.6.0"
"semver": "^7.3.8"
},
"devDependencies": {
"atob": "^2.1.2",
Expand All @@ -59,8 +53,8 @@
"sinon": "^6.0.0"
},
"engines": {
"npm": ">=1.4.28",
"node": ">=4"
"npm": ">=6",
"node": ">=12"
},
"files": [
"lib",
Expand Down
81 changes: 56 additions & 25 deletions sign.js
View file
@@ -1,20 +1,15 @@
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, isBoolean, isInteger, isNumber, isPlainObject, isString, once} = require('lodash')
const { KeyObject, createSecretKey, createPrivateKey } = 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' },
Expand All @@ -26,10 +21,11 @@ var sign_options_schema = {
jwtid: { isValid: isString, message: '"jwtid" must be a string' },
noTimestamp: { isValid: isBoolean, message: '"noTimestamp" must be a boolean' },
keyid: { isValid: isString, message: '"keyid" must be a string' },
mutatePayload: { isValid: isBoolean, message: '"mutatePayload" must be a boolean' }
mutatePayload: { isValid: isBoolean, message: '"mutatePayload" must be a boolean' },
allowInsecureKeySizes: { isValid: isBoolean, message: '"allowInsecureKeySizes" 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' }
Expand All @@ -41,7 +37,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 + '"');
Expand All @@ -62,14 +58,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',
Expand All @@ -87,10 +83,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
Expand All @@ -107,6 +103,32 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) {
return failure(new Error('secretOrPrivateKey must have a value'));
}

if (secretOrPrivateKey != null && !(secretOrPrivateKey instanceof KeyObject)) {
try {
secretOrPrivateKey = createPrivateKey(secretOrPrivateKey)
} catch (_) {
try {
secretOrPrivateKey = createSecretKey(typeof secretOrPrivateKey === 'string' ? Buffer.from(secretOrPrivateKey) : secretOrPrivateKey)
} catch (_) {
return failure(new Error('secretOrPrivateKey is not valid key material'));
}
}
}

if (header.alg.startsWith('HS') && secretOrPrivateKey.type !== 'secret') {
return failure(new Error((`secretOrPrivateKey must be a symmetric key when using ${header.alg}`)))
} else if (/^(?:RS|PS|ES)/.test(header.alg)) {
if (secretOrPrivateKey.type !== 'private') {
return failure(new Error((`secretOrPrivateKey must be an asymmetric key when using ${header.alg}`)))
}
if (!options.allowInsecureKeySizes &&
!header.alg.startsWith('ES') &&
secretOrPrivateKey.asymmetricKeyDetails !== undefined && //KeyObject.asymmetricKeyDetails is supported in Node 15+
secretOrPrivateKey.asymmetricKeyDetails.modulusLength < 2048) {
return failure(new Error(`secretOrPrivateKey has a minimum key size of 2048 bits for ${header.alg}`));
}
}

if (typeof payload === 'undefined') {
return failure(new Error('payload is required'));
} else if (isObjectPayload) {
Expand All @@ -120,7 +142,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';
});

Expand All @@ -144,7 +166,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;
Expand Down Expand Up @@ -177,7 +199,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.'));
Expand All @@ -186,7 +208,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);
Expand All @@ -198,9 +220,18 @@ module.exports = function (payload, secretOrPrivateKey, options, callback) {
encoding: encoding
}).once('error', callback)
.once('done', function (signature) {
// TODO: Remove in favor of the modulus length check before signing once node 15+ is the minimum supported version
if(!options.allowInsecureKeySizes && /^(?:RS|PS)/.test(header.alg) && signature.length < 256) {
return callback(new Error(`secretOrPrivateKey has a minimum key size of 2048 bits for ${header.alg}`))
}
callback(null, signature);
});
} else {
return jws.sign({header: header, payload: payload, secret: secretOrPrivateKey, encoding: encoding});
let signature = jws.sign({header: header, payload: payload, secret: secretOrPrivateKey, encoding: encoding});
// TODO: Remove in favor of the modulus length check before signing once node 15+ is the minimum supported version
if(!options.allowInsecureKeySizes && /^(?:RS|PS)/.test(header.alg) && signature.length < 256) {
throw new Error(`secretOrPrivateKey has a minimum key size of 2048 bits for ${header.alg}`)
}
return signature
}
};
16 changes: 16 additions & 0 deletions test/async_sign.tests.js
View file
Expand Up @@ -2,6 +2,7 @@ var jwt = require('../index');
var expect = require('chai').expect;
var jws = require('jws');
var PS_SUPPORTED = require('../lib/psSupported');
const {generateKeyPairSync} = require("crypto");

describe('signing a token asynchronously', function() {

Expand Down Expand Up @@ -59,6 +60,21 @@ describe('signing a token asynchronously', function() {
});
});

it('should not work for RS algorithms when modulus length is less than 2048 when allowInsecureKeySizes is false or not set', function(done) {
const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 1024 });

jwt.sign({ foo: 'bar' }, privateKey, { algorithm: 'RS256' }, function (err) {
expect(err).to.be.ok;
done();
});
});

it('should work for RS algorithms when modulus length is less than 2048 when allowInsecureKeySizes is true', function(done) {
const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 1024 });

jwt.sign({ foo: 'bar' }, privateKey, { algorithm: 'RS256', allowInsecureKeySizes: true }, done);
});

if (PS_SUPPORTED) {
it('should return error when secret is not a cert for PS256', function(done) {
//this throw an error because the secret is not a cert and PS256 requires a cert.
Expand Down

0 comments on commit ecdf6cc

Please sign in to comment.