Skip to content

Commit f974aaf

Browse files
committed
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.
1 parent 73fa9ab commit f974aaf

File tree

1 file changed

+28
-30
lines changed

1 file changed

+28
-30
lines changed

src/crypto/crypto_common.cc

Lines changed: 28 additions & 30 deletions
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>());
@@ -1125,8 +1128,8 @@ MaybeLocal<Object> GetEphemeralKey(Environment* env, const SSLPointer& ssl) {
11251128
{
11261129
const char* curve_name;
11271130
if (kid == EVP_PKEY_EC) {
1128-
ECKeyPointer ec(EVP_PKEY_get1_EC_KEY(key.get()));
1129-
int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec.get()));
1131+
OSSL3_CONST EC_KEY* ec = EVP_PKEY_get0_EC_KEY(key.get());
1132+
int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
11301133
curve_name = OBJ_nid2sn(nid);
11311134
} else {
11321135
curve_name = OBJ_nid2sn(kid);
@@ -1285,24 +1288,24 @@ MaybeLocal<Object> X509ToObject(
12851288
return MaybeLocal<Object>();
12861289
}
12871290

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

13021305
if (rsa) {
13031306
const BIGNUM* n;
13041307
const BIGNUM* e;
1305-
RSA_get0_key(rsa.get(), &n, &e, nullptr);
1308+
RSA_get0_key(rsa, &n, &e, nullptr);
13061309
if (!Set<Value>(context,
13071310
info,
13081311
env->modulus_string(),
@@ -1319,7 +1322,7 @@ MaybeLocal<Object> X509ToObject(
13191322
return MaybeLocal<Object>();
13201323
}
13211324
} else if (ec) {
1322-
const EC_GROUP* group = EC_KEY_get0_group(ec.get());
1325+
const EC_GROUP* group = EC_KEY_get0_group(ec);
13231326

13241327
if (!Set<Value>(
13251328
context, info, env->bits_string(), GetECGroupBits(env, group)) ||
@@ -1348,11 +1351,6 @@ MaybeLocal<Object> X509ToObject(
13481351
}
13491352
}
13501353

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

0 commit comments

Comments
 (0)