Skip to content

Commit 5e72bd3

Browse files
jasnelladuh95
jasnell
authored and
aduh95
committed
src: eliminate ManagedEVPPkey
Prior to this change, the ManagedEVPPkey class added an additional layer of abstraction to the EVP_PKEY class that wasn't strictly necessary. Previously we had: KeyObjectHandle -> std::shared_ptr<KeyObjectData> -> ManagedEVPPkey -> EVPKeyPointer After this change we have: KeyObjectHandle -> KeyObjectData -> EVPKeyPointer The `KeyObjectData` class no longer needs to be wrapped in std::shared_ptr but it will hold the underlying EVPKeyPointer in a std::shared_ptr. This greatly simplifies the abstraction and provides an overall reduction in code and complexity, although the changeset in this PR is fairly extensive to get there. This refactor is being done to simplify the codebase as part of the process of extracting crypto functionality to the separate ncrypto dep. PR-URL: #54751 Reviewed-By: Yagiz Nizipli <[email protected]>
1 parent de73d1e commit 5e72bd3

29 files changed

+603
-712
lines changed

‎src/crypto/README.md

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,10 @@ threadpool).
149149
Refer to `crypto_keys.h` and `crypto_keys.cc` for all code relating to the
150150
core key objects.
151151

152-
#### `ManagedEVPPKey`
153-
154-
The `ManagedEVPPKey` class is a smart pointer for OpenSSL `EVP_PKEY`
155-
structures. These manage the lifecycle of Public and Private key pairs.
156-
157152
#### `KeyObjectData`
158153

159154
`KeyObjectData` is an internal thread-safe structure used to wrap either
160-
a `ManagedEVPPKey` (for Public or Private keys) or a `ByteSource` containing
155+
a `EVPKeyPointer` (for Public or Private keys) or a `ByteSource` containing
161156
a Secret key.
162157

163158
#### `KeyObjectHandle`

