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 targos committed May 13, 2020
1 parent c1b7674 commit 71bccdd
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
Expand Up @@ -5164,6 +5164,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 @@ -5179,10 +5187,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
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 71bccdd

Please sign in to comment.