Skip to content

Commit

Permalink
src: use EVPKeyPointer in more places
Browse files Browse the repository at this point in the history 
Rejoice, the code base is now free of manual EVP_PKEY_free() calls!

PR-URL: nodejs#26632
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@bnoordhuis @danbev
bnoordhuis authored and danbev committed Mar 18, 2019
1 parent 0f745bf commit 3610155
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 52 deletions.
71 changes: 28 additions & 43 deletions src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2233,16 +2233,17 @@ void SSLWrap<Base>::GetEphemeralKeyInfo(

Local<Object> info = Object::New(env->isolate());

EVP_PKEY* key;

if (SSL_get_server_tmp_key(w->ssl_.get(), &key)) {
int kid = EVP_PKEY_id(key);
EVP_PKEY* raw_key;
if (SSL_get_server_tmp_key(w->ssl_.get(), &raw_key)) {
EVPKeyPointer key(raw_key);
int kid = EVP_PKEY_id(key.get());
switch (kid) {
case EVP_PKEY_DH:
info->Set(context, env->type_string(),
FIXED_ONE_BYTE_STRING(env->isolate(), "DH")).FromJust();
info->Set(context, env->size_string(),
Integer::New(env->isolate(), EVP_PKEY_bits(key))).FromJust();
Integer::New(env->isolate(), EVP_PKEY_bits(key.get())))
.FromJust();
break;
case EVP_PKEY_EC:
// TODO(shigeki) Change this to EVP_PKEY_X25519 and add EVP_PKEY_X448
Expand All @@ -2251,7 +2252,7 @@ void SSLWrap<Base>::GetEphemeralKeyInfo(
{
const char* curve_name;
if (kid == EVP_PKEY_EC) {
EC_KEY* ec = EVP_PKEY_get1_EC_KEY(key);
EC_KEY* ec = EVP_PKEY_get1_EC_KEY(key.get());
int nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
curve_name = OBJ_nid2sn(nid);
EC_KEY_free(ec);
Expand All @@ -2265,11 +2266,10 @@ void SSLWrap<Base>::GetEphemeralKeyInfo(
curve_name)).FromJust();
info->Set(context, env->size_string(),
Integer::New(env->isolate(),
EVP_PKEY_bits(key))).FromJust();
EVP_PKEY_bits(key.get()))).FromJust();
}
break;
}
EVP_PKEY_free(key);
}

