Skip to content

Commit

Permalink
crypto: fix generateKeyPair with encoding 'jwk'
Browse files Browse the repository at this point in the history 
Fixes: #39205

PR-URL: #39319
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@himself65 @targos
himself65 authored and targos committed Aug 2, 2021
1 parent 7a731ef commit cbf6a01
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 58 deletions.
5 changes: 4 additions & 1 deletion lib/internal/crypto/keygen.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const {
SecretKeyObject,
parsePublicKeyEncoding,
parsePrivateKeyEncoding,
isJwk
} = require('internal/crypto/keys');

const {
Expand Down Expand Up @@ -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);
}
Expand Down
41 changes: 10 additions & 31 deletions lib/internal/crypto/keys.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const {
kKeyTypePrivate,
kKeyFormatPEM,
kKeyFormatDER,
kKeyFormatJWK,
kKeyEncodingPKCS1,
kKeyEncodingPKCS8,
kKeyEncodingSPKI,
Expand All @@ -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');
Expand Down Expand Up @@ -151,7 +150,6 @@ const {

const kAsymmetricKeyType = Symbol('kAsymmetricKeyType');
const kAsymmetricKeyDetails = Symbol('kAsymmetricKeyDetails');
const kAsymmetricKeyJWKProperties = Symbol('kAsymmetricKeyJWKProperties');

function normalizeKeyDetails(details = {}) {
if (details.publicExponent !== undefined) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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);
}

Expand Down Expand Up @@ -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 };
}

Expand Down Expand Up @@ -766,4 +744,5 @@ module.exports = {
PrivateKeyObject,
isKeyObject,
isCryptoKey,
isJwk,
};
2 changes: 0 additions & 2 deletions lib/internal/errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
28 changes: 28 additions & 0 deletions src/crypto/crypto_ec.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,34 @@ Maybe<bool> ExportJWKEcKey(
return Nothing<bool>();
}

Local<String> 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<bool>();
}
}
if (target->Set(
env->context(),
env->jwk_crv_string(),
crv_name).IsNothing()) {
return Nothing<bool>();
}

if (key->GetKeyType() == kKeyTypePrivate) {
const BIGNUM* pvt = EC_KEY_get0_private_key(ec);
return SetEncodedValue(
Expand Down
57 changes: 34 additions & 23 deletions src/crypto/crypto_keys.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ void GetKeyFormatAndTypeFromJs(
config->type_ = Just<PKEncodingType>(static_cast<PKEncodingType>(
args[*offset + 1].As<Int32>()->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<PKEncodingType>();
}
Expand Down Expand Up @@ -487,9 +491,7 @@ Maybe<bool> ExportJWKAsymmetricKey(
std::shared_ptr<KeyObjectData> key,
Local<Object> 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
Expand All @@ -499,7 +501,7 @@ Maybe<bool> 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);
}

Expand Down Expand Up @@ -605,6 +607,21 @@ static inline Maybe<bool> Tristate(bool b) {
return b ? Just(true) : Nothing<bool>();
}

Maybe<bool> ExportJWKInner(Environment* env,
std::shared_ptr<KeyObjectData> key,
Local<Value> result) {
switch (key->GetKeyType()) {
case kKeyTypeSecret:
return ExportJWKSecretKey(env, key, result.As<Object>());
case kKeyTypePublic:
// Fall through
case kKeyTypePrivate:
return ExportJWKAsymmetricKey(env, key, result.As<Object>());
default:
UNREACHABLE();
}
}

Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
Environment* env,
ManagedEVPPKey key,
Expand All @@ -617,6 +634,11 @@ Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
std::shared_ptr<KeyObjectData> data =
KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
} else if (config.format_ == kKeyFormatJWK) {
std::shared_ptr<KeyObjectData> 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));
Expand All @@ -632,6 +654,11 @@ Maybe<bool> ManagedEVPPKey::ToEncodedPrivateKey(
std::shared_ptr<KeyObjectData> data =
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
} else if (config.format_ == kKeyFormatJWK) {
std::shared_ptr<KeyObjectData> 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));
Expand Down Expand Up @@ -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<Object>())
.IsNothing()) {
return;
}
break;
case kKeyTypePublic:
// Fall through
case kKeyTypePrivate:
if (ExportJWKAsymmetricKey(env, key->Data(), args[0].As<Object>())
.IsNothing()) {
return;
}
break;
default:
UNREACHABLE();
}
ExportJWKInner(env, key->Data(), args[0]);

args.GetReturnValue().Set(args[0]);
}
Expand Down Expand Up @@ -1380,6 +1390,7 @@ void Initialize(Environment* env, Local<Object> 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);
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/crypto_keys.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ enum PKEncodingType {

enum PKFormatType {
kKeyFormatDER,
kKeyFormatPEM
kKeyFormatPEM,
kKeyFormatJWK
};

enum KeyType {
Expand Down
3 changes: 3 additions & 0 deletions src/node_errors.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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") \
Expand Down
Loading

0 comments on commit cbf6a01

Please sign in to comment.