crypto: refactor array buffer view validation

This is just a refactoring to reduce code and computational overhead. PR-URL: #29683 Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Oct 9, 2019 · 75f3b28 · 75f3b28
1 parent 3153dd6
commit 75f3b28
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 63 deletions.
diff --git 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
@@ -22,7 +22,7 @@ const {
 const {
   getDefaultEncoding,
   kHandle,
-  toBuf
+  getArrayBufferView
 } = require('internal/crypto/util');
 
 const { isArrayBufferView } = require('internal/util/types');
@@ -105,31 +105,17 @@ 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);
 }
 
 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);
   }
 
   const ret = this[kHandle].update(data, inputEncoding);

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -131,9 +131,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,