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>
Linkgoron authored and targos committed Apr 29, 2021

Verified

This commit was signed with the committer’s verified signature. The key has expired.
colombod Diego Colombo
1 parent f455e08 commit 4b073b0
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
@@ -40,6 +40,7 @@ const {
const { customPromisifyArgs } = require('internal/util');

const {
isInt32,
isUint32,
validateCallback,
validateString,
@@ -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(
@@ -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);
}

@@ -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(
77 changes: 76 additions & 1 deletion test/parallel/test-crypto-keygen.js
Original file line number Diff line number Diff line change
@@ -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
@@ -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',
@@ -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(() => {

0 comments on commit 4b073b0

Please sign in to comment.