Skip to content

Commit

Permalink
crypto: refactor array buffer view validation
Browse files Browse the repository at this point in the history
This is just a refactoring to reduce code and computational overhead.

PR-URL: #29683
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
BridgeAR committed Oct 1, 2019
1 parent 204248a commit eb05d68
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 63 deletions.
29 changes: 7 additions & 22 deletions lib/internal/crypto/certificate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down
23 changes: 5 additions & 18 deletions lib/internal/crypto/cipher.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const {
const {
getDefaultEncoding,
kHandle,
toBuf
getArrayBufferView
} = require('internal/crypto/util');

const { isArrayBufferView } = require('internal/util/types');
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/crypto/pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions lib/internal/crypto/scrypt.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const {
} = require('internal/errors').codes;
const {
getDefaultEncoding,
validateArrayBufferView,
getArrayBufferView,
} = require('internal/crypto/util');

const defaults = {
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 3 additions & 6 deletions lib/internal/crypto/sig.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ const {
const {
getDefaultEncoding,
kHandle,
toBuf,
validateArrayBufferView,
getArrayBufferView,
} = require('internal/crypto/util');
const {
preparePrivateKey,
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -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);
Expand Down
23 changes: 15 additions & 8 deletions lib/internal/crypto/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -94,10 +101,10 @@ function validateArrayBufferView(buffer, name) {
);
}
return buffer;
}
});

module.exports = {
validateArrayBufferView,
getArrayBufferView,
getCiphers,
getCurves,
getDefaultEncoding,
Expand Down
3 changes: 0 additions & 3 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit eb05d68

Please sign in to comment.