From cbf6a01c177c5acbe3a606cffe4dfb5b8007964b Mon Sep 17 00:00:00 2001 From: himself65 Date: Fri, 9 Jul 2021 12:07:32 +0800 Subject: [PATCH] crypto: fix `generateKeyPair` with encoding 'jwk' Fixes: https://github.com/nodejs/node/issues/39205 PR-URL: https://github.com/nodejs/node/pull/39319 Reviewed-By: Filip Skokan Reviewed-By: James M Snell --- lib/internal/crypto/keygen.js | 5 +- lib/internal/crypto/keys.js | 41 +++------- lib/internal/errors.js | 2 - src/crypto/crypto_ec.cc | 28 +++++++ src/crypto/crypto_keys.cc | 57 ++++++++------ src/crypto/crypto_keys.h | 3 +- src/node_errors.h | 3 + test/parallel/test-crypto-keygen.js | 114 ++++++++++++++++++++++++++++ 8 files changed, 195 insertions(+), 58 deletions(-) diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js index 06490e24a9c24f..49a7f044cb66cd 100644 --- a/lib/internal/crypto/keygen.js +++ b/lib/internal/crypto/keygen.js @@ -31,6 +31,7 @@ const { SecretKeyObject, parsePublicKeyEncoding, parsePrivateKeyEncoding, + isJwk } = require('internal/crypto/keys'); const { @@ -60,7 +61,9 @@ const { const { isArrayBufferView } = require('internal/util/types'); function wrapKey(key, ctor) { - if (typeof key === 'string' || isArrayBufferView(key)) + if (typeof key === 'string' || + isArrayBufferView(key) || + isJwk(key)) return key; return new ctor(key); } diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index c24b2d14eb5001..27e28942fdaa8a 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -17,6 +17,7 @@ const { kKeyTypePrivate, kKeyFormatPEM, kKeyFormatDER, + kKeyFormatJWK, kKeyEncodingPKCS1, kKeyEncodingPKCS8, kKeyEncodingSPKI, @@ -37,8 +38,6 @@ const { ERR_INVALID_ARG_VALUE, ERR_OUT_OF_RANGE, ERR_OPERATION_FAILED, - ERR_CRYPTO_JWK_UNSUPPORTED_CURVE, - ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE, ERR_CRYPTO_INVALID_JWK, } } = require('internal/errors'); @@ -151,7 +150,6 @@ const { const kAsymmetricKeyType = Symbol('kAsymmetricKeyType'); const kAsymmetricKeyDetails = Symbol('kAsymmetricKeyDetails'); - const kAsymmetricKeyJWKProperties = Symbol('kAsymmetricKeyJWKProperties'); function normalizeKeyDetails(details = {}) { if (details.publicExponent !== undefined) { @@ -189,28 +187,6 @@ const { return {}; } } - - [kAsymmetricKeyJWKProperties]() { - switch (this.asymmetricKeyType) { - case 'rsa': return {}; - case 'ec': - switch (this.asymmetricKeyDetails.namedCurve) { - case 'prime256v1': return { crv: 'P-256' }; - case 'secp256k1': return { crv: 'secp256k1' }; - case 'secp384r1': return { crv: 'P-384' }; - case 'secp521r1': return { crv: 'P-521' }; - default: - throw new ERR_CRYPTO_JWK_UNSUPPORTED_CURVE( - this.asymmetricKeyDetails.namedCurve); - } - case 'ed25519': return { crv: 'Ed25519' }; - case 'ed448': return { crv: 'Ed448' }; - case 'x25519': return { crv: 'X25519' }; - case 'x448': return { crv: 'X448' }; - default: - throw new ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE(); - } - } } class PublicKeyObject extends AsymmetricKeyObject { @@ -220,8 +196,7 @@ const { export(options) { if (options && options.format === 'jwk') { - const properties = this[kAsymmetricKeyJWKProperties](); - return this[kHandle].exportJwk(properties); + return this[kHandle].exportJwk({}); } const { format, @@ -242,8 +217,7 @@ const { throw new ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS( 'jwk', 'does not support encryption'); } - const properties = this[kAsymmetricKeyJWKProperties](); - return this[kHandle].exportJwk(properties); + return this[kHandle].exportJwk({}); } const { format, @@ -265,6 +239,8 @@ function parseKeyFormat(formatStr, defaultFormat, optionName) { return kKeyFormatPEM; else if (formatStr === 'der') return kKeyFormatDER; + else if (formatStr === 'jwk') + return kKeyFormatJWK; throw new ERR_INVALID_ARG_VALUE(optionName, formatStr); } @@ -305,12 +281,14 @@ function parseKeyFormatAndType(enc, keyType, isPublic, objName) { isInput ? kKeyFormatPEM : undefined, option('format', objName)); + const isRequired = (!isInput || + format === kKeyFormatDER) && + format !== kKeyFormatJWK; const type = parseKeyType(typeStr, - !isInput || format === kKeyFormatDER, + isRequired, keyType, isPublic, option('type', objName)); - return { format, type }; } @@ -766,4 +744,5 @@ module.exports = { PrivateKeyObject, isKeyObject, isCryptoKey, + isJwk, }; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 6fb884c7df4167..28ae792cc36130 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -923,8 +923,6 @@ E('ERR_CRYPTO_INVALID_JWK', 'Invalid JWK data', TypeError); E('ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE', 'Invalid key object type %s, expected %s.', TypeError); E('ERR_CRYPTO_INVALID_STATE', 'Invalid state for operation %s', Error); -E('ERR_CRYPTO_JWK_UNSUPPORTED_CURVE', 'Unsupported JWK EC curve: %s.', Error); -E('ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE', 'Unsupported JWK Key Type.', Error); E('ERR_CRYPTO_PBKDF2_ERROR', 'PBKDF2 error', Error); E('ERR_CRYPTO_SCRYPT_INVALID_PARAMETER', 'Invalid scrypt parameter', Error); E('ERR_CRYPTO_SCRYPT_NOT_SUPPORTED', 'Scrypt algorithm not supported', Error); diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index f0ad45529e0039..2abe570080e21d 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -740,6 +740,34 @@ Maybe ExportJWKEcKey( return Nothing(); } + Local crv_name; + const int nid = EC_GROUP_get_curve_name(group); + switch (nid) { + case NID_X9_62_prime256v1: + crv_name = OneByteString(env->isolate(), "P-256"); + break; + case NID_secp256k1: + crv_name = OneByteString(env->isolate(), "secp256k1"); + break; + case NID_secp384r1: + crv_name = OneByteString(env->isolate(), "P-384"); + break; + case NID_secp521r1: + crv_name = OneByteString(env->isolate(), "P-521"); + break; + default: { + THROW_ERR_CRYPTO_JWK_UNSUPPORTED_CURVE( + env, "Unsupported JWK EC curve: %s.", OBJ_nid2sn(nid)); + return Nothing(); + } + } + if (target->Set( + env->context(), + env->jwk_crv_string(), + crv_name).IsNothing()) { + return Nothing(); + } + if (key->GetKeyType() == kKeyTypePrivate) { const BIGNUM* pvt = EC_KEY_get0_private_key(ec); return SetEncodedValue( diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 21cc988cee6a18..821a06b1bd1417 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -61,7 +61,11 @@ void GetKeyFormatAndTypeFromJs( config->type_ = Just(static_cast( args[*offset + 1].As()->Value())); } else { - CHECK(context == kKeyContextInput && config->format_ == kKeyFormatPEM); + CHECK( + (context == kKeyContextInput && + config->format_ == kKeyFormatPEM) || + (context == kKeyContextGenerate && + config->format_ == kKeyFormatJWK)); CHECK(args[*offset + 1]->IsNullOrUndefined()); config->type_ = Nothing(); } @@ -487,9 +491,7 @@ Maybe ExportJWKAsymmetricKey( std::shared_ptr key, Local target) { switch (EVP_PKEY_id(key->GetAsymmetricKey().get())) { - case EVP_PKEY_RSA: - // Fall through - case EVP_PKEY_RSA_PSS: return ExportJWKRsaKey(env, key, target); + case EVP_PKEY_RSA: return ExportJWKRsaKey(env, key, target); case EVP_PKEY_EC: return ExportJWKEcKey(env, key, target); case EVP_PKEY_ED25519: // Fall through @@ -499,7 +501,7 @@ Maybe ExportJWKAsymmetricKey( // Fall through case EVP_PKEY_X448: return ExportJWKEdKey(env, key, target); } - THROW_ERR_CRYPTO_INVALID_KEYTYPE(env); + THROW_ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE(env); return Just(false); } @@ -605,6 +607,21 @@ static inline Maybe Tristate(bool b) { return b ? Just(true) : Nothing(); } +Maybe ExportJWKInner(Environment* env, + std::shared_ptr key, + Local result) { + switch (key->GetKeyType()) { + case kKeyTypeSecret: + return ExportJWKSecretKey(env, key, result.As()); + case kKeyTypePublic: + // Fall through + case kKeyTypePrivate: + return ExportJWKAsymmetricKey(env, key, result.As()); + default: + UNREACHABLE(); + } +} + Maybe ManagedEVPPKey::ToEncodedPublicKey( Environment* env, ManagedEVPPKey key, @@ -617,6 +634,11 @@ Maybe ManagedEVPPKey::ToEncodedPublicKey( std::shared_ptr data = KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key)); return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out)); + } else if (config.format_ == kKeyFormatJWK) { + std::shared_ptr data = + KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key)); + *out = Object::New(env->isolate()); + return ExportJWKInner(env, data, *out); } return Tristate(WritePublicKey(env, key.get(), config).ToLocal(out)); @@ -632,6 +654,11 @@ Maybe ManagedEVPPKey::ToEncodedPrivateKey( std::shared_ptr data = KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key)); return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out)); + } else if (config.format_ == kKeyFormatJWK) { + std::shared_ptr data = + KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key)); + *out = Object::New(env->isolate()); + return ExportJWKInner(env, data, *out); } return Tristate(WritePrivateKey(env, key.get(), config).ToLocal(out)); @@ -1211,24 +1238,7 @@ void KeyObjectHandle::ExportJWK( CHECK(args[0]->IsObject()); - switch (key->Data()->GetKeyType()) { - case kKeyTypeSecret: - if (ExportJWKSecretKey(env, key->Data(), args[0].As()) - .IsNothing()) { - return; - } - break; - case kKeyTypePublic: - // Fall through - case kKeyTypePrivate: - if (ExportJWKAsymmetricKey(env, key->Data(), args[0].As()) - .IsNothing()) { - return; - } - break; - default: - UNREACHABLE(); - } + ExportJWKInner(env, key->Data(), args[0]); args.GetReturnValue().Set(args[0]); } @@ -1380,6 +1390,7 @@ void Initialize(Environment* env, Local target) { NODE_DEFINE_CONSTANT(target, kKeyEncodingSEC1); NODE_DEFINE_CONSTANT(target, kKeyFormatDER); NODE_DEFINE_CONSTANT(target, kKeyFormatPEM); + NODE_DEFINE_CONSTANT(target, kKeyFormatJWK); NODE_DEFINE_CONSTANT(target, kKeyTypeSecret); NODE_DEFINE_CONSTANT(target, kKeyTypePublic); NODE_DEFINE_CONSTANT(target, kKeyTypePrivate); diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h index 3662b3a3b8688b..df3ab8ab181437 100644 --- a/src/crypto/crypto_keys.h +++ b/src/crypto/crypto_keys.h @@ -31,7 +31,8 @@ enum PKEncodingType { enum PKFormatType { kKeyFormatDER, - kKeyFormatPEM + kKeyFormatPEM, + kKeyFormatJWK }; enum KeyType { diff --git a/src/node_errors.h b/src/node_errors.h index 96659f3a400826..0f70fe81b9aa1c 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -49,6 +49,8 @@ void OnFatalError(const char* location, const char* message); V(ERR_CRYPTO_INVALID_SCRYPT_PARAMS, RangeError) \ V(ERR_CRYPTO_INVALID_STATE, Error) \ V(ERR_CRYPTO_INVALID_TAG_LENGTH, RangeError) \ + V(ERR_CRYPTO_JWK_UNSUPPORTED_CURVE, Error) \ + V(ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE, Error) \ V(ERR_CRYPTO_OPERATION_FAILED, Error) \ V(ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH, RangeError) \ V(ERR_CRYPTO_UNKNOWN_CIPHER, Error) \ @@ -136,6 +138,7 @@ ERRORS_WITH_CODE(V) V(ERR_CRYPTO_INVALID_SCRYPT_PARAMS, "Invalid scrypt params") \ V(ERR_CRYPTO_INVALID_STATE, "Invalid state") \ V(ERR_CRYPTO_INVALID_TAG_LENGTH, "Invalid taglength") \ + V(ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE, "Unsupported JWK Key Type.") \ V(ERR_CRYPTO_OPERATION_FAILED, "Operation failed") \ V(ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH, \ "Input buffers must have the same byte length") \ diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index d9fb6489786685..ed5986e6bfd421 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/test/parallel/test-crypto-keygen.js @@ -598,6 +598,94 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); passphrase: 'top secret' }); })); + + // Test async elliptic curve key generation with 'jwk' encoding + [ + ['ec', ['P-384', 'P-256', 'P-521', 'secp256k1']], + ['rsa'], + ['ed25519'], + ['ed448'], + ['x25519'], + ['x448'], + ].forEach((types) => { + const [type, options] = types; + switch (type) { + case 'ec': { + return options.forEach((curve) => { + generateKeyPair(type, { + namedCurve: curve, + publicKeyEncoding: { + format: 'jwk' + }, + privateKeyEncoding: { + format: 'jwk' + } + }, common.mustSucceed((publicKey, privateKey) => { + assert.strictEqual(typeof publicKey, 'object'); + assert.strictEqual(typeof privateKey, 'object'); + assert.strictEqual(publicKey.x, privateKey.x); + assert.strictEqual(publicKey.y, privateKey.y); + assert(!publicKey.d); + assert(privateKey.d); + assert.strictEqual(publicKey.kty, 'EC'); + assert.strictEqual(publicKey.kty, privateKey.kty); + assert.strictEqual(publicKey.crv, curve); + assert.strictEqual(publicKey.crv, privateKey.crv); + })); + }); + } + case 'rsa': { + return generateKeyPair(type, { + modulusLength: 4096, + publicKeyEncoding: { + format: 'jwk' + }, + privateKeyEncoding: { + format: 'jwk' + } + }, common.mustSucceed((publicKey, privateKey) => { + assert.strictEqual(typeof publicKey, 'object'); + assert.strictEqual(typeof privateKey, 'object'); + assert.strictEqual(publicKey.kty, 'RSA'); + assert.strictEqual(publicKey.kty, privateKey.kty); + assert.strictEqual(typeof publicKey.n, 'string'); + assert.strictEqual(publicKey.n, privateKey.n); + assert.strictEqual(typeof publicKey.e, 'string'); + assert.strictEqual(publicKey.e, privateKey.e); + assert.strictEqual(typeof privateKey.d, 'string'); + assert.strictEqual(typeof privateKey.p, 'string'); + assert.strictEqual(typeof privateKey.q, 'string'); + assert.strictEqual(typeof privateKey.dp, 'string'); + assert.strictEqual(typeof privateKey.dq, 'string'); + assert.strictEqual(typeof privateKey.qi, 'string'); + })); + } + case 'ed25519': + case 'ed448': + case 'x25519': + case 'x448': { + generateKeyPair(type, { + publicKeyEncoding: { + format: 'jwk' + }, + privateKeyEncoding: { + format: 'jwk' + } + }, common.mustSucceed((publicKey, privateKey) => { + assert.strictEqual(typeof publicKey, 'object'); + assert.strictEqual(typeof privateKey, 'object'); + assert.strictEqual(publicKey.x, privateKey.x); + assert(!publicKey.d); + assert(privateKey.d); + assert.strictEqual(publicKey.kty, 'OKP'); + assert.strictEqual(publicKey.kty, privateKey.kty); + const expectedCrv = `${type.charAt(0).toUpperCase()}${type.slice(1)}`; + assert.strictEqual(publicKey.crv, expectedCrv); + assert.strictEqual(publicKey.crv, privateKey.crv); + })); + } + } + }); } // Test invalid parameter encoding. @@ -621,6 +709,32 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); message: "The property 'options.paramEncoding' is invalid. " + "Received 'otherEncoding'" }); + assert.throws(() => generateKeyPairSync('dsa', { + modulusLength: 4096, + publicKeyEncoding: { + format: 'jwk' + }, + privateKeyEncoding: { + format: 'jwk' + } + }), { + name: 'Error', + code: 'ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE', + message: 'Unsupported JWK Key Type.' + }); + assert.throws(() => generateKeyPairSync('ec', { + namedCurve: 'secp224r1', + publicKeyEncoding: { + format: 'jwk' + }, + privateKeyEncoding: { + format: 'jwk' + } + }), { + name: 'Error', + code: 'ERR_CRYPTO_JWK_UNSUPPORTED_CURVE', + message: 'Unsupported JWK EC curve: secp224r1.' + }); } {