Skip to content

Commit

Permalink
crypto: check DiffieHellman p and g params
Browse files Browse the repository at this point in the history
It's possible to pass in the prime and generator params as buffers
but that mode of input wasn't as rigorously checked as numeric input.

PR-URL: #32739
Fixes: #32738
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis authored and addaleax committed Apr 28, 2020
1 parent 0694401 commit 38146e7
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 2 deletions.
25 changes: 23 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5136,6 +5136,14 @@ bool DiffieHellman::Init(int primeLength, int g) {

bool DiffieHellman::Init(const char* p, int p_len, int g) {
dh_.reset(DH_new());
if (p_len <= 0) {
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
return false;
}
if (g <= 1) {
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
return false;
}
BIGNUM* bn_p =
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
BIGNUM* bn_g = BN_new();
Expand All @@ -5151,10 +5159,23 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {

bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
dh_.reset(DH_new());
BIGNUM* bn_p =
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
if (p_len <= 0) {
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
return false;
}
if (g_len <= 0) {
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
return false;
}
BIGNUM* bn_g =
BN_bin2bn(reinterpret_cast<const unsigned char*>(g), g_len, nullptr);
if (BN_is_zero(bn_g) || BN_is_one(bn_g)) {
BN_free(bn_g);
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
return false;
}
BIGNUM* bn_p =
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
if (!DH_set0_pqg(dh_.get(), bn_p, nullptr, bn_g)) {
BN_free(bn_p);
BN_free(bn_g);
Expand Down
31 changes: 31 additions & 0 deletions test/parallel/test-crypto-dh.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,37 @@ for (const bits of [-1, 0, 1]) {
});
}

// Through a fluke of history, g=0 defaults to DH_GENERATOR (2).
{
const g = 0;
crypto.createDiffieHellman('abcdef', g);
crypto.createDiffieHellman('abcdef', 'hex', g);
}

for (const g of [-1, 1]) {
const ex = {
code: 'ERR_OSSL_DH_BAD_GENERATOR',
name: 'Error',
message: /bad generator/,
};
assert.throws(() => crypto.createDiffieHellman('abcdef', g), ex);
assert.throws(() => crypto.createDiffieHellman('abcdef', 'hex', g), ex);
}

crypto.createDiffieHellman('abcdef', Buffer.from([2])); // OK

for (const g of [Buffer.from([]),
Buffer.from([0]),
Buffer.from([1])]) {
const ex = {
code: 'ERR_OSSL_DH_BAD_GENERATOR',
name: 'Error',
message: /bad generator/,
};
assert.throws(() => crypto.createDiffieHellman('abcdef', g), ex);
assert.throws(() => crypto.createDiffieHellman('abcdef', 'hex', g), ex);
}

{
const DiffieHellman = crypto.DiffieHellman;
const dh = DiffieHellman(p1, 'buffer');
Expand Down

0 comments on commit 38146e7

Please sign in to comment.