Skip to content

Commit 50b1cde

Browse files
tniessenaddaleax
tniessen
authored and
addaleax
committed
src: store key data in separate class
This separates key handles from the actual key data: +-----------------+ | NativeKeyObject | +-----------------+ ^ extends | +-----------------+ +-----------------+ +---------------+ | KeyObject (JS) | -> | KeyObjectHandle | -> | KeyObjectData | +-----------------+ +-----------------+ +---------------+ PR-URL: #33360 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent bf3aaa3 commit 50b1cde

File tree

3 files changed

+116
-105
lines changed

3 files changed

+116
-105
lines changed

‎lib/internal/crypto/keys.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,23 +331,23 @@ function createSecretKey(key) {
331331
key = prepareSecretKey(key, true);
332332
if (key.byteLength === 0)
333333
throw new ERR_OUT_OF_RANGE('key.byteLength', '> 0', key.byteLength);
334-
const handle = new KeyObjectHandle(kKeyTypeSecret);
335-
handle.init(key);
334+
const handle = new KeyObjectHandle();
335+
handle.init(kKeyTypeSecret, key);
336336
return new SecretKeyObject(handle);
337337
}
338338

339339
function createPublicKey(key) {
340340
const { format, type, data } = prepareAsymmetricKey(key, kCreatePublic);
341-
const handle = new KeyObjectHandle(kKeyTypePublic);
342-
handle.init(data, format, type);
341+
const handle = new KeyObjectHandle();
342+
handle.init(kKeyTypePublic, data, format, type);
343343
return new PublicKeyObject(handle);
344344
}
345345

346346
function createPrivateKey(key) {
347347
const { format, type, data, passphrase } =
348348
prepareAsymmetricKey(key, kCreatePrivate);
349-
const handle = new KeyObjectHandle(kKeyTypePrivate);
350-
handle.init(data, format, type, passphrase);
349+
const handle = new KeyObjectHandle();
350+
handle.init(kKeyTypePrivate, data, format, type, passphrase);
351351
return new PrivateKeyObject(handle);
352352
}
353353

‎src/node_crypto.cc

Lines changed: 86 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -2890,7 +2890,8 @@ ByteSource ByteSource::FromSymmetricKeyObjectHandle(Local<Value> handle) {
28902890
CHECK(handle->IsObject());
28912891
KeyObjectHandle* key = Unwrap<KeyObjectHandle>(handle.As<Object>());
28922892
CHECK_NOT_NULL(key);
2893-
return Foreign(key->GetSymmetricKey(), key->GetSymmetricKeySize());
2893+
return Foreign(key->Data()->GetSymmetricKey(),
2894+
key->Data()->GetSymmetricKeySize());
28942895
}
28952896

28962897
ByteSource::ByteSource(const char* data, char* allocated_data, size_t size)
@@ -3036,9 +3037,9 @@ static ManagedEVPPKey GetPrivateKeyFromJs(
30363037
CHECK(args[*offset]->IsObject() && allow_key_object);
30373038
KeyObjectHandle* key;
30383039
ASSIGN_OR_RETURN_UNWRAP(&key, args[*offset].As<Object>(), ManagedEVPPKey());
3039-
CHECK_EQ(key->GetKeyType(), kKeyTypePrivate);
3040+
CHECK_EQ(key->Data()->GetKeyType(), kKeyTypePrivate);
30403041
(*offset) += 4;
3041-
return key->GetAsymmetricKey();
3042+
return key->Data()->GetAsymmetricKey();
30423043
}
30433044
}
30443045

@@ -3096,9 +3097,9 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
30963097
CHECK(args[*offset]->IsObject());
30973098
KeyObjectHandle* key = Unwrap<KeyObjectHandle>(args[*offset].As<Object>());
30983099
CHECK_NOT_NULL(key);
3099-
CHECK_NE(key->GetKeyType(), kKeyTypeSecret);
3100+
CHECK_NE(key->Data()->GetKeyType(), kKeyTypeSecret);
31003101
(*offset) += 4;
3101-
return key->GetAsymmetricKey();
3102+
return key->Data()->GetAsymmetricKey();
31023103
}
31033104
}
31043105

