FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: fix error handling for CryptoJob::ToResult · nodejs/node@6ad0b6f · GitHub

/ node Public

Commit 6ad0b6f

Browse files
tniessen
authored and
MylesBorins
committed
src: fix error handling for CryptoJob::ToResult
PR-URL: #37076 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent ad7e344 commit 6ad0b6f

4 files changed

Lines changed: 69 additions & 10 deletions

File tree

‎src/crypto/crypto_keygen.h‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,13 @@ class KeyGenJob final : public CryptoJob<KeyGenTraits> {
9696
Environment* env = AsyncWrap::env();
9797
CryptoErrorStore* errors = CryptoJob<KeyGenTraits>::errors();
9898
AdditionalParams* params = CryptoJob<KeyGenTraits>::params();
99-
if (status_ == KeyGenJobStatus::OK &&
100-
LIKELY(!KeyGenTraits::EncodeKey(env, params, result).IsNothing())) {
101-
*err = Undefined(env->isolate());
102-
return v8::Just(true);
99+
100+
if (status_ == KeyGenJobStatus::OK) {
101+
v8::Maybe<bool> ret = KeyGenTraits::EncodeKey(env, params, result);
102+
if (ret.IsJust() && ret.FromJust()) {
103+
*err = Undefined(env->isolate());
104+
}
105+
return ret;
103106
}
104107

105108
if (errors->Empty())

‎src/crypto/crypto_keys.cc‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,11 @@ size_t ManagedEVPPKey::size_of_public_key() const {
602602
pkey_.get(), nullptr, &len) == 1) ? len : 0;
603603
}
604604

605+
// This maps true to Just<bool>(true) and false to Nothing<bool>().
606+
static inline Maybe<bool> Tristate(bool b) {
607+
return b ? Just(true) : Nothing<bool>();
608+
}
609+
605610
Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
606611
Environment* env,
607612
ManagedEVPPKey key,
@@ -613,9 +618,10 @@ Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
613618
// private key.
614619
std::shared_ptr<KeyObjectData> data =
615620
KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
616-
return Just(KeyObjectHandle::Create(env, data).ToLocal(out));
621+
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
617622
}
618-
return Just(WritePublicKey(env, key.get(), config).ToLocal(out));
623+
624+
return Tristate(WritePublicKey(env, key.get(), config).ToLocal(out));
619625
}
620626

621627
Maybe<bool> ManagedEVPPKey::ToEncodedPrivateKey(
@@ -627,10 +633,10 @@ Maybe<bool> ManagedEVPPKey::ToEncodedPrivateKey(
627633
if (config.output_key_object_) {
628634
std::shared_ptr<KeyObjectData> data =
629635
KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
630-
return Just(KeyObjectHandle::Create(env, data).ToLocal(out));
636+
return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
631637
}
632638

633-
return Just(WritePrivateKey(env, key.get(), config).ToLocal(out));
639+
return Tristate(WritePrivateKey(env, key.get(), config).ToLocal(out));
634640
}
635641

636642
NonCopyableMaybe<PrivateKeyEncodingConfig>

‎src/crypto/crypto_util.h‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,27 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork {
349349
if (status == UV_ECANCELED) return;
350350
v8::HandleScope handle_scope(env->isolate());
351351
v8::Context::Scope context_scope(env->context());
352+
353+
// TODO(tniessen): Remove the exception handling logic here as soon as we
354+
// can verify that no code path in ToResult will ever throw an exception.
355+
v8::Local<v8::Value> exception;
352356
v8::Local<v8::Value> args[2];
353-
if (ptr->ToResult(&args[0], &args[1]).FromJust())
357+
{
358+
node::errors::TryCatchScope try_catch(env);
359+
v8::Maybe<bool> ret = ptr->ToResult(&args[0], &args[1]);
360+
if (!ret.IsJust()) {
361+
CHECK(try_catch.HasCaught());
362+
exception = try_catch.Exception();
363+
} else if (!ret.FromJust()) {
364+
return;
365+
}
366+
}
367+
368+
if (exception.IsEmpty()) {
354369
ptr->MakeCallback(env->ondone_string(), arraysize(args), args);
370+
} else {
371+
ptr->MakeCallback(env->ondone_string(), 1, &exception);
372+
}
355373
}
356374

357375
virtual v8::Maybe<bool> ToResult(
@@ -384,7 +402,8 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork {
384402
v8::Local<v8::Value> ret[2];
385403
env->PrintSyncTrace();
386404
job->DoThreadPoolWork();
387-
if (job->ToResult(&ret[0], &ret[1]).FromJust()) {
405+
v8::Maybe<bool> result = job->ToResult(&ret[0], &ret[1]);
406+
if (result.IsJust() && result.FromJust()) {
388407
args.GetReturnValue().Set(
389408
v8::Array::New(env->isolate(), ret, arraysize(ret)));
390409
}

‎test/parallel/test-crypto-keygen.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const {
1212
createVerify,
1313
generateKeyPair,
1414
generateKeyPairSync,
15+
getCurves,
1516
publicEncrypt,
1617
privateDecrypt,
1718
sign,
@@ -1314,3 +1315,33 @@ if (!common.hasOpenSSL3) {
13141315
);
13151316
}
13161317
}
1318+
1319+
{
1320+
// This test creates EC key pairs on curves without associated OIDs.
1321+
// Specifying a key encoding should not crash.
1322+
1323+
if (process.versions.openssl >= '1.1.1i') {
1324+
for (const namedCurve of ['Oakley-EC2N-3', 'Oakley-EC2N-4']) {
1325+
if (!getCurves().includes(namedCurve))
1326+
continue;
1327+
1328+
const params = {
1329+
namedCurve,
1330+
publicKeyEncoding: {
1331+
format: 'der',
1332+
type: 'spki'
1333+
}
1334+
};
1335+
1336+
assert.throws(() => {
1337+
generateKeyPairSync('ec', params);
1338+
}, {
1339+
code: 'ERR_OSSL_EC_MISSING_OID'
1340+
});
1341+
1342+
generateKeyPair('ec', params, common.mustCall((err) => {
1343+
assert.strictEqual(err.code, 'ERR_OSSL_EC_MISSING_OID');
1344+
}));
1345+
}
1346+
}
1347+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL