Skip to content

Commit 8ca4db1

Browse files
panvapanva
committed
fixup! crypto: support OpenSSL STORE private keys
1 parent 9d23b6b commit 8ca4db1

5 files changed

Lines changed: 196 additions & 63 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ namespace {
7575
using BignumCtxPointer = DeleteFnPtr<BN_CTX, BN_CTX_free>;
7676
using BignumGenCallbackPointer = DeleteFnPtr<BN_GENCB, BN_GENCB_free>;
7777
using NetscapeSPKIPointer = DeleteFnPtr<NETSCAPE_SPKI, NETSCAPE_SPKI_free>;
78+
using X509PubKeyPointer = DeleteFnPtr<X509_PUBKEY, X509_PUBKEY_free>;
7879

7980
static constexpr int kX509NameFlagsRFC2253WithinUtf8JSON =
8081
XN_FLAG_RFC2253 & ~ASN1_STRFLGS_ESC_MSB & ~ASN1_STRFLGS_ESC_CTRL;
@@ -2755,6 +2756,8 @@ Result<BIOPointer, bool> EVPKeyPointer::writePrivateKey(
27552756
#else
27562757
RSA* rsa = EVP_PKEY_get0_RSA(get());
27572758
#endif
2759+
if (rsa == nullptr) return Result<BIOPointer, bool>(false);
2760+
27582761
switch (config.format) {
27592762
case PKFormatType::PEM: {
27602763
err = PEM_write_bio_RSAPrivateKey(
@@ -2818,6 +2821,8 @@ Result<BIOPointer, bool> EVPKeyPointer::writePrivateKey(
28182821
#else
28192822
EC_KEY* ec = EVP_PKEY_get0_EC_KEY(get());
28202823
#endif
2824+
if (ec == nullptr) return Result<BIOPointer, bool>(false);
2825+
28212826
switch (config.format) {
28222827
case PKFormatType::PEM: {
28232828
err = PEM_write_bio_ECPrivateKey(
@@ -2871,6 +2876,8 @@ Result<BIOPointer, bool> EVPKeyPointer::writePublicKey(
28712876
#else
28722877
RSA* rsa = EVP_PKEY_get0_RSA(get());
28732878
#endif
2879+
if (rsa == nullptr) return Result<BIOPointer, bool>(false);
2880+
28742881
if (config.format == ncrypto::EVPKeyPointer::PKFormatType::PEM) {
28752882
// Encode PKCS#1 as PEM.
28762883
if (PEM_write_bio_RSAPublicKey(bio.get(), rsa) != 1) {
@@ -2890,7 +2897,14 @@ Result<BIOPointer, bool> EVPKeyPointer::writePublicKey(
28902897

28912898
if (config.format == ncrypto::EVPKeyPointer::PKFormatType::PEM) {
28922899
// Encode SPKI as PEM.
2893-
if (PEM_write_bio_PUBKEY(bio.get(), get()) != 1) {
2900+
X509_PUBKEY* pubkey = nullptr;
2901+
if (X509_PUBKEY_set(&pubkey, get()) != 1) {
2902+
X509_PUBKEY_free(pubkey);
2903+
return Result<BIOPointer, bool>(false,
2904+
mark_pop_error_on_return.peekError());
2905+
}
2906+
X509PubKeyPointer pubkey_ptr(pubkey);
2907+
if (PEM_write_bio_X509_PUBKEY(bio.get(), pubkey_ptr.get()) != 1) {
28942908
return Result<BIOPointer, bool>(false,
28952909
mark_pop_error_on_return.peekError());
28962910
}
@@ -2959,14 +2973,45 @@ std::optional<uint32_t> EVPKeyPointer::getBytesOfRS() const {
29592973

29602974
if (id == EVP_PKEY_DSA) {
29612975
const DSA* dsa_key = EVP_PKEY_get0_DSA(get());
2976+
bool has_bits = false;
29622977
// Both r and s are computed mod q, so their width is limited by that of q.
2963-
bits = BignumPointer::GetBitCount(DSA_get0_q(dsa_key));
2978+
if (dsa_key != nullptr) {
2979+
const BIGNUM* q = DSA_get0_q(dsa_key);
2980+
if (q != nullptr) {
2981+
bits = BignumPointer::GetBitCount(q);
2982+
has_bits = true;
2983+
}
2984+
}
2985+
#if OPENSSL_VERSION_MAJOR >= 3 && !defined(OPENSSL_IS_BORINGSSL)
2986+
if (!has_bits &&
2987+
EVP_PKEY_get_int_param(get(), OSSL_PKEY_PARAM_FFC_QBITS, &bits) == 1) {
2988+
has_bits = true;
2989+
}
2990+
#endif
2991+
if (!has_bits) return std::nullopt;
29642992
} else if (id == EVP_PKEY_EC) {
2965-
bits = EC_GROUP_order_bits(ECKeyPointer::GetGroup(*this));
2993+
const EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(get());
2994+
bool has_bits = false;
2995+
if (ec_key != nullptr) {
2996+
const EC_GROUP* group = ECKeyPointer::GetGroup(ec_key);
2997+
if (group != nullptr) {
2998+
bits = EC_GROUP_order_bits(group);
2999+
has_bits = true;
3000+
}
3001+
}
3002+
#if OPENSSL_VERSION_MAJOR >= 3 && !defined(OPENSSL_IS_BORINGSSL)
3003+
if (!has_bits &&
3004+
EVP_PKEY_get_int_param(get(), OSSL_PKEY_PARAM_BITS, &bits) == 1) {
3005+
has_bits = true;
3006+
}
3007+
#endif
3008+
if (!has_bits) return std::nullopt;
29663009
} else {
29673010
return std::nullopt;
29683011
}
29693012

3013+
if (bits <= 0) return std::nullopt;
3014+
29703015
return (bits + 7) / 8;
29713016
}
29723017

@@ -2997,16 +3042,18 @@ EVPKeyPointer::operator Dsa() const {
29973042

29983043
bool EVPKeyPointer::validateDsaParameters() const {
29993044
if (!pkey_) return false;
3000-
/* Validate DSA2 parameters from FIPS 186-4 */
3045+
/* Validate DSA2 parameters from FIPS 186-4 */
30013046
#if OPENSSL_VERSION_MAJOR >= 3
30023047
if (EVP_default_properties_is_fips_enabled(nullptr) && EVP_PKEY_DSA == id()) {
30033048
#else
30043049
if (FIPS_mode() && EVP_PKEY_DSA == id()) {
30053050
#endif
30063051
const DSA* dsa = EVP_PKEY_get0_DSA(pkey_.get());
3052+
if (dsa == nullptr) return false;
30073053
const BIGNUM* p;
30083054
const BIGNUM* q;
30093055
DSA_get0_pqg(dsa, &p, &q, nullptr);
3056+
if (p == nullptr || q == nullptr) return false;
30103057
int L = BignumPointer::GetBitCount(p);
30113058
int N = BignumPointer::GetBitCount(q);
30123059

‎src/crypto/crypto_dsa.cc‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,22 @@
11
#include "crypto/crypto_dsa.h"
2+
#include "async_wrap-inl.h"
23
#include "crypto/crypto_keys.h"
34
#include "crypto/crypto_util.h"
4-
#include "async_wrap-inl.h"
55
#include "env-inl.h"
66
#include "memory_tracker-inl.h"
77
#include "threadpoolwork-inl.h"
88
#include "v8.h"
99

1010
#include <openssl/bn.h>
11+
#include <openssl/core_names.h>
1112
#include <openssl/dsa.h>
13+
#include <openssl/evp.h>
1214

1315
#include <cstdio>
1416

1517
namespace node {
1618

19+
using ncrypto::BignumPointer;
1720
using ncrypto::Dsa;
1821
using ncrypto::EVPKeyCtxPointer;
1922
using v8::FunctionCallbackInfo;
@@ -62,7 +65,7 @@ Maybe<void> DsaKeyGenTraits::AdditionalConfig(
6265
const FunctionCallbackInfo<Value>& args,
6366
unsigned int* offset,
6467
DsaKeyPairGenConfig* params) {
65-
CHECK(args[*offset]->IsUint32()); // modulus bits
68+
CHECK(args[*offset]->IsUint32()); // modulus bits
6669
CHECK(args[*offset + 1]->IsInt32()); // divisor bits
6770

6871
params->params.modulus_bits = args[*offset].As<Uint32>()->Value();
@@ -74,16 +77,10 @@ Maybe<void> DsaKeyGenTraits::AdditionalConfig(
7477
return JustVoid();
7578
}
7679

77-
bool GetDsaKeyDetail(Environment* env,
78-
const KeyObjectData& key,
79-
Local<Object> target) {
80-
if (!key) return false;
81-
Dsa dsa = key.GetAsymmetricKey();
82-
if (!dsa) return false;
83-
84-
size_t modulus_length = dsa.getModulusLength();
85-
size_t divisor_length = dsa.getDivisorLength();
86-
80+
static bool SetDsaKeyDetail(Environment* env,
81+
Local<Object> target,
82+
size_t modulus_length,
83+
size_t divisor_length) {
8784
return target
8885
->Set(env->context(),
8986
env->modulus_length_string(),
@@ -98,6 +95,38 @@ bool GetDsaKeyDetail(Environment* env,
9895
.IsJust();
9996
}
10097

98+
bool GetDsaKeyDetail(Environment* env,
99+
const KeyObjectData& key,
100+
Local<Object> target) {
101+
if (!key) return false;
102+
const auto& m_pkey = key.GetAsymmetricKey();
103+
Dsa dsa = m_pkey;
104+
if (!dsa) {
105+
#if OPENSSL_VERSION_MAJOR >= 3 && !defined(OPENSSL_IS_BORINGSSL)
106+
BIGNUM* p = nullptr;
107+
BIGNUM* q = nullptr;
108+
BignumPointer p_ptr;
109+
BignumPointer q_ptr;
110+
if (EVP_PKEY_get_bn_param(m_pkey.get(), OSSL_PKEY_PARAM_FFC_P, &p) == 1) {
111+
p_ptr.reset(p);
112+
}
113+
if (EVP_PKEY_get_bn_param(m_pkey.get(), OSSL_PKEY_PARAM_FFC_Q, &q) == 1) {
114+
q_ptr.reset(q);
115+
}
116+
if (p_ptr && q_ptr) {
117+
return SetDsaKeyDetail(env,
118+
target,
119+
BignumPointer::GetBitCount(p_ptr.get()),
120+
BignumPointer::GetBitCount(q_ptr.get()));
121+
}
122+
#endif
123+
return false;
124+
}
125+
126+
return SetDsaKeyDetail(
127+
env, target, dsa.getModulusLength(), dsa.getDivisorLength());
128+
}
129+
101130
namespace DSAAlg {
102131
void Initialize(Environment* env, Local<Object> target) {
103132
DsaKeyPairGenJob::Initialize(env, target);

‎src/crypto/crypto_ec.cc‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111
#include "v8.h"
1212

1313
#include <openssl/bn.h>
14+
#include <openssl/core_names.h>
1415
#include <openssl/ec.h>
1516
#include <openssl/ecdh.h>
17+
#include <openssl/evp.h>
1618

1719
#include <algorithm>
1820

@@ -470,14 +472,14 @@ bool ExportJWKEcKey(Environment* env,
470472
CHECK_EQ(m_pkey.id(), EVP_PKEY_EC);
471473

472474
const EC_KEY* ec = m_pkey;
473-
CHECK_NOT_NULL(ec);
475+
if (ec == nullptr) return false;
474476

475477
const auto pub = ECKeyPointer::GetPublicKey(ec);
476478
const auto group = ECKeyPointer::GetGroup(ec);
477479

478480
int degree_bits = EC_GROUP_get_degree(group);
479481
int degree_bytes =
480-
(degree_bits / CHAR_BIT) + (7 + (degree_bits % CHAR_BIT)) / 8;
482+
(degree_bits / CHAR_BIT) + (7 + (degree_bits % CHAR_BIT)) / 8;
481483

482484
auto x = BignumPointer::New();
483485
auto y = BignumPointer::New();
@@ -752,7 +754,25 @@ bool GetEcKeyDetail(Environment* env,
752754
CHECK_EQ(m_pkey.id(), EVP_PKEY_EC);
753755

754756
const EC_KEY* ec = m_pkey;
755-
CHECK_NOT_NULL(ec);
757+
if (ec == nullptr) {
758+
#if OPENSSL_VERSION_MAJOR >= 3 && !defined(OPENSSL_IS_BORINGSSL)
759+
char group_name[80];
760+
size_t group_name_len = 0;
761+
if (EVP_PKEY_get_utf8_string_param(m_pkey.get(),
762+
OSSL_PKEY_PARAM_GROUP_NAME,
763+
group_name,
764+
sizeof(group_name),
765+
&group_name_len) == 1 &&
766+
group_name_len > 0) {
767+
return target
768+
->Set(env->context(),
769+
env->named_curve_string(),
770+
OneByteString(env->isolate(), group_name, group_name_len))
771+
.IsJust();
772+
}
773+
#endif
774+
return true;
775+
}
756776

757777
const auto group = ECKeyPointer::GetGroup(ec);
758778
int nid = EC_GROUP_get_curve_name(group);
@@ -764,18 +784,5 @@ bool GetEcKeyDetail(Environment* env,
764784
.IsJust();
765785
}
766786

767-
// WebCrypto requires a different format for ECDSA signatures than
768-
// what OpenSSL produces, so we need to convert between them. The
769-
// implementation here is a adapted from Chromium's impl here:
770-
// https://github.com/chromium/chromium/blob/7af6cfd/components/webcrypto/algorithms/ecdsa.cc
771-
772-
size_t GroupOrderSize(const EVPKeyPointer& key) {
773-
const EC_KEY* ec = key;
774-
CHECK_NOT_NULL(ec);
775-
auto order = BignumPointer::New();
776-
CHECK(order);
777-
CHECK(EC_GROUP_get_order(ECKeyPointer::GetGroup(ec), order.get(), nullptr));
778-
return order.byteLength();
779-
}
780787
} // namespace crypto
781788
} // namespace node

‎src/crypto/crypto_keys.cc‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,11 @@ bool KeyObjectData::ToEncodedPublicKey(
387387
const auto& pkey = GetAsymmetricKey();
388388
if (pkey.id() == EVP_PKEY_EC) {
389389
const EC_KEY* ec_key = pkey;
390-
CHECK_NOT_NULL(ec_key);
390+
if (ec_key == nullptr) {
391+
THROW_ERR_CRYPTO_OPERATION_FAILED(env,
392+
"Failed to export EC public key");
393+
return false;
394+
}
391395
auto form = static_cast<point_conversion_form_t>(config.ec_point_form);
392396
const auto group = ECKeyPointer::GetGroup(ec_key);
393397
const auto point = ECKeyPointer::GetPublicKey(ec_key);
@@ -433,9 +437,17 @@ bool KeyObjectData::ToEncodedPrivateKey(
433437
const auto& pkey = GetAsymmetricKey();
434438
if (pkey.id() == EVP_PKEY_EC) {
435439
const EC_KEY* ec_key = pkey;
436-
CHECK_NOT_NULL(ec_key);
440+
if (ec_key == nullptr) {
441+
THROW_ERR_CRYPTO_OPERATION_FAILED(env,
442+
"Failed to export EC private key");
443+
return false;
444+
}
437445
const BIGNUM* private_key = ECKeyPointer::GetPrivateKey(ec_key);
438-
CHECK_NOT_NULL(private_key);
446+
if (private_key == nullptr) {
447+
THROW_ERR_CRYPTO_OPERATION_FAILED(env,
448+
"Failed to export EC private key");
449+
return false;
450+
}
439451
const auto group = ECKeyPointer::GetGroup(ec_key);
440452
auto order = BignumPointer::New();
441453
CHECK(order);
@@ -1539,7 +1551,10 @@ void KeyObjectHandle::ExportECPublicRaw(
15391551
}
15401552

15411553
const EC_KEY* ec_key = m_pkey;
1542-
CHECK_NOT_NULL(ec_key);
1554+
if (ec_key == nullptr) {
1555+
return THROW_ERR_CRYPTO_OPERATION_FAILED(env,
1556+
"Failed to export EC public key");
1557+
}
15431558

15441559
CHECK(args[0]->IsInt32());
15451560
auto form =
@@ -1570,10 +1585,16 @@ void KeyObjectHandle::ExportECPrivateRaw(
15701585
}
15711586

15721587
const EC_KEY* ec_key = m_pkey;
1573-
CHECK_NOT_NULL(ec_key);
1588+
if (ec_key == nullptr) {
1589+
return THROW_ERR_CRYPTO_OPERATION_FAILED(env,
1590+
"Failed to export EC private key");
1591+
}
15741592

15751593
const BIGNUM* private_key = ECKeyPointer::GetPrivateKey(ec_key);
1576-
CHECK_NOT_NULL(private_key);
1594+
if (private_key == nullptr) {
1595+
return THROW_ERR_CRYPTO_OPERATION_FAILED(env,
1596+
"Failed to export EC private key");
1597+
}
15771598

15781599
const auto group = ECKeyPointer::GetGroup(ec_key);
15791600
auto order = BignumPointer::New();
@@ -1630,6 +1651,8 @@ void KeyObjectHandle::ExportJWK(
16301651

16311652
if (ExportJWKInner(env, key->Data(), args[0], args[1]->IsTrue())) {
16321653
args.GetReturnValue().Set(args[0]);
1654+
} else if (!env->isolate()->HasPendingException()) {
1655+
THROW_ERR_CRYPTO_OPERATION_FAILED(env, "Failed to export JWK");
16331656
}
16341657
}
16351658

0 commit comments

Comments
 (0)