Skip to content

Commit d302537

Browse files
authored
tls: avoid taking ownership of OpenSSL objects
It is often unnecessary to obtain (shared) ownership of OpenSSL objects in this code, and it generally is more costly to do so as opposed to just obtaining a pointer to the respective OpenSSL object. Therefore, this patch replaces various OpenSSL function calls that take ownership with ones that do not. PR-URL: #53436 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 15b3902 commit d302537

File tree

1 file changed

+28
-30
lines changed

1 file changed

+28
-30
lines changed

src/crypto/crypto_common.cc

+28-30
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@
2121
#include <string>
2222
#include <unordered_map>
2323

24+
// Some OpenSSL 1.1.1 functions unnecessarily operate on and return non-const
25+
// pointers, whereas the same functions in OpenSSL 3 use const pointers.
26+
#if OPENSSL_VERSION_MAJOR >= 3
27+
#define OSSL3_CONST const
28+
#else
29+
#define OSSL3_CONST
30+
#endif
31+
2432
namespace node {
2533

2634
using v8::Array;
@@ -425,20 +433,15 @@ MaybeLocal<Value> GetCurveName(Environment* env, const int nid) {
425433
MaybeLocal<Value>(Undefined(env->isolate()));
426434
}
427435

428-
MaybeLocal<Value> GetECPubKey(
429-
Environment* env,
430-
const EC_GROUP* group,
431-
const ECPointer& ec) {
432-
const EC_POINT* pubkey = EC_KEY_get0_public_key(ec.get());
436+
MaybeLocal<Value> GetECPubKey(Environment* env,
437+
const EC_GROUP* group,
438+
OSSL3_CONST EC_KEY* ec) {
439+
const EC_POINT* pubkey = EC_KEY_get0_public_key(ec);
433440
if (pubkey == nullptr)
434441
return Undefined(env->isolate());
435442

436-
return ECPointToBuffer(
437-
env,
438-
group,
439-
pubkey,
440-
EC_KEY_get_conv_form(ec.get()),
441-
nullptr).FromMaybe(Local<Object>());
443+
return ECPointToBuffer(env, group, pubkey, EC_KEY_get_conv_form(ec), nullptr)
444+
.FromMaybe(Local<Object>());
442445
}
443446

444447
MaybeLocal<Value> GetECGroupBits(Environment* env, const EC_GROUP* group) {
@@ -452,8 +455,8 @@ MaybeLocal<Value> GetECGroupBits(Environment* env, const EC_GROUP* group) {
452455
return Integer::New(env->isolate(), bits);
453456
}
454457

455-
MaybeLocal<Object> GetPubKey(Environment* env, const RSAPointer& rsa) {
456-
int size = i2d_RSA_PUBKEY(rsa.get(), nullptr);
458+
MaybeLocal<Object> GetPubKey(Environment* env, OSSL3_CONST RSA* rsa) {
459+
int size = i2d_RSA_PUBKEY(rsa, nullptr);
457460
CHECK_GE(size, 0);
458461

459462
std::unique_ptr<BackingStore> bs;
@@ -463,7 +466,7 @@ MaybeLocal<Object> GetPubKey(Environment* env, const RSAPointer& rsa) {
463466
}
464467

465468
unsigned char* serialized = reinterpret_cast<unsigned char*>(bs->Data());
466-
CHECK_GE(i2d_RSA_PUBKEY(rsa.get(), &serialized), 0);
469+
CHECK_GE(i2d_RSA_PUBKEY(rsa, &serialized), 0);
467470

468471
Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), std::move(bs));
469472
return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local<Object>());
@@ -1124,8 +1127,8 @@ MaybeLocal<Object> GetEphemeralKey(Environment* env, const SSLPointer& ssl) {
11241127
{
11251128
const char* curve_name;
11261129
if (kid == EVP_PKEY_EC) {
1127-
ECKeyPointer ec(EVP_PKEY_get1_EC_KEY(key.get()));
1128-
int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec.get()));
1130+
OSSL3_CONST EC_KEY* ec = EVP_PKEY_get0_EC_KEY(key.get());
1131+
int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
11291132
curve_name = OBJ_nid2sn(nid);
11301133
} else {
11311134
curve_name = OBJ_nid2sn(kid);
@@ -1284,24 +1287,24 @@ MaybeLocal<Object> X509ToObject(
12841287
return MaybeLocal<Object>();
12851288
}
12861289

1287-
EVPKeyPointer pkey(X509_get_pubkey(cert));
1288-
RSAPointer rsa;
1289-
ECPointer ec;
1290-
if (pkey) {
1291-
switch (EVP_PKEY_id(pkey.get())) {
1290+
OSSL3_CONST EVP_PKEY* pkey = X509_get0_pubkey(cert);
1291+
OSSL3_CONST RSA* rsa = nullptr;
1292+
OSSL3_CONST EC_KEY* ec = nullptr;
1293+
if (pkey != nullptr) {
1294+
switch (EVP_PKEY_id(pkey)) {
12921295
case EVP_PKEY_RSA:
1293-
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
1296+
rsa = EVP_PKEY_get0_RSA(pkey);
12941297
break;
12951298
case EVP_PKEY_EC:
1296-
ec.reset(EVP_PKEY_get1_EC_KEY(pkey.get()));
1299+
ec = EVP_PKEY_get0_EC_KEY(pkey);
12971300
break;
12981301
}
12991302
}
13001303

13011304
if (rsa) {
13021305
const BIGNUM* n;
13031306
const BIGNUM* e;
1304-
RSA_get0_key(rsa.get(), &n, &e, nullptr);
1307+
RSA_get0_key(rsa, &n, &e, nullptr);
13051308
if (!Set<Value>(context,
13061309
info,
13071310
env->modulus_string(),
@@ -1318,7 +1321,7 @@ MaybeLocal<Object> X509ToObject(
13181321
return MaybeLocal<Object>();
13191322
}
13201323
} else if (ec) {
1321-
const EC_GROUP* group = EC_KEY_get0_group(ec.get());
1324+
const EC_GROUP* group = EC_KEY_get0_group(ec);
13221325

13231326
if (!Set<Value>(
13241327
context, info, env->bits_string(), GetECGroupBits(env, group)) ||
@@ -1347,11 +1350,6 @@ MaybeLocal<Object> X509ToObject(
13471350
}
13481351
}
13491352

1350-
// pkey, rsa, and ec pointers are no longer needed.
1351-
pkey.reset();
1352-
rsa.reset();
1353-
ec.reset();
1354-
13551353
if (!Set<Value>(context,
13561354
info,
13571355
env->valid_from_string(),

0 commit comments

Comments
 (0)