From 6d3e24c10d8e56465e821da9d6689a7a586de849 Mon Sep 17 00:00:00 2001 From: Mohammed Keyvanzadeh Date: Mon, 4 Apr 2022 15:08:35 +0430 Subject: [PATCH] crypto: cleanup validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Many of the validations could be simplified and cleaned up by using validators and to keep consistency. PR-URL: https://github.com/nodejs/node/pull/39841 Reviewed-By: Antoine du Hamel Reviewed-By: Michaël Zasso --- lib/internal/crypto/keygen.js | 61 ++++---- test/parallel/test-crypto-keygen.js | 227 ++++++++++++++++++++++------ 2 files changed, 210 insertions(+), 78 deletions(-) diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js index a043a17f48deec..c6909b227d0c15 100644 --- a/lib/internal/crypto/keygen.js +++ b/lib/internal/crypto/keygen.js @@ -41,13 +41,14 @@ const { const { customPromisifyArgs } = require('internal/util'); const { - isInt32, - isUint32, validateFunction, + validateBuffer, validateString, validateInteger, validateObject, validateOneOf, + validateInt32, + validateUint32, } = require('internal/validators'); const { @@ -174,16 +175,13 @@ function createJob(mode, type, options) { { validateObject(options, 'options'); const { modulusLength } = options; - if (!isUint32(modulusLength)) - throw new ERR_INVALID_ARG_VALUE('options.modulusLength', modulusLength); + validateUint32(modulusLength, 'options.modulusLength'); let { publicExponent } = options; if (publicExponent == null) { publicExponent = 0x10001; - } else if (!isUint32(publicExponent)) { - throw new ERR_INVALID_ARG_VALUE( - 'options.publicExponent', - publicExponent); + } else { + validateUint32(publicExponent, 'options.publicExponent'); } if (type === 'rsa') { @@ -201,22 +199,20 @@ function createJob(mode, type, options) { const pendingDeprecation = getOptionValue('--pending-deprecation'); - if (saltLength !== undefined && (!isInt32(saltLength) || saltLength < 0)) - throw new ERR_INVALID_ARG_VALUE('options.saltLength', saltLength); - if (hashAlgorithm !== undefined && typeof hashAlgorithm !== 'string') - throw new ERR_INVALID_ARG_VALUE('options.hashAlgorithm', hashAlgorithm); - if (mgf1HashAlgorithm !== undefined && - typeof mgf1HashAlgorithm !== 'string') - throw new ERR_INVALID_ARG_VALUE('options.mgf1HashAlgorithm', - mgf1HashAlgorithm); + if (saltLength !== undefined) + validateInt32(saltLength, 'options.saltLength', 0); + if (hashAlgorithm !== undefined) + validateString(hashAlgorithm, 'options.hashAlgorithm'); + if (mgf1HashAlgorithm !== undefined) + validateString(mgf1HashAlgorithm, 'options.mgf1HashAlgorithm'); if (hash !== undefined) { pendingDeprecation && process.emitWarning( '"options.hash" is deprecated, ' + 'use "options.hashAlgorithm" instead.', 'DeprecationWarning', 'DEP0154'); - if (typeof hash !== 'string' || - (hashAlgorithm && hash !== hashAlgorithm)) { + validateString(hash, 'options.hash'); + if (hashAlgorithm && hash !== hashAlgorithm) { throw new ERR_INVALID_ARG_VALUE('options.hash', hash); } } @@ -226,8 +222,8 @@ function createJob(mode, type, options) { 'use "options.mgf1HashAlgorithm" instead.', 'DeprecationWarning', 'DEP0154'); - if (typeof mgf1Hash !== 'string' || - (mgf1HashAlgorithm && mgf1Hash !== mgf1HashAlgorithm)) { + validateString(mgf1Hash, 'options.mgf1Hash'); + if (mgf1HashAlgorithm && mgf1Hash !== mgf1HashAlgorithm) { throw new ERR_INVALID_ARG_VALUE('options.mgf1Hash', mgf1Hash); } } @@ -246,15 +242,13 @@ function createJob(mode, type, options) { { validateObject(options, 'options'); const { modulusLength } = options; - if (!isUint32(modulusLength)) - throw new ERR_INVALID_ARG_VALUE('options.modulusLength', modulusLength); + validateUint32(modulusLength, 'options.modulusLength'); let { divisorLength } = options; if (divisorLength == null) { divisorLength = -1; - } else if (!isInt32(divisorLength) || divisorLength < 0) { - throw new ERR_INVALID_ARG_VALUE('options.divisorLength', divisorLength); - } + } else + validateInt32(divisorLength, 'options.divisorLength', 0); return new DsaKeyPairGenJob( mode, @@ -266,8 +260,7 @@ function createJob(mode, type, options) { { validateObject(options, 'options'); const { namedCurve } = options; - if (typeof namedCurve !== 'string') - throw new ERR_INVALID_ARG_VALUE('options.namedCurve', namedCurve); + validateString(namedCurve, 'options.namedCurve'); let { paramEncoding } = options; if (paramEncoding == null || paramEncoding === 'named') paramEncoding = OPENSSL_EC_NAMED_CURVE; @@ -315,8 +308,8 @@ function createJob(mode, type, options) { throw new ERR_INCOMPATIBLE_OPTION_PAIR('group', 'primeLength'); if (generator != null) throw new ERR_INCOMPATIBLE_OPTION_PAIR('group', 'generator'); - if (typeof group !== 'string') - throw new ERR_INVALID_ARG_VALUE('options.group', group); + + validateString(group, 'options.group'); return new DhKeyPairGenJob(mode, group, ...encoding); } @@ -324,19 +317,17 @@ function createJob(mode, type, options) { if (prime != null) { if (primeLength != null) throw new ERR_INCOMPATIBLE_OPTION_PAIR('prime', 'primeLength'); - if (!isArrayBufferView(prime)) - throw new ERR_INVALID_ARG_VALUE('options.prime', prime); + + validateBuffer(prime, 'options.prime'); } else if (primeLength != null) { - if (!isInt32(primeLength) || primeLength < 0) - throw new ERR_INVALID_ARG_VALUE('options.primeLength', primeLength); + validateInt32(primeLength, 'options.primeLength', 0); } else { throw new ERR_MISSING_OPTION( 'At least one of the group, prime, or primeLength options'); } if (generator != null) { - if (!isInt32(generator) || generator < 0) - throw new ERR_INVALID_ARG_VALUE('options.generator', generator); + validateInt32(generator, 'options.generator', 0); } return new DhKeyPairGenJob( mode, diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index 9d03fff9d4a61b..6db4bf1f957ce7 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/test/parallel/test-crypto-keygen.js @@ -1122,57 +1122,189 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); } } +function addNumericalSeparator(val) { + val = String(val); + let res = ''; + let i = val.length; + const start = val[0] === '-' ? 1 : 0; + for (; i >= start + 4; i -= 3) { + res = `_${val.slice(i - 3, i)}${res}`; + } + return `${val.slice(0, i)}${res}`; +} + + // Test RSA parameters. { - // Test invalid modulus lengths. - for (const modulusLength of [undefined, null, 'a', true, {}, [], 512.1, -1]) { + // Test invalid modulus lengths. (non-number) + for (const modulusLength of [undefined, null, 'a', true, {}, []]) { assert.throws(() => generateKeyPair('rsa', { modulusLength }, common.mustNotCall()), { name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.modulusLength' is invalid. " + + code: 'ERR_INVALID_ARG_TYPE', + message: + 'The "options.modulusLength" property must be of type number.' + + common.invalidArgTypeHelper(modulusLength) + }); + } + + // Test invalid modulus lengths. (non-integer) + for (const modulusLength of [512.1, 1.3, 1.1, 5000.9, 100.5]) { + assert.throws(() => generateKeyPair('rsa', { + modulusLength + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.modulusLength" is out of range. ' + + 'It must be an integer. ' + `Received ${inspect(modulusLength)}` }); } - // Test invalid exponents. - for (const publicExponent of ['a', true, {}, [], 3.5, -1]) { + // Test invalid modulus lengths. (out of range) + for (const modulusLength of [-1, -9, 4294967297]) { + assert.throws(() => generateKeyPair('rsa', { + modulusLength + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.modulusLength" is out of range. ' + + 'It must be >= 0 && < 4294967296. ' + + `Received ${addNumericalSeparator(modulusLength)}` + }); + } + + // Test invalid exponents. (non-number) + for (const publicExponent of ['a', true, {}, []]) { assert.throws(() => generateKeyPair('rsa', { modulusLength: 4096, publicExponent }, common.mustNotCall()), { name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.publicExponent' is invalid. " + + code: 'ERR_INVALID_ARG_TYPE', + message: + 'The "options.publicExponent" property must be of type number.' + + common.invalidArgTypeHelper(publicExponent) + }); + } + + // Test invalid exponents. (non-integer) + for (const publicExponent of [3.5, 1.1, 50.5, 510.5]) { + assert.throws(() => generateKeyPair('rsa', { + modulusLength: 4096, + publicExponent + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.publicExponent" is out of range. ' + + 'It must be an integer. ' + `Received ${inspect(publicExponent)}` }); } + + // Test invalid exponents. (out of range) + for (const publicExponent of [-5, -3, 4294967297]) { + assert.throws(() => generateKeyPair('rsa', { + modulusLength: 4096, + publicExponent + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.publicExponent" is out of range. ' + + 'It must be >= 0 && < 4294967296. ' + + `Received ${addNumericalSeparator(publicExponent)}` + }); + } } // Test DSA parameters. { - // Test invalid modulus lengths. - for (const modulusLength of [undefined, null, 'a', true, {}, [], 4096.1]) { + // Test invalid modulus lengths. (non-number) + for (const modulusLength of [undefined, null, 'a', true, {}, []]) { assert.throws(() => generateKeyPair('dsa', { modulusLength }, common.mustNotCall()), { name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.modulusLength' is invalid. " + + code: 'ERR_INVALID_ARG_TYPE', + message: + 'The "options.modulusLength" property must be of type number.' + + common.invalidArgTypeHelper(modulusLength) + }); + } + + // Test invalid modulus lengths. (non-integer) + for (const modulusLength of [512.1, 1.3, 1.1, 5000.9, 100.5]) { + assert.throws(() => generateKeyPair('dsa', { + modulusLength + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.modulusLength" is out of range. ' + + 'It must be an integer. ' + `Received ${inspect(modulusLength)}` }); } - // Test invalid divisor lengths. - for (const divisorLength of ['a', true, {}, [], 4096.1, 2147483648, -1]) { + // Test invalid modulus lengths. (out of range) + for (const modulusLength of [-1, -9, 4294967297]) { + assert.throws(() => generateKeyPair('dsa', { + modulusLength + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.modulusLength" is out of range. ' + + 'It must be >= 0 && < 4294967296. ' + + `Received ${addNumericalSeparator(modulusLength)}` + }); + } + + // Test invalid divisor lengths. (non-number) + for (const divisorLength of ['a', true, {}, []]) { assert.throws(() => generateKeyPair('dsa', { modulusLength: 2048, divisorLength }, common.mustNotCall()), { name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.divisorLength' is invalid. " + + code: 'ERR_INVALID_ARG_TYPE', + message: + 'The "options.divisorLength" property must be of type number.' + + common.invalidArgTypeHelper(divisorLength) + }); + } + + // Test invalid divisor lengths. (non-integer) + for (const divisorLength of [4096.1, 5.1, 6.9, 9.5]) { + assert.throws(() => generateKeyPair('dsa', { + modulusLength: 2048, + divisorLength + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.divisorLength" is out of range. ' + + 'It must be an integer. ' + + `Received ${inspect(divisorLength)}` + }); + } + + // Test invalid divisor lengths. (out of range) + for (const divisorLength of [-6, -9, 2147483648]) { + assert.throws(() => generateKeyPair('dsa', { + modulusLength: 2048, + divisorLength + }, common.mustNotCall()), { + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: + 'The value of "options.divisorLength" is out of range. ' + + 'It must be >= 0 && <= 2147483647. ' + `Received ${inspect(divisorLength)}` }); } @@ -1202,9 +1334,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); }); }, { name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.namedCurve' is invalid. " + - `Received ${inspect(namedCurve)}` + code: 'ERR_INVALID_ARG_TYPE', + message: + 'The "options.namedCurve" property must be of type string.' + + common.invalidArgTypeHelper(namedCurve) }); } @@ -1293,9 +1426,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); primeLength: 2147483648 }, common.mustNotCall()); }, { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.primeLength' is invalid. " + + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "options.primeLength" is out of range. ' + + 'It must be >= 0 && <= 2147483647. ' + 'Received 2147483648', }); @@ -1304,9 +1438,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); primeLength: -1 }, common.mustNotCall()); }, { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.primeLength' is invalid. " + + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "options.primeLength" is out of range. ' + + 'It must be >= 0 && <= 2147483647. ' + 'Received -1', }); @@ -1316,9 +1451,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); generator: 2147483648, }, common.mustNotCall()); }, { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.generator' is invalid. " + + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "options.generator" is out of range. ' + + 'It must be >= 0 && <= 2147483647. ' + 'Received 2147483648', }); @@ -1328,9 +1464,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); generator: -1, }, common.mustNotCall()); }, { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.generator' is invalid. " + + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "options.generator" is out of range. ' + + 'It must be >= 0 && <= 2147483647. ' + 'Received -1', }); @@ -1389,9 +1526,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); }); }, { name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.hashAlgorithm' is invalid. " + - `Received ${inspect(hashValue)}` + code: 'ERR_INVALID_ARG_TYPE', + message: + 'The "options.hashAlgorithm" property must be of type string.' + + common.invalidArgTypeHelper(hashValue) }); } @@ -1404,9 +1542,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); mgf1HashAlgorithm: 'sha256' }, common.mustNotCall()); }, { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.saltLength' is invalid. " + + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "options.saltLength" is out of range. ' + + 'It must be >= 0 && <= 2147483647. ' + 'Received 2147483648' }); @@ -1418,9 +1557,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); mgf1HashAlgorithm: 'sha256' }, common.mustNotCall()); }, { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.saltLength' is invalid. " + + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "options.saltLength" is out of range. ' + + 'It must be >= 0 && <= 2147483647. ' + 'Received -1' }); @@ -1534,9 +1674,10 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); }, { name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: "The property 'options.mgf1HashAlgorithm' is invalid. " + - `Received ${inspect(mgf1HashAlgorithm)}` + code: 'ERR_INVALID_ARG_TYPE', + message: + 'The "options.mgf1HashAlgorithm" property must be of type string.' + + common.invalidArgTypeHelper(mgf1HashAlgorithm) } );