return args.GetReturnValue().Set(info);
Expand Down Expand Up @@ -3138,7 +3138,7 @@ static ManagedEVPPKey GetPrivateKeyFromJs(
ParsePrivateKey(config.Release(), key.get(), key.size());
if (!pkey)
ThrowCryptoError(env, ERR_get_error(), "Failed to read private key");
return ManagedEVPPKey(pkey.release());
return ManagedEVPPKey(std::move(pkey));
} else {
CHECK(args[*offset]->IsObject() && allow_key_object);
KeyObject* key;
Expand Down Expand Up @@ -3197,7 +3197,7 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
}
if (!pkey)
ThrowCryptoError(env, ERR_get_error(), "Failed to read asymmetric key");
return ManagedEVPPKey(pkey.release());
return ManagedEVPPKey(std::move(pkey));
} else {
CHECK(args[*offset]->IsObject());
KeyObject* key = Unwrap<KeyObject>(args[*offset].As<Object>());
Expand Down Expand Up @@ -3287,42 +3287,27 @@ static MaybeLocal<Value> WritePrivateKey(
return BIOToStringOrBuffer(env, bio.get(), config.format_);
}

ManagedEVPPKey::ManagedEVPPKey() : pkey_(nullptr) {}

ManagedEVPPKey::ManagedEVPPKey(EVP_PKEY* pkey) : pkey_(pkey) {}
ManagedEVPPKey::ManagedEVPPKey(EVPKeyPointer&& pkey) : pkey_(std::move(pkey)) {}

ManagedEVPPKey::ManagedEVPPKey(const ManagedEVPPKey& key) : pkey_(nullptr) {
*this = key;
ManagedEVPPKey::ManagedEVPPKey(const ManagedEVPPKey& that) {
*this = that;
}

ManagedEVPPKey::ManagedEVPPKey(ManagedEVPPKey&& key) {
*this = key;
}
ManagedEVPPKey& ManagedEVPPKey::operator=(const ManagedEVPPKey& that) {
pkey_.reset(that.get());

ManagedEVPPKey::~ManagedEVPPKey() {
EVP_PKEY_free(pkey_);
}

ManagedEVPPKey& ManagedEVPPKey::operator=(const ManagedEVPPKey& key) {
EVP_PKEY_free(pkey_);
pkey_ = key.pkey_;
EVP_PKEY_up_ref(pkey_);
return *this;
}
if (pkey_)
EVP_PKEY_up_ref(pkey_.get());

ManagedEVPPKey& ManagedEVPPKey::operator=(ManagedEVPPKey&& key) {
EVP_PKEY_free(pkey_);
pkey_ = key.pkey_;
key.pkey_ = nullptr;
return *this;
}

ManagedEVPPKey::operator bool() const {
return pkey_ != nullptr;
return !!pkey_;
}

EVP_PKEY* ManagedEVPPKey::get() const {
return pkey_;
return pkey_.get();
}

Local<Function> KeyObject::Initialize(Environment* env, Local<Object> target) {
Expand Down Expand Up @@ -5704,13 +5689,13 @@ class DSAKeyPairGenerationConfig : public KeyPairGenerationConfig {
}
}

EVP_PKEY* params = nullptr;
if (EVP_PKEY_paramgen(param_ctx.get(), &params) <= 0)
EVP_PKEY* raw_params = nullptr;
if (EVP_PKEY_paramgen(param_ctx.get(), &raw_params) <= 0)
return nullptr;
EVPKeyPointer params(raw_params);
param_ctx.reset();

EVPKeyCtxPointer key_ctx(EVP_PKEY_CTX_new(params, nullptr));
EVP_PKEY_free(params);
EVPKeyCtxPointer key_ctx(EVP_PKEY_CTX_new(params.get(), nullptr));
return key_ctx;
}

Expand Down Expand Up @@ -5739,13 +5724,13 @@ class ECKeyPairGenerationConfig : public KeyPairGenerationConfig {
if (EVP_PKEY_CTX_set_ec_param_enc(param_ctx.get(), param_encoding_) <= 0)
return nullptr;

EVP_PKEY* params = nullptr;
if (EVP_PKEY_paramgen(param_ctx.get(), &params) <= 0)
EVP_PKEY* raw_params = nullptr;
if (EVP_PKEY_paramgen(param_ctx.get(), &raw_params) <= 0)
return nullptr;
EVPKeyPointer params(raw_params);
param_ctx.reset();

EVPKeyCtxPointer key_ctx(EVP_PKEY_CTX_new(params, nullptr));
EVP_PKEY_free(params);
EVPKeyCtxPointer key_ctx(EVP_PKEY_CTX_new(params.get(), nullptr));
return key_ctx;
}

Expand Down Expand Up @@ -5793,7 +5778,7 @@ class GenerateKeyPairJob : public CryptoJob {
EVP_PKEY* pkey = nullptr;
if (EVP_PKEY_keygen(ctx.get(), &pkey) != 1)
return false;
pkey_ = ManagedEVPPKey(pkey);
pkey_ = ManagedEVPPKey(EVPKeyPointer(pkey));
return true;
}

Expand Down
14 changes: 5 additions & 9 deletions src/node_crypto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,20 +421,16 @@ enum KeyType {
// use.
class ManagedEVPPKey {
public:
ManagedEVPPKey();
explicit ManagedEVPPKey(EVP_PKEY* pkey);
ManagedEVPPKey(const ManagedEVPPKey& key);
ManagedEVPPKey(ManagedEVPPKey&& key);
~ManagedEVPPKey();

ManagedEVPPKey& operator=(const ManagedEVPPKey& key);
ManagedEVPPKey& operator=(ManagedEVPPKey&& key);
ManagedEVPPKey() = default;
explicit ManagedEVPPKey(EVPKeyPointer&& pkey);
ManagedEVPPKey(const ManagedEVPPKey& that);
ManagedEVPPKey& operator=(const ManagedEVPPKey& that);

operator bool() const;
EVP_PKEY* get() const;

private:
EVP_PKEY* pkey_;
EVPKeyPointer pkey_;
};

class KeyObject : public BaseObject {
Expand Down

0 comments on commit 3610155

Please sign in to comment.