Skip to content

Commit f695c61

Browse files
panvapanva
committed
fixup! crypto: split OpenSSL 3, BoringSSL, and legacy backends
1 parent a277df2 commit f695c61

4 files changed

Lines changed: 163 additions & 56 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 130 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,15 @@ EVPKeyPointer NewDhPKey(const char* group_name,
260260

261261
bool GetDhParams(const EVP_PKEY* pkey,
262262
DeleteFnPtr<BIGNUM, BN_free>* p,
263-
DeleteFnPtr<BIGNUM, BN_free>* g) {
263+
DeleteFnPtr<BIGNUM, BN_free>* g,
264+
DeleteFnPtr<BIGNUM, BN_free>* q = nullptr,
265+
DeleteFnPtr<BIGNUM, BN_free>* j = nullptr) {
264266
return GetPKeyBnParam(pkey, OSSL_PKEY_PARAM_FFC_P, p) &&
265-
GetPKeyBnParam(pkey, OSSL_PKEY_PARAM_FFC_G, g);
267+
GetPKeyBnParam(pkey, OSSL_PKEY_PARAM_FFC_G, g) &&
268+
(q == nullptr ||
269+
GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_FFC_Q, q)) &&
270+
(j == nullptr ||
271+
GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_FFC_COFACTOR, j));
266272
}
267273

268274
bool GetDhKeys(const EVP_PKEY* pkey,
@@ -1849,6 +1855,94 @@ bool GenerateDhPublicKey(BignumPointer* out,
18491855
*out = std::move(pub);
18501856
return true;
18511857
}
1858+
1859+
std::optional<int> CheckDhParams(const BIGNUM* p,
1860+
const BIGNUM* g,
1861+
const BIGNUM* q,
1862+
const BIGNUM* j) {
1863+
if (p == nullptr || g == nullptr) return std::nullopt;
1864+
1865+
const int p_bits = BN_num_bits(p);
1866+
if (p_bits > OPENSSL_DH_CHECK_MAX_MODULUS_BITS) return std::nullopt;
1867+
1868+
int codes = 0;
1869+
if (!BN_is_odd(p)) {
1870+
codes |= static_cast<int>(DHPointer::CheckResult::P_NOT_PRIME);
1871+
}
1872+
if (BN_is_negative(g) || BN_is_zero(g) || BN_is_one(g)) {
1873+
codes |= static_cast<int>(DHPointer::CheckResult::NOT_SUITABLE_GENERATOR);
1874+
}
1875+
if (p_bits < 512) {
1876+
codes |= static_cast<int>(DHPointer::CheckResult::MODULUS_TOO_SMALL);
1877+
}
1878+
if (p_bits > OPENSSL_DH_MAX_MODULUS_BITS) {
1879+
codes |= static_cast<int>(DHPointer::CheckResult::MODULUS_TOO_LARGE);
1880+
}
1881+
1882+
BignumCtxPointer ctx(BN_CTX_new());
1883+
if (!ctx) return std::nullopt;
1884+
1885+
auto tmp1 = BignumPointer::New();
1886+
auto tmp2 = BignumPointer::New();
1887+
if (!tmp1 || !tmp2) return std::nullopt;
1888+
1889+
if (BN_copy(tmp1.get(), p) == nullptr || BN_sub_word(tmp1.get(), 1) != 1) {
1890+
return std::nullopt;
1891+
}
1892+
if (BN_cmp(g, tmp1.get()) >= 0) {
1893+
codes |= static_cast<int>(DHPointer::CheckResult::NOT_SUITABLE_GENERATOR);
1894+
}
1895+
1896+
bool q_good = false;
1897+
if (q != nullptr) {
1898+
if (BN_ucmp(p, q) > 0) {
1899+
q_good = true;
1900+
} else {
1901+
codes |= static_cast<int>(DHPointer::CheckResult::INVALID_Q);
1902+
}
1903+
}
1904+
1905+
if (q_good) {
1906+
if (BN_cmp(g, BN_value_one()) <= 0 || BN_cmp(g, p) >= 0) {
1907+
codes |= static_cast<int>(DHPointer::CheckResult::NOT_SUITABLE_GENERATOR);
1908+
} else if (BN_mod_exp(tmp1.get(), g, q, p, ctx.get()) != 1) {
1909+
return std::nullopt;
1910+
} else if (!BN_is_one(tmp1.get())) {
1911+
codes |= static_cast<int>(DHPointer::CheckResult::NOT_SUITABLE_GENERATOR);
1912+
}
1913+
1914+
const int q_is_prime = BN_check_prime(q, ctx.get(), nullptr);
1915+
if (q_is_prime < 0) return std::nullopt;
1916+
if (q_is_prime == 0) {
1917+
codes |= static_cast<int>(DHPointer::CheckResult::Q_NOT_PRIME);
1918+
}
1919+
1920+
if (BN_div(tmp1.get(), tmp2.get(), p, q, ctx.get()) != 1) {
1921+
return std::nullopt;
1922+
}
1923+
if (!BN_is_one(tmp2.get())) {
1924+
codes |= static_cast<int>(DHPointer::CheckResult::INVALID_Q);
1925+
}
1926+
if (j != nullptr && BN_cmp(j, tmp1.get()) != 0) {
1927+
codes |= static_cast<int>(DHPointer::CheckResult::INVALID_J);
1928+
}
1929+
}
1930+
1931+
const int p_is_prime = BN_check_prime(p, ctx.get(), nullptr);
1932+
if (p_is_prime < 0) return std::nullopt;
1933+
if (p_is_prime == 0) {
1934+
codes |= static_cast<int>(DHPointer::CheckResult::P_NOT_PRIME);
1935+
} else if (q == nullptr) {
1936+
if (BN_rshift1(tmp1.get(), p) != 1) return std::nullopt;
1937+
const int q_is_prime = BN_check_prime(tmp1.get(), ctx.get(), nullptr);
1938+
if (q_is_prime < 0) return std::nullopt;
1939+
if (q_is_prime == 0) {
1940+
codes |= static_cast<int>(DHPointer::CheckResult::P_NOT_SAFE_PRIME);
1941+
}
1942+
}
1943+
1944+
return codes;
1945+
}
18521946
#endif
18531947
} // namespace
18541948

