Skip to content

Commit

Permalink
src: refactor to avoid using a moved object
Browse files Browse the repository at this point in the history
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: #44269
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
  • Loading branch information
tniessen authored and ruyadorno committed Aug 22, 2022
1 parent a2707d0 commit 3dadc95
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 30 deletions.
16 changes: 6 additions & 10 deletions src/crypto/crypto_keygen.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,12 @@ struct KeyPairGenTraits final {
AdditionalParameters* params,
v8::Local<v8::Value>* result) {
v8::Local<v8::Value> keys[2];
if (ManagedEVPPKey::ToEncodedPublicKey(
env,
std::move(params->key),
params->public_key_encoding,
&keys[0]).IsNothing() ||
ManagedEVPPKey::ToEncodedPrivateKey(
env,
std::move(params->key),
params->private_key_encoding,
&keys[1]).IsNothing()) {
if (params->key
.ToEncodedPublicKey(env, params->public_key_encoding, &keys[0])
.IsNothing() ||
params->key
.ToEncodedPrivateKey(env, params->private_key_encoding, &keys[1])
.IsNothing()) {
return v8::Nothing<bool>();
}
*result = v8::Array::New(env->isolate(), keys, arraysize(keys));
Expand Down
18 changes: 8 additions & 10 deletions src/crypto/crypto_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -633,44 +633,42 @@ Maybe<bool> ExportJWKInner(Environment* env,

Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
Environment* env,
ManagedEVPPKey key,
const PublicKeyEncodingConfig& config,
Local<Value>* out) {
if (!key) return Nothing<bool>();
if (!*this) return Nothing<bool>();
if (config.output_key_object_) {
// Note that this has the downside of containing sensitive data of the
// private key.
std::shared_ptr<KeyObjectData> data =
KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
KeyObjectData::CreateAsymmetric(kKeyTypePublic, *this);
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
} else if (config.format_ == kKeyFormatJWK) {
std::shared_ptr<KeyObjectData> data =
KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
KeyObjectData::CreateAsymmetric(kKeyTypePublic, *this);
*out = Object::New(env->isolate());
return ExportJWKInner(env, data, *out, false);
}

return Tristate(WritePublicKey(env, key.get(), config).ToLocal(out));
return Tristate(WritePublicKey(env, get(), config).ToLocal(out));
}

Maybe<bool> ManagedEVPPKey::ToEncodedPrivateKey(
Environment* env,
ManagedEVPPKey key,
const PrivateKeyEncodingConfig& config,
Local<Value>* out) {
if (!key) return Nothing<bool>();
if (!*this) return Nothing<bool>();
if (config.output_key_object_) {
std::shared_ptr<KeyObjectData> data =
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, *this);
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
} else if (config.format_ == kKeyFormatJWK) {
std::shared_ptr<KeyObjectData> data =
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, *this);
*out = Object::New(env->isolate());
return ExportJWKInner(env, data, *out, false);
}

return Tristate(WritePrivateKey(env, key.get(), config).ToLocal(out));
return Tristate(WritePrivateKey(env, get(), config).ToLocal(out));
}

NonCopyableMaybe<PrivateKeyEncodingConfig>
Expand Down
16 changes: 6 additions & 10 deletions src/crypto/crypto_keys.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,13 @@ class ManagedEVPPKey : public MemoryRetainer {
unsigned int* offset,
bool allow_key_object);

static v8::Maybe<bool> ToEncodedPublicKey(
Environment* env,
ManagedEVPPKey key,
const PublicKeyEncodingConfig& config,
v8::Local<v8::Value>* out);
v8::Maybe<bool> ToEncodedPublicKey(Environment* env,
const PublicKeyEncodingConfig& config,
v8::Local<v8::Value>* out);

static v8::Maybe<bool> ToEncodedPrivateKey(
Environment* env,
ManagedEVPPKey key,
const PrivateKeyEncodingConfig& config,
v8::Local<v8::Value>* out);
v8::Maybe<bool> ToEncodedPrivateKey(Environment* env,
const PrivateKeyEncodingConfig& config,
v8::Local<v8::Value>* out);

private:
size_t size_of_private_key() const;
Expand Down

0 comments on commit 3dadc95

Please sign in to comment.