★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/26278/filesNouvelle recherche | Portail wanayoo
Skip to content

/node

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

crypto: allow deriving public from private keys #26278

Closed
wants to merge 2 commits into from
Closed

crypto: allow deriving public from private keys #26278

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67f2fe8
crypto: allow deriving public from private keys
tniessen Jan 26, 2019
73cb9a2
Update doc/api/crypto.md
BridgeAR Mar 5, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+79 −26
Diff settings

Always

Just for now
Copy path View file
17 doc/api/crypto.md
@@ -1813,28 +1813,35 @@ must be an object with the properties described above.
<!-- YAML
added: v11.6.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26278
description: The `key` argument can now be a `KeyObject` with type
`private`.
- version: v11.7.0
pr-url: https://github.com/nodejs/node/pull/25217
description: The `key` argument can now be a private key.
-->
* `key` {Object | string | Buffer}
* `key` {Object | string | Buffer | KeyObject}
- `key`: {string | Buffer}
- `format`: {string} Must be `'pem'` or `'der'`. **Default:** `'pem'`.
- `type`: {string} Must be `'pkcs1'` or `'spki'`. This option is required
only if the `format` is `'der'`.
* Returns: {KeyObject}

Creates and returns a new key object containing a public key. If `key` is a
string or `Buffer`, `format` is assumed to be `'pem'`; otherwise, `key`
must be an object with the properties described above.
string or `Buffer`, `format` is assumed to be `'pem'`; if `key` is a `KeyObject`
with type `'private'`, the public key is derived from the given private key;
otherwise, `key` must be an object with the properties described above.

If the format is `'pem'`, the `'key'` may also be an X.509 certificate.

Because public keys can be derived from private keys, a private key may be
passed instead of a public key. In that case, this function behaves as if
[`crypto.createPrivateKey()`][] had been called, except that the type of the
returned `KeyObject` will be `public` and that the private key cannot be
extracted from the returned `KeyObject`.
returned `KeyObject` will be `'public'` and that the private key cannot be
extracted from the returned `KeyObject`. Similarly, if a `KeyObject` with type
This conversation was marked as resolved by tniessen

This comment has been minimized.

Sign in to view
Copy link
@panva

panva Mar 5, 2019

Contributor

Is there a cost related to this action? When we pass a 'private' key to functions needing a 'public' one, is the key being derived over and over?

Should an API be encouraged to prepare and cache the public KeyObject and pass that one instead?

This comment has been minimized.

Sign in to view
Copy link
@tniessen

tniessen Mar 5, 2019

Author Member

The only cost is the creation of two or three JavaScript objects. In cryptography, we often speak about "deriving the key", but in this case, that does not actually happen. For all supported key types (RSA, DSA, EC), the public key is a part of the private key, so OpenSSL just keeps a memory reference to the private key instead of actually creating a public key. Node.js makes sure that only the public portion of the referenced key can be accessed.

Should an API be encouraged to prepare and cache the public KeyObject and pass that one instead?

I don't think calling createPublicKey(privateKeyObject) takes a considerable amount of time, but I can try to benchmark it later. In general, key objects should be cached, but in this case, it should be sufficient to cache the private key object. (You can also pass the private key object instead of a public key object to all APIs that take public keys, so there is usually little need to explicitely derive a public key object.) Is that okay? :)

This comment has been minimized.

Sign in to view
Copy link
@panva

panva Mar 5, 2019 •
edited

Contributor

Yes, absolutely. I assumed this to be the case, just wanted to make sure. I'm making use of KeyObjects only in my JOSE module (++ JWK / creating keys from components support!) and when i've got private one at hand i pass that one to the crypto operation. I was wondering if there would be any reason to do createPublic(private) before calling crypto instead, but since it's essentially just "cherry-picking" what it needs to from an already prepared openssl key representation that's not needed. Thank you for confirming.

This comment has been minimized.

Sign in to view
Copy link
@tniessen

tniessen Mar 5, 2019

Author Member

Exactly, there is no performance gain in calling createPublic(private)! Thanks for the excellent question!
`'private'` is given, a new `KeyObject` with type `'public'` will be returned
This conversation was marked as resolved by sam-github

This comment has been minimized.

Sign in to view
Copy link
@sam-github

sam-github Feb 25, 2019

Member

nit: I would rephrase to remove "private keys" and "KeyObjects of type 'private'" being described seperately, and emphasize that private keys cannot be derived from public key objects, ever (regardless of how they are created).

Suggestion:

