Skip to content

Commit 49f2ae2

Browse files
panvaaduh95
panva
authored and
aduh95
committed
crypto: cleanse provider private key copies
Clear provider-exported RSA, EC, and DH private BIGNUMs before freeing them. Also cleanse OSSL_PARAM builder copies and the plaintext DER intermediate used for encrypted traditional PEM output. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #64547 Backport-PR-URL: #65087 Refs: #64211 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent a5d4ac8 commit 49f2ae2

2 files changed

Lines changed: 45 additions & 35 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,19 @@ const EVP_MD* GetDigestCtxMd(const EVP_MD_CTX* ctx) {
9494

9595
#if NCRYPTO_USE_OPENSSL3_PROVIDER
9696
using OSSLParamBldPointer = DeleteFnPtr<OSSL_PARAM_BLD, OSSL_PARAM_BLD_free>;
97-
using OSSLParamPointer = DeleteFnPtr<OSSL_PARAM, OSSL_PARAM_free>;
97+
struct OSSLParamDeleter {
98+
void operator()(OSSL_PARAM* params) const {
99+
if (params == nullptr) return;
100+
for (OSSL_PARAM* param = params; param->key != nullptr; param++) {
101+
if (param->data != nullptr && param->data_type != OSSL_PARAM_UTF8_PTR &&
102+
param->data_type != OSSL_PARAM_OCTET_PTR) {
103+
OPENSSL_cleanse(param->data, param->data_size);
104+
}
105+
}
106+
OSSL_PARAM_free(params);
107+
}
108+
};
109+
using OSSLParamPointer = std::unique_ptr<OSSL_PARAM, OSSLParamDeleter>;
98110
struct OpenSSLBufferDeleter {
99111
void operator()(unsigned char* pointer) const { OPENSSL_free(pointer); }
100112
};
@@ -106,9 +118,8 @@ static constexpr int kX509NameFlagsRFC2253WithinUtf8JSON =
106118
XN_FLAG_RFC2253 & ~ASN1_STRFLGS_ESC_MSB & ~ASN1_STRFLGS_ESC_CTRL;
107119

108120
#if NCRYPTO_USE_OPENSSL3_PROVIDER
109-
bool GetPKeyBnParam(const EVP_PKEY* pkey,
110-
const char* name,
111-
DeleteFnPtr<BIGNUM, BN_free>* out) {
121+
template <typename Pointer>
122+
bool GetPKeyBnParam(const EVP_PKEY* pkey, const char* name, Pointer* out) {
112123
BIGNUM* bn = nullptr;
113124
if (pkey == nullptr) return false;
114125
if (EVP_PKEY_get_bn_param(pkey, name, &bn) == 1) {
@@ -135,9 +146,10 @@ bool GetPKeyBnParam(const EVP_PKEY* pkey,
135146
return true;
136147
}
137148

149+
template <typename Pointer>
138150
bool GetOptionalPKeyBnParam(const EVP_PKEY* pkey,
139151
const char* name,
140-
DeleteFnPtr<BIGNUM, BN_free>* out) {
152+
Pointer* out) {
141153
BIGNUM* bn = nullptr;
142154
if (pkey == nullptr) {
143155
out->reset();
@@ -273,9 +285,11 @@ bool GetDhParams(const EVP_PKEY* pkey,
273285

274286
bool GetDhKeys(const EVP_PKEY* pkey,
275287
DeleteFnPtr<BIGNUM, BN_free>* pub,
276-
DeleteFnPtr<BIGNUM, BN_free>* priv) {
277-
return GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_PUB_KEY, pub) &&
278-
GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_PRIV_KEY, priv);
288+
DeleteFnPtr<BIGNUM, BN_clear_free>* priv) {
289+
return (pub == nullptr ||
290+
GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_PUB_KEY, pub)) &&
291+
(priv == nullptr ||
292+
GetOptionalPKeyBnParam(pkey, OSSL_PKEY_PARAM_PRIV_KEY, priv));
279293
}
280294
#endif
281295

@@ -2290,8 +2304,7 @@ DataPointer DHPointer::getPublicKey() const {
22902304
if (!dh_) return {};
22912305

22922306
DeleteFnPtr<BIGNUM, BN_free> pub_key;
2293-
DeleteFnPtr<BIGNUM, BN_free> pvt_key;
2294-
if (!GetDhKeys(dh_.get(), &pub_key, &pvt_key)) return {};
2307+
if (!GetDhKeys(dh_.get(), &pub_key, nullptr)) return {};
22952308
return BignumPointer::Encode(pub_key.get());
22962309
#else
22972310
const BIGNUM* pub_key;
@@ -2306,9 +2319,8 @@ DataPointer DHPointer::getPrivateKey() const {
23062319
if (pvt_key_) return pvt_key_.encode();
23072320
if (!dh_) return {};
23082321

2309-
DeleteFnPtr<BIGNUM, BN_free> pub_key;
2310-
DeleteFnPtr<BIGNUM, BN_free> pvt_key;
2311-
if (!GetDhKeys(dh_.get(), &pub_key, &pvt_key)) return {};
2322+
DeleteFnPtr<BIGNUM, BN_clear_free> pvt_key;
2323+
if (!GetDhKeys(dh_.get(), nullptr, &pvt_key)) return {};
23122324
return BignumPointer::Encode(pvt_key.get());
23132325
#else
23142326
const BIGNUM* pvt_key;
@@ -2323,9 +2335,8 @@ bool DHPointer::hasPrivateKey() const {
23232335
if (pvt_key_) return true;
23242336
if (!dh_) return false;
23252337

2326-
DeleteFnPtr<BIGNUM, BN_free> pub_key;
2327-
DeleteFnPtr<BIGNUM, BN_free> pvt_key;
2328-
if (!GetDhKeys(dh_.get(), &pub_key, &pvt_key)) return false;
2338+
DeleteFnPtr<BIGNUM, BN_clear_free> pvt_key;
2339+
if (!GetDhKeys(dh_.get(), nullptr, &pvt_key)) return false;
23292340
return pvt_key != nullptr;
23302341
#else
23312342
const BIGNUM* pvt_key = nullptr;
@@ -2361,7 +2372,7 @@ DataPointer DHPointer::generateKeys() {
23612372
DeleteFnPtr<BIGNUM, BN_free> p;
23622373
DeleteFnPtr<BIGNUM, BN_free> g;
23632374
DeleteFnPtr<BIGNUM, BN_free> pub_key;
2364-
DeleteFnPtr<BIGNUM, BN_free> pvt_key;
2375+
DeleteFnPtr<BIGNUM, BN_clear_free> pvt_key;
23652376
if (!GetDhParams(dh_.get(), &p, &g) ||
23662377
!GetDhKeys(dh_.get(), &pub_key, &pvt_key)) {
23672378
return {};
@@ -2496,9 +2507,8 @@ bool DHPointer::setPublicKey(BignumPointer&& key) {
24962507
return true;
24972508
}
24982509

2499-
DeleteFnPtr<BIGNUM, BN_free> pub_key;
2500-
DeleteFnPtr<BIGNUM, BN_free> pvt_key;
2501-
if (!GetDhKeys(dh_.get(), &pub_key, &pvt_key)) {
2510+
DeleteFnPtr<BIGNUM, BN_clear_free> pvt_key;
2511+
if (!GetDhKeys(dh_.get(), nullptr, &pvt_key)) {
25022512
return false;
25032513
}
25042514
EVPKeyPointer pkey;
@@ -2535,8 +2545,7 @@ bool DHPointer::setPrivateKey(BignumPointer&& key) {
25352545
}
25362546

25372547
DeleteFnPtr<BIGNUM, BN_free> pub_key;
2538-
DeleteFnPtr<BIGNUM, BN_free> pvt_key;
2539-
if (!GetDhKeys(dh_.get(), &pub_key, &pvt_key)) {
2548+
if (!GetDhKeys(dh_.get(), &pub_key, nullptr)) {
25402549
return false;
25412550
}
25422551
EVPKeyPointer pkey;
@@ -3537,12 +3546,13 @@ bool WriteEncryptedTraditionalPEM(BIO* bio,
35373546
size_t der_len = 0;
35383547
OSSLEncoderCtxPointer ctx(OSSL_ENCODER_CTX_new_for_pkey(
35393548
pkey, OSSL_KEYMGMT_SELECT_KEYPAIR, "DER", "pkcs1", nullptr));
3540-
if (!ctx || OSSL_ENCODER_to_data(ctx.get(), &der, &der_len) != 1) {
3541-
return false;
3542-
}
3549+
if (!ctx) return false;
3550+
3551+
const int result = OSSL_ENCODER_to_data(ctx.get(), &der, &der_len);
3552+
DataPointer der_storage(der, der_len);
3553+
if (result != 1) return false;
35433554

3544-
OpenSSLBufferPointer der_storage(der);
3545-
DERView der_view{der_storage.get(), der_len};
3555+
DERView der_view{der_storage.get<const unsigned char>(), der_len};
35463556
return PEM_ASN1_write_bio(
35473557
WriteDERView,
35483558
PEM_STRING_RSA,
@@ -4977,7 +4987,7 @@ bool ECKeyPointer::generate() {
49774987
if (EVP_PKEY_keygen(ctx.get(), &raw) != 1) return false;
49784988
EVPKeyPointer pkey(raw);
49794989

4980-
DeleteFnPtr<BIGNUM, BN_free> priv;
4990+
DeleteFnPtr<BIGNUM, BN_clear_free> priv;
49814991
if (!GetPKeyBnParam(pkey.get(), OSSL_PKEY_PARAM_PRIV_KEY, &priv)) {
49824992
return false;
49834993
}

‎deps/ncrypto/ncrypto.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -641,12 +641,12 @@ class Rsa final {
641641
bool rsa_ = false;
642642
DeleteFnPtr<BIGNUM, BN_free> n_;
643643
DeleteFnPtr<BIGNUM, BN_free> e_;
644-
DeleteFnPtr<BIGNUM, BN_free> d_;
645-
DeleteFnPtr<BIGNUM, BN_free> p_;
646-
DeleteFnPtr<BIGNUM, BN_free> q_;
647-
DeleteFnPtr<BIGNUM, BN_free> dp_;
648-
DeleteFnPtr<BIGNUM, BN_free> dq_;
649-
DeleteFnPtr<BIGNUM, BN_free> qi_;
644+
DeleteFnPtr<BIGNUM, BN_clear_free> d_;
645+
DeleteFnPtr<BIGNUM, BN_clear_free> p_;
646+
DeleteFnPtr<BIGNUM, BN_clear_free> q_;
647+
DeleteFnPtr<BIGNUM, BN_clear_free> dp_;
648+
DeleteFnPtr<BIGNUM, BN_clear_free> dq_;
649+
DeleteFnPtr<BIGNUM, BN_clear_free> qi_;
650650
std::optional<PssParams> pss_params_;
651651
#else
652652
OSSL3_CONST RSA* rsa_;
@@ -1634,7 +1634,7 @@ class ECKeyPointer final {
16341634
#if NCRYPTO_USE_OPENSSL3_PROVIDER
16351635
DeleteFnPtr<EC_GROUP, EC_GROUP_free> group_;
16361636
DeleteFnPtr<EC_POINT, EC_POINT_free> pub_;
1637-
DeleteFnPtr<BIGNUM, BN_free> priv_;
1637+
DeleteFnPtr<BIGNUM, BN_clear_free> priv_;
16381638
#else
16391639
DeleteFnPtr<EC_KEY, EC_KEY_free> key_;
16401640
#endif

0 commit comments

Comments
 (0)