From 30e5c46ecf00a498e65a551ced88bc897531c2a4 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sat, 5 Sep 2020 14:23:03 +0200 Subject: [PATCH] feat: decrypt allowlists for both key management and content encryption BREAKING CHANGE: the `JWE.decrypt` option `algorithms` was removed and replaced with contentEncryptionAlgorithms (handles `enc` allowlist) and keyManagementAlgorithms (handles `alg` allowlist) --- docs/README.md | 9 +++-- lib/jwe/decrypt.js | 43 ++++++++++++++++----- test/jwe/sanity.test.js | 83 +++++++++++++++++++++++++---------------- types/index.d.ts | 3 +- 4 files changed, 91 insertions(+), 47 deletions(-) diff --git a/docs/README.md b/docs/README.md index f72153d3a9..f712d09d46 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1488,9 +1488,12 @@ operation. Syntax) matches. Any `JWK.asKey()` compatible input also works. `` instances are recommended for performance purposes when re-using the same key for every operation. - `options`: `` - - `algorithms`: `string[]` Array of Algorithms to accept, when the JWE does not use an - Key Management algorithm from this list the decryption will fail. **Default:** 'undefined' - - accepts all algorithms available on the keys + - `contentEncryptionAlgorithms`: `string[]` Array of algorithms to accept as the `enc` (content + encryption), when the JWE does not use an Key Management algorithm from this list the decryption + will fail. **Default:** 'undefined' - accepts all content encryption algorithms. + - `keyManagementAlgorithms`: `string[]` Array of algorithms to accept as the `alg` (key management), + when the JWE does not use an Key Management algorithm from this list the decryption will fail. + **Default:** 'undefined' - accepts all algorithms available on the key or key store. - `complete`: `` When true returns an object with the parsed headers, verified AAD, the content encryption key, the key that was used to unwrap or derive the content encryption key, and cleartext instead of only the cleartext. diff --git a/lib/jwe/decrypt.js b/lib/jwe/decrypt.js index 318bc0e577..fb5da68aaa 100644 --- a/lib/jwe/decrypt.js +++ b/lib/jwe/decrypt.js @@ -37,17 +37,26 @@ const combineHeader = (prot = {}, unprotected = {}, header = {}) => { } } +const validateAlgorithms = (algorithms, option) => { + if (algorithms !== undefined && (!Array.isArray(algorithms) || algorithms.some(s => typeof s !== 'string' || !s))) { + throw new TypeError(`"${option}" option must be an array of non-empty strings`) + } + + if (!algorithms) { + return undefined + } + + return new Set(algorithms) +} + /* * @public */ -const jweDecrypt = (skipValidateHeaders, serialization, jwe, key, { crit = [], complete = false, algorithms } = {}) => { +const jweDecrypt = (skipValidateHeaders, serialization, jwe, key, { crit = [], complete = false, keyManagementAlgorithms, contentEncryptionAlgorithms } = {}) => { key = getKey(key, true) - if (algorithms !== undefined && (!Array.isArray(algorithms) || algorithms.some(s => typeof s !== 'string' || !s))) { - throw new TypeError('"algorithms" option must be an array of non-empty strings') - } else if (algorithms) { - algorithms = new Set(algorithms) - } + keyManagementAlgorithms = validateAlgorithms(keyManagementAlgorithms, 'keyManagementAlgorithms') + contentEncryptionAlgorithms = validateAlgorithms(contentEncryptionAlgorithms, 'contentEncryptionAlgorithms') if (!Array.isArray(crit) || crit.some(s => typeof s !== 'string' || !s)) { throw new TypeError('"crit" option must be an array of non-empty strings') @@ -82,8 +91,12 @@ const jweDecrypt = (skipValidateHeaders, serialization, jwe, key, { crit = [], c ;({ alg, enc } = opts) - if (algorithms && !algorithms.has(alg === 'dir' ? enc : alg)) { - throw new errors.JOSEAlgNotWhitelisted('alg not whitelisted') + if (keyManagementAlgorithms && !keyManagementAlgorithms.has(alg)) { + throw new errors.JOSEAlgNotWhitelisted('key management algorithm not whitelisted') + } + + if (contentEncryptionAlgorithms && !contentEncryptionAlgorithms.has(enc)) { + throw new errors.JOSEAlgNotWhitelisted('content encryption algorithm not whitelisted') } if (key instanceof KeyStore) { @@ -106,7 +119,12 @@ const jweDecrypt = (skipValidateHeaders, serialization, jwe, key, { crit = [], c const errs = [] for (const key of keys) { try { - return jweDecrypt(true, serialization, jwe, key, { crit, complete, algorithms: algorithms ? [...algorithms] : undefined }) + return jweDecrypt(true, serialization, jwe, key, { + crit, + complete, + contentEncryptionAlgorithms: contentEncryptionAlgorithms ? [...contentEncryptionAlgorithms] : undefined, + keyManagementAlgorithms: keyManagementAlgorithms ? [...keyManagementAlgorithms] : undefined + }) } catch (err) { errs.push(err) continue @@ -187,7 +205,12 @@ const jweDecrypt = (skipValidateHeaders, serialization, jwe, key, { crit = [], c const errs = [] for (const recipient of recipients) { try { - return jweDecrypt(true, 'flattened', { ...root, ...recipient }, key, { crit, complete, algorithms: algorithms ? [...algorithms] : undefined }) + return jweDecrypt(true, 'flattened', { ...root, ...recipient }, key, { + crit, + complete, + contentEncryptionAlgorithms: contentEncryptionAlgorithms ? [...contentEncryptionAlgorithms] : undefined, + keyManagementAlgorithms: keyManagementAlgorithms ? [...keyManagementAlgorithms] : undefined + }) } catch (err) { errs.push(err) continue diff --git a/test/jwe/sanity.test.js b/test/jwe/sanity.test.js index 3409da2152..edfa4bb945 100644 --- a/test/jwe/sanity.test.js +++ b/test/jwe/sanity.test.js @@ -3,22 +3,41 @@ const test = require('ava') const base64url = require('../../lib/help/base64url') const { JWKS, JWK: { generateSync }, JWE, errors } = require('../..') -test('algorithms option be an array of strings', t => { +test('keyManagementAlgorithms option be an array of strings', t => { ;[{}, new Object(), false, null, Infinity, 0, '', Buffer.from('foo')].forEach((val) => { // eslint-disable-line no-new-object t.throws(() => { JWE.decrypt({ header: { alg: 'HS256' }, payload: 'foo', ciphertext: 'bar' - }, generateSync('oct'), { algorithms: val }) - }, { instanceOf: TypeError, message: '"algorithms" option must be an array of non-empty strings' }) + }, generateSync('oct'), { keyManagementAlgorithms: val }) + }, { instanceOf: TypeError, message: '"keyManagementAlgorithms" option must be an array of non-empty strings' }) t.throws(() => { JWE.decrypt({ header: { alg: 'HS256' }, payload: 'foo', ciphertext: 'bar' - }, generateSync('oct'), { algorithms: [val] }) - }, { instanceOf: TypeError, message: '"algorithms" option must be an array of non-empty strings' }) + }, generateSync('oct'), { keyManagementAlgorithms: [val] }) + }, { instanceOf: TypeError, message: '"keyManagementAlgorithms" option must be an array of non-empty strings' }) + }) +}) + +test('contentEncryptionAlgorithms option be an array of strings', t => { + ;[{}, new Object(), false, null, Infinity, 0, '', Buffer.from('foo')].forEach((val) => { // eslint-disable-line no-new-object + t.throws(() => { + JWE.decrypt({ + header: { alg: 'HS256' }, + payload: 'foo', + ciphertext: 'bar' + }, generateSync('oct'), { contentEncryptionAlgorithms: val }) + }, { instanceOf: TypeError, message: '"contentEncryptionAlgorithms" option must be an array of non-empty strings' }) + t.throws(() => { + JWE.decrypt({ + header: { alg: 'HS256' }, + payload: 'foo', + ciphertext: 'bar' + }, generateSync('oct'), { contentEncryptionAlgorithms: [val] }) + }, { instanceOf: TypeError, message: '"contentEncryptionAlgorithms" option must be an array of non-empty strings' }) }) }) @@ -433,42 +452,40 @@ test('JWE prot, unprot and per-recipient headers must be disjoint', t => { }, { instanceOf: errors.JWEInvalid, code: 'ERR_JWE_INVALID', message: 'JWE Shared Protected, JWE Shared Unprotected and JWE Per-Recipient Header Parameter names must be disjoint' }) }) -if (!('electron' in process.versions)) { - test('JWE decrypt algorithms whitelist', t => { - const k = generateSync('oct') - const jwe = JWE.encrypt('foo', k, { alg: 'PBES2-HS256+A128KW' }) - JWE.decrypt(jwe, k, { algorithms: ['PBES2-HS256+A128KW', 'PBES2-HS384+A192KW'] }) +test('JWE decrypt keyManagementAlgorithms whitelist', t => { + const k = generateSync('oct', 128) + const jwe = JWE.encrypt('foo', k, { alg: 'A128GCMKW' }) + JWE.decrypt(jwe, k, { keyManagementAlgorithms: ['A128GCMKW', 'A192GCMKW'] }) - t.throws(() => { - JWE.decrypt(jwe, k, { algorithms: ['PBES2-HS384+A192KW'] }) - }, { instanceOf: errors.JOSEAlgNotWhitelisted, code: 'ERR_JOSE_ALG_NOT_WHITELISTED', message: 'alg not whitelisted' }) - }) + t.throws(() => { + JWE.decrypt(jwe, k, { keyManagementAlgorithms: ['A192GCMKW'] }) + }, { instanceOf: errors.JOSEAlgNotWhitelisted, code: 'ERR_JOSE_ALG_NOT_WHITELISTED', message: 'key management algorithm not whitelisted' }) +}) - test('JWE decrypt algorithms whitelist with a keystore', t => { - const k = generateSync('oct') - const k2 = generateSync('oct') - const ks = new JWKS.KeyStore(k, k2) +test('JWE decrypt keyManagementAlgorithms whitelist with a keystore', t => { + const k = generateSync('oct') + const k2 = generateSync('oct', 128) + const ks = new JWKS.KeyStore(k, k2) - const jwe = JWE.encrypt('foo', k2, { alg: 'PBES2-HS256+A128KW' }) - JWE.decrypt(jwe, ks, { algorithms: ['PBES2-HS256+A128KW', 'PBES2-HS384+A192KW'] }) + const jwe = JWE.encrypt('foo', k2, { alg: 'A128GCMKW' }) + JWE.decrypt(jwe, ks, { keyManagementAlgorithms: ['A128GCMKW', 'A192GCMKW'] }) - t.throws(() => { - JWE.decrypt(jwe, ks, { algorithms: ['PBES2-HS384+A192KW'] }) - }, { instanceOf: errors.JOSEAlgNotWhitelisted, code: 'ERR_JOSE_ALG_NOT_WHITELISTED' }) - }) -} + t.throws(() => { + JWE.decrypt(jwe, ks, { keyManagementAlgorithms: ['A192GCMKW'] }) + }, { instanceOf: errors.JOSEAlgNotWhitelisted, code: 'ERR_JOSE_ALG_NOT_WHITELISTED' }) +}) -test('JWE decrypt algorithms whitelist with direct encryption', t => { +test('JWE decrypt contentEncryptionAlgorithms whitelist', t => { const k = generateSync('oct') const jwe = JWE.encrypt('foo', k, { alg: 'dir' }) - JWE.decrypt(jwe, k, { algorithms: ['A128CBC-HS256'] }) + JWE.decrypt(jwe, k, { contentEncryptionAlgorithms: ['A128CBC-HS256'] }) t.throws(() => { - JWE.decrypt(jwe, k, { algorithms: ['PBES2-HS384+A192KW'] }) + JWE.decrypt(jwe, k, { contentEncryptionAlgorithms: ['PBES2-HS384+A192KW'] }) }, { instanceOf: errors.JOSEAlgNotWhitelisted, code: 'ERR_JOSE_ALG_NOT_WHITELISTED' }) }) -test('JWE decrypt algorithms whitelist (multi-recipient)', t => { +test('JWE decrypt keyManagementAlgorithms whitelist (multi-recipient)', t => { const k = generateSync('oct') const k2 = generateSync('RSA') @@ -477,12 +494,12 @@ test('JWE decrypt algorithms whitelist (multi-recipient)', t => { encrypt.recipient(k2) const jwe = encrypt.encrypt('general') - JWE.decrypt(jwe, k, { algorithms: ['electron' in process.versions ? 'A256GCMKW' : 'A256KW'] }) - JWE.decrypt(jwe, k2, { algorithms: ['RSA-OAEP'] }) + JWE.decrypt(jwe, k, { keyManagementAlgorithms: ['electron' in process.versions ? 'A256GCMKW' : 'A256KW'] }) + JWE.decrypt(jwe, k2, { keyManagementAlgorithms: ['RSA-OAEP'] }) let err err = t.throws(() => { - JWE.decrypt(jwe, k, { algorithms: ['RSA-OAEP'] }) + JWE.decrypt(jwe, k, { keyManagementAlgorithms: ['RSA-OAEP'] }) }, { instanceOf: errors.JOSEMultiError, code: 'ERR_JOSE_MULTIPLE_ERRORS' }) ;[...err].forEach((e, i) => { if (i === 0) { @@ -493,7 +510,7 @@ test('JWE decrypt algorithms whitelist (multi-recipient)', t => { }) err = t.throws(() => { - JWE.decrypt(jwe, k2, { algorithms: ['electron' in process.versions ? 'A256GCMKW' : 'A256KW'] }) + JWE.decrypt(jwe, k2, { keyManagementAlgorithms: ['electron' in process.versions ? 'A256GCMKW' : 'A256KW'] }) }, { instanceOf: errors.JOSEMultiError, code: 'ERR_JOSE_MULTIPLE_ERRORS' }) ;[...err].forEach((e, i) => { if (i === 0) { diff --git a/types/index.d.ts b/types/index.d.ts index f7f8929587..9c4ff9af4d 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -399,7 +399,8 @@ export namespace JWE { interface DecryptOptions { complete?: boolean; crit?: string[]; - algorithms?: string[]; + contentEncryptionAlgorithms?: string[]; + keyManagementAlgorithms?: string[]; } interface completeDecrypt {