Skip to content

Commit

Permalink
tls: avoid taking ownership of OpenSSL objects
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tniessen committed Jun 12, 2024
1 parent 73fa9ab commit 906bd1f
Showing 1 changed file with 20 additions and 30 deletions.
50 changes: 20 additions & 30 deletions src/crypto/crypto_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -425,20 +425,15 @@ MaybeLocal<Value> GetCurveName(Environment* env, const int nid) {
MaybeLocal<Value>(Undefined(env->isolate()));
}

MaybeLocal<Value> GetECPubKey(
Environment* env,
const EC_GROUP* group,
const ECPointer& ec) {
const EC_POINT* pubkey = EC_KEY_get0_public_key(ec.get());
MaybeLocal<Value> GetECPubKey(Environment* env,
const EC_GROUP* group,
const EC_KEY* ec) {
const EC_POINT* pubkey = EC_KEY_get0_public_key(ec);
if (pubkey == nullptr)
return Undefined(env->isolate());

return ECPointToBuffer(
env,
group,
pubkey,
EC_KEY_get_conv_form(ec.get()),
nullptr).FromMaybe(Local<Object>());
return ECPointToBuffer(env, group, pubkey, EC_KEY_get_conv_form(ec), nullptr)
.FromMaybe(Local<Object>());
}

MaybeLocal<Value> GetECGroupBits(Environment* env, const EC_GROUP* group) {
Expand All @@ -452,8 +447,8 @@ MaybeLocal<Value> GetECGroupBits(Environment* env, const EC_GROUP* group) {
return Integer::New(env->isolate(), bits);
}

MaybeLocal<Object> GetPubKey(Environment* env, const RSAPointer& rsa) {
int size = i2d_RSA_PUBKEY(rsa.get(), nullptr);
MaybeLocal<Object> GetPubKey(Environment* env, const RSA* rsa) {
int size = i2d_RSA_PUBKEY(rsa, nullptr);
CHECK_GE(size, 0);

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

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

Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), std::move(bs));
return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local<Object>());
Expand Down Expand Up @@ -1125,8 +1120,8 @@ MaybeLocal<Object> GetEphemeralKey(Environment* env, const SSLPointer& ssl) {
{
const char* curve_name;
if (kid == EVP_PKEY_EC) {
ECKeyPointer ec(EVP_PKEY_get1_EC_KEY(key.get()));
int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec.get()));
const EC_KEY* ec = EVP_PKEY_get0_EC_KEY(key.get());
int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
curve_name = OBJ_nid2sn(nid);
} else {
curve_name = OBJ_nid2sn(kid);
Expand Down Expand Up @@ -1285,24 +1280,24 @@ MaybeLocal<Object> X509ToObject(
return MaybeLocal<Object>();
}

EVPKeyPointer pkey(X509_get_pubkey(cert));
RSAPointer rsa;
ECPointer ec;
if (pkey) {
switch (EVP_PKEY_id(pkey.get())) {
const EVP_PKEY* pkey = X509_get0_pubkey(cert);
const RSA* rsa = nullptr;
const EC_KEY* ec = nullptr;
if (pkey != nullptr) {
switch (EVP_PKEY_id(pkey)) {
case EVP_PKEY_RSA:
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
rsa = EVP_PKEY_get0_RSA(pkey);
break;
case EVP_PKEY_EC:
ec.reset(EVP_PKEY_get1_EC_KEY(pkey.get()));
ec = EVP_PKEY_get0_EC_KEY(pkey);
break;
}
}

if (rsa) {
const BIGNUM* n;
const BIGNUM* e;
RSA_get0_key(rsa.get(), &n, &e, nullptr);
RSA_get0_key(rsa, &n, &e, nullptr);
if (!Set<Value>(context,
info,
env->modulus_string(),
Expand All @@ -1319,7 +1314,7 @@ MaybeLocal<Object> X509ToObject(
return MaybeLocal<Object>();
}
} else if (ec) {
const EC_GROUP* group = EC_KEY_get0_group(ec.get());
const EC_GROUP* group = EC_KEY_get0_group(ec);

if (!Set<Value>(
context, info, env->bits_string(), GetECGroupBits(env, group)) ||
Expand Down Expand Up @@ -1348,11 +1343,6 @@ MaybeLocal<Object> X509ToObject(
}
}

// pkey, rsa, and ec pointers are no longer needed.
pkey.reset();
rsa.reset();
ec.reset();

if (!Set<Value>(context,
info,
env->valid_from_string(),
Expand Down

0 comments on commit 906bd1f

Please sign in to comment.