From b634469c380cffc1504306cf4a6f6f641b064e72 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 12 Feb 2021 15:54:06 +0100 Subject: [PATCH] crypto: refactor to avoid unsafe array iteration PR-URL: https://github.com/nodejs/node/pull/37364 Reviewed-By: James M Snell Reviewed-By: Zijian Liu --- lib/internal/crypto/aes.js | 7 ++++--- lib/internal/crypto/diffiehellman.js | 9 +++++---- lib/internal/crypto/dsa.js | 8 ++++---- lib/internal/crypto/ec.js | 12 +++++++----- lib/internal/crypto/hkdf.js | 2 +- lib/internal/crypto/keygen.js | 11 ++++++----- lib/internal/crypto/keys.js | 20 +++++++++++++++++--- lib/internal/crypto/pbkdf2.js | 2 +- lib/internal/crypto/random.js | 6 +++--- lib/internal/crypto/rsa.js | 14 ++++++++------ lib/internal/crypto/scrypt.js | 2 +- 11 files changed, 57 insertions(+), 36 deletions(-) diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js index ab3ee099cd0d69..76b3b90b753e68 100644 --- a/lib/internal/crypto/aes.js +++ b/lib/internal/crypto/aes.js @@ -8,6 +8,7 @@ const { ArrayPrototypePush, MathFloor, Promise, + ReflectApply, SafeSet, TypedArrayPrototypeSlice, } = primordials; @@ -252,12 +253,12 @@ async function aesImportKey( extractable, keyUsages) { const { name } = algorithm; - const checkUsages = ['wrapKey', 'unwrapKey']; + const usagesSet = new SafeSet(keyUsages); + const checkUsages = [usagesSet, 'wrapKey', 'unwrapKey']; if (name !== 'AES-KW') ArrayPrototypePush(checkUsages, 'encrypt', 'decrypt'); - const usagesSet = new SafeSet(keyUsages); - if (hasAnyNotIn(usagesSet, ...checkUsages)) { + if (ReflectApply(hasAnyNotIn, null, checkUsages)) { throw lazyDOMException( 'Unsupported key usage for an AES key', 'SyntaxError'); diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js index bcf9b1e5090d1a..a23974a531e402 100644 --- a/lib/internal/crypto/diffiehellman.js +++ b/lib/internal/crypto/diffiehellman.js @@ -2,10 +2,12 @@ const { ArrayBufferPrototypeSlice, + ArrayPrototypePush, FunctionPrototypeCall, MathFloor, ObjectDefineProperty, Promise, + ReflectApply, SafeSet, } = primordials; @@ -347,16 +349,15 @@ function deriveBitsDH(publicKey, privateKey, callback) { } function verifyAcceptableDhKeyUse(name, type, usages) { - let checkSet; + const args = [usages]; switch (type) { case 'private': - checkSet = ['deriveBits', 'deriveKey']; + ArrayPrototypePush(args, 'deriveBits', 'deriveKey'); break; case 'public': - checkSet = []; break; } - if (hasAnyNotIn(usages, ...checkSet)) { + if (ReflectApply(hasAnyNotIn, null, args)) { throw lazyDOMException( `Unsupported key usage for an ${name} key`, 'SyntaxError'); diff --git a/lib/internal/crypto/dsa.js b/lib/internal/crypto/dsa.js index 0baabc4680b108..17a255fa77727c 100644 --- a/lib/internal/crypto/dsa.js +++ b/lib/internal/crypto/dsa.js @@ -51,16 +51,16 @@ const { } = require('internal/crypto/util'); function verifyAcceptableDsaKeyUse(name, type, usages) { - let checkSet; + let check; switch (type) { case 'private': - checkSet = ['sign']; + check = 'sign'; break; case 'public': - checkSet = ['verify']; + check = 'verify'; break; } - if (hasAnyNotIn(usages, ...checkSet)) { + if (hasAnyNotIn(usages, check)) { throw lazyDOMException( `Unsupported key usage for an ${name} key`, 'SyntaxError'); diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index 24ccac8c38e059..68ef854d7eb4f8 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -1,8 +1,10 @@ 'use strict'; const { + ArrayPrototypePush, ObjectKeys, Promise, + ReflectApply, SafeSet, } = primordials; @@ -59,10 +61,10 @@ const { } = require('internal/crypto/keys'); function verifyAcceptableEcKeyUse(name, type, usages) { - let checkSet; + const args = [usages]; switch (name) { case 'ECDH': - checkSet = ['deriveKey', 'deriveBits']; + ArrayPrototypePush(args, 'deriveKey', 'deriveBits'); break; case 'NODE-ED25519': // Fall through @@ -71,14 +73,14 @@ function verifyAcceptableEcKeyUse(name, type, usages) { case 'ECDSA': switch (type) { case 'private': - checkSet = ['sign']; + ArrayPrototypePush(args, 'sign'); break; case 'public': - checkSet = ['verify']; + ArrayPrototypePush(args, 'verify'); break; } } - if (hasAnyNotIn(usages, ...checkSet)) { + if (ReflectApply(hasAnyNotIn, null, args)) { throw lazyDOMException( `Unsupported key usage for a ${name} key`, 'SyntaxError'); diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js index 7949fa33d2e077..7400d0c815f84f 100644 --- a/lib/internal/crypto/hkdf.js +++ b/lib/internal/crypto/hkdf.js @@ -134,7 +134,7 @@ function hkdfSync(hash, key, salt, info, length) { } = validateParameters(hash, key, salt, info, length)); const job = new HKDFJob(kCryptoJobSync, hash, key, salt, info, length); - const [err, bits] = job.run(); + const { 0: err, 1: bits } = job.run(); if (err !== undefined) throw err; diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js index 56f06f7a79fe2a..1e944d76fac66d 100644 --- a/lib/internal/crypto/keygen.js +++ b/lib/internal/crypto/keygen.js @@ -3,6 +3,7 @@ const { FunctionPrototypeCall, ObjectDefineProperty, + SafeArrayIterator, } = primordials; const { @@ -75,7 +76,7 @@ function generateKeyPair(type, options, callback) { job.ondone = (error, result) => { if (error) return FunctionPrototypeCall(callback, job, error); // If no encoding was chosen, return key objects instead. - let [pubkey, privkey] = result; + let { 0: pubkey, 1: privkey } = result; pubkey = wrapKey(pubkey, PublicKeyObject); privkey = wrapKey(privkey, PrivateKeyObject); FunctionPrototypeCall(callback, job, null, pubkey, privkey); @@ -97,11 +98,11 @@ function handleError(ret) { if (ret == null) return; // async - const [err, keys] = ret; + const { 0: err, 1: keys } = ret; if (err !== undefined) throw err; - const [publicKey, privateKey] = keys; + const { 0: publicKey, 1: privateKey } = keys; // If no encoding was chosen, return key objects instead. return { @@ -156,7 +157,7 @@ function parseKeyEncoding(keyType, options = {}) { function createJob(mode, type, options) { validateString(type, 'type'); - const encoding = parseKeyEncoding(type, options); + const encoding = new SafeArrayIterator(parseKeyEncoding(type, options)); if (options !== undefined) validateObject(options, 'options'); @@ -341,7 +342,7 @@ function handleGenerateKeyError(ret) { if (ret === undefined) return; // async - const [err, key] = ret; + const { 0: err, 1: key } = ret; if (err !== undefined) throw err; diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index c4c60f3d226de7..140f4ad6df7a1d 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -2,6 +2,7 @@ const { ArrayFrom, + ArrayPrototypeSlice, ObjectDefineProperty, ObjectSetPrototypeOf, Symbol, @@ -160,6 +161,11 @@ const [ } class AsymmetricKeyObject extends KeyObject { + // eslint-disable-next-line no-useless-constructor + constructor(type, handle) { + super(type, handle); + } + get asymmetricKeyType() { return this[kAsymmetricKeyType] || (this[kAsymmetricKeyType] = this[kHandle].getAsymmetricKeyType()); @@ -390,13 +396,21 @@ function getKeyObjectHandle(key, ctx) { } function getKeyTypes(allowKeyObject, bufferOnly = false) { - return [ + const types = [ 'ArrayBuffer', 'Buffer', 'TypedArray', 'DataView', - ...(!bufferOnly ? ['string'] : []), - ...(!bufferOnly && allowKeyObject ? ['KeyObject', 'CryptoKey'] : [])]; + 'string', // Only if bufferOnly == false + 'KeyObject', // Only if allowKeyObject == true && bufferOnly == false + 'CryptoKey', // Only if allowKeyObject == true && bufferOnly == false + ]; + if (bufferOnly) { + return ArrayPrototypeSlice(types, 0, 4); + } else if (!allowKeyObject) { + return ArrayPrototypeSlice(types, 0, 5); + } + return types; } function prepareAsymmetricKey(key, ctx) { diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js index 1017063b07952d..0e516113776a73 100644 --- a/lib/internal/crypto/pbkdf2.js +++ b/lib/internal/crypto/pbkdf2.js @@ -74,7 +74,7 @@ function pbkdf2Sync(password, salt, iterations, keylen, digest) { keylen, digest); - const [err, result] = job.run(); + const { 0: err, 1: result } = job.run(); if (err !== undefined) throw err; diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js index 45be27edfe56c0..e82aeb54564ae1 100644 --- a/lib/internal/crypto/random.js +++ b/lib/internal/crypto/random.js @@ -128,7 +128,7 @@ function randomFillSync(buf, offset = 0, size) { offset, size); - const [ err ] = job.run(); + const err = job.run()[0]; if (err) throw err; @@ -471,7 +471,7 @@ function generatePrimeSync(size, options = {}) { validateUint32(size, 'size', true); const job = createRandomPrimeJob(kCryptoJobSync, size, options); - const [err, prime] = job.run(); + const { 0: err, 1: prime } = job.run(); if (err) throw err; return job.result(prime); @@ -548,7 +548,7 @@ function checkPrimeSync(candidate, options = {}) { validateUint32(checks, 'options.checks'); const job = new CheckPrimeJob(kCryptoJobSync, candidate, checks); - const [err, result] = job.run(); + const { 0: err, 1: result } = job.run(); if (err) throw err; diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index 1c90b57f439177..809747ea011373 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -1,7 +1,9 @@ 'use strict'; const { + ArrayPrototypePush, Promise, + ReflectApply, SafeSet, Uint8Array, } = primordials; @@ -71,29 +73,29 @@ const kRsaVariants = { }; function verifyAcceptableRsaKeyUse(name, type, usages) { - let checkSet; + const args = [usages]; switch (name) { case 'RSA-OAEP': switch (type) { case 'private': - checkSet = ['decrypt', 'unwrapKey']; + ArrayPrototypePush(args, 'decrypt', 'unwrapKey'); break; case 'public': - checkSet = ['encrypt', 'wrapKey']; + ArrayPrototypePush(args, 'encrypt', 'wrapKey'); break; } break; default: switch (type) { case 'private': - checkSet = ['sign']; + ArrayPrototypePush(args, 'sign'); break; case 'public': - checkSet = ['verify']; + ArrayPrototypePush(args, 'verify'); break; } } - if (hasAnyNotIn(usages, ...checkSet)) { + if (ReflectApply(hasAnyNotIn, null, args)) { throw lazyDOMException( `Unsupported key usage for an ${name} key`, 'SyntaxError'); diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js index 39305c15d8a4ba..97bc93a6143d19 100644 --- a/lib/internal/crypto/scrypt.js +++ b/lib/internal/crypto/scrypt.js @@ -74,7 +74,7 @@ function scryptSync(password, salt, keylen, options = defaults) { ({ password, salt, keylen } = options); const job = new ScryptJob( kCryptoJobSync, password, salt, N, r, p, maxmem, keylen); - const [err, result] = job.run(); + const { 0: err, 1: result } = job.run(); if (err !== undefined) throw err;