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>
nodejs · Sep 27, 2020 · 6de3aff · 6de3aff
1 parent 48a8339
commit 6de3aff
Show file tree

Hide file tree

Showing 7 changed files with 252 additions and 52 deletions.
diff --git a/doc/api/crypto.md b/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

diff --git a/doc/api/worker_threads.md b/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

diff --git a/lib/internal/crypto/keys.js b/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;
 

diff --git a/src/env.h b/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)                        \

diff --git a/src/node_crypto.cc b/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_)