From eb05d6881521a969ad605b137e0a06847dd01971 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 23 Sep 2019 23:35:24 +0200 Subject: [PATCH] crypto: refactor array buffer view validation This is just a refactoring to reduce code and computational overhead. PR-URL: https://github.com/nodejs/node/pull/29683 Reviewed-By: James M Snell --- lib/internal/crypto/certificate.js | 29 +++++++---------------------- lib/internal/crypto/cipher.js | 23 +++++------------------ lib/internal/crypto/pbkdf2.js | 6 +++--- lib/internal/crypto/scrypt.js | 6 +++--- lib/internal/crypto/sig.js | 9 +++------ lib/internal/crypto/util.js | 23 +++++++++++++++-------- lib/internal/validators.js | 3 --- 7 files changed, 36 insertions(+), 63 deletions(-) diff --git a/lib/internal/crypto/certificate.js b/lib/internal/crypto/certificate.js index d828bef0f6ae2d..b9a86b00702366 100644 --- a/lib/internal/crypto/certificate.js +++ b/lib/internal/crypto/certificate.js @@ -9,11 +9,8 @@ const { validateBuffer } = require('internal/validators'); -const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes; -const { isArrayBufferView } = require('internal/util/types'); - const { - toBuf + getArrayBufferView } = require('internal/crypto/util'); function verifySpkac(spkac) { @@ -22,27 +19,15 @@ function verifySpkac(spkac) { } function exportPublicKey(spkac, encoding) { - spkac = toBuf(spkac, encoding); - if (!isArrayBufferView(spkac)) { - throw new ERR_INVALID_ARG_TYPE( - 'spkac', - ['string', 'Buffer', 'TypedArray', 'DataView'], - spkac - ); - } - return certExportPublicKey(spkac); + return certExportPublicKey( + getArrayBufferView(spkac, 'spkac', encoding) + ); } function exportChallenge(spkac, encoding) { - spkac = toBuf(spkac, encoding); - if (!isArrayBufferView(spkac)) { - throw new ERR_INVALID_ARG_TYPE( - 'spkac', - ['string', 'Buffer', 'TypedArray', 'DataView'], - spkac - ); - } - return certExportChallenge(spkac); + return certExportChallenge( + getArrayBufferView(spkac, 'spkac', encoding) + ); } // For backwards compatibility reasons, this cannot be converted into a diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index 48c5ba23c73e0c..4eb1aee69c9ecc 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -22,7 +22,7 @@ const { const { getDefaultEncoding, kHandle, - toBuf + getArrayBufferView } = require('internal/crypto/util'); const { isArrayBufferView } = require('internal/util/types'); @@ -105,20 +105,9 @@ function createCipherBase(cipher, credential, options, decipher, iv) { LazyTransform.call(this, options); } -function invalidArrayBufferView(name, value) { - return new ERR_INVALID_ARG_TYPE( - name, - ['string', 'Buffer', 'TypedArray', 'DataView'], - value - ); -} - function createCipher(cipher, password, options, decipher) { validateString(cipher, 'cipher'); - password = toBuf(password); - if (!isArrayBufferView(password)) { - throw invalidArrayBufferView('password', password); - } + password = getArrayBufferView(password, 'password'); createCipherBase.call(this, cipher, password, options, decipher); } @@ -126,10 +115,7 @@ function createCipher(cipher, password, options, decipher) { function createCipherWithIV(cipher, key, options, decipher, iv) { validateString(cipher, 'cipher'); key = prepareSecretKey(key); - iv = toBuf(iv); - if (iv !== null && !isArrayBufferView(iv)) { - throw invalidArrayBufferView('iv', iv); - } + iv = iv === null ? null : getArrayBufferView(iv, 'iv'); createCipherBase.call(this, cipher, key, options, decipher, iv); } @@ -164,7 +150,8 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) { outputEncoding = outputEncoding || encoding; if (typeof data !== 'string' && !isArrayBufferView(data)) { - throw invalidArrayBufferView('data', data); + throw new ERR_INVALID_ARG_TYPE( + 'data', ['string', 'Buffer', 'TypedArray', 'DataView'], data); } validateEncoding(data, inputEncoding); diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js index d8c4bc9b518dc1..1c2288b65f1a25 100644 --- a/lib/internal/crypto/pbkdf2.js +++ b/lib/internal/crypto/pbkdf2.js @@ -13,7 +13,7 @@ const { } = require('internal/errors').codes; const { getDefaultEncoding, - validateArrayBufferView, + getArrayBufferView, } = require('internal/crypto/util'); function pbkdf2(password, salt, iterations, keylen, digest, callback) { @@ -64,8 +64,8 @@ function check(password, salt, iterations, keylen, digest) { digest = defaultDigest(); } - password = validateArrayBufferView(password, 'password'); - salt = validateArrayBufferView(salt, 'salt'); + password = getArrayBufferView(password, 'password'); + salt = getArrayBufferView(salt, 'salt'); validateUint32(iterations, 'iterations', 0); validateUint32(keylen, 'keylen', 0); diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js index e2751f8fa58261..cd18f671b18b73 100644 --- a/lib/internal/crypto/scrypt.js +++ b/lib/internal/crypto/scrypt.js @@ -11,7 +11,7 @@ const { } = require('internal/errors').codes; const { getDefaultEncoding, - validateArrayBufferView, + getArrayBufferView, } = require('internal/crypto/util'); const defaults = { @@ -72,8 +72,8 @@ function check(password, salt, keylen, options) { if (_scrypt === undefined) throw new ERR_CRYPTO_SCRYPT_NOT_SUPPORTED(); - password = validateArrayBufferView(password, 'password'); - salt = validateArrayBufferView(salt, 'salt'); + password = getArrayBufferView(password, 'password'); + salt = getArrayBufferView(salt, 'salt'); validateUint32(keylen, 'keylen'); let { N, r, p, maxmem } = defaults; diff --git a/lib/internal/crypto/sig.js b/lib/internal/crypto/sig.js index b6a3376e735094..9b9c32e59c8fd0 100644 --- a/lib/internal/crypto/sig.js +++ b/lib/internal/crypto/sig.js @@ -17,8 +17,7 @@ const { const { getDefaultEncoding, kHandle, - toBuf, - validateArrayBufferView, + getArrayBufferView, } = require('internal/crypto/util'); const { preparePrivateKey, @@ -47,8 +46,7 @@ Sign.prototype._write = function _write(chunk, encoding, callback) { Sign.prototype.update = function update(data, encoding) { encoding = encoding || getDefaultEncoding(); - data = validateArrayBufferView(toBuf(data, encoding), - 'data'); + data = getArrayBufferView(data, 'data', encoding); this[kHandle].update(data); return this; }; @@ -154,8 +152,7 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) { const pssSaltLength = getSaltLength(options); - signature = validateArrayBufferView(toBuf(signature, sigEncoding), - 'signature'); + signature = getArrayBufferView(signature, 'signature', sigEncoding); return this[kHandle].verify(data, format, type, passphrase, signature, rsaPadding, pssSaltLength); diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js index 544d44669ae466..8bf66e9cdf3268 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -13,10 +13,13 @@ const { } = internalBinding('constants').crypto; const { - ERR_CRYPTO_ENGINE_UNKNOWN, - ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH, - ERR_INVALID_ARG_TYPE, -} = require('internal/errors').codes; + hideStackFrames, + codes: { + ERR_CRYPTO_ENGINE_UNKNOWN, + ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH, + ERR_INVALID_ARG_TYPE, + } +} = require('internal/errors'); const { validateString } = require('internal/validators'); const { Buffer } = require('buffer'); const { @@ -84,8 +87,12 @@ function timingSafeEqual(buf1, buf2) { return _timingSafeEqual(buf1, buf2); } -function validateArrayBufferView(buffer, name) { - buffer = toBuf(buffer); +const getArrayBufferView = hideStackFrames((buffer, name, encoding) => { + if (typeof buffer === 'string') { + if (encoding === 'buffer') + encoding = 'utf8'; + return Buffer.from(buffer, encoding); + } if (!isArrayBufferView(buffer)) { throw new ERR_INVALID_ARG_TYPE( name, @@ -94,10 +101,10 @@ function validateArrayBufferView(buffer, name) { ); } return buffer; -} +}); module.exports = { - validateArrayBufferView, + getArrayBufferView, getCiphers, getCurves, getDefaultEncoding, diff --git a/lib/internal/validators.js b/lib/internal/validators.js index b5c1477bccd8a5..7fd4d5180e9231 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -132,9 +132,6 @@ function validateSignalName(signal, name = 'signal') { } } -// TODO(BridgeAR): We have multiple validation functions that call -// `require('internal/utils').toBuf()` before validating for array buffer views. -// Those should likely also be consolidated in here. const validateBuffer = hideStackFrames((buffer, name = 'buffer') => { if (!isArrayBufferView(buffer)) { throw new ERR_INVALID_ARG_TYPE(name,