diff --git a/src/crypto/crypto_keygen.h b/src/crypto/crypto_keygen.h index e96d850cee1b5a..59e7e2ce7cec4e 100644 --- a/src/crypto/crypto_keygen.h +++ b/src/crypto/crypto_keygen.h @@ -96,10 +96,13 @@ class KeyGenJob final : public CryptoJob { Environment* env = AsyncWrap::env(); CryptoErrorStore* errors = CryptoJob::errors(); AdditionalParams* params = CryptoJob::params(); - if (status_ == KeyGenJobStatus::OK && - LIKELY(!KeyGenTraits::EncodeKey(env, params, result).IsNothing())) { - *err = Undefined(env->isolate()); - return v8::Just(true); + + if (status_ == KeyGenJobStatus::OK) { + v8::Maybe ret = KeyGenTraits::EncodeKey(env, params, result); + if (ret.IsJust() && ret.FromJust()) { + *err = Undefined(env->isolate()); + } + return ret; } if (errors->Empty()) diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index f72ca8322db960..4e8408086e9bff 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -602,6 +602,11 @@ size_t ManagedEVPPKey::size_of_public_key() const { pkey_.get(), nullptr, &len) == 1) ? len : 0; } +// This maps true to Just(true) and false to Nothing(). +static inline Maybe Tristate(bool b) { + return b ? Just(true) : Nothing(); +} + Maybe ManagedEVPPKey::ToEncodedPublicKey( Environment* env, ManagedEVPPKey key, @@ -613,9 +618,10 @@ Maybe ManagedEVPPKey::ToEncodedPublicKey( // private key. std::shared_ptr data = KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key)); - return Just(KeyObjectHandle::Create(env, data).ToLocal(out)); + return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out)); } - return Just(WritePublicKey(env, key.get(), config).ToLocal(out)); + + return Tristate(WritePublicKey(env, key.get(), config).ToLocal(out)); } Maybe ManagedEVPPKey::ToEncodedPrivateKey( @@ -627,10 +633,10 @@ Maybe ManagedEVPPKey::ToEncodedPrivateKey( if (config.output_key_object_) { std::shared_ptr data = KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key)); - return Just(KeyObjectHandle::Create(env, data).ToLocal(out)); + return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out)); } - return Just(WritePrivateKey(env, key.get(), config).ToLocal(out)); + return Tristate(WritePrivateKey(env, key.get(), config).ToLocal(out)); } NonCopyableMaybe diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 928c0cd38969be..27bb6310b884d1 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -349,9 +349,27 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork { if (status == UV_ECANCELED) return; v8::HandleScope handle_scope(env->isolate()); v8::Context::Scope context_scope(env->context()); + + // TODO(tniessen): Remove the exception handling logic here as soon as we + // can verify that no code path in ToResult will ever throw an exception. + v8::Local exception; v8::Local args[2]; - if (ptr->ToResult(&args[0], &args[1]).FromJust()) + { + node::errors::TryCatchScope try_catch(env); + v8::Maybe ret = ptr->ToResult(&args[0], &args[1]); + if (!ret.IsJust()) { + CHECK(try_catch.HasCaught()); + exception = try_catch.Exception(); + } else if (!ret.FromJust()) { + return; + } + } + + if (exception.IsEmpty()) { ptr->MakeCallback(env->ondone_string(), arraysize(args), args); + } else { + ptr->MakeCallback(env->ondone_string(), 1, &exception); + } } virtual v8::Maybe ToResult( @@ -384,7 +402,8 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork { v8::Local ret[2]; env->PrintSyncTrace(); job->DoThreadPoolWork(); - if (job->ToResult(&ret[0], &ret[1]).FromJust()) { + v8::Maybe result = job->ToResult(&ret[0], &ret[1]); + if (result.IsJust() && result.FromJust()) { args.GetReturnValue().Set( v8::Array::New(env->isolate(), ret, arraysize(ret))); } diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index 55f878971b90cc..def42f393a2ada 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/test/parallel/test-crypto-keygen.js @@ -12,6 +12,7 @@ const { createVerify, generateKeyPair, generateKeyPairSync, + getCurves, publicEncrypt, privateDecrypt, sign, @@ -1314,3 +1315,33 @@ if (!common.hasOpenSSL3) { ); } } + +{ + // This test creates EC key pairs on curves without associated OIDs. + // Specifying a key encoding should not crash. + + if (process.versions.openssl >= '1.1.1i') { + for (const namedCurve of ['Oakley-EC2N-3', 'Oakley-EC2N-4']) { + if (!getCurves().includes(namedCurve)) + continue; + + const params = { + namedCurve, + publicKeyEncoding: { + format: 'der', + type: 'spki' + } + }; + + assert.throws(() => { + generateKeyPairSync('ec', params); + }, { + code: 'ERR_OSSL_EC_MISSING_OID' + }); + + generateKeyPair('ec', params, common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_OSSL_EC_MISSING_OID'); + })); + } + } +}