Skip to content

Commit

Permalink
crypto: handle invalid prepareAsymmetricKey JWK inputs
Browse files Browse the repository at this point in the history
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
  • Loading branch information
panva authored and juanarbol committed Oct 7, 2022
1 parent e5725c2 commit 230af80
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 7 deletions.
5 changes: 4 additions & 1 deletion lib/internal/crypto/keygen.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const {
SecretKeyObject,
parsePublicKeyEncoding,
parsePrivateKeyEncoding,
isJwk
} = require('internal/crypto/keys');

const {
Expand Down Expand Up @@ -66,6 +65,10 @@ const { isArrayBufferView } = require('internal/util/types');
const { getOptionValue } = require('internal/options');
const pendingDeprecation = getOptionValue('--pending-deprecation');

function isJwk(obj) {
return obj != null && obj.kty !== undefined;
}

function wrapKey(key, ctor) {
if (typeof key === 'string' ||
isArrayBufferView(key) ||
Expand Down
11 changes: 5 additions & 6 deletions lib/internal/crypto/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -526,14 +526,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(
Expand Down Expand Up @@ -723,10 +727,6 @@ function isCryptoKey(obj) {
return obj != null && obj[kKeyObject] !== undefined;
}

function isJwk(obj) {
return obj != null && obj.kty !== undefined;
}

module.exports = {
// Public API.
createSecretKey,
Expand All @@ -748,5 +748,4 @@ module.exports = {
PrivateKeyObject,
isKeyObject,
isCryptoKey,
isJwk,
};
12 changes: 12 additions & 0 deletions test/parallel/test-crypto-key-objects.js
Original file line number Diff line number Diff line change
Expand Up @@ -870,3 +870,15 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
assert(!first.privateKey.equals(second.privateKey));
assert(!first.privateKey.equals(second.publicKey));
}

{
// 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/ });
}
}
18 changes: 18 additions & 0 deletions test/parallel/test-crypto-sign-verify.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/ });
}
}

0 comments on commit 230af80

Please sign in to comment.