@@ -3205,6 +3206,48 @@ EVP_PKEY* ManagedEVPPKey::get() const {
32053206
return pkey_.get();
32063207
}
32073208

3209+
KeyObjectData* KeyObjectData::CreateSecret(v8::Local<v8::ArrayBufferView> abv) {
3210+
size_t key_len = abv->ByteLength();
3211+
char* mem = MallocOpenSSL<char>(key_len);
3212+
abv->CopyContents(mem, key_len);
3213+
KeyObjectData* data = new KeyObjectData();
3214+
data->key_type_ = kKeyTypeSecret;
3215+
data->symmetric_key_ = std::unique_ptr<char, std::function<void(char*)>>(mem,
3216+
[key_len](char* p) {
3217+
OPENSSL_clear_free(p, key_len);
3218+
});
3219+
data->symmetric_key_len_ = key_len;
3220+
return data;
3221+
}
3222+
3223+
KeyObjectData* KeyObjectData::CreateAsymmetric(KeyType key_type,
3224+
const ManagedEVPPKey& pkey) {
3225+
CHECK(pkey);
3226+
KeyObjectData* data = new KeyObjectData();
3227+
data->key_type_ = key_type;
3228+
data->asymmetric_key_ = pkey;
3229+
return data;
3230+
}
3231+
3232+
KeyType KeyObjectData::GetKeyType() const {
3233+
return key_type_;
3234+
}
3235+
3236+
ManagedEVPPKey KeyObjectData::GetAsymmetricKey() const {
3237+
CHECK_NE(key_type_, kKeyTypeSecret);
3238+
return asymmetric_key_;
3239+
}
3240+
3241+
const char* KeyObjectData::GetSymmetricKey() const {
3242+
CHECK_EQ(key_type_, kKeyTypeSecret);
3243+
return symmetric_key_.get();
3244+
}
3245+
3246+
size_t KeyObjectData::GetSymmetricKeySize() const {
3247+
CHECK_EQ(key_type_, kKeyTypeSecret);
3248+
return symmetric_key_len_;
3249+
}
3250+
32083251
Local<Function> KeyObjectHandle::Initialize(Environment* env,
32093252
Local<Object> target) {
32103253
Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
@@ -3241,46 +3284,23 @@ MaybeLocal<Object> KeyObjectHandle::Create(Environment* env,
32413284

32423285
KeyObjectHandle* key = Unwrap<KeyObjectHandle>(obj);
32433286
CHECK_NOT_NULL(key);
3244-
if (key_type == kKeyTypePublic)
3245-
key->InitPublic(pkey);
3246-
else
3247-
key->InitPrivate(pkey);
3287+
key->data_.reset(KeyObjectData::CreateAsymmetric(key_type, pkey));
32483288
return obj;
32493289
}
32503290

3251-
ManagedEVPPKey KeyObjectHandle::GetAsymmetricKey() const {
3252-
CHECK_NE(key_type_, kKeyTypeSecret);
3253-
return this->asymmetric_key_;
3254-
}
3255-
3256-
const char* KeyObjectHandle::GetSymmetricKey() const {
3257-
CHECK_EQ(key_type_, kKeyTypeSecret);
3258-
return this->symmetric_key_.get();
3259-
}
3260-
3261-
size_t KeyObjectHandle::GetSymmetricKeySize() const {
3262-
CHECK_EQ(key_type_, kKeyTypeSecret);
3263-
return this->symmetric_key_len_;
3291+
const KeyObjectData* KeyObjectHandle::Data() {
3292+
return data_.get();
32643293
}
32653294

32663295
void KeyObjectHandle::New(const FunctionCallbackInfo<Value>& args) {
32673296
CHECK(args.IsConstructCall());
3268-
CHECK(args[0]->IsInt32());
3269-
KeyType key_type = static_cast<KeyType>(args[0].As<Uint32>()->Value());
32703297
Environment* env = Environment::GetCurrent(args);
3271-
new KeyObjectHandle(env, args.This(), key_type);
3272-
}
3273-
3274-
KeyType KeyObjectHandle::GetKeyType() const {
3275-
return this->key_type_;
3298+
new KeyObjectHandle(env, args.This());
32763299
}
32773300

32783301
KeyObjectHandle::KeyObjectHandle(Environment* env,
3279-
Local<Object> wrap,
3280-
KeyType key_type)
3281-
: BaseObject(env, wrap),
3282-
key_type_(key_type),
3283-
symmetric_key_(nullptr, nullptr) {
3302+
Local<Object> wrap)
3303+
: BaseObject(env, wrap) {
32843304
MakeWeak();
32853305
}
32863306

@@ -3289,66 +3309,45 @@ void KeyObjectHandle::Init(const FunctionCallbackInfo<Value>& args) {
32893309
ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder());
32903310
MarkPopErrorOnReturn mark_pop_error_on_return;
32913311

3312+
CHECK(args[0]->IsInt32());
3313+
KeyType type = static_cast<KeyType>(args[0].As<Uint32>()->Value());
3314+
32923315
unsigned int offset;
32933316
ManagedEVPPKey pkey;
32943317

3295-
switch (key->key_type_) {
3318+
switch (type) {
32963319
case kKeyTypeSecret:
3297-
CHECK_EQ(args.Length(), 1);
3298-
CHECK(args[0]->IsArrayBufferView());
3299-
key->InitSecret(args[0].As<ArrayBufferView>());
3320+
CHECK_EQ(args.Length(), 2);
3321+
CHECK(args[1]->IsArrayBufferView());
3322+
key->data_.reset(
3323+
KeyObjectData::CreateSecret(args[1].As<ArrayBufferView>()));
33003324
break;
33013325
case kKeyTypePublic:
3302-
CHECK_EQ(args.Length(), 3);
3326+
CHECK_EQ(args.Length(), 4);
33033327

3304-
offset = 0;
3328+
offset = 1;
33053329
pkey = GetPublicOrPrivateKeyFromJs(args, &offset);
33063330
if (!pkey)
33073331
return;
3308-
key->InitPublic(pkey);
3332+
key->data_.reset(KeyObjectData::CreateAsymmetric(type, pkey));
33093333
break;
33103334
case kKeyTypePrivate:
3311-
CHECK_EQ(args.Length(), 4);
3335+
CHECK_EQ(args.Length(), 5);
33123336

3313-
offset = 0;
3337+
offset = 1;
33143338
pkey = GetPrivateKeyFromJs(args, &offset, false);
33153339
if (!pkey)
33163340
return;
3317-
key->InitPrivate(pkey);
3341+
key->data_.reset(KeyObjectData::CreateAsymmetric(type, pkey));
33183342
break;
33193343
default:
33203344
CHECK(false);
33213345
}
33223346
}
33233347

3324-
void KeyObjectHandle::InitSecret(Local<ArrayBufferView> abv) {
3325-
CHECK_EQ(this->key_type_, kKeyTypeSecret);
3326-
3327-
size_t key_len = abv->ByteLength();
3328-
char* mem = MallocOpenSSL<char>(key_len);
3329-
abv->CopyContents(mem, key_len);
3330-
this->symmetric_key_ = std::unique_ptr<char, std::function<void(char*)>>(mem,
3331-
[key_len](char* p) {
3332-
OPENSSL_clear_free(p, key_len);
3333-
});
3334-
this->symmetric_key_len_ = key_len;
3335-
}
3336-
3337-
void KeyObjectHandle::InitPublic(const ManagedEVPPKey& pkey) {
3338-
CHECK_EQ(this->key_type_, kKeyTypePublic);
3339-
CHECK(pkey);
3340-
this->asymmetric_key_ = pkey;
3341-
}
3342-
3343-
void KeyObjectHandle::InitPrivate(const ManagedEVPPKey& pkey) {
3344-
CHECK_EQ(this->key_type_, kKeyTypePrivate);
3345-
CHECK(pkey);
3346-
this->asymmetric_key_ = pkey;
3347-
}
3348-
33493348
Local<Value> KeyObjectHandle::GetAsymmetricKeyType() const {
3350-
CHECK_NE(this->key_type_, kKeyTypeSecret);
3351-
switch (EVP_PKEY_id(this->asymmetric_key_.get())) {
3349+
const ManagedEVPPKey& key = data_->GetAsymmetricKey();
3350+
switch (EVP_PKEY_id(key.get())) {
33523351
case EVP_PKEY_RSA:
33533352
return env()->crypto_rsa_string();
33543353
case EVP_PKEY_RSA_PSS:
@@ -3384,24 +3383,27 @@ void KeyObjectHandle::GetSymmetricKeySize(
33843383
const FunctionCallbackInfo<Value>& args) {
33853384
KeyObjectHandle* key;
33863385
ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder());
3387-
args.GetReturnValue().Set(static_cast<uint32_t>(key->GetSymmetricKeySize()));
3386+
args.GetReturnValue().Set(
3387+
static_cast<uint32_t>(key->Data()->GetSymmetricKeySize()));
33883388
}
33893389

33903390
void KeyObjectHandle::Export(const FunctionCallbackInfo<Value>& args) {
33913391
KeyObjectHandle* key;
33923392
ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder());
33933393

3394+
KeyType type = key->Data()->GetKeyType();
3395+
33943396
MaybeLocal<Value> result;
3395-
if (key->key_type_ == kKeyTypeSecret) {
3397+
if (type == kKeyTypeSecret) {
33963398
result = key->ExportSecretKey();
3397-
} else if (key->key_type_ == kKeyTypePublic) {
3399+
} else if (type == kKeyTypePublic) {
33983400
unsigned int offset = 0;
33993401
PublicKeyEncodingConfig config =
34003402
GetPublicKeyEncodingFromJs(args, &offset, kKeyContextExport);
34013403
CHECK_EQ(offset, static_cast<unsigned int>(args.Length()));
34023404
result = key->ExportPublicKey(config);
34033405
} else {
3404-
CHECK_EQ(key->key_type_, kKeyTypePrivate);
3406+
CHECK_EQ(type, kKeyTypePrivate);
34053407
unsigned int offset = 0;
34063408
NonCopyableMaybe<PrivateKeyEncodingConfig> config =
34073409
GetPrivateKeyEncodingFromJs(args, &offset, kKeyContextExport);
@@ -3416,18 +3418,19 @@ void KeyObjectHandle::Export(const FunctionCallbackInfo<Value>& args) {
34163418
}
34173419

34183420
Local<Value> KeyObjectHandle::ExportSecretKey() const {
3419-
return Buffer::Copy(env(), symmetric_key_.get(), symmetric_key_len_)
3420-
.ToLocalChecked();
3421+
const char* buf = data_->GetSymmetricKey();
3422+
unsigned int len = data_->GetSymmetricKeySize();
3423+
return Buffer::Copy(env(), buf, len).ToLocalChecked();
34213424
}
34223425

34233426
MaybeLocal<Value> KeyObjectHandle::ExportPublicKey(
34243427
const PublicKeyEncodingConfig& config) const {
3425-
return WritePublicKey(env(), asymmetric_key_.get(), config);
3428+
return WritePublicKey(env(), data_->GetAsymmetricKey().get(), config);
34263429
}
34273430

34283431
MaybeLocal<Value> KeyObjectHandle::ExportPrivateKey(
34293432
const PrivateKeyEncodingConfig& config) const {
3430-
return WritePrivateKey(env(), asymmetric_key_.get(), config);
3433+
return WritePrivateKey(env(), data_->GetAsymmetricKey().get(), config);
34313434
}
34323435

34333436
void NativeKeyObject::New(const FunctionCallbackInfo<Value>& args) {
@@ -6772,13 +6775,13 @@ void StatelessDiffieHellman(const FunctionCallbackInfo<Value>& args) {
67726775
CHECK(args[0]->IsObject() && args[1]->IsObject());
67736776
KeyObjectHandle* our_key_object;
67746777
ASSIGN_OR_RETURN_UNWRAP(&our_key_object, args[0].As<Object>());
6775-
CHECK_EQ(our_key_object->GetKeyType(), kKeyTypePrivate);
6778+
CHECK_EQ(our_key_object->Data()->GetKeyType(), kKeyTypePrivate);
67766779
KeyObjectHandle* their_key_object;
67776780
ASSIGN_OR_RETURN_UNWRAP(&their_key_object, args[1].As<Object>());
6778-
CHECK_NE(their_key_object->GetKeyType(), kKeyTypeSecret);
6781+
CHECK_NE(their_key_object->Data()->GetKeyType(), kKeyTypeSecret);
67796782

6780-
ManagedEVPPKey our_key = our_key_object->GetAsymmetricKey();
6781-
ManagedEVPPKey their_key = their_key_object->GetAsymmetricKey();
6783+
ManagedEVPPKey our_key = our_key_object->Data()->GetAsymmetricKey();
6784+
ManagedEVPPKey their_key = their_key_object->Data()->GetAsymmetricKey();
67826785

67836786
AllocatedBuffer out = StatelessDiffieHellman(env, our_key, their_key);
67846787
if (out.size() == 0)

‎src/node_crypto.h

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,27 @@ class ManagedEVPPKey {
407407
EVPKeyPointer pkey_;
408408
};
409409

410+
class KeyObjectData {
411+
public:
412+
static KeyObjectData* CreateSecret(v8::Local<v8::ArrayBufferView> abv);
413+
static KeyObjectData* CreateAsymmetric(KeyType type,
414+
const ManagedEVPPKey& pkey);
415+
416+
KeyType GetKeyType() const;
417+
418+
// These functions allow unprotected access to the raw key material and should
419+
// only be used to implement cryptograohic operations requiring the key.
420+
ManagedEVPPKey GetAsymmetricKey() const;
421+
const char* GetSymmetricKey() const;
422+
size_t GetSymmetricKeySize() const;
423+
424+
private:
425+
KeyType key_type_;
426+
std::unique_ptr<char, std::function<void(char*)>> symmetric_key_;
427+
unsigned int symmetric_key_len_;
428+
ManagedEVPPKey asymmetric_key_;
429+
};
430+
410431
class KeyObjectHandle : public BaseObject {
411432
public:
412433
static v8::Local<v8::Function> Initialize(Environment* env,
@@ -421,21 +442,12 @@ class KeyObjectHandle : public BaseObject {
421442
SET_MEMORY_INFO_NAME(KeyObjectHandle)
422443
SET_SELF_SIZE(KeyObjectHandle)
423444

424-
KeyType GetKeyType() const;
425-
426-
// These functions allow unprotected access to the raw key material and should
427-
// only be used to implement cryptograohic operations requiring the key.
428-
ManagedEVPPKey GetAsymmetricKey() const;
429-
const char* GetSymmetricKey() const;
430-
size_t GetSymmetricKeySize() const;
445+
const KeyObjectData* Data();
431446

432447
protected:
433448
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
434449

435450
static void Init(const v8::FunctionCallbackInfo<v8::Value>& args);
436-
void InitSecret(v8::Local<v8::ArrayBufferView> abv);
437-
void InitPublic(const ManagedEVPPKey& pkey);
438-
void InitPrivate(const ManagedEVPPKey& pkey);
439451

440452
static void GetAsymmetricKeyType(
441453
const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -452,14 +464,10 @@ class KeyObjectHandle : public BaseObject {
452464
const PrivateKeyEncodingConfig& config) const;
453465

454466
KeyObjectHandle(Environment* env,
455-
v8::Local<v8::Object> wrap,
456-
KeyType key_type);
467+
v8::Local<v8::Object> wrap);
457468

458469
private:
459-
const KeyType key_type_;
460-
std::unique_ptr<char, std::function<void(char*)>> symmetric_key_;
461-
unsigned int symmetric_key_len_;
462-
ManagedEVPPKey asymmetric_key_;
470+
std::unique_ptr<KeyObjectData> data_;
463471
};
464472

465473
class NativeKeyObject : public BaseObject {

0 commit comments

Comments
 (0)