Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing KeyObjects via postMessage #33360

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,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.
Expand All @@ -1229,6 +1233,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
Expand Down Expand Up @@ -3518,6 +3526,7 @@ the `crypto`, `tls`, and `https` modules and are generally specific to OpenSSL.
[`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
Expand Down
8 changes: 6 additions & 2 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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');
Expand Down Expand Up @@ -849,6 +852,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
Expand Down
152 changes: 86 additions & 66 deletions lib/internal/crypto/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ const {
} = primordials;

const {
KeyObject: KeyObjectHandle,
KeyObjectHandle,
createNativeKeyObjectClass,
kKeyTypeSecret,
kKeyTypePublic,
kKeyTypePrivate,
Expand Down Expand Up @@ -42,80 +43,99 @@ for (const m of [[kKeyEncodingPKCS1, 'pkcs1'], [kKeyEncodingPKCS8, 'pkcs8'],
[kKeyEncodingSPKI, 'spki'], [kKeyEncodingSEC1, 'sec1']])
encodingNames[m[0]] = m[1];

class KeyObject {
constructor(type, handle) {
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);

this[kKeyType] = type;

ObjectDefineProperty(this, kHandle, {
value: handle,
enumerable: false,
configurable: false,
writable: false
});
}

get type() {
return this[kKeyType];
}
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);
}

class SecretKeyObject extends KeyObject {
constructor(handle) {
super('secret', 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++.
// The creation requires a callback to make sure that the NativeKeyObject
// base class cannot exist without the other KeyObject implementations.
const [
KeyObject,
SecretKeyObject,
PublicKeyObject,
PrivateKeyObject
] = createNativeKeyObjectClass((NativeKeyObject) => {
// Publicly visible KeyObject class.
class KeyObject extends NativeKeyObject {
constructor(type, handle) {
super(checkKeyTypeAndHandle(type, handle) || handle);

this[kKeyType] = type;

ObjectDefineProperty(this, kHandle, {
value: handle,
enumerable: false,
configurable: false,
writable: false
});
}

get symmetricKeySize() {
return this[kHandle].getSymmetricKeySize();
get type() {
return this[kKeyType];
Copy link
Member

@himself65 himself65 May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use private filed? I wouldn't like to add an extra symbol which is only used once, in this simple class KeyObject

like:

  class KeyObject extends NativeKeyObject {
    #type

    constructor(type, handle) {
      super(checkKeyTypeAndHandle(type, handle) || handle);

      this.#type = type;

      ObjectDefineProperty(this, kHandle, {
        value: handle,
        enumerable: false,
        configurable: false,
        writable: false
      });
    }

    get type() {
      return this.#type;
    }
  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's pre-existing code, but I'll consider it. I guess the same is true for a lot of existing class fields in core.

}
}

export() {
return this[kHandle].export();
}
}
class SecretKeyObject extends KeyObject {
constructor(handle) {
super('secret', handle);
}

const kAsymmetricKeyType = Symbol('kAsymmetricKeyType');
get symmetricKeySize() {
return this[kHandle].getSymmetricKeySize();
}

class AsymmetricKeyObject extends KeyObject {
get asymmetricKeyType() {
return this[kAsymmetricKeyType] ||
(this[kAsymmetricKeyType] = this[kHandle].getAsymmetricKeyType());
export() {
return this[kHandle].export();
}
}
}

class PublicKeyObject extends AsymmetricKeyObject {
constructor(handle) {
super('public', handle);
}
const kAsymmetricKeyType = Symbol('kAsymmetricKeyType');

export(encoding) {
const {
format,
type
} = parsePublicKeyEncoding(encoding, this.asymmetricKeyType);
return this[kHandle].export(format, type);
class AsymmetricKeyObject extends KeyObject {
get asymmetricKeyType() {
return this[kAsymmetricKeyType] ||
(this[kAsymmetricKeyType] = this[kHandle].getAsymmetricKeyType());
}
}
}

class PrivateKeyObject extends AsymmetricKeyObject {
constructor(handle) {
super('private', handle);
class PublicKeyObject extends AsymmetricKeyObject {
constructor(handle) {
super('public', handle);
}

export(encoding) {
const {
format,
type
} = parsePublicKeyEncoding(encoding, this.asymmetricKeyType);
return this[kHandle].export(format, type);
}
}

export(encoding) {
const {
format,
type,
cipher,
passphrase
} = parsePrivateKeyEncoding(encoding, this.asymmetricKeyType);
return this[kHandle].export(format, type, cipher, passphrase);
class PrivateKeyObject extends AsymmetricKeyObject {
constructor(handle) {
super('private', handle);
}

export(encoding) {
const {
format,
type,
cipher,
passphrase
} = parsePrivateKeyEncoding(encoding, this.asymmetricKeyType);
return this[kHandle].export(format, type, cipher, passphrase);
}
}
}

return [KeyObject, SecretKeyObject, PublicKeyObject, PrivateKeyObject];
});

function parseKeyFormat(formatStr, defaultFormat, optionName) {
if (formatStr === undefined && defaultFormat !== undefined)
Expand Down Expand Up @@ -314,23 +334,23 @@ function createSecretKey(key) {
key = prepareSecretKey(key, true);
if (key.byteLength === 0)
throw new ERR_OUT_OF_RANGE('key.byteLength', '> 0', key.byteLength);
const handle = new KeyObjectHandle(kKeyTypeSecret);
handle.init(key);
const handle = new KeyObjectHandle();
handle.init(kKeyTypeSecret, key);
return new SecretKeyObject(handle);
}

function createPublicKey(key) {
const { format, type, data } = prepareAsymmetricKey(key, kCreatePublic);
const handle = new KeyObjectHandle(kKeyTypePublic);
handle.init(data, format, type);
const handle = new KeyObjectHandle();
handle.init(kKeyTypePublic, data, format, type);
return new PublicKeyObject(handle);
}

function createPrivateKey(key) {
const { format, type, data, passphrase } =
prepareAsymmetricKey(key, kCreatePrivate);
const handle = new KeyObjectHandle(kKeyTypePrivate);
handle.init(data, format, type, passphrase);
const handle = new KeyObjectHandle();
handle.init(kKeyTypePrivate, data, format, type, passphrase);
return new PrivateKeyObject(handle);
}

Expand Down
4 changes: 4 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,10 @@ constexpr size_t kFsStatsBufferLength =
V(async_hooks_promise_resolve_function, v8::Function) \
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) \
Expand Down
Loading