fix!: Prevent accidental use of insecure key sizes & misconfiguration…

… 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`
auth0 · Nov 29, 2022 · ecdf6cc · ecdf6cc
1 parent 8345030
commit ecdf6cc
Show file tree

Hide file tree

Showing 10 changed files with 267 additions and 138 deletions.
diff --git 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
+      - node-v12
+      - node-v14
+      - node-v16
+      - node-v18
diff --git a/README.md b/README.md
@@ -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) |
 
 
@@ -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`:
 
@@ -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
 
 
 
@@ -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.
@@ -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
 

diff --git a/package.json b/package.json
@@ -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",
@@ -59,8 +53,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
@@ -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' },
@@ -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' }
@@ -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 + '"');
@@ -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',
@@ -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
@@ -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) {
@@ -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';
     });
 
@@ -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;
@@ -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.'));
@@ -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);
@@ -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
   }
 };
diff --git a/test/async_sign.tests.js b/test/async_sign.tests.js
@@ -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() {
 
@@ -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.