From 15ec3819440f266e7d548709ce2c097f8dfd68d8 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 13 Mar 2019 11:46:47 +0100 Subject: [PATCH] src: use EVPKeyPointer in more places MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rejoice, the code base is now free of manual EVP_PKEY_free() calls! PR-URL: https://github.com/nodejs/node/pull/26632 Reviewed-By: Colin Ihrig Reviewed-By: Tobias Nießen Reviewed-By: Minwoo Jung Reviewed-By: James M Snell --- src/node_crypto.cc | 71 ++++++++++++++++++---------------------------- src/node_crypto.h | 14 ++++----- 2 files changed, 33 insertions(+), 52 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index c6de29693ffc53..885f02b8436cbd 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2206,16 +2206,17 @@ void SSLWrap::GetEphemeralKeyInfo( Local 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 @@ -2224,7 +2225,7 @@ void SSLWrap::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); @@ -2238,11 +2239,10 @@ void SSLWrap::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); @@ -3110,7 +3110,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; @@ -3169,7 +3169,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(args[*offset].As()); @@ -3259,42 +3259,27 @@ static MaybeLocal 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 KeyObject::Initialize(Environment* env, Local target) { @@ -5672,13 +5657,13 @@ class DSAKeyPairGenerationConfig : public KeyPairGenerationConfig { } } - EVP_PKEY* params = nullptr; - if (EVP_PKEY_paramgen(param_ctx.get(), ¶ms) <= 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; } @@ -5707,13 +5692,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(), ¶ms) <= 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; } @@ -5761,7 +5746,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; } diff --git a/src/node_crypto.h b/src/node_crypto.h index 1c66b318a2e97d..a85b0cdeb98b82 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -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 {