crypto: allow KeyObjects in postMessage

This change allows sharing KeyObjects between threads via postMessage.
The receiver acquires a new KeyObject and a new KeyObjectHandle, but
refers to the same KeyObjectData:

+-------------------+
| NativeKeyObject 1 | ------------------------------------------+
+-------------------+                                           |
        ^                                                       |
     extends                                                    |
        |                                                       |
+-------------------+    +-------------------+                  |
| KeyObject 1  (JS) | -> | KeyObjectHandle 1 | --------------+  |
+-------------------+    +-------------------+               |  |
                                                             |  |
                                                             |  |
                                                             |  |
                                                             |  |
                                                             |  |
+-------------------+                                        |  |
| NativeKeyObject 2 | ------------------------------------+  |  |
+-------------------+                                     |  |  |
        ^                                                 |  |  |
     extends                                              |  |  |
        |                                                 |  |  |
+-------------------+    +-------------------+            |  |  |
| KeyObject 2  (JS) | -> | KeyObjectHandle 2 | --------+  |  |  |
+-------------------+    +-------------------+         |  |  |  |
                                                       |  |  |  |
                                                       |  |  |  |
                                                       |  |  |  |
                                                       |  |  |  |
                                                       |  |  |  |
+-------------------+                                  |  |  |  |
| NativeKeyObject 3 | ------------------------------+  |  |  |  |
+-------------------+                               |  |  |  |  |
        ^                                           |  |  |  |  |
     extends                                        |  |  |  |  |
        |                                           v  v  v  v  v
+-------------------+    +-------------------+    +---------------+
| KeyObject 3  (JS) | -> | KeyObjectHandle 3 | -> | KeyObjectData |
+-------------------+    +-------------------+    +---------------+

Co-authored-by: Anna Henningsen <anna@addaleax.net>

PR-URL: #33360
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: tniessen

doc/api/crypto.md

@@ -1215,6 +1215,10 @@ This can be called many times with new data as it is streamed.
 <!-- YAML
 added: v11.6.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/33360
+    description: Instances of this class can now be passed to worker threads
+                 using `postMessage`.
   - version: v11.13.0
     pr-url: https://github.com/nodejs/node/pull/26438
     description: This class is now exported.
@@ -1230,6 +1234,10 @@ keyword.
 Most applications should consider using the new `KeyObject` API instead of
 passing keys as strings or `Buffer`s due to improved security features.
 
+`KeyObject` instances can be passed to other threads via [`postMessage()`][].
+The receiver obtains a cloned `KeyObject`, and the `KeyObject` does not need to
+be listed in the `transferList` argument.
+
 ### `keyObject.asymmetricKeyType`
 <!-- YAML
 added: v11.6.0
@@ -3560,6 +3568,7 @@ See the [list of SSL OP Flags][] for details.
 [`hmac.digest()`]: #crypto_hmac_digest_encoding
 [`hmac.update()`]: #crypto_hmac_update_data_inputencoding
 [`keyObject.export()`]: #crypto_keyobject_export_options
+[`postMessage()`]: worker_threads.html#worker_threads_port_postmessage_value_transferlist
 [`sign.sign()`]: #crypto_sign_sign_privatekey_outputencoding
 [`sign.update()`]: #crypto_sign_update_data_inputencoding
 [`stream.Writable` options]: stream.html#stream_new_stream_writable_options

doc/api/worker_threads.md

@@ -328,6 +328,9 @@ are part of the channel.
 <!-- YAML
 added: v10.5.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/33360
+    description: Added `KeyObject` to the list of cloneable types.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/33772
     description: Added `FileHandle` to the list of transferable types.
@@ -348,8 +351,8 @@ In particular, the significant differences to `JSON` are:
 * `value` may contain typed arrays, both using `ArrayBuffer`s
    and `SharedArrayBuffer`s.
 * `value` may contain [`WebAssembly.Module`][] instances.
