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

crypto: add keyObject.params for asymmetric keys #30045

Closed
wants to merge 3 commits into from
Closed

crypto: add keyObject.params for asymmetric keys #30045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af2887c
crypto: add keyObject.params for asymmetric keys
tniessen Oct 20, 2019
ca028d9
fixup! crypto: add keyObject.params for asymmetric keys
tniessen Oct 21, 2019
bfdddd0
fixup! crypto: add keyObject.params for asymmetric keys
tniessen Oct 31, 2019
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
20 changes: 20 additions & 0 deletions doc/api/crypto.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,26 @@ encryption mechanism, PEM-level encryption is not supported when encrypting
a PKCS#8 key. See [RFC 5208][] for PKCS#8 encryption and [RFC 1421][] for
PKCS#1 and SEC1 encryption.

### keyObject.params
<!-- YAML
added: REPLACEME
-->

* {Object}

This property exists only on asymmetric keys. Depending on the type of the key,
this object contains information about the key. None of the information obtained
through this property can be used to uniquely identify a key or to compromise
the security of the key.

For `'rsa'` and `'rsa-pss'` keys, this object has the property `modulusLength`.

For `'dsa'` keys, this object has the properties `modulusLength` and
`divisorLength`.

For `'ec'` keys with a known curve, this object has the string property
`namedCurve`.

### keyObject.symmetricKeySize
<!-- YAML
added: v11.6.0
Expand Down
6 changes: 6 additions & 0 deletions lib/internal/crypto/keys.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,18 @@ class SecretKeyObject extends KeyObject {
}

const kAsymmetricKeyType = Symbol('kAsymmetricKeyType');
const kParams = Symbol('kParams');

class AsymmetricKeyObject extends KeyObject {
get asymmetricKeyType() {
return this[kAsymmetricKeyType] ||
(this[kAsymmetricKeyType] = this[kHandle].getAsymmetricKeyType());
}

get params() {
return this[kParams] ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but could you rewrite this as:

if (this[kParams] === undefined)
  this[kParams] = this[kHandle].getAsymmetricParams();
return this[kParams];

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest something similar at first but it's locally consistent, get asymmetricKeyType() uses the same style.

(this[kParams] = this[kHandle].getAsymmetricParams());
}
}

class PublicKeyObject extends AsymmetricKeyObject {
Expand Down
68 changes: 68 additions & 0 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3761,6 +3761,8 @@ Local<Function> KeyObject::Initialize(Environment* env, Local<Object> target) {
GetSymmetricKeySize);
env->SetProtoMethodNoSideEffect(t, "getAsymmetricKeyType",
GetAsymmetricKeyType);
env->SetProtoMethodNoSideEffect(t, "getAsymmetricParams",
GetAsymmetricParams);
env->SetProtoMethod(t, "export", Export);

auto function = t->GetFunction(env->context()).ToLocalChecked();
Expand Down Expand Up @@ -3912,6 +3914,72 @@ void KeyObject::GetAsymmetricKeyType(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(key->GetAsymmetricKeyType());
}

static inline void SetParam(Environment* env, Local<Object> params,
const char* key, Local<Value> value) {
bool ok = params->DefineOwnProperty(
env->context(),
String::NewFromUtf8(env->isolate(), key, NewStringType::kNormal)
.ToLocalChecked(),
value,
PropertyAttribute::ReadOnly).ToChecked();
CHECK(ok);
}

Local<Value> KeyObject::GetAsymmetricParams() const {
CHECK_NE(key_type_, kKeyTypeSecret);

Local<Object> params = Object::New(env()->isolate());
EVP_PKEY* pkey = asymmetric_key_.get();
RSA* rsa;
DSA* dsa;
EC_KEY* ec;
const EC_GROUP* ec_group;
int ec_curve_id;
const char* ec_curve_name;

switch (EVP_PKEY_id(pkey)) {
case EVP_PKEY_RSA_PSS:
// TODO(tniessen): Also retrieve additional PSS params as soon as OpenSSL
// provides a way to do that.
case EVP_PKEY_RSA:
// TODO(tniessen): This should really be EVP_PKEY_get0_RSA, but OpenSSL does
// not support that for RSA-PSS.
rsa = reinterpret_cast<RSA*>(EVP_PKEY_get0(pkey));
SetParam(env(), params, "modulusLength",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: add the string to PER_ISOLATE_STRING_PROPERTIES in env.h. Ditto the other strings.

Integer::New(env()->isolate(), RSA_bits(rsa)));
// TODO(tniessen): Add publicExponent.
break;
case EVP_PKEY_DSA:
dsa = EVP_PKEY_get0_DSA(pkey);
SetParam(env(), params, "modulusLength",
Integer::New(env()->isolate(), DSA_bits(dsa)));
SetParam(env(), params, "divisorLength",
Integer::New(env()->isolate(), BN_num_bits(DSA_get0_q(dsa))));
break;
case EVP_PKEY_EC:
ec = EVP_PKEY_get0_EC_KEY(pkey);
ec_group = EC_KEY_get0_group(ec);
ec_curve_id = EC_GROUP_get_curve_name(ec_group);
if (ec_curve_id != NID_undef) {
ec_curve_name = OBJ_nid2sn(ec_curve_id);
if (ec_curve_name != nullptr) {
SetParam(env(), params, "namedCurve",
String::NewFromUtf8(env()->isolate(), ec_curve_name,
NewStringType::kNormal).ToLocalChecked());
}
}
break;
}
return params;
}
tniessen marked this conversation as resolved.
Show resolved Hide resolved

void KeyObject::GetAsymmetricParams(const FunctionCallbackInfo<Value>& args) {
KeyObject* key;
ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder());

args.GetReturnValue().Set(key->GetAsymmetricParams());
}

