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

crypto: always accept private keys as public keys #25217

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
16 changes: 16 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -1369,6 +1369,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 @@ -1409,6 +1412,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 @@ -1819,6 +1825,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 @@ -1833,6 +1843,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