@@ -1908,6 +2002,14 @@ void DHPointer::reset(
19082002

19092003
#if NCRYPTO_USE_OPENSSL3_PROVIDER
19102004
EVP_PKEY* DHPointer::release() {
2005+
if (!dh_ && p_ && g_) {
2006+
auto pkey =
2007+
group_name_ != nullptr
2008+
? NewDhPKey(group_name_, pub_key_.get(), pvt_key_.get())
2009+
: NewDhPKey(p_.get(), g_.get(), pub_key_.get(), pvt_key_.get());
2010+
if (!pkey) return nullptr;
2011+
dh_.reset(pkey.release());
2012+
}
19112013
p_.reset();
19122014
g_.reset();
19132015
pub_key_.reset();
@@ -2021,41 +2123,27 @@ DHPointer::CheckResult DHPointer::check() {
20212123

20222124
DeleteFnPtr<BIGNUM, BN_free> p;
20232125
DeleteFnPtr<BIGNUM, BN_free> g;
2126+
DeleteFnPtr<BIGNUM, BN_free> q;
2127+
DeleteFnPtr<BIGNUM, BN_free> j;
20242128
const BIGNUM* p_bn = p_.get();
20252129
const BIGNUM* g_bn = g_.get();
2026-
if ((p_bn == nullptr || g_bn == nullptr) && !GetDhParams(dh_.get(), &p, &g)) {
2130+
const BIGNUM* q_bn = nullptr;
2131+
const BIGNUM* j_bn = nullptr;
2132+
if ((p_bn == nullptr || g_bn == nullptr) &&
2133+
!GetDhParams(dh_.get(), &p, &g, &q, &j)) {
20272134
return DHPointer::CheckResult::CHECK_FAILED;
20282135
}
20292136
if (p_bn == nullptr) p_bn = p.get();
20302137
if (g_bn == nullptr) g_bn = g.get();
2138+
q_bn = q.get();
2139+
j_bn = j.get();
20312140
if (p_bn == nullptr || g_bn == nullptr) {
20322141
return DHPointer::CheckResult::CHECK_FAILED;
20332142
}
20342143

2035-
int codes = 0;
2036-
BignumCtxPointer ctx(BN_CTX_new());
2037-
if (!ctx) return DHPointer::CheckResult::CHECK_FAILED;
2038-
2039-
const int p_is_prime = BN_check_prime(p_bn, ctx.get(), nullptr);
2040-
if (p_is_prime < 0) return DHPointer::CheckResult::CHECK_FAILED;
2041-
if (p_is_prime == 0) codes |= static_cast<int>(CheckResult::P_NOT_PRIME);
2042-
2043-
auto q = BignumPointer::New();
2044-
if (!q || BN_copy(q.get(), p_bn) == nullptr || BN_sub_word(q.get(), 1) != 1 ||
2045-
BN_rshift1(q.get(), q.get()) != 1) {
2046-
return DHPointer::CheckResult::CHECK_FAILED;
2047-
}
2048-
const int q_is_prime = BN_check_prime(q.get(), ctx.get(), nullptr);
2049-
if (q_is_prime < 0) return DHPointer::CheckResult::CHECK_FAILED;
2050-
if (q_is_prime == 0) {
2051-
codes |= static_cast<int>(CheckResult::P_NOT_SAFE_PRIME);
2052-
}
2053-
2054-
if (BN_cmp(g_bn, BN_value_one()) <= 0 || BN_cmp(g_bn, p_bn) >= 0) {
2055-
codes |= static_cast<int>(CheckResult::NOT_SUITABLE_GENERATOR);
2056-
}
2057-
2058-
return static_cast<CheckResult>(codes);
2144+
auto codes = CheckDhParams(p_bn, g_bn, q_bn, j_bn);
2145+
if (!codes) return DHPointer::CheckResult::CHECK_FAILED;
2146+
return static_cast<CheckResult>(*codes);
20592147
#else
20602148
int codes = 0;
20612149
if (DH_check(dh_.get(), &codes) != 1)
@@ -2097,7 +2185,18 @@ DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey(
20972185
return DHPointer::CheckPublicKeyResult::TOO_LARGE;
20982186
}
20992187

2100-
if (p_) return CheckPublicKeyResult::NONE;
2188+
if (p_) {
2189+
if (group_name_ == nullptr) return CheckPublicKeyResult::NONE;
2190+
2191+
auto peer = NewDhPKey(group_name_, pub_key.get());
2192+
if (!peer) return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
2193+
EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(peer.get(), nullptr));
2194+
if (!ctx) return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
2195+
if (EVP_PKEY_public_check(ctx.get()) != 1) {
2196+
return DHPointer::CheckPublicKeyResult::INVALID;
2197+
}
2198+
return CheckPublicKeyResult::NONE;
2199+
}
21012200

21022201
auto peer = NewDhPKey(p_bn, g_bn, pub_key.get());
21032202
if (!peer) return DHPointer::CheckPublicKeyResult::CHECK_FAILED;
@@ -2260,14 +2359,9 @@ DataPointer DHPointer::generateKeys() {
22602359
}
22612360

22622361
if (pvt_key != nullptr) {
2263-
auto generated_pub_key = BignumPointer::New();
2264-
BignumCtxPointer ctx(BN_CTX_new());
2265-
if (!generated_pub_key || !ctx ||
2266-
BN_mod_exp(generated_pub_key.get(),
2267-
g.get(),
2268-
pvt_key.get(),
2269-
p.get(),
2270-
ctx.get()) != 1) {
2362+
BignumPointer generated_pub_key;
2363+
if (!GenerateDhPublicKey(
2364+
&generated_pub_key, p.get(), g.get(), pvt_key.get())) {
22712365
return {};
22722366
}
22732367

‎deps/ncrypto/ncrypto.gyp‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,6 @@
2323
],
2424
},
2525
'targets': [
26-
{
27-
'target_name': 'ncrypto_engine',
28-
'type': 'static_library',
29-
'include_dirs': ['.'],
30-
'defines': [
31-
'NCRYPTO_BSSL_LIBDECREPIT_MISSING=<(ncrypto_bssl_libdecrepit_missing)',
32-
],
33-
'sources': [ '<@(ncrypto_engine_sources)' ],
34-
'conditions': [
35-
['openssl_is_boringssl=="false" and openssl_version >= 0x3000000f', {
36-
'defines': [ '<@(ncrypto_engine_defines)' ],
37-
}],
38-
['node_shared_openssl=="false"', {
39-
'dependencies': [
40-
'../openssl/openssl.gyp:openssl'
41-
]
42-
}],
43-
]
44-
},
4526
{
4627
'target_name': 'ncrypto',
4728
'type': 'static_library',
@@ -80,5 +61,28 @@
8061
}],
8162
]
8263
},
83-
]
64+
],
65+
'conditions': [
66+
['openssl_is_boringssl=="false" and openssl_version >= 0x3000000f', {
67+
'targets': [
68+
{
69+
'target_name': 'ncrypto_engine',
70+
'type': 'static_library',
71+
'include_dirs': ['.'],
72+
'defines': [
73+
'NCRYPTO_BSSL_LIBDECREPIT_MISSING=<(ncrypto_bssl_libdecrepit_missing)',
74+
'<@(ncrypto_engine_defines)',
75+
],
76+
'sources': [ '<@(ncrypto_engine_sources)' ],
77+
'conditions': [
78+
['node_shared_openssl=="false"', {
79+
'dependencies': [
80+
'../openssl/openssl.gyp:openssl'
81+
]
82+
}],
83+
]
84+
},
85+
],
86+
}],
87+
],
8488
}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,6 +1215,8 @@ class DHPointer final {
12151215
// Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE
12161216
INVALID_Q = 0x20,
12171217
INVALID_J = 0x40,
1218+
MODULUS_TOO_SMALL = 0x80,
1219+
MODULUS_TOO_LARGE = 0x100,
12181220
#endif
12191221
CHECK_FAILED = 512,
12201222
};

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ if (!common.hasCrypto)
55

66
const assert = require('assert');
77
const crypto = require('crypto');
8+
const { hasOpenSSL3 } = require('../common/crypto');
89

910
// Second OAKLEY group, see
1011
// https://github.com/nodejs/node-v0.x-archive/issues/2338 and
@@ -21,6 +22,12 @@ const bad_dh = process.features.openssl_is_boringssl ?
2122
crypto.createDiffieHellman('02', 'hex');
2223
assert.notStrictEqual(bad_dh.verifyError, 0);
2324

25+
if (hasOpenSSL3) {
26+
const smallSafePrime = crypto.createDiffieHellman(
27+
Buffer.from([23]), Buffer.from([2]));
28+
assert.notStrictEqual(smallSafePrime.verifyError, 0);
29+
}
30+
2431
const availableCurves = new Set(crypto.getCurves());
2532
const availableHashes = new Set(crypto.getHashes());
2633

0 commit comments

Comments
 (0)