void KeyObject::GetSymmetricKeySize(const FunctionCallbackInfo<Value>& args) {
KeyObject* key;
ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder());
Expand Down
3 changes: 3 additions & 0 deletions src/node_crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,9 @@ class KeyObject : public BaseObject {
static void GetAsymmetricKeyType(
const v8::FunctionCallbackInfo<v8::Value>& args);
v8::Local<v8::Value> GetAsymmetricKeyType() const;
static void GetAsymmetricParams(
const v8::FunctionCallbackInfo<v8::Value>& args);
v8::Local<v8::Value> GetAsymmetricParams() const;

static void GetSymmetricKeySize(
const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
10 changes: 9 additions & 1 deletion test/parallel/test-crypto-key-objects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,20 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
assert.strictEqual(publicKey.type, 'public');
assert.strictEqual(publicKey.asymmetricKeyType, 'rsa');
assert.strictEqual(publicKey.symmetricKeySize, undefined);
assert.strictEqual(publicKey.params.modulusLength, 2048);

const privateKey = createPrivateKey(privatePem);
assert.strictEqual(privateKey.type, 'private');
assert.strictEqual(privateKey.asymmetricKeyType, 'rsa');
assert.strictEqual(privateKey.symmetricKeySize, undefined);
assert.strictEqual(privateKey.params.modulusLength, 2048);

// 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);
assert.strictEqual(derivedPublicKey.params.modulusLength, 2048);

// Test exporting with an invalid options object, this should throw.
for (const opt of [undefined, null, 'foo', 0, NaN]) {
Expand Down Expand Up @@ -293,6 +296,8 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
assert.strictEqual(publicKey.type, 'public');
assert.strictEqual(publicKey.asymmetricKeyType, 'dsa');
assert.strictEqual(publicKey.symmetricKeySize, undefined);
assert.strictEqual(publicKey.params.modulusLength, 1088);
assert.strictEqual(publicKey.params.divisorLength, 160);

const privateKey = createPrivateKey({
key: privateDsa,
Expand All @@ -302,7 +307,8 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
assert.strictEqual(privateKey.type, 'private');
assert.strictEqual(privateKey.asymmetricKeyType, 'dsa');
assert.strictEqual(privateKey.symmetricKeySize, undefined);

assert.strictEqual(privateKey.params.modulusLength, 1088);
assert.strictEqual(privateKey.params.divisorLength, 160);
}

{
Expand All @@ -318,9 +324,11 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',

assert.strictEqual(publicKey.type, 'public');
assert.strictEqual(publicKey.asymmetricKeyType, 'rsa-pss');
assert.strictEqual(publicKey.params.modulusLength, 2048);

assert.strictEqual(privateKey.type, 'private');
assert.strictEqual(privateKey.asymmetricKeyType, 'rsa-pss');
assert.strictEqual(privateKey.params.modulusLength, 2048);

for (const key of [privatePem, privateKey]) {
// Any algorithm should work.
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-crypto-keygen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,12 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
assert.strictEqual(typeof publicKey, 'object');
assert.strictEqual(publicKey.type, 'public');
assert.strictEqual(publicKey.asymmetricKeyType, 'rsa');
assert.strictEqual(publicKey.params.modulusLength, 512);

assert.strictEqual(typeof privateKey, 'object');
assert.strictEqual(privateKey.type, 'private');
assert.strictEqual(privateKey.asymmetricKeyType, 'rsa');
assert.strictEqual(privateKey.params.modulusLength, 512);
}

{
Expand Down Expand Up @@ -465,6 +467,22 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);

testSignVerify(publicKey, { key: privateKey, passphrase: 'secret' });
}));

generateKeyPair('ec', {
namedCurve: 'secp521r1'
}, common.mustCall((err, publicKey, privateKey) => {
assert.ifError(err);

assert.strictEqual(publicKey.type, 'public');
assert.strictEqual(publicKey.asymmetricKeyType, 'ec');
assert.strictEqual(publicKey.params.namedCurve, 'secp521r1');

assert.strictEqual(privateKey.type, 'private');
assert.strictEqual(privateKey.asymmetricKeyType, 'ec');
assert.strictEqual(privateKey.params.namedCurve, 'secp521r1');

testSignVerify(publicKey, privateKey);
}));
}

{
Expand Down Expand Up @@ -643,6 +661,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
assert.strictEqual(typeof publicKey, 'object');
assert.strictEqual(publicKey.type, 'public');
assert.strictEqual(publicKey.asymmetricKeyType, 'rsa');
assert.strictEqual(publicKey.params.modulusLength, 1024);

// The private key should still be a string.
assert.strictEqual(typeof privateKey, 'string');
Expand All @@ -667,6 +686,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
assert.strictEqual(typeof privateKey, 'object');
assert.strictEqual(privateKey.type, 'private');
assert.strictEqual(privateKey.asymmetricKeyType, 'rsa');
assert.strictEqual(privateKey.params.modulusLength, 1024);

testEncryptDecrypt(publicKey, privateKey);
testSignVerify(publicKey, privateKey);
Expand Down