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: don't crash on unknown keyObject.asymmetricKeyType #26786

Closed
wants to merge 7 commits into from
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -1129,18 +1129,23 @@ passing keys as strings or `Buffer`s due to improved security features.
<!-- YAML
added: v11.6.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26786
description: This property now returns `undefined` for KeyObject
instances of unrecognized type instead of aborting.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26774
description: Added support for `'x25519'` and `'x448'`
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26319
description: Added support for `'ed25519'` and `'ed448'`
description: Added support for `'ed25519'` and `'ed448'`.
-->
* {string}

For asymmetric keys, this property represents the type of the embedded key
(`'rsa'`, `'dsa'`, `'ec'`, `'ed25519'`, `'ed448'`, `'x25519'` or `'x448'`).
This property is `undefined` for symmetric keys.
This property is `undefined` for unrecognized `KeyObject` types and symmetric
keys.

### keyObject.export([options])
<!-- YAML
Expand Down
4 changes: 2 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3643,7 +3643,7 @@ void KeyObject::InitPrivate(const ManagedEVPPKey& pkey) {
this->asymmetric_key_ = pkey;
}

Local<String> KeyObject::GetAsymmetricKeyType() const {
Local<Value> KeyObject::GetAsymmetricKeyType() const {
CHECK_NE(this->key_type_, kKeyTypeSecret);
switch (EVP_PKEY_id(this->asymmetric_key_.get())) {
case EVP_PKEY_RSA:
Expand All @@ -3661,7 +3661,7 @@ Local<String> KeyObject::GetAsymmetricKeyType() const {
case EVP_PKEY_X448:
return env()->crypto_x448_string();
default:
CHECK(false);
return Undefined(env()->isolate());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ class KeyObject : public BaseObject {

static void GetAsymmetricKeyType(
const v8::FunctionCallbackInfo<v8::Value>& args);
v8::Local<v8::String> GetAsymmetricKeyType() const;
v8::Local<v8::Value> GetAsymmetricKeyType() const;

static void GetSymmetricKeySize(
const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
28 changes: 28 additions & 0 deletions test/fixtures/test_unknown_privkey.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEugIBADALBgkqhkiG9w0BAQoEggSmMIIEogIBAAKCAQEAwMSNbT9SbSHvXmPt
j1x2Ipk1tUM06301UD91xGcA0232zrIQcKjsPM7bE6YXN0zRxfLJUqalewCk80Ct
6V+E5XtMHUFQt1Ne8HW9U930KnfnQEyU8UwRPoWWeZQhs+sa8ZfggtfN7gq4/wiS
KFNNtSJb24NKoLis31P0nILGC4/JewgE0QaFUoOL+Oc3dMhwWg9/H64sSjhI/SGW
9Sv3M6WcSn7vQCe8oM2vslf3Xm8rHNqZMlXujs7zhRtcr5alKz9BwMJIoGouQrk9
9cgupdYsddgNh2bC4TQR9BMKHj8tV5Uf3Pbf4EoZOFffCbyBZxmKtsYsmhh2FDLA
RzNhKwIDAQABAoIBAHBRlj4ziSmBfmG3Q/ImY8chEkQ9lpYn7GqHr2zyv25yQj6J
Tj72jj+YH9pBCoH0Rr5aCqgX5Y/X/kSmSS8TsvGrd9wL9KX88/KUB+7YAq7EEoBK
nvZB5kJRwC2y/DhDIv3mCrDyYVDz+nrPWaoZb8u861zqEQ+4yzGNT5fqMs8Ewm8A
hxg3GA2R6FC2CymZO884XOxlVac6SNURfA+U+xrcMIXbXpok2Z5eh/kMOeIKwmL0
QEO0U6DEnZm4rJjywu8fEkKbX00YfaDQaiGzRZfvmzkTPIQemXPWARdIvFtJU8Fx
OWWeMumJD1KiU9ISW4e76l7F8UOviT6jEg9rxFECgYEA96WCEIB+O4aO7+s56kOv
vQkEXn959lz7e++S9AV3R19PpBCh50l5v9NSjGQlA4FU4AdBB5EmiX/bLZRHFwHI
KLDsMFuq9id3OPHYIzFP4YjVHTGRPZToJHwy4ePIdZEaeJHY39EEz8oHsSSJlLdm
o0417RsFAfApW2VN63c3JFcCgYEAx0Um/ATsT4ELguVQ+XlquLQdS12XS7zjcwWv
PL8UyooSxcjcbLcJB6DRWXM0NOry7KPUCIF4m3KSjIZypV/v2KVFPCfD3vxZcdB7
xgccqXJMUx7MSs9AMZXTtv5hG7RS5z+ig7Yi/6nzBm21jKYKbFDbqfq8MSfiR6cT
KjR+RU0CgYAm/iFnlcPKfZpd/mylDTlLi3Lrqii6+NMEJam+0GmCjGhOzeugLjqE
ULLLtiz5y1Bg4eOEXH9z4PTSzWkQH1Czz3+w8Y4OqhIknjfI+se4HEJqEVbsGlke
/YtJdAMpN8qyN0ytmQyn5wilBLrA9surZPIqvjlgn77zTBUjwSamiwKBgAqIVS8s
83CgWYNpq4YELOfmXUYGhGC0czE5M7H6R5cNBUD/BOeaJRgKIAaiWDgT0xM+9Y4d
icptm+Fhmd2z3HGPCsHLOEco/3FMm74z0ggCypX6IsIxgiscyDv75hYYyej/LA/a
KK9qxDWqxtXQUOy4uWOapSfT+9ndst2gOKxhAoGAVFcfedCLxummgTtZE91n59pL
TWTk4GgYpWyv6XbHjYrFW2y18qmn0hmEpO+440So0NmGGDtNnPYNUKY/MPjHScwC
FoZMFqqnkmshXz0uDx3gMQK2JDmdF+s3VwZq4Rtb3NJ9v4/WMgWftxaUpAm1/aRC
IHc67mAAez4i8fg2wTQ=
-----END PRIVATE KEY-----
7 changes: 7 additions & 0 deletions test/parallel/test-crypto-key-objects.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
});
}

{
// This should not cause a crash: https://github.com/nodejs/node/pull/26786
const pem = fixtures.readSync('test_unknown_privkey.pem', 'ascii');
const key = createPrivateKey(pem);
assert.strictEqual(key.asymmetricKeyType, undefined);
}

[
{ private: fixtures.readSync('test_ed25519_privkey.pem', 'ascii'),
public: fixtures.readSync('test_ed25519_pubkey.pem', 'ascii'),
Expand Down