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.

Sign up for GitHub

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

Jump to bottom

Allow passing KeyObjects via postMessage #33360

Closed
wants to merge 6 commits into from
Closed

Allow passing KeyObjects via postMessage #33360

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
67d36ca
src: rename internal key handles to KeyObjectHandle
tniessen May 10, 2020
be4f686
src: add NativeKeyObject base class
tniessen May 10, 2020
023856d
src: store key data in separate class
tniessen May 11, 2020
43a87c7
crypto: allow KeyObjects in postMessage
tniessen May 11, 2020
94b74e5
fixup! crypto: allow KeyObjects in postMessage
addaleax Jun 20, 2020
c5d8024
fixup! crypto: allow KeyObjects in postMessage
tniessen Jun 22, 2020
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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
View file
Open in desktop
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