Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: crash in crypto.createDiffieHellman #27766

Merged
merged 1 commit into from Feb 18, 2021
Merged

fix: crash in crypto.createDiffieHellman #27766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 27 additions & 23 deletions patches/node/fix_comment_out_incompatible_crypto_modules.patch
View file
Open in desktop
Expand Up @@ -9,58 +9,62 @@ with what's exposed through BoringSSL. I plan to upstream parts of this or
otherwise introduce shims to reduce friction.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index c373533ce85241f86d64eab8a49af79f935acdeb..454fff5ada0c271db7fb975f809c84d87773dcd6 100644
index c373533ce85241f86d64eab8a49af79f935acdeb..19932c2a0abf454ddb1848b0b2cd19e3e9b2cfe9 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -5145,6 +5145,7 @@ bool DiffieHellman::Init(int primeLength, int g) {

@@ -5146,11 +5146,11 @@ bool DiffieHellman::Init(int primeLength, int g) {
bool DiffieHellman::Init(const char* p, int p_len, int g) {
dh_.reset(DH_new());
+#if 0
if (p_len <= 0) {
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
- BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
+ OPENSSL_PUT_ERROR(BN, BN_R_BITS_TOO_SMALL);
return false;
@@ -5153,6 +5154,7 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
}
if (g <= 1) {
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
return false;
}
+#endif
BIGNUM* bn_p =
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
BIGNUM* bn_g = BN_new();
@@ -5168,6 +5170,7 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {

@@ -5169,18 +5169,18 @@ 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());
+#if 0
if (p_len <= 0) {
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
- BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
+ OPENSSL_PUT_ERROR(BN, BN_R_BITS_TOO_SMALL);
return false;
@@ -5190,6 +5193,7 @@ bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
}
if (g_len <= 0) {
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+ OPENSSL_PUT_ERROR(DH, 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);
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
return false;
}
+#endif
return VerifyContext();
}

@@ -6157,6 +6161,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
BIGNUM* bn_p =
@@ -6157,6 +6157,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
EVPKeyCtxPointer Setup() override {
EVPKeyPointer params;
if (prime_info_.fixed_value_) {
+#if 0
DHPointer dh(DH_new());
if (!dh)
return nullptr;
@@ -6173,6 +6178,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
@@ -6173,6 +6174,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
params = EVPKeyPointer(EVP_PKEY_new());
CHECK(params);
EVP_PKEY_assign_DH(params.get(), dh.release());
+#endif
} else {
EVPKeyCtxPointer param_ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DH, nullptr));
if (!param_ctx)
@@ -6180,7 +6186,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
@@ -6180,7 +6182,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {

if (EVP_PKEY_paramgen_init(param_ctx.get()) <= 0)
return nullptr;
Expand All @@ -69,7 +73,7 @@ index c373533ce85241f86d64eab8a49af79f935acdeb..454fff5ada0c271db7fb975f809c84d8
if (EVP_PKEY_CTX_set_dh_paramgen_prime_len(param_ctx.get(),
prime_info_.prime_size_) <= 0)
return nullptr;
@@ -6188,7 +6194,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
@@ -6188,7 +6190,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
if (EVP_PKEY_CTX_set_dh_paramgen_generator(param_ctx.get(),
generator_) <= 0)
return nullptr;
Expand Down
14 changes: 14 additions & 0 deletions spec/node-spec.js
View file
Open in desktop
Expand Up @@ -303,6 +303,20 @@ describe('node feature', () => {
expect(cipherText).to.equal(result);
}
});

it('does not crash when using crypto.diffieHellman() constructors', () => {
const crypto = require('crypto');

crypto.createDiffieHellman('abc');
crypto.createDiffieHellman('abc', 2);

// Needed to test specific DiffieHellman ctors.

// eslint-disable-next-line no-octal
crypto.createDiffieHellman('abc', Buffer.from([02]));
// eslint-disable-next-line no-octal
crypto.createDiffieHellman('abc', '123');
});
});

describe('process.stdout', () => {
Expand Down