‎src/crypto/crypto_aes.cc

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,13 @@ namespace {
3131
// The key_data must be a secret key.
3232
// On success, this function sets out to a new ByteSource
3333
// instance containing the results and returns WebCryptoCipherStatus::OK.
34-
WebCryptoCipherStatus AES_Cipher(
35-
Environment* env,
36-
KeyObjectData* key_data,
37-
WebCryptoCipherMode cipher_mode,
38-
const AESCipherConfig& params,
39-
const ByteSource& in,
40-
ByteSource* out) {
41-
CHECK_NOT_NULL(key_data);
42-
CHECK_EQ(key_data->GetKeyType(), kKeyTypeSecret);
34+
WebCryptoCipherStatus AES_Cipher(Environment* env,
35+
const KeyObjectData& key_data,
36+
WebCryptoCipherMode cipher_mode,
37+
const AESCipherConfig& params,
38+
const ByteSource& in,
39+
ByteSource* out) {
40+
CHECK_EQ(key_data.GetKeyType(), kKeyTypeSecret);
4341

4442
const int mode = EVP_CIPHER_mode(params.cipher);
4543

@@ -69,14 +67,13 @@ WebCryptoCipherStatus AES_Cipher(
6967
return WebCryptoCipherStatus::FAILED;
7068
}
7169

72-
if (!EVP_CIPHER_CTX_set_key_length(
73-
ctx.get(),
74-
key_data->GetSymmetricKeySize()) ||
70+
if (!EVP_CIPHER_CTX_set_key_length(ctx.get(),
71+
key_data.GetSymmetricKeySize()) ||
7572
!EVP_CipherInit_ex(
7673
ctx.get(),
7774
nullptr,
7875
nullptr,
79-
reinterpret_cast<const unsigned char*>(key_data->GetSymmetricKey()),
76+
reinterpret_cast<const unsigned char*>(key_data.GetSymmetricKey()),
8077
params.iv.data<unsigned char>(),
8178
encrypt)) {
8279
return WebCryptoCipherStatus::FAILED;
@@ -217,21 +214,20 @@ std::vector<unsigned char> BlockWithZeroedCounter(
217214
return new_counter_block;
218215
}
219216

220-
WebCryptoCipherStatus AES_CTR_Cipher2(
221-
KeyObjectData* key_data,
222-
WebCryptoCipherMode cipher_mode,
223-
const AESCipherConfig& params,
224-
const ByteSource& in,
225-
unsigned const char* counter,
226-
unsigned char* out) {
217+
WebCryptoCipherStatus AES_CTR_Cipher2(const KeyObjectData& key_data,
218+
WebCryptoCipherMode cipher_mode,
219+
const AESCipherConfig& params,
220+
const ByteSource& in,
221+
unsigned const char* counter,
222+
unsigned char* out) {
227223
CipherCtxPointer ctx(EVP_CIPHER_CTX_new());
228224
const bool encrypt = cipher_mode == kWebCryptoCipherEncrypt;
229225

230226
if (!EVP_CipherInit_ex(
231227
ctx.get(),
232228
params.cipher,
233229
nullptr,
234-
reinterpret_cast<const unsigned char*>(key_data->GetSymmetricKey()),
230+
reinterpret_cast<const unsigned char*>(key_data.GetSymmetricKey()),
235231
counter,
236232
encrypt)) {
237233
// Cipher init failed
@@ -259,13 +255,12 @@ WebCryptoCipherStatus AES_CTR_Cipher2(
259255
return WebCryptoCipherStatus::OK;
260256
}
261257

262-
WebCryptoCipherStatus AES_CTR_Cipher(
263-
Environment* env,
264-
KeyObjectData* key_data,
265-
WebCryptoCipherMode cipher_mode,
266-
const AESCipherConfig& params,
267-
const ByteSource& in,
268-
ByteSource* out) {
258+
WebCryptoCipherStatus AES_CTR_Cipher(Environment* env,
259+
const KeyObjectData& key_data,
260+
WebCryptoCipherMode cipher_mode,
261+
const AESCipherConfig& params,
262+
const ByteSource& in,
263+
ByteSource* out) {
269264
auto num_counters = BignumPointer::New();
270265
if (!BN_lshift(num_counters.get(), BignumPointer::One(), params.length))
271266
return WebCryptoCipherStatus::FAILED;
@@ -518,16 +513,15 @@ Maybe<bool> AESCipherTraits::AdditionalConfig(
518513
return Just(true);
519514
}
520515

521-
WebCryptoCipherStatus AESCipherTraits::DoCipher(
522-
Environment* env,
523-
std::shared_ptr<KeyObjectData> key_data,
524-
WebCryptoCipherMode cipher_mode,
525-
const AESCipherConfig& params,
526-
const ByteSource& in,
527-
ByteSource* out) {
516+
WebCryptoCipherStatus AESCipherTraits::DoCipher(Environment* env,
517+
const KeyObjectData& key_data,
518+
WebCryptoCipherMode cipher_mode,
519+
const AESCipherConfig& params,
520+
const ByteSource& in,
521+
ByteSource* out) {
528522
#define V(name, fn, _) \
529523
case kKeyVariantAES_##name: \
530-
return fn(env, key_data.get(), cipher_mode, params, in, out);
524+
return fn(env, key_data, cipher_mode, params, in, out);
531525
switch (params.variant) {
532526
VARIANTS(V)
533527
default:

‎src/crypto/crypto_aes.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,12 @@ struct AESCipherTraits final {
6767
WebCryptoCipherMode cipher_mode,
6868
AESCipherConfig* config);
6969

70-
static WebCryptoCipherStatus DoCipher(
71-
Environment* env,
72-
std::shared_ptr<KeyObjectData> key_data,
73-
WebCryptoCipherMode cipher_mode,
74-
const AESCipherConfig& params,
75-
const ByteSource& in,
76-
ByteSource* out);
70+
static WebCryptoCipherStatus DoCipher(Environment* env,
71+
const KeyObjectData& key_data,
72+
WebCryptoCipherMode cipher_mode,
73+
const AESCipherConfig& params,
74+
const ByteSource& in,
75+
ByteSource* out);
7776
};
7877

7978
using AESCryptoJob = CipherJob<AESCipherTraits>;

‎src/crypto/crypto_cipher.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -990,7 +990,7 @@ template <PublicKeyCipher::Operation operation,
990990
PublicKeyCipher::EVP_PKEY_cipher_t EVP_PKEY_cipher>
991991
bool PublicKeyCipher::Cipher(
992992
Environment* env,
993-
const ManagedEVPPKey& pkey,
993+
const EVPKeyPointer& pkey,
994994
int padding,
995995
const EVP_MD* digest,
996996
const ArrayBufferOrViewContents<unsigned char>& oaep_label,
@@ -1057,8 +1057,9 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
10571057
Environment* env = Environment::GetCurrent(args);
10581058

10591059
unsigned int offset = 0;
1060-
ManagedEVPPKey pkey =
1061-
ManagedEVPPKey::GetPublicOrPrivateKeyFromJs(args, &offset);
1060+
auto data = KeyObjectData::GetPublicOrPrivateKeyFromJs(args, &offset);
1061+
if (!data) return;
1062+
const auto& pkey = data.GetAsymmetricKey();
10621063
if (!pkey)
10631064
return;
10641065

‎src/crypto/crypto_cipher.h

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class PublicKeyCipher {
110110
EVP_PKEY_cipher_init_t EVP_PKEY_cipher_init,
111111
EVP_PKEY_cipher_t EVP_PKEY_cipher>
112112
static bool Cipher(Environment* env,
113-
const ManagedEVPPKey& pkey,
113+
const EVPKeyPointer& pkey,
114114
int padding,
115115
const EVP_MD* digest,
116116
const ArrayBufferOrViewContents<unsigned char>& oaep_label,
@@ -195,27 +195,23 @@ class CipherJob final : public CryptoJob<CipherTraits> {
195195
CryptoJob<CipherTraits>::RegisterExternalReferences(New, registry);
196196
}
197197

198-
CipherJob(
199-
Environment* env,
200-
v8::Local<v8::Object> object,
201-
CryptoJobMode mode,
202-
KeyObjectHandle* key,
203-
WebCryptoCipherMode cipher_mode,
204-
const ArrayBufferOrViewContents<char>& data,
205-
AdditionalParams&& params)
206-
: CryptoJob<CipherTraits>(
207-
env,
208-
object,
209-
AsyncWrap::PROVIDER_CIPHERREQUEST,
210-
mode,
211-
std::move(params)),
212-
key_(key->Data()),
198+
CipherJob(Environment* env,
199+
v8::Local<v8::Object> object,
200+
CryptoJobMode mode,
201+
KeyObjectHandle* key,
202+
WebCryptoCipherMode cipher_mode,
203+
const ArrayBufferOrViewContents<char>& data,
204+
AdditionalParams&& params)
205+
: CryptoJob<CipherTraits>(env,
206+
object,
207+
AsyncWrap::PROVIDER_CIPHERREQUEST,
208+
mode,
209+
std::move(params)),
210+
key_(key->Data().addRef()),
213211
cipher_mode_(cipher_mode),
214-
in_(mode == kCryptoJobAsync
215-
? data.ToCopy()
216-
: data.ToByteSource()) {}
212+
in_(mode == kCryptoJobAsync ? data.ToCopy() : data.ToByteSource()) {}
217213

218-
std::shared_ptr<KeyObjectData> key() const { return key_; }
214+
const KeyObjectData& key() const { return key_; }
219215

220216
WebCryptoCipherMode cipher_mode() const { return cipher_mode_; }
221217

@@ -278,7 +274,7 @@ class CipherJob final : public CryptoJob<CipherTraits> {
278274
}
279275

280276
private:
281-
std::shared_ptr<KeyObjectData> key_;
277+
KeyObjectData key_;
282278
WebCryptoCipherMode cipher_mode_;
283279
ByteSource in_;
284280
ByteSource out_;

‎src/crypto/crypto_context.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -613,15 +613,14 @@ void SecureContext::SetKeylogCallback(KeylogCb cb) {
613613
SSL_CTX_set_keylog_callback(ctx_.get(), cb);
614614
}
615615

616-
Maybe<void> SecureContext::UseKey(Environment* env,
617-
std::shared_ptr<KeyObjectData> key) {
618-
if (key->GetKeyType() != KeyType::kKeyTypePrivate) {
616+
Maybe<void> SecureContext::UseKey(Environment* env, const KeyObjectData& key) {
617+
if (key.GetKeyType() != KeyType::kKeyTypePrivate) {
619618
THROW_ERR_CRYPTO_INVALID_KEYTYPE(env);
620619
return Nothing<void>();
621620
}
622621

623622
ClearErrorOnReturn clear_error_on_return;
624-
if (!SSL_CTX_use_PrivateKey(ctx_.get(), key->GetAsymmetricKey().get())) {
623+
if (!SSL_CTX_use_PrivateKey(ctx_.get(), key.GetAsymmetricKey().get())) {
625624
ThrowCryptoError(env, ERR_get_error(), "SSL_CTX_use_PrivateKey");
626625
return Nothing<void>();
627626
}

‎src/crypto/crypto_context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class SecureContext final : public BaseObject {
5959

6060
v8::Maybe<void> AddCert(Environment* env, BIOPointer&& bio);
6161
v8::Maybe<void> SetCRL(Environment* env, const BIOPointer& bio);
62-
v8::Maybe<void> UseKey(Environment* env, std::shared_ptr<KeyObjectData> key);
62+
v8::Maybe<void> UseKey(Environment* env, const KeyObjectData& key);
6363

6464
void SetCACert(const BIOPointer& bio);
6565
void SetRootCerts();

‎src/crypto/crypto_dh.cc

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -432,30 +432,30 @@ Maybe<bool> DHKeyExportTraits::AdditionalConfig(
432432
}
433433

434434
WebCryptoKeyExportStatus DHKeyExportTraits::DoExport(
435-
std::shared_ptr<KeyObjectData> key_data,
435+
const KeyObjectData& key_data,
436436
WebCryptoKeyFormat format,
437437
const DHKeyExportConfig& params,
438438
ByteSource* out) {
439-
CHECK_NE(key_data->GetKeyType(), kKeyTypeSecret);
439+
CHECK_NE(key_data.GetKeyType(), kKeyTypeSecret);
440440

441441
switch (format) {
442442
case kWebCryptoKeyFormatPKCS8:
443-
if (key_data->GetKeyType() != kKeyTypePrivate)
443+
if (key_data.GetKeyType() != kKeyTypePrivate)
444444
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
445-
return PKEY_PKCS8_Export(key_data.get(), out);
445+
return PKEY_PKCS8_Export(key_data, out);
446446
case kWebCryptoKeyFormatSPKI:
447-
if (key_data->GetKeyType() != kKeyTypePublic)
447+
if (key_data.GetKeyType() != kKeyTypePublic)
448448
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
449-
return PKEY_SPKI_Export(key_data.get(), out);
449+
return PKEY_SPKI_Export(key_data, out);
450450
default:
451451
UNREACHABLE();
452452
}
453453
}
454454

455455
namespace {
456-
ByteSource StatelessDiffieHellmanThreadsafe(const ManagedEVPPKey& our_key,
457-
const ManagedEVPPKey& their_key) {
458-
auto dp = DHPointer::stateless(our_key.pkey(), their_key.pkey());
456+
ByteSource StatelessDiffieHellmanThreadsafe(const EVPKeyPointer& our_key,
457+
const EVPKeyPointer& their_key) {
458+
auto dp = DHPointer::stateless(our_key, their_key);
459459
if (!dp) return {};
460460

461461
return ByteSource::Allocated(dp.release());
@@ -467,13 +467,13 @@ void Stateless(const FunctionCallbackInfo<Value>& args) {
467467
CHECK(args[0]->IsObject() && args[1]->IsObject());
468468
KeyObjectHandle* our_key_object;
469469
ASSIGN_OR_RETURN_UNWRAP(&our_key_object, args[0].As<Object>());
470-
CHECK_EQ(our_key_object->Data()->GetKeyType(), kKeyTypePrivate);
470+
CHECK_EQ(our_key_object->Data().GetKeyType(), kKeyTypePrivate);
471471
KeyObjectHandle* their_key_object;
472472
ASSIGN_OR_RETURN_UNWRAP(&their_key_object, args[1].As<Object>());
473-
CHECK_NE(their_key_object->Data()->GetKeyType(), kKeyTypeSecret);
473+
CHECK_NE(their_key_object->Data().GetKeyType(), kKeyTypeSecret);
474474

475-
ManagedEVPPKey our_key = our_key_object->Data()->GetAsymmetricKey();
476-
ManagedEVPPKey their_key = their_key_object->Data()->GetAsymmetricKey();
475+
const auto& our_key = our_key_object->Data().GetAsymmetricKey();
476+
const auto& their_key = their_key_object->Data().GetAsymmetricKey();
477477

478478
Local<Value> out;
479479
if (!StatelessDiffieHellmanThreadsafe(our_key, their_key)
@@ -503,14 +503,14 @@ Maybe<bool> DHBitsTraits::AdditionalConfig(
503503
ASSIGN_OR_RETURN_UNWRAP(&public_key, args[offset], Nothing<bool>());
504504
ASSIGN_OR_RETURN_UNWRAP(&private_key, args[offset + 1], Nothing<bool>());
505505

506-
if (private_key->Data()->GetKeyType() != kKeyTypePrivate ||
507-
public_key->Data()->GetKeyType() != kKeyTypePublic) {
506+
if (private_key->Data().GetKeyType() != kKeyTypePrivate ||
507+
public_key->Data().GetKeyType() != kKeyTypePublic) {
508508
THROW_ERR_CRYPTO_INVALID_KEYTYPE(env);
509509
return Nothing<bool>();
510510
}
511511

512-
params->public_key = public_key->Data();
513-
params->private_key = private_key->Data();
512+
params->public_key = public_key->Data().addRef();
513+
params->private_key = private_key->Data().addRef();
514514

515515
return Just(true);
516516
}
@@ -528,18 +528,15 @@ bool DHBitsTraits::DeriveBits(
528528
Environment* env,
529529
const DHBitsConfig& params,
530530
ByteSource* out) {
531-
*out = StatelessDiffieHellmanThreadsafe(
532-
params.private_key->GetAsymmetricKey(),
533-
params.public_key->GetAsymmetricKey());
531+
*out = StatelessDiffieHellmanThreadsafe(params.private_key.GetAsymmetricKey(),
532+
params.public_key.GetAsymmetricKey());
534533
return true;
535534
}
536535

537-
Maybe<bool> GetDhKeyDetail(
538-
Environment* env,
539-
std::shared_ptr<KeyObjectData> key,
540-
Local<Object> target) {
541-
ManagedEVPPKey pkey = key->GetAsymmetricKey();
542-
CHECK_EQ(EVP_PKEY_id(pkey.get()), EVP_PKEY_DH);
536+
Maybe<bool> GetDhKeyDetail(Environment* env,
537+
const KeyObjectData& key,
538+
Local<Object> target) {
539+
CHECK_EQ(EVP_PKEY_id(key.GetAsymmetricKey().get()), EVP_PKEY_DH);
543540
return Just(true);
544541
}
545542

0 commit comments

Comments
 (0)