Skip to content

Commit

Permalink
crypto: always accept private keys as public keys
Browse files Browse the repository at this point in the history
Some APIs already accept private keys instead of public keys. This
changes all relevant crypto APIs to do so.

PR-URL: nodejs#25217
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
  • Loading branch information
tniessen authored and refack committed Jan 10, 2019
1 parent cac7eb7 commit c528de9
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 42 deletions.
16 changes: 16 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -1379,6 +1379,9 @@ This can be called many times with new data as it is streamed.
<!-- YAML
added: v0.1.92
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/25217
description: The key can now be a private key.
- version: v8.0.0
pr-url: https://github.com/nodejs/node/pull/11705
description: Support for RSASSA-PSS and additional options was added.
Expand Down Expand Up @@ -1419,6 +1422,9 @@ The `verify` object can not be used again after `verify.verify()` has been
called. Multiple calls to `verify.verify()` will result in an error being
thrown.

Because public keys can be derived from private keys, a private key may
be passed instead of a public key.

## `crypto` module methods and properties

### crypto.constants
Expand Down Expand Up @@ -1829,6 +1835,10 @@ must be an object with the properties described above.
### crypto.createPublicKey(key)
<!-- YAML
added: v11.6.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/25217
description: The `key` argument can now be a private key.
-->
* `key` {Object | string | Buffer}
- `key`: {string | Buffer}
Expand All @@ -1843,6 +1853,12 @@ 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`.

### crypto.createSecretKey(key)
<!-- YAML
added: v11.6.0
Expand Down
7 changes: 1 addition & 6 deletions lib/internal/crypto/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,6 @@ function prepareAsymmetricKey(key, isPublic, allowKeyObject = true) {
}
}

function preparePublicKey(key, allowKeyObject) {
return prepareAsymmetricKey(key, true, allowKeyObject);
}

function preparePrivateKey(key, allowKeyObject) {
return prepareAsymmetricKey(key, false, allowKeyObject);
}
Expand Down Expand Up @@ -300,7 +296,7 @@ function createSecretKey(key) {
}

function createPublicKey(key) {
const { format, type, data } = preparePublicKey(key, false);
const { format, type, data } = preparePublicOrPrivateKey(key, false);
const handle = new KeyObjectHandle(kKeyTypePublic);
handle.init(data, format, type);
return new PublicKeyObject(handle);
Expand All @@ -326,7 +322,6 @@ module.exports = {
// These are designed for internal use only and should not be exposed.
parsePublicKeyEncoding,
parsePrivateKeyEncoding,
preparePublicKey,
preparePrivateKey,
preparePublicOrPrivateKey,
prepareSecretKey,
Expand Down
9 changes: 5 additions & 4 deletions lib/internal/crypto/sig.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const {
} = require('internal/crypto/util');
const {
preparePrivateKey,
preparePublicKey
preparePublicOrPrivateKey
} = require('internal/crypto/keys');
const { Writable } = require('stream');

Expand Down Expand Up @@ -112,8 +112,9 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
const {
data,
format,
type
} = preparePublicKey(options, true);
type,
passphrase
} = preparePublicOrPrivateKey(options, true);

sigEncoding = sigEncoding || getDefaultEncoding();

Expand All @@ -125,7 +126,7 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
signature = validateArrayBufferView(toBuf(signature, sigEncoding),
'signature');

return this[kHandle].verify(data, format, type, signature,
return this[kHandle].verify(data, format, type, passphrase, signature,
rsaPadding, pssSaltLength);
};

Expand Down
28 changes: 2 additions & 26 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3012,30 +3012,6 @@ static PublicKeyEncodingConfig GetPublicKeyEncodingFromJs(
return result;
}

static ManagedEVPPKey GetPublicKeyFromJs(
const FunctionCallbackInfo<Value>& args,
unsigned int* offset,
bool allow_key_object) {
if (args[*offset]->IsString() || Buffer::HasInstance(args[*offset])) {
Environment* env = Environment::GetCurrent(args);
ByteSource key = ByteSource::FromStringOrBuffer(env, args[(*offset)++]);
PublicKeyEncodingConfig config =
GetPublicKeyEncodingFromJs(args, offset, kKeyContextInput);
EVPKeyPointer pkey;
ParsePublicKey(&pkey, config, key.get(), key.size());
if (!pkey)
ThrowCryptoError(env, ERR_get_error(), "Failed to read public key");
return ManagedEVPPKey(pkey.release());
} else {
CHECK(args[*offset]->IsObject() && allow_key_object);
KeyObject* key;
ASSIGN_OR_RETURN_UNWRAP(&key, args[*offset].As<Object>(), ManagedEVPPKey());
CHECK_EQ(key->GetKeyType(), kKeyTypePublic);
(*offset) += 3;
return key->GetAsymmetricKey();
}
}

static NonCopyableMaybe<PrivateKeyEncodingConfig> GetPrivateKeyEncodingFromJs(
const FunctionCallbackInfo<Value>& args,
unsigned int* offset,
Expand Down Expand Up @@ -3397,7 +3373,7 @@ void KeyObject::Init(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(args.Length(), 3);

offset = 0;
pkey = GetPublicKeyFromJs(args, &offset, false);
pkey = GetPublicOrPrivateKeyFromJs(args, &offset, false);
if (!pkey)
return;
key->InitPublic(pkey);
Expand Down Expand Up @@ -4679,7 +4655,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());

unsigned int offset = 0;
ManagedEVPPKey pkey = GetPublicKeyFromJs(args, &offset, true);
ManagedEVPPKey pkey = GetPublicOrPrivateKeyFromJs(args, &offset, true);

char* hbuf = Buffer::Data(args[offset]);
ssize_t hlen = Buffer::Length(args[offset]);
Expand Down
16 changes: 10 additions & 6 deletions test/parallel/test-crypto-keygen.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,23 @@ function assertApproximateSize(key, expectedSize) {
function testEncryptDecrypt(publicKey, privateKey) {
const message = 'Hello Node.js world!';
const plaintext = Buffer.from(message, 'utf8');
const ciphertext = publicEncrypt(publicKey, plaintext);
const received = privateDecrypt(privateKey, ciphertext);
assert.strictEqual(received.toString('utf8'), message);
for (const key of [publicKey, privateKey]) {
const ciphertext = publicEncrypt(key, plaintext);
const received = privateDecrypt(privateKey, ciphertext);
assert.strictEqual(received.toString('utf8'), message);
}
}

// Tests that a key pair can be used for signing / verification.
function testSignVerify(publicKey, privateKey) {
const message = 'Hello Node.js world!';
const signature = createSign('SHA256').update(message)
.sign(privateKey, 'hex');
const okay = createVerify('SHA256').update(message)
.verify(publicKey, signature, 'hex');
assert(okay);
for (const key of [publicKey, privateKey]) {
const okay = createVerify('SHA256').update(message)
.verify(key, signature, 'hex');
assert(okay);
}
}

// Constructs a regular expression for a PEM-encoded key with the given label.
Expand Down

0 comments on commit c528de9

Please sign in to comment.