Skip to content

Commit 46b523f

Browse files
tniessenguangwong
authored andcommitted
src: refactor to avoid using a moved object
KeyPairGenTraits::EncodeKey should not use an object that it previously moved. Make ManagedEVPPKey::ToEncoded(Public|Private)Key non-static members of ManagedEVPPKey and call them on the key object instead. PR-URL: nodejs/node#44269 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Feng Yu <[email protected]>
1 parent 2e91460 commit 46b523f

File tree

3 files changed

+20
-30
lines changed

3 files changed

+20
-30
lines changed

src/crypto/crypto_keygen.h

+6-10
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,12 @@ struct KeyPairGenTraits final {
183183
AdditionalParameters* params,
184184
v8::Local<v8::Value>* result) {
185185
v8::Local<v8::Value> keys[2];
186-
if (ManagedEVPPKey::ToEncodedPublicKey(
187-
env,
188-
std::move(params->key),
189-
params->public_key_encoding,
190-
&keys[0]).IsNothing() ||
191-
ManagedEVPPKey::ToEncodedPrivateKey(
192-
env,
193-
std::move(params->key),
194-
params->private_key_encoding,
195-
&keys[1]).IsNothing()) {
186+
if (params->key
187+
.ToEncodedPublicKey(env, params->public_key_encoding, &keys[0])
188+
.IsNothing() ||
189+
params->key
190+
.ToEncodedPrivateKey(env, params->private_key_encoding, &keys[1])
191+
.IsNothing()) {
196192
return v8::Nothing<bool>();
197193
}
198194
*result = v8::Array::New(env->isolate(), keys, arraysize(keys));

src/crypto/crypto_keys.cc

+8-10
Original file line numberDiff line numberDiff line change
@@ -632,44 +632,42 @@ Maybe<bool> ExportJWKInner(Environment* env,
632632

633633
Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
634634
Environment* env,
635-
ManagedEVPPKey key,
636635
const PublicKeyEncodingConfig& config,
637636
Local<Value>* out) {
638-
if (!key) return Nothing<bool>();
637+
if (!*this) return Nothing<bool>();
639638
if (config.output_key_object_) {
640639
// Note that this has the downside of containing sensitive data of the
641640
// private key.
642641
std::shared_ptr<KeyObjectData> data =
643-
KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
642+
KeyObjectData::CreateAsymmetric(kKeyTypePublic, *this);
644643
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
645644
} else if (config.format_ == kKeyFormatJWK) {
646645
std::shared_ptr<KeyObjectData> data =
647-
KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
646+
KeyObjectData::CreateAsymmetric(kKeyTypePublic, *this);
648647
*out = Object::New(env->isolate());
649648
return ExportJWKInner(env, data, *out, false);
650649
}
651650

652-
return Tristate(WritePublicKey(env, key.get(), config).ToLocal(out));
651+
return Tristate(WritePublicKey(env, get(), config).ToLocal(out));
653652
}
654653

655654
Maybe<bool> ManagedEVPPKey::ToEncodedPrivateKey(
656655
Environment* env,
657-
ManagedEVPPKey key,
658656
const PrivateKeyEncodingConfig& config,
659657
Local<Value>* out) {
660-
if (!key) return Nothing<bool>();
658+
if (!*this) return Nothing<bool>();
661659
if (config.output_key_object_) {
662660
std::shared_ptr<KeyObjectData> data =
663-
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
661+
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, *this);
664662
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
665663
} else if (config.format_ == kKeyFormatJWK) {
666664
std::shared_ptr<KeyObjectData> data =
667-
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
665+
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, *this);
668666
*out = Object::New(env->isolate());
669667
return ExportJWKInner(env, data, *out, false);
670668
}
671669

672-
return Tristate(WritePrivateKey(env, key.get(), config).ToLocal(out));
670+
return Tristate(WritePrivateKey(env, get(), config).ToLocal(out));
673671
}
674672

675673
NonCopyableMaybe<PrivateKeyEncodingConfig>

src/crypto/crypto_keys.h

+6-10
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,13 @@ class ManagedEVPPKey : public MemoryRetainer {
112112
unsigned int* offset,
113113
bool allow_key_object);
114114

115-
static v8::Maybe<bool> ToEncodedPublicKey(
116-
Environment* env,
117-
ManagedEVPPKey key,
118-
const PublicKeyEncodingConfig& config,
119-
v8::Local<v8::Value>* out);
115+
v8::Maybe<bool> ToEncodedPublicKey(Environment* env,
116+
const PublicKeyEncodingConfig& config,
117+
v8::Local<v8::Value>* out);
120118

121-
static v8::Maybe<bool> ToEncodedPrivateKey(
122-
Environment* env,
123-
ManagedEVPPKey key,
124-
const PrivateKeyEncodingConfig& config,
125-
v8::Local<v8::Value>* out);
119+
v8::Maybe<bool> ToEncodedPrivateKey(Environment* env,
120+
const PrivateKeyEncodingConfig& config,
121+
v8::Local<v8::Value>* out);
126122

127123
private:
128124
size_t size_of_private_key() const;

0 commit comments

Comments
 (0)