-* `value` may not contain native (C++-backed) objects other than `MessagePort`s
-  and [`FileHandle`][]s.
+* `value` may not contain native (C++-backed) objects other than `MessagePort`s,
+  [`FileHandle`][]s, and [`KeyObject`][]s.
 
 ```js
 const { MessageChannel } = require('worker_threads');
@@ -846,6 +849,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
 [`EventEmitter`]: events.html
 [`EventTarget`]: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget
 [`FileHandle`]: fs.html#fs_class_filehandle
+[`KeyObject`]: crypto.html#crypto_class_keyobject
 [`MessagePort`]: #worker_threads_class_messageport
 [`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
 [`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array

lib/internal/crypto/keys.js

@@ -43,6 +43,13 @@ for (const m of [[kKeyEncodingPKCS1, 'pkcs1'], [kKeyEncodingPKCS8, 'pkcs8'],
                  [kKeyEncodingSPKI, 'spki'], [kKeyEncodingSEC1, 'sec1']])
   encodingNames[m[0]] = m[1];
 
+function checkKeyTypeAndHandle(type, handle) {
+  if (type !== 'secret' && type !== 'public' && type !== 'private')
+    throw new ERR_INVALID_ARG_VALUE('type', type);
+  if (typeof handle !== 'object' || !(handle instanceof KeyObjectHandle))
+    throw new ERR_INVALID_ARG_TYPE('handle', 'object', handle);
+}
+
 // Creating the KeyObject class is a little complicated due to inheritance
 // and that fact that KeyObjects should be transferrable between threads,
 // which requires the KeyObject base class to be implemented in C++.
@@ -57,11 +64,7 @@ const [
   // Publicly visible KeyObject class.
   class KeyObject extends NativeKeyObject {
     constructor(type, handle) {
-      super();
-      if (type !== 'secret' && type !== 'public' && type !== 'private')
-        throw new ERR_INVALID_ARG_VALUE('type', type);
-      if (typeof handle !== 'object')
-        throw new ERR_INVALID_ARG_TYPE('handle', 'object', handle);
+      super(checkKeyTypeAndHandle(type, handle) || handle);
 
       this[kKeyType] = type;
 

src/env.h

@@ -452,6 +452,9 @@ constexpr size_t kFsStatsBufferLength =
   V(buffer_prototype_object, v8::Object)                                       \
   V(crypto_key_object_constructor, v8::Function)                               \
   V(crypto_key_object_handle_constructor, v8::Function)                        \
+  V(crypto_key_object_private_constructor, v8::Function)                       \
+  V(crypto_key_object_public_constructor, v8::Function)                        \
+  V(crypto_key_object_secret_constructor, v8::Function)                        \
   V(domexception_function, v8::Function)                                       \
   V(enhance_fatal_stack_after_inspector, v8::Function)                         \
   V(enhance_fatal_stack_before_inspector, v8::Function)                        \

src/node_crypto.cc

@@ -3206,27 +3206,24 @@ EVP_PKEY* ManagedEVPPKey::get() const {
   return pkey_.get();
 }
 
-KeyObjectData* KeyObjectData::CreateSecret(v8::Local<v8::ArrayBufferView> abv) {
+std::shared_ptr<KeyObjectData> KeyObjectData::CreateSecret(
+    Local<ArrayBufferView> abv) {
   size_t key_len = abv->ByteLength();
   char* mem = MallocOpenSSL<char>(key_len);
   abv->CopyContents(mem, key_len);
-  KeyObjectData* data = new KeyObjectData();
-  data->key_type_ = kKeyTypeSecret;
-  data->symmetric_key_ = std::unique_ptr<char, std::function<void(char*)>>(mem,
+  return std::shared_ptr<KeyObjectData>(new KeyObjectData(
+      std::unique_ptr<char, std::function<void(char*)>>(mem,
           [key_len](char* p) {
             OPENSSL_clear_free(p, key_len);
-          });
-  data->symmetric_key_len_ = key_len;
-  return data;
+          }),
+      key_len));
 }
 
-KeyObjectData* KeyObjectData::CreateAsymmetric(KeyType key_type,
-                                               const ManagedEVPPKey& pkey) {
+std::shared_ptr<KeyObjectData> KeyObjectData::CreateAsymmetric(
+    KeyType key_type,
+    const ManagedEVPPKey& pkey) {
   CHECK(pkey);
-  KeyObjectData* data = new KeyObjectData();
-  data->key_type_ = key_type;
-  data->asymmetric_key_ = pkey;
-  return data;
+  return std::shared_ptr<KeyObjectData>(new KeyObjectData(key_type, pkey));
 }
 
 KeyType KeyObjectData::GetKeyType() const {
@@ -3270,26 +3267,24 @@ Local<Function> KeyObjectHandle::Initialize(Environment* env,
   return function;
 }
 
-MaybeLocal<Object> KeyObjectHandle::Create(Environment* env,
-                                           KeyType key_type,
-                                           const ManagedEVPPKey& pkey) {
-  CHECK_NE(key_type, kKeyTypeSecret);
-  Local<Value> type = Integer::New(env->isolate(), key_type);
+MaybeLocal<Object> KeyObjectHandle::Create(
+    Environment* env,
+    std::shared_ptr<KeyObjectData> data) {
   Local<Object> obj;
   if (!env->crypto_key_object_handle_constructor()
-           ->NewInstance(env->context(), 1, &type)
+           ->NewInstance(env->context(), 0, nullptr)
            .ToLocal(&obj)) {
     return MaybeLocal<Object>();
   }
 
   KeyObjectHandle* key = Unwrap<KeyObjectHandle>(obj);
   CHECK_NOT_NULL(key);
-  key->data_.reset(KeyObjectData::CreateAsymmetric(key_type, pkey));
+  key->data_ = data;
   return obj;
 }
 
-const KeyObjectData* KeyObjectHandle::Data() {
-  return data_.get();
+const std::shared_ptr<KeyObjectData>& KeyObjectHandle::Data() {
+  return data_;
 }
 
 void KeyObjectHandle::New(const FunctionCallbackInfo<Value>& args) {
@@ -3319,8 +3314,7 @@ void KeyObjectHandle::Init(const FunctionCallbackInfo<Value>& args) {
   case kKeyTypeSecret:
     CHECK_EQ(args.Length(), 2);
     CHECK(args[1]->IsArrayBufferView());
-    key->data_.reset(
-        KeyObjectData::CreateSecret(args[1].As<ArrayBufferView>()));
+    key->data_ = KeyObjectData::CreateSecret(args[1].As<ArrayBufferView>());
     break;
   case kKeyTypePublic:
     CHECK_EQ(args.Length(), 4);
@@ -3329,7 +3323,7 @@ void KeyObjectHandle::Init(const FunctionCallbackInfo<Value>& args) {
     pkey = GetPublicOrPrivateKeyFromJs(args, &offset);
     if (!pkey)
       return;
-    key->data_.reset(KeyObjectData::CreateAsymmetric(type, pkey));
+    key->data_ = KeyObjectData::CreateAsymmetric(type, pkey);
     break;
   case kKeyTypePrivate:
     CHECK_EQ(args.Length(), 5);
@@ -3338,7 +3332,7 @@ void KeyObjectHandle::Init(const FunctionCallbackInfo<Value>& args) {
     pkey = GetPrivateKeyFromJs(args, &offset, false);
     if (!pkey)
       return;
-    key->data_.reset(KeyObjectData::CreateAsymmetric(type, pkey));
+    key->data_ = KeyObjectData::CreateAsymmetric(type, pkey);
     break;
   default:
     CHECK(false);
@@ -3434,7 +3428,50 @@ MaybeLocal<Value> KeyObjectHandle::ExportPrivateKey(
 }
 
 void NativeKeyObject::New(const FunctionCallbackInfo<Value>& args) {
-  CHECK_EQ(args.Length(), 0);
+  Environment* env = Environment::GetCurrent(args);
+  CHECK_EQ(args.Length(), 1);
+  CHECK(args[0]->IsObject());
+  KeyObjectHandle* handle = Unwrap<KeyObjectHandle>(args[0].As<Object>());
+  new NativeKeyObject(env, args.This(), handle->Data());
+}
+
+BaseObjectPtr<BaseObject> NativeKeyObject::KeyObjectTransferData::Deserialize(
+        Environment* env,
+        Local<Context> context,
+        std::unique_ptr<worker::TransferData> self) {
+  if (context != env->context()) {
+    THROW_ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE(env);
+    return {};
+  }
+
+  Local<Value> handle = KeyObjectHandle::Create(env, data_).ToLocalChecked();
+  Local<Function> key_ctor;
+  switch (data_->GetKeyType()) {
+    case kKeyTypeSecret:
+      key_ctor = env->crypto_key_object_secret_constructor();
+      break;
+    case kKeyTypePublic:
+      key_ctor = env->crypto_key_object_public_constructor();
+      break;
+    case kKeyTypePrivate:
+      key_ctor = env->crypto_key_object_private_constructor();
+      break;
+    default:
+      CHECK(false);
+  }
+
+  Local<Value> key =
+      key_ctor->NewInstance(context, 1, &handle).ToLocalChecked();
+  return BaseObjectPtr<BaseObject>(Unwrap<KeyObjectHandle>(key.As<Object>()));
+}
+
+BaseObject::TransferMode NativeKeyObject::GetTransferMode() const {
+  return BaseObject::TransferMode::kCloneable;
+}
+
+std::unique_ptr<worker::TransferData> NativeKeyObject::CloneForMessaging()
+    const {
+  return std::make_unique<KeyObjectTransferData>(handle_data_);
 }
 
 static void CreateNativeKeyObjectClass(
@@ -3448,13 +3485,23 @@ static void CreateNativeKeyObjectClass(
   Local<FunctionTemplate> t = env->NewFunctionTemplate(NativeKeyObject::New);
   t->InstanceTemplate()->SetInternalFieldCount(
       KeyObjectHandle::kInternalFieldCount);
+  t->Inherit(BaseObject::GetConstructorTemplate(env));
 
   Local<Value> ctor = t->GetFunction(env->context()).ToLocalChecked();
 
   Local<Value> recv = Undefined(env->isolate());
-  Local<Value> ret =
-      callback.As<Function>()->Call(env->context(), recv, 1, &ctor)
-      .ToLocalChecked();
+  Local<Value> ret_v;
+  if (!callback.As<Function>()->Call(
+          env->context(), recv, 1, &ctor).ToLocal(&ret_v)) {
+    return;
+  }
+  Local<Array> ret = ret_v.As<Array>();
+  if (!ret->Get(env->context(), 1).ToLocal(&ctor)) return;
+  env->set_crypto_key_object_secret_constructor(ctor.As<Function>());
+  if (!ret->Get(env->context(), 2).ToLocal(&ctor)) return;
+  env->set_crypto_key_object_public_constructor(ctor.As<Function>());
+  if (!ret->Get(env->context(), 3).ToLocal(&ctor)) return;
+  env->set_crypto_key_object_private_constructor(ctor.As<Function>());
   args.GetReturnValue().Set(ret);
 }
 
@@ -6318,8 +6365,9 @@ class GenerateKeyPairJob : public CryptoJob {
     if (public_key_encoding_.output_key_object_) {
       // Note that this has the downside of containing sensitive data of the
       // private key.
-      if (!KeyObjectHandle::Create(env(), kKeyTypePublic, pkey_)
-               .ToLocal(pubkey))
+      std::shared_ptr<KeyObjectData> data =
+          KeyObjectData::CreateAsymmetric(kKeyTypePublic, pkey_);
+      if (!KeyObjectHandle::Create(env(), data).ToLocal(pubkey))
         return false;
     } else {
       if (!WritePublicKey(env(), pkey_.get(), public_key_encoding_)
@@ -6329,8 +6377,9 @@ class GenerateKeyPairJob : public CryptoJob {
 
     // Now do the same for the private key.
     if (private_key_encoding_.output_key_object_) {
-      if (!KeyObjectHandle::Create(env(), kKeyTypePrivate, pkey_)
-              .ToLocal(privkey))
+      std::shared_ptr<KeyObjectData> data =
+          KeyObjectData::CreateAsymmetric(kKeyTypePrivate, pkey_);
+      if (!KeyObjectHandle::Create(env(), data).ToLocal(privkey))
         return false;
     } else {
       if (!WritePrivateKey(env(), pkey_.get(), private_key_encoding_)

src/node_crypto.h

@@ -30,6 +30,7 @@
 #include "env.h"
 #include "base_object.h"
 #include "util.h"
+#include "node_messaging.h"
 
 #include "v8.h"
 
@@ -407,11 +408,13 @@ class ManagedEVPPKey {
   EVPKeyPointer pkey_;
 };
 
+// Objects of this class can safely be shared among threads.
 class KeyObjectData {
  public:
-  static KeyObjectData* CreateSecret(v8::Local<v8::ArrayBufferView> abv);
-  static KeyObjectData* CreateAsymmetric(KeyType type,
-                                         const ManagedEVPPKey& pkey);
+  static std::shared_ptr<KeyObjectData> CreateSecret(
+      v8::Local<v8::ArrayBufferView> abv);
+  static std::shared_ptr<KeyObjectData> CreateAsymmetric(
+      KeyType type, const ManagedEVPPKey& pkey);
 
   KeyType GetKeyType() const;
 
@@ -422,10 +425,23 @@ class KeyObjectData {
   size_t GetSymmetricKeySize() const;
 
  private:
-  KeyType key_type_;
-  std::unique_ptr<char, std::function<void(char*)>> symmetric_key_;
-  unsigned int symmetric_key_len_;
-  ManagedEVPPKey asymmetric_key_;
+  KeyObjectData(std::unique_ptr<char, std::function<void(char*)>> symmetric_key,
+                unsigned int symmetric_key_len)
+      : key_type_(KeyType::kKeyTypeSecret),
+        symmetric_key_(std::move(symmetric_key)),
+        symmetric_key_len_(symmetric_key_len),
+        asymmetric_key_() {}
+
+  KeyObjectData(KeyType type, const ManagedEVPPKey& pkey)
+      : key_type_(type),
+        symmetric_key_(),
+        symmetric_key_len_(0),
+        asymmetric_key_{pkey} {}
+
+  const KeyType key_type_;
+  const std::unique_ptr<char, std::function<void(char*)>> symmetric_key_;
+  const unsigned int symmetric_key_len_;
+  const ManagedEVPPKey asymmetric_key_;
 };
 
 class KeyObjectHandle : public BaseObject {
@@ -434,15 +450,14 @@ class KeyObjectHandle : public BaseObject {
                                             v8::Local<v8::Object> target);
 
   static v8::MaybeLocal<v8::Object> Create(Environment* env,
-                                           KeyType type,
-                                           const ManagedEVPPKey& pkey);
+                                           std::shared_ptr<KeyObjectData> data);
 
   // TODO(tniessen): track the memory used by OpenSSL types
   SET_NO_MEMORY_INFO()
   SET_MEMORY_INFO_NAME(KeyObjectHandle)
   SET_SELF_SIZE(KeyObjectHandle)
 
-  const KeyObjectData* Data();
+  const std::shared_ptr<KeyObjectData>& Data();
 
  protected:
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -467,7 +482,7 @@ class KeyObjectHandle : public BaseObject {
                   v8::Local<v8::Object> wrap);
 
  private:
-  std::unique_ptr<KeyObjectData> data_;
+  std::shared_ptr<KeyObjectData> data_;
 };
 
 class NativeKeyObject : public BaseObject {
@@ -477,6 +492,36 @@ class NativeKeyObject : public BaseObject {
   SET_NO_MEMORY_INFO()
   SET_MEMORY_INFO_NAME(NativeKeyObject)
   SET_SELF_SIZE(NativeKeyObject)
+
+  class KeyObjectTransferData : public worker::TransferData {
+   public:
+    explicit KeyObjectTransferData(const std::shared_ptr<KeyObjectData>& data)
+        : data_(data) {}
+
+    BaseObjectPtr<BaseObject> Deserialize(
+        Environment* env,
+        v8::Local<v8::Context> context,
+        std::unique_ptr<worker::TransferData> self) override;
+
+    SET_MEMORY_INFO_NAME(KeyObjectTransferData)
+    SET_SELF_SIZE(KeyObjectTransferData)
+    SET_NO_MEMORY_INFO()
+
+   private:
+    std::shared_ptr<KeyObjectData> data_;
+  };
+
+  BaseObject::TransferMode GetTransferMode() const override;
+  std::unique_ptr<worker::TransferData> CloneForMessaging() const override;
+
+ private:
+  NativeKeyObject(Environment* env,
+                  v8::Local<v8::Object> wrap,
+                  const std::shared_ptr<KeyObjectData>& handle_data)
+    : BaseObject(env, wrap),
+      handle_data_(handle_data) {}
+
+  std::shared_ptr<KeyObjectData> handle_data_;
 };
 
 class CipherBase : public BaseObject {

test/parallel/test-crypto-key-objects-messageport.js

@@ -0,0 +1,87 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const { createSecretKey, generateKeyPairSync, randomBytes } = require('crypto');
+const { createContext } = require('vm');
+const {
+  MessageChannel,
+  Worker,
+  moveMessagePortToContext,
+  parentPort
+} = require('worker_threads');
+
+function keyToString(key) {
+  let ret;
+  if (key.type === 'secret') {
+    ret = key.export().toString('hex');
+  } else {
+    ret = key.export({ type: 'pkcs1', format: 'pem' });
+  }
+  return ret;
+}
+
+// Worker threads simply reply with their representation of the received key.
+if (process.env.HAS_STARTED_WORKER) {
+  return parentPort.once('message', ({ key }) => {
+    parentPort.postMessage(keyToString(key));
+  });
+}
+
+// Don't use isMainThread to allow running this test inside a worker.
+process.env.HAS_STARTED_WORKER = 1;
+
+// The main thread generates keys and passes them to worker threads.
+const secretKey = createSecretKey(randomBytes(32));
+const { publicKey, privateKey } = generateKeyPairSync('rsa', {
+  modulusLength: 1024
+});
+
+// Get immutable representations of all keys.
+const keys = [secretKey, publicKey, privateKey]
+             .map((key) => [key, keyToString(key)]);
+
+for (const [key, repr] of keys) {
+  {
+    // Test 1: No context change.
+    const { port1, port2 } = new MessageChannel();
+
+    port1.postMessage({ key });
+    assert.strictEqual(keyToString(key), repr);
+
+    port2.once('message', common.mustCall(({ key }) => {
+      assert.strictEqual(keyToString(key), repr);
+    }));
+  }
+
+  {
+    // Test 2: Across threads.
+    const worker = new Worker(__filename);
+    worker.once('message', common.mustCall((receivedRepresentation) => {
+      assert.strictEqual(receivedRepresentation, repr);
+    }));
+    worker.on('disconnect', () => console.log('disconnect'));
+    worker.postMessage({ key });
+  }
+
+  {
+    // Test 3: Across contexts (should not work).
+    const { port1, port2 } = new MessageChannel();
+    const context = createContext();
+    const port2moved = moveMessagePortToContext(port2, context);
+    assert(!(port2moved instanceof Object));
+
+    // TODO(addaleax): Switch this to a 'messageerror' event once MessagePort
+    // implements EventTarget fully and in a cross-context manner.
+    port2moved.emit = common.mustCall((name, err) => {
+      assert.strictEqual(name, 'messageerror');
+      assert.strictEqual(err.code, 'ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE');
+    });
+
+    port2moved.start();
+    port1.postMessage({ key });
+    port1.close();
+  }
+}