From 50b1cde872e64b1e4fa65660d25190cf8d0083bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 11 May 2020 13:05:38 +0000 Subject: [PATCH] src: store key data in separate class This separates key handles from the actual key data: +-----------------+ | NativeKeyObject | +-----------------+ ^ extends | +-----------------+ +-----------------+ +---------------+ | KeyObject (JS) | -> | KeyObjectHandle | -> | KeyObjectData | +-----------------+ +-----------------+ +---------------+ PR-URL: https://github.com/nodejs/node/pull/33360 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- lib/internal/crypto/keys.js | 12 +-- src/node_crypto.cc | 169 ++++++++++++++++++------------------ src/node_crypto.h | 40 +++++---- 3 files changed, 116 insertions(+), 105 deletions(-) diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index 99cff1891b0b01..61d3859553230d 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -331,23 +331,23 @@ function createSecretKey(key) { key = prepareSecretKey(key, true); if (key.byteLength === 0) throw new ERR_OUT_OF_RANGE('key.byteLength', '> 0', key.byteLength); - const handle = new KeyObjectHandle(kKeyTypeSecret); - handle.init(key); + const handle = new KeyObjectHandle(); + handle.init(kKeyTypeSecret, key); return new SecretKeyObject(handle); } function createPublicKey(key) { const { format, type, data } = prepareAsymmetricKey(key, kCreatePublic); - const handle = new KeyObjectHandle(kKeyTypePublic); - handle.init(data, format, type); + const handle = new KeyObjectHandle(); + handle.init(kKeyTypePublic, data, format, type); return new PublicKeyObject(handle); } function createPrivateKey(key) { const { format, type, data, passphrase } = prepareAsymmetricKey(key, kCreatePrivate); - const handle = new KeyObjectHandle(kKeyTypePrivate); - handle.init(data, format, type, passphrase); + const handle = new KeyObjectHandle(); + handle.init(kKeyTypePrivate, data, format, type, passphrase); return new PrivateKeyObject(handle); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 40cbbdf1bc6c04..0976fbea7daf46 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2890,7 +2890,8 @@ ByteSource ByteSource::FromSymmetricKeyObjectHandle(Local handle) { CHECK(handle->IsObject()); KeyObjectHandle* key = Unwrap(handle.As()); CHECK_NOT_NULL(key); - return Foreign(key->GetSymmetricKey(), key->GetSymmetricKeySize()); + return Foreign(key->Data()->GetSymmetricKey(), + key->Data()->GetSymmetricKeySize()); } ByteSource::ByteSource(const char* data, char* allocated_data, size_t size) @@ -3036,9 +3037,9 @@ static ManagedEVPPKey GetPrivateKeyFromJs( CHECK(args[*offset]->IsObject() && allow_key_object); KeyObjectHandle* key; ASSIGN_OR_RETURN_UNWRAP(&key, args[*offset].As(), ManagedEVPPKey()); - CHECK_EQ(key->GetKeyType(), kKeyTypePrivate); + CHECK_EQ(key->Data()->GetKeyType(), kKeyTypePrivate); (*offset) += 4; - return key->GetAsymmetricKey(); + return key->Data()->GetAsymmetricKey(); } } @@ -3096,9 +3097,9 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs( CHECK(args[*offset]->IsObject()); KeyObjectHandle* key = Unwrap(args[*offset].As()); CHECK_NOT_NULL(key); - CHECK_NE(key->GetKeyType(), kKeyTypeSecret); + CHECK_NE(key->Data()->GetKeyType(), kKeyTypeSecret); (*offset) += 4; - return key->GetAsymmetricKey(); + return key->Data()->GetAsymmetricKey(); } } @@ -3205,6 +3206,48 @@ EVP_PKEY* ManagedEVPPKey::get() const { return pkey_.get(); } +KeyObjectData* KeyObjectData::CreateSecret(v8::Local abv) { + size_t key_len = abv->ByteLength(); + char* mem = MallocOpenSSL(key_len); + abv->CopyContents(mem, key_len); + KeyObjectData* data = new KeyObjectData(); + data->key_type_ = kKeyTypeSecret; + data->symmetric_key_ = std::unique_ptr>(mem, + [key_len](char* p) { + OPENSSL_clear_free(p, key_len); + }); + data->symmetric_key_len_ = key_len; + return data; +} + +KeyObjectData* KeyObjectData::CreateAsymmetric(KeyType key_type, + const ManagedEVPPKey& pkey) { + CHECK(pkey); + KeyObjectData* data = new KeyObjectData(); + data->key_type_ = key_type; + data->asymmetric_key_ = pkey; + return data; +} + +KeyType KeyObjectData::GetKeyType() const { + return key_type_; +} + +ManagedEVPPKey KeyObjectData::GetAsymmetricKey() const { + CHECK_NE(key_type_, kKeyTypeSecret); + return asymmetric_key_; +} + +const char* KeyObjectData::GetSymmetricKey() const { + CHECK_EQ(key_type_, kKeyTypeSecret); + return symmetric_key_.get(); +} + +size_t KeyObjectData::GetSymmetricKeySize() const { + CHECK_EQ(key_type_, kKeyTypeSecret); + return symmetric_key_len_; +} + Local KeyObjectHandle::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); @@ -3241,46 +3284,23 @@ MaybeLocal KeyObjectHandle::Create(Environment* env, KeyObjectHandle* key = Unwrap(obj); CHECK_NOT_NULL(key); - if (key_type == kKeyTypePublic) - key->InitPublic(pkey); - else - key->InitPrivate(pkey); + key->data_.reset(KeyObjectData::CreateAsymmetric(key_type, pkey)); return obj; } -ManagedEVPPKey KeyObjectHandle::GetAsymmetricKey() const { - CHECK_NE(key_type_, kKeyTypeSecret); - return this->asymmetric_key_; -} - -const char* KeyObjectHandle::GetSymmetricKey() const { - CHECK_EQ(key_type_, kKeyTypeSecret); - return this->symmetric_key_.get(); -} - -size_t KeyObjectHandle::GetSymmetricKeySize() const { - CHECK_EQ(key_type_, kKeyTypeSecret); - return this->symmetric_key_len_; +const KeyObjectData* KeyObjectHandle::Data() { + return data_.get(); } void KeyObjectHandle::New(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); - CHECK(args[0]->IsInt32()); - KeyType key_type = static_cast(args[0].As()->Value()); Environment* env = Environment::GetCurrent(args); - new KeyObjectHandle(env, args.This(), key_type); -} - -KeyType KeyObjectHandle::GetKeyType() const { - return this->key_type_; + new KeyObjectHandle(env, args.This()); } KeyObjectHandle::KeyObjectHandle(Environment* env, - Local wrap, - KeyType key_type) - : BaseObject(env, wrap), - key_type_(key_type), - symmetric_key_(nullptr, nullptr) { + Local wrap) + : BaseObject(env, wrap) { MakeWeak(); } @@ -3289,66 +3309,45 @@ void KeyObjectHandle::Init(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder()); MarkPopErrorOnReturn mark_pop_error_on_return; + CHECK(args[0]->IsInt32()); + KeyType type = static_cast(args[0].As()->Value()); + unsigned int offset; ManagedEVPPKey pkey; - switch (key->key_type_) { + switch (type) { case kKeyTypeSecret: - CHECK_EQ(args.Length(), 1); - CHECK(args[0]->IsArrayBufferView()); - key->InitSecret(args[0].As()); + CHECK_EQ(args.Length(), 2); + CHECK(args[1]->IsArrayBufferView()); + key->data_.reset( + KeyObjectData::CreateSecret(args[1].As())); break; case kKeyTypePublic: - CHECK_EQ(args.Length(), 3); + CHECK_EQ(args.Length(), 4); - offset = 0; + offset = 1; pkey = GetPublicOrPrivateKeyFromJs(args, &offset); if (!pkey) return; - key->InitPublic(pkey); + key->data_.reset(KeyObjectData::CreateAsymmetric(type, pkey)); break; case kKeyTypePrivate: - CHECK_EQ(args.Length(), 4); + CHECK_EQ(args.Length(), 5); - offset = 0; + offset = 1; pkey = GetPrivateKeyFromJs(args, &offset, false); if (!pkey) return; - key->InitPrivate(pkey); + key->data_.reset(KeyObjectData::CreateAsymmetric(type, pkey)); break; default: CHECK(false); } } -void KeyObjectHandle::InitSecret(Local abv) { - CHECK_EQ(this->key_type_, kKeyTypeSecret); - - size_t key_len = abv->ByteLength(); - char* mem = MallocOpenSSL(key_len); - abv->CopyContents(mem, key_len); - this->symmetric_key_ = std::unique_ptr>(mem, - [key_len](char* p) { - OPENSSL_clear_free(p, key_len); - }); - this->symmetric_key_len_ = key_len; -} - -void KeyObjectHandle::InitPublic(const ManagedEVPPKey& pkey) { - CHECK_EQ(this->key_type_, kKeyTypePublic); - CHECK(pkey); - this->asymmetric_key_ = pkey; -} - -void KeyObjectHandle::InitPrivate(const ManagedEVPPKey& pkey) { - CHECK_EQ(this->key_type_, kKeyTypePrivate); - CHECK(pkey); - this->asymmetric_key_ = pkey; -} - Local KeyObjectHandle::GetAsymmetricKeyType() const { - CHECK_NE(this->key_type_, kKeyTypeSecret); - switch (EVP_PKEY_id(this->asymmetric_key_.get())) { + const ManagedEVPPKey& key = data_->GetAsymmetricKey(); + switch (EVP_PKEY_id(key.get())) { case EVP_PKEY_RSA: return env()->crypto_rsa_string(); case EVP_PKEY_RSA_PSS: @@ -3384,24 +3383,27 @@ void KeyObjectHandle::GetSymmetricKeySize( const FunctionCallbackInfo& args) { KeyObjectHandle* key; ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder()); - args.GetReturnValue().Set(static_cast(key->GetSymmetricKeySize())); + args.GetReturnValue().Set( + static_cast(key->Data()->GetSymmetricKeySize())); } void KeyObjectHandle::Export(const FunctionCallbackInfo& args) { KeyObjectHandle* key; ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder()); + KeyType type = key->Data()->GetKeyType(); + MaybeLocal result; - if (key->key_type_ == kKeyTypeSecret) { + if (type == kKeyTypeSecret) { result = key->ExportSecretKey(); - } else if (key->key_type_ == kKeyTypePublic) { + } else if (type == kKeyTypePublic) { unsigned int offset = 0; PublicKeyEncodingConfig config = GetPublicKeyEncodingFromJs(args, &offset, kKeyContextExport); CHECK_EQ(offset, static_cast(args.Length())); result = key->ExportPublicKey(config); } else { - CHECK_EQ(key->key_type_, kKeyTypePrivate); + CHECK_EQ(type, kKeyTypePrivate); unsigned int offset = 0; NonCopyableMaybe config = GetPrivateKeyEncodingFromJs(args, &offset, kKeyContextExport); @@ -3416,18 +3418,19 @@ void KeyObjectHandle::Export(const FunctionCallbackInfo& args) { } Local KeyObjectHandle::ExportSecretKey() const { - return Buffer::Copy(env(), symmetric_key_.get(), symmetric_key_len_) - .ToLocalChecked(); + const char* buf = data_->GetSymmetricKey(); + unsigned int len = data_->GetSymmetricKeySize(); + return Buffer::Copy(env(), buf, len).ToLocalChecked(); } MaybeLocal KeyObjectHandle::ExportPublicKey( const PublicKeyEncodingConfig& config) const { - return WritePublicKey(env(), asymmetric_key_.get(), config); + return WritePublicKey(env(), data_->GetAsymmetricKey().get(), config); } MaybeLocal KeyObjectHandle::ExportPrivateKey( const PrivateKeyEncodingConfig& config) const { - return WritePrivateKey(env(), asymmetric_key_.get(), config); + return WritePrivateKey(env(), data_->GetAsymmetricKey().get(), config); } void NativeKeyObject::New(const FunctionCallbackInfo& args) { @@ -6772,13 +6775,13 @@ void StatelessDiffieHellman(const FunctionCallbackInfo& args) { CHECK(args[0]->IsObject() && args[1]->IsObject()); KeyObjectHandle* our_key_object; ASSIGN_OR_RETURN_UNWRAP(&our_key_object, args[0].As()); - CHECK_EQ(our_key_object->GetKeyType(), kKeyTypePrivate); + CHECK_EQ(our_key_object->Data()->GetKeyType(), kKeyTypePrivate); KeyObjectHandle* their_key_object; ASSIGN_OR_RETURN_UNWRAP(&their_key_object, args[1].As()); - CHECK_NE(their_key_object->GetKeyType(), kKeyTypeSecret); + CHECK_NE(their_key_object->Data()->GetKeyType(), kKeyTypeSecret); - ManagedEVPPKey our_key = our_key_object->GetAsymmetricKey(); - ManagedEVPPKey their_key = their_key_object->GetAsymmetricKey(); + ManagedEVPPKey our_key = our_key_object->Data()->GetAsymmetricKey(); + ManagedEVPPKey their_key = their_key_object->Data()->GetAsymmetricKey(); AllocatedBuffer out = StatelessDiffieHellman(env, our_key, their_key); if (out.size() == 0) diff --git a/src/node_crypto.h b/src/node_crypto.h index f3a9e83b9f27ca..4d746b12b1d46a 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -407,6 +407,27 @@ class ManagedEVPPKey { EVPKeyPointer pkey_; }; +class KeyObjectData { + public: + static KeyObjectData* CreateSecret(v8::Local abv); + static KeyObjectData* CreateAsymmetric(KeyType type, + const ManagedEVPPKey& pkey); + + KeyType GetKeyType() const; + + // These functions allow unprotected access to the raw key material and should + // only be used to implement cryptograohic operations requiring the key. + ManagedEVPPKey GetAsymmetricKey() const; + const char* GetSymmetricKey() const; + size_t GetSymmetricKeySize() const; + + private: + KeyType key_type_; + std::unique_ptr> symmetric_key_; + unsigned int symmetric_key_len_; + ManagedEVPPKey asymmetric_key_; +}; + class KeyObjectHandle : public BaseObject { public: static v8::Local Initialize(Environment* env, @@ -421,21 +442,12 @@ class KeyObjectHandle : public BaseObject { SET_MEMORY_INFO_NAME(KeyObjectHandle) SET_SELF_SIZE(KeyObjectHandle) - KeyType GetKeyType() const; - - // These functions allow unprotected access to the raw key material and should - // only be used to implement cryptograohic operations requiring the key. - ManagedEVPPKey GetAsymmetricKey() const; - const char* GetSymmetricKey() const; - size_t GetSymmetricKeySize() const; + const KeyObjectData* Data(); protected: static void New(const v8::FunctionCallbackInfo& args); static void Init(const v8::FunctionCallbackInfo& args); - void InitSecret(v8::Local abv); - void InitPublic(const ManagedEVPPKey& pkey); - void InitPrivate(const ManagedEVPPKey& pkey); static void GetAsymmetricKeyType( const v8::FunctionCallbackInfo& args); @@ -452,14 +464,10 @@ class KeyObjectHandle : public BaseObject { const PrivateKeyEncodingConfig& config) const; KeyObjectHandle(Environment* env, - v8::Local wrap, - KeyType key_type); + v8::Local wrap); private: - const KeyType key_type_; - std::unique_ptr> symmetric_key_; - unsigned int symmetric_key_len_; - ManagedEVPPKey asymmetric_key_; + std::unique_ptr data_; }; class NativeKeyObject : public BaseObject {