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]>
noslate-project · Jan 3, 2023 · 46b523f · 46b523f
1 parent 2e91460
commit 46b523f
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 30 deletions.
diff --git a/src/crypto/crypto_keygen.h b/src/crypto/crypto_keygen.h
@@ -183,16 +183,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));

diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
@@ -632,44 +632,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>

diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h
@@ -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;