Skip to content

Commit

Permalink
crypto: fix generateKeyPair type checks
Browse files Browse the repository at this point in the history
Change saltLength, divisorLength, primeLength and generator
checks in generateKeyPair to int32 from uint32, to align
with c++ code.

fixes: #38358

PR-URL: #38364
Fixes: #38358
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Linkgoron authored and targos committed Apr 29, 2021
1 parent f455e08 commit 4b073b0
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 5 deletions.
9 changes: 5 additions & 4 deletions lib/internal/crypto/keygen.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const {
const { customPromisifyArgs } = require('internal/util');

const {
isInt32,
isUint32,
validateCallback,
validateString,
Expand Down Expand Up @@ -194,7 +195,7 @@ function createJob(mode, type, options) {
throw new ERR_INVALID_ARG_VALUE('options.hash', hash);
if (mgf1Hash !== undefined && typeof mgf1Hash !== 'string')
throw new ERR_INVALID_ARG_VALUE('options.mgf1Hash', mgf1Hash);
if (saltLength !== undefined && !isUint32(saltLength))
if (saltLength !== undefined && (!isInt32(saltLength) || saltLength < 0))
throw new ERR_INVALID_ARG_VALUE('options.saltLength', saltLength);

return new RsaKeyPairGenJob(
Expand All @@ -217,7 +218,7 @@ function createJob(mode, type, options) {
let { divisorLength } = options;
if (divisorLength == null) {
divisorLength = -1;
} else if (!isUint32(divisorLength)) {
} else if (!isInt32(divisorLength) || divisorLength < 0) {
throw new ERR_INVALID_ARG_VALUE('options.divisorLength', divisorLength);
}

Expand Down Expand Up @@ -292,15 +293,15 @@ function createJob(mode, type, options) {
if (!isArrayBufferView(prime))
throw new ERR_INVALID_ARG_VALUE('options.prime', prime);
} else if (primeLength != null) {
if (!isUint32(primeLength))
if (!isInt32(primeLength) || primeLength < 0)
throw new ERR_INVALID_ARG_VALUE('options.primeLength', primeLength);
} else {
throw new ERR_MISSING_OPTION(
'At least one of the group, prime, or primeLength options');
}

if (generator != null) {
if (!isUint32(generator))
if (!isInt32(generator) || generator < 0)
throw new ERR_INVALID_ARG_VALUE('options.generator', generator);
}
return new DhKeyPairGenJob(
Expand Down
77 changes: 76 additions & 1 deletion test/parallel/test-crypto-keygen.js
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
}

// Test invalid divisor lengths.
for (const divisorLength of ['a', true, {}, [], 4096.1]) {
for (const divisorLength of ['a', true, {}, [], 4096.1, 2147483648, -1]) {
assert.throws(() => generateKeyPair('dsa', {
modulusLength: 2048,
divisorLength
Expand Down Expand Up @@ -1081,6 +1081,52 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
message: 'Unknown DH group'
});

assert.throws(() => {
generateKeyPair('dh', {
primeLength: 2147483648
}, common.mustNotCall());
}, {
name: 'TypeError',
code: 'ERR_INVALID_ARG_VALUE',
message: "The property 'options.primeLength' is invalid. " +
'Received 2147483648',
});

assert.throws(() => {
generateKeyPair('dh', {
primeLength: -1
}, common.mustNotCall());
}, {
name: 'TypeError',
code: 'ERR_INVALID_ARG_VALUE',
message: "The property 'options.primeLength' is invalid. " +
'Received -1',
});

assert.throws(() => {
generateKeyPair('dh', {
primeLength: 2,
generator: 2147483648,
}, common.mustNotCall());
}, {
name: 'TypeError',
code: 'ERR_INVALID_ARG_VALUE',
message: "The property 'options.generator' is invalid. " +
'Received 2147483648',
});

assert.throws(() => {
generateKeyPair('dh', {
primeLength: 2,
generator: -1,
}, common.mustNotCall());
}, {
name: 'TypeError',
code: 'ERR_INVALID_ARG_VALUE',
message: "The property 'options.generator' is invalid. " +
'Received -1',
});

// Test incompatible options.
const allOpts = {
group: 'modp5',
Expand Down Expand Up @@ -1142,6 +1188,35 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
});
}

// too long salt length
assert.throws(() => {
generateKeyPair('rsa-pss', {
modulusLength: 512,
saltLength: 2147483648,
hash: 'sha256',
mgf1Hash: 'sha256'
}, common.mustNotCall());
}, {
name: 'TypeError',
code: 'ERR_INVALID_ARG_VALUE',
message: "The property 'options.saltLength' is invalid. " +
'Received 2147483648'
});

assert.throws(() => {
generateKeyPair('rsa-pss', {
modulusLength: 512,
saltLength: -1,
hash: 'sha256',
mgf1Hash: 'sha256'
}, common.mustNotCall());
}, {
name: 'TypeError',
code: 'ERR_INVALID_ARG_VALUE',
message: "The property 'options.saltLength' is invalid. " +
'Received -1'
});

// Invalid private key type.
for (const type of ['foo', 'spki']) {
assert.throws(() => {
Expand Down

0 comments on commit 4b073b0

Please sign in to comment.