Because public keys can be computed from private keys, a private key or KeyObject of type 'private' may be passed instead of a public key. When used this way, the argument types are the same as [crypto.createPrivateKey()][], but a KeyObject of type 'public' is returned. Note that the inverse is never possible, a private key cannot be derived from a public key or KeyObject.

This comment has been minimized.

Sign in to view
Copy link
@tniessen

tniessen Mar 3, 2019

Author Member

I like your description, but since private key objects cannot be passed to crypto.createPrivateKey (yet), I am afraid it might be incorrect (but maybe that's just my interpretation of it).
and it will be impossible to extract the private key from the returned object.

### crypto.createSecretKey(key)
<!-- YAML
Copy path View file
50 lib/internal/crypto/keys.js
@@ -26,6 +26,12 @@ const { isArrayBufferView } = require('internal/util/types');

const kKeyType = Symbol('kKeyType');

// Key input contexts.
const kConsumePublic = 0;
const kConsumePrivate = 1;
const kCreatePublic = 2;
const kCreatePrivate = 3;

const encodingNames = [];
for (const m of [[kKeyEncodingPKCS1, 'pkcs1'], [kKeyEncodingPKCS8, 'pkcs8'],
[kKeyEncodingSPKI, 'spki'], [kKeyEncodingSEC1, 'sec1']])
@@ -203,7 +209,7 @@ function parseKeyEncoding(enc, keyType, isPublic, objName) {
// when this is used to parse an input encoding and must be a valid key type if
// used to parse an output encoding.
function parsePublicKeyEncoding(enc, keyType, objName) {
return parseKeyFormatAndType(enc, keyType, true, objName);
return parseKeyEncoding(enc, keyType, keyType ? true : undefined, objName);
This conversation was marked as resolved by sam-github

This comment has been minimized.

Sign in to view
Copy link
@sam-github

sam-github Feb 25, 2019

Member

The code must function correctly, you tested it, but its not clear from the code near here why if keyType === 'secret', the third arg, isPublic, is set to true. Maybe that can never happen? If so, an assert would make it clear.

nit: it looks like the ternary could be reexpressed as !!keyType.

This comment has been minimized.

Sign in to view
Copy link
@tniessen

tniessen Mar 2, 2019

Author Member

The code must function correctly, you tested it, but its not clear from the code near here why if keyType === 'secret', the third arg, isPublic, is set to true. Maybe that can never happen? If so, an assert would make it clear.

keyType refers to the type of the asymmetric key (e.g. 'rsa' or 'dsa'), not to 'public', 'private' etc. It is mostly used to distinguish between input / output encoding and to ensure that PKCS#1 is only used if keyType === 'rsa' etc. Sorry about the confusion.

nit: it looks like the ternary could be reexpressed as !!keyType.

I think we still need all three possible values (true, false, undefined). undefined means that it can either be true or false.
}

// Parses the private key encoding based on an object. keyType must be undefined
@@ -213,26 +219,31 @@ function parsePrivateKeyEncoding(enc, keyType, objName) {
return parseKeyEncoding(enc, keyType, false, objName);
}

function getKeyObjectHandle(key, isPublic, allowKeyObject) {
if (!allowKeyObject) {
function getKeyObjectHandle(key, ctx) {
if (ctx === kCreatePrivate) {
throw new ERR_INVALID_ARG_TYPE(
'key',
['string', 'Buffer', 'TypedArray', 'DataView'],
key
);
}
if (isPublic != null) {
const expectedType = isPublic ? 'public' : 'private';
if (key.type !== expectedType)
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(key.type, expectedType);

if (key.type !== 'private') {
if (ctx === kConsumePrivate || ctx === kCreatePublic)
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(key.type, 'private');
if (key.type !== 'public') {
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(key.type,
'private or public');
}
}

return key[kHandle];
}

function prepareAsymmetricKey(key, isPublic, allowKeyObject = true) {
function prepareAsymmetricKey(key, ctx) {
if (isKeyObject(key)) {
// Best case: A key object, as simple as that.
return { data: getKeyObjectHandle(key, isPublic, allowKeyObject) };
return { data: getKeyObjectHandle(key, ctx) };
} else if (typeof key === 'string' || isArrayBufferView(key)) {
// Expect PEM by default, mostly for backward compatibility.
return { format: kKeyFormatPEM, data: key };
@@ -241,32 +252,32 @@ function prepareAsymmetricKey(key, isPublic, allowKeyObject = true) {
// The 'key' property can be a KeyObject as well to allow specifying
// additional options such as padding along with the key.
if (isKeyObject(data))
return { data: getKeyObjectHandle(data, isPublic, allowKeyObject) };
return { data: getKeyObjectHandle(data, ctx) };
// Either PEM or DER using PKCS#1 or SPKI.
if (!isStringOrBuffer(data)) {
throw new ERR_INVALID_ARG_TYPE(
'key',
['string', 'Buffer', 'TypedArray', 'DataView',
...(allowKeyObject ? ['KeyObject'] : [])],
...(ctx !== kCreatePrivate ? ['KeyObject'] : [])],
key);
}
return { data, ...parseKeyEncoding(key, undefined, isPublic) };
return { data, ...parseKeyEncoding(key, undefined) };
} else {
throw new ERR_INVALID_ARG_TYPE(
'key',
['string', 'Buffer', 'TypedArray', 'DataView',
...(allowKeyObject ? ['KeyObject'] : [])],
...(ctx !== kCreatePrivate ? ['KeyObject'] : [])],
key
);
}
}

function preparePrivateKey(key, allowKeyObject) {
return prepareAsymmetricKey(key, false, allowKeyObject);
function preparePrivateKey(key) {
return prepareAsymmetricKey(key, kConsumePrivate);
}

function preparePublicOrPrivateKey(key, allowKeyObject) {
return prepareAsymmetricKey(key, undefined, allowKeyObject);
function preparePublicOrPrivateKey(key) {
return prepareAsymmetricKey(key, kConsumePublic);
}

function prepareSecretKey(key, bufferOnly = false) {
@@ -296,14 +307,15 @@ function createSecretKey(key) {
}

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

function createPrivateKey(key) {
const { format, type, data, passphrase } = preparePrivateKey(key, false);
const { format, type, data, passphrase } =
prepareAsymmetricKey(key, kCreatePrivate);
const handle = new KeyObjectHandle(kKeyTypePrivate);
handle.init(data, format, type, passphrase);
return new PrivateKeyObject(handle);
Copy path View file
2 src/node_crypto.cc
@@ -3414,7 +3414,7 @@ void KeyObject::Init(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(args.Length(), 3);

offset = 0;
pkey = GetPublicOrPrivateKeyFromJs(args, &offset, false);
pkey = GetPublicOrPrivateKeyFromJs(args, &offset, true);
if (!pkey)
return;
key->InitPublic(pkey);
Copy path View file
36 test/parallel/test-crypto-key-objects.js
@@ -59,9 +59,27 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
}

{
// Passing an existing key object should throw.
// Passing an existing public key object to createPublicKey should throw.
const publicKey = createPublicKey(publicPem);
common.expectsError(() => createPublicKey(publicKey), {
type: TypeError,
code: 'ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE',
message: 'Invalid key object type public, expected private.'
});

// Constructing a private key from a public key should be impossible, even
// if the public key was derived from a private key.
common.expectsError(() => createPrivateKey(createPublicKey(privatePem)), {
type: TypeError,
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "key" argument must be one of type string, Buffer, ' +
'TypedArray, or DataView. Received type object'
});

// Similarly, passing an existing private key object to createPrivateKey
// should throw.
const privateKey = createPrivateKey(privatePem);
common.expectsError(() => createPrivateKey(privateKey), {
type: TypeError,
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "key" argument must be one of type string, Buffer, ' +
@@ -80,6 +98,12 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
assert.strictEqual(privateKey.asymmetricKeyType, 'rsa');
assert.strictEqual(privateKey.symmetricKeySize, undefined);

// It should be possible to derive a public key from a private key.
const derivedPublicKey = createPublicKey(privateKey);
assert.strictEqual(derivedPublicKey.type, 'public');
assert.strictEqual(derivedPublicKey.asymmetricKeyType, 'rsa');
assert.strictEqual(derivedPublicKey.symmetricKeySize, undefined);

const publicDER = publicKey.export({
format: 'der',
type: 'pkcs1'
@@ -95,8 +119,18 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');

const plaintext = Buffer.from('Hello world', 'utf8');
const ciphertexts = [
// Encrypt using the public key.
publicEncrypt(publicKey, plaintext),
publicEncrypt({ key: publicKey }, plaintext),

// Encrypt using the private key.
publicEncrypt(privateKey, plaintext),
publicEncrypt({ key: privateKey }, plaintext),

// Encrypt using a public key derived from the private key.
publicEncrypt(derivedPublicKey, plaintext),
publicEncrypt({ key: derivedPublicKey }, plaintext),

// Test distinguishing PKCS#1 public and private keys based on the
// DER-encoded data only.
publicEncrypt({ format: 'der', type: 'pkcs1', key: publicDER }, plaintext),
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.