From 960b15793efbb80b092e29efedeb7e9004da3273 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sat, 3 Sep 2022 14:36:20 +0200 Subject: [PATCH] crypto: handle invalid prepareAsymmetricKey JWK inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #44471 PR-URL: https://github.com/nodejs/node/pull/44475 Reviewed-By: Rich Trott Reviewed-By: Tobias Nießen Reviewed-By: Mohammed Keyvanzadeh --- lib/internal/crypto/keygen.js | 5 ++++- lib/internal/crypto/keys.js | 11 +++++------ test/parallel/test-crypto-key-objects.js | 12 ++++++++++++ test/parallel/test-crypto-sign-verify.js | 18 ++++++++++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js index 8f8fbfb1c0893d..05b1150a203b75 100644 --- a/lib/internal/crypto/keygen.js +++ b/lib/internal/crypto/keygen.js @@ -31,7 +31,6 @@ const { SecretKeyObject, parsePublicKeyEncoding, parsePrivateKeyEncoding, - isJwk } = require('internal/crypto/keys'); const { @@ -66,6 +65,10 @@ const { isArrayBufferView } = require('internal/util/types'); const { getOptionValue } = require('internal/options'); +function isJwk(obj) { + return obj != null && obj.kty !== undefined; +} + function wrapKey(key, ctor) { if (typeof key === 'string' || isArrayBufferView(key) || diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index e31b8019c71a69..4303dc44fe60d7 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -525,14 +525,18 @@ function prepareAsymmetricKey(key, ctx) { return { format: kKeyFormatPEM, data: getArrayBufferOrView(key, 'key') }; } else if (typeof key === 'object') { const { key: data, encoding, format } = key; + // The 'key' property can be a KeyObject as well to allow specifying // additional options such as padding along with the key. if (isKeyObject(data)) return { data: getKeyObjectHandle(data, ctx) }; else if (isCryptoKey(data)) return { data: getKeyObjectHandle(data[kKeyObject], ctx) }; - else if (isJwk(data) && format === 'jwk') + else if (format === 'jwk') { + validateObject(data, 'key.key'); return { data: getKeyObjectHandleFromJwk(data, ctx), format: 'jwk' }; + } + // Either PEM or DER using PKCS#1 or SPKI. if (!isStringOrBuffer(data)) { throw new ERR_INVALID_ARG_TYPE( @@ -720,10 +724,6 @@ function isCryptoKey(obj) { return obj != null && obj[kKeyObject] !== undefined; } -function isJwk(obj) { - return obj != null && obj.kty !== undefined; -} - module.exports = { // Public API. createSecretKey, @@ -745,5 +745,4 @@ module.exports = { PrivateKeyObject, isKeyObject, isCryptoKey, - isJwk, }; diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js index 63f1c4ac4b4c9f..93e7cca3fbbdbd 100644 --- a/test/parallel/test-crypto-key-objects.js +++ b/test/parallel/test-crypto-key-objects.js @@ -868,3 +868,15 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem', assert(!first.equals(third)); assert(!third.equals(first)); } + +{ + // This should not cause a crash: https://github.com/nodejs/node/issues/44471 + for (const key of ['', 'foo', null, undefined, true, Boolean]) { + assert.throws(() => { + createPublicKey({ key, format: 'jwk' }); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /The "key\.key" property must be of type object/ }); + assert.throws(() => { + createPrivateKey({ key, format: 'jwk' }); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /The "key\.key" property must be of type object/ }); + } +} diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js index b2c14b1efcd68b..74c0ff53eb18b7 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -756,3 +756,21 @@ assert.throws( message: /digest too big for rsa key/ }); } + +{ + // This should not cause a crash: https://github.com/nodejs/node/issues/44471 + for (const key of ['', 'foo', null, undefined, true, Boolean]) { + assert.throws(() => { + crypto.verify('sha256', 'foo', { key, format: 'jwk' }, Buffer.alloc(0)); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /The "key\.key" property must be of type object/ }); + assert.throws(() => { + crypto.createVerify('sha256').verify({ key, format: 'jwk' }, Buffer.alloc(0)); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /The "key\.key" property must be of type object/ }); + assert.throws(() => { + crypto.sign('sha256', 'foo', { key, format: 'jwk' }); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /The "key\.key" property must be of type object/ }); + assert.throws(() => { + crypto.createSign('sha256').sign({ key, format: 'jwk' }); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /The "key\.key" property must be of type object/ }); + } +}