From 9ab4a7e5ce5d57b5218829466657853eb16be747 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 6 Dec 2019 13:45:40 +0100 Subject: [PATCH] src: move BaseObject subclass dtors/ctors out of node_crypto.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally landed in the QUIC repo Move constructor and destructors for subclasses of `BaseObject` from node_crypto.h to node_crypto.cc. This removes the need to include base_object-inl.h when using node_crypto.h in some cases. Original review metadata: ``` PR-URL: https://github.com/nodejs/quic/pull/220 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell ``` PR-URL: https://github.com/nodejs/node/pull/31872 Reviewed-By: Sam Roberts Reviewed-By: David Carlier Reviewed-By: Colin Ihrig Reviewed-By: Denys Otrishko Reviewed-By: Tobias Nießen --- src/node_crypto.cc | 79 +++++++++++++++++++++++++++++++++++++ src/node_crypto.h | 98 +++++++++------------------------------------- 2 files changed, 97 insertions(+), 80 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index fa1997fa3dc45a..5017e25e3d4a12 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -531,6 +531,24 @@ void SecureContext::Initialize(Environment* env, Local target) { env->set_secure_context_constructor_template(t); } +SecureContext::SecureContext(Environment* env, v8::Local wrap) + : BaseObject(env, wrap) { + MakeWeak(); + env->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize); +} + +inline void SecureContext::Reset() { + if (ctx_ != nullptr) { + env()->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize); + } + ctx_.reset(); + cert_.reset(); + issuer_.reset(); +} + +SecureContext::~SecureContext() { + Reset(); +} void SecureContext::New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -3816,6 +3834,15 @@ KeyType KeyObject::GetKeyType() const { return this->key_type_; } +KeyObject::KeyObject(Environment* env, + v8::Local wrap, + KeyType key_type) + : BaseObject(env, wrap), + key_type_(key_type), + symmetric_key_(nullptr, nullptr) { + MakeWeak(); +} + void KeyObject::Init(const FunctionCallbackInfo& args) { KeyObject* key; ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder()); @@ -3958,6 +3985,17 @@ MaybeLocal KeyObject::ExportPrivateKey( return WritePrivateKey(env(), asymmetric_key_.get(), config); } +CipherBase::CipherBase(Environment* env, + v8::Local wrap, + CipherKind kind) + : BaseObject(env, wrap), + ctx_(nullptr), + kind_(kind), + auth_tag_state_(kAuthTagUnknown), + auth_tag_len_(kNoAuthTagLength), + pending_auth_failed_(false) { + MakeWeak(); +} void CipherBase::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); @@ -4579,6 +4617,11 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } +Hmac::Hmac(Environment* env, v8::Local wrap) + : BaseObject(env, wrap), + ctx_(nullptr) { + MakeWeak(); +} void Hmac::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); @@ -4697,6 +4740,13 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(rc.ToLocalChecked()); } +Hash::Hash(Environment* env, v8::Local wrap) + : BaseObject(env, wrap), + mdctx_(nullptr), + has_md_(false), + md_value_(nullptr) { + MakeWeak(); +} void Hash::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); @@ -4711,6 +4761,10 @@ void Hash::Initialize(Environment* env, Local target) { t->GetFunction(env->context()).ToLocalChecked()).Check(); } +Hash::~Hash() { + if (md_value_ != nullptr) + OPENSSL_clear_free(md_value_, md_len_); +} void Hash::New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -4934,6 +4988,10 @@ void CheckThrow(Environment* env, SignBase::Error error) { } } +SignBase::SignBase(Environment* env, v8::Local wrap) + : BaseObject(env, wrap) { +} + void SignBase::CheckThrow(SignBase::Error error) { node::crypto::CheckThrow(env(), error); } @@ -4957,6 +5015,9 @@ static bool ApplyRSAOptions(const ManagedEVPPKey& pkey, } +Sign::Sign(Environment* env, v8::Local wrap) : SignBase(env, wrap) { + MakeWeak(); +} void Sign::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); @@ -5277,6 +5338,11 @@ void SignOneShot(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(signature.ToBuffer().ToLocalChecked()); } +Verify::Verify(Environment* env, v8::Local wrap) : + SignBase(env, wrap) { + MakeWeak(); +} + void Verify::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); @@ -5580,6 +5646,10 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } +DiffieHellman::DiffieHellman(Environment* env, v8::Local wrap) + : BaseObject(env, wrap), verifyError_(0) { + MakeWeak(); +} void DiffieHellman::Initialize(Environment* env, Local target) { auto make = [&] (Local name, FunctionCallback callback) { @@ -5947,6 +6017,15 @@ void ECDH::Initialize(Environment* env, Local target) { t->GetFunction(env->context()).ToLocalChecked()).Check(); } +ECDH::ECDH(Environment* env, v8::Local wrap, ECKeyPointer&& key) + : BaseObject(env, wrap), + key_(std::move(key)), + group_(EC_KEY_get0_group(key_.get())) { + MakeWeak(); + CHECK_NOT_NULL(group_); +} + +ECDH::~ECDH() {} void ECDH::New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); diff --git a/src/node_crypto.h b/src/node_crypto.h index bc973b0b3ad9e2..b2bb6b0f25a939 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -84,11 +84,9 @@ extern void UseExtraCaCerts(const std::string& file); void InitCryptoOnce(); -class SecureContext : public BaseObject { +class SecureContext final : public BaseObject { public: - ~SecureContext() override { - Reset(); - } + ~SecureContext() override; static void Initialize(Environment* env, v8::Local target); @@ -177,20 +175,8 @@ class SecureContext : public BaseObject { HMAC_CTX* hctx, int enc); - SecureContext(Environment* env, v8::Local wrap) - : BaseObject(env, wrap) { - MakeWeak(); - env->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize); - } - - inline void Reset() { - if (ctx_ != nullptr) { - env()->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize); - } - ctx_.reset(); - cert_.reset(); - issuer_.reset(); - } + SecureContext(Environment* env, v8::Local wrap); + void Reset(); }; // SSLWrap implicitly depends on the inheriting class' handle having an @@ -460,14 +446,7 @@ class KeyObject : public BaseObject { v8::MaybeLocal ExportPrivateKey( const PrivateKeyEncodingConfig& config) const; - KeyObject(Environment* env, - v8::Local wrap, - KeyType key_type) - : BaseObject(env, wrap), - key_type_(key_type), - symmetric_key_(nullptr, nullptr) { - MakeWeak(); - } + KeyObject(Environment* env, v8::Local wrap, KeyType key_type); private: const KeyType key_type_; @@ -541,17 +520,7 @@ class CipherBase : public BaseObject { static void SetAuthTag(const v8::FunctionCallbackInfo& args); static void SetAAD(const v8::FunctionCallbackInfo& args); - CipherBase(Environment* env, - v8::Local wrap, - CipherKind kind) - : BaseObject(env, wrap), - ctx_(nullptr), - kind_(kind), - auth_tag_state_(kAuthTagUnknown), - auth_tag_len_(kNoAuthTagLength), - pending_auth_failed_(false) { - MakeWeak(); - } + CipherBase(Environment* env, v8::Local wrap, CipherKind kind); private: DeleteFnPtr ctx_; @@ -581,18 +550,16 @@ class Hmac : public BaseObject { static void HmacUpdate(const v8::FunctionCallbackInfo& args); static void HmacDigest(const v8::FunctionCallbackInfo& args); - Hmac(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - ctx_(nullptr) { - MakeWeak(); - } + Hmac(Environment* env, v8::Local wrap); private: DeleteFnPtr ctx_; }; -class Hash : public BaseObject { +class Hash final : public BaseObject { public: + ~Hash() override; + static void Initialize(Environment* env, v8::Local target); // TODO(joyeecheung): track the memory used by OpenSSL types @@ -608,18 +575,7 @@ class Hash : public BaseObject { static void HashUpdate(const v8::FunctionCallbackInfo& args); static void HashDigest(const v8::FunctionCallbackInfo& args); - Hash(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - mdctx_(nullptr), - has_md_(false), - md_value_(nullptr) { - MakeWeak(); - } - - ~Hash() override { - if (md_value_ != nullptr) - OPENSSL_clear_free(md_value_, md_len_); - } + Hash(Environment* env, v8::Local wrap); private: EVPMDPointer mdctx_; @@ -641,9 +597,7 @@ class SignBase : public BaseObject { kSignMalformedSignature } Error; - SignBase(Environment* env, v8::Local wrap) - : BaseObject(env, wrap) { - } + SignBase(Environment* env, v8::Local wrap); Error Init(const char* sign_type); Error Update(const char* data, int len); @@ -689,9 +643,7 @@ class Sign : public SignBase { static void SignUpdate(const v8::FunctionCallbackInfo& args); static void SignFinal(const v8::FunctionCallbackInfo& args); - Sign(Environment* env, v8::Local wrap) : SignBase(env, wrap) { - MakeWeak(); - } + Sign(Environment* env, v8::Local wrap); }; class Verify : public SignBase { @@ -710,9 +662,7 @@ class Verify : public SignBase { static void VerifyUpdate(const v8::FunctionCallbackInfo& args); static void VerifyFinal(const v8::FunctionCallbackInfo& args); - Verify(Environment* env, v8::Local wrap) : SignBase(env, wrap) { - MakeWeak(); - } + Verify(Environment* env, v8::Local wrap); }; class PublicKeyCipher { @@ -769,11 +719,7 @@ class DiffieHellman : public BaseObject { static void VerifyErrorGetter( const v8::FunctionCallbackInfo& args); - DiffieHellman(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - verifyError_(0) { - MakeWeak(); - } + DiffieHellman(Environment* env, v8::Local wrap); // TODO(joyeecheung): track the memory used by OpenSSL types SET_NO_MEMORY_INFO() @@ -792,11 +738,9 @@ class DiffieHellman : public BaseObject { DHPointer dh_; }; -class ECDH : public BaseObject { +class ECDH final : public BaseObject { public: - ~ECDH() override { - group_ = nullptr; - } + ~ECDH() override; static void Initialize(Environment* env, v8::Local target); static ECPointPointer BufferToPoint(Environment* env, @@ -809,13 +753,7 @@ class ECDH : public BaseObject { SET_SELF_SIZE(ECDH) protected: - ECDH(Environment* env, v8::Local wrap, ECKeyPointer&& key) - : BaseObject(env, wrap), - key_(std::move(key)), - group_(EC_KEY_get0_group(key_.get())) { - MakeWeak(); - CHECK_NOT_NULL(group_); - } + ECDH(Environment* env, v8::Local wrap, ECKeyPointer&& key); static void New(const v8::FunctionCallbackInfo& args); static void GenerateKeys(const v8::FunctionCallbackInfo& args);