Skip to content

Commit 2c9fb14

Browse files
stefanmbrvagg
authored andcommitted
crypto: Improve error checking and reporting
Added checks where necessary to prevent hard crashes and gave precedence to returning the OpenSSL error strings instead of generic error strings. PR-URL: #3753 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 530bb91 commit 2c9fb14

File tree

1 file changed

+26
-16
lines changed

1 file changed

+26
-16
lines changed

src/node_crypto.cc

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3394,10 +3394,14 @@ void Hmac::HmacInit(const char* hash_type, const char* key, int key_len) {
33943394
return env()->ThrowError("Unknown message digest");
33953395
}
33963396
HMAC_CTX_init(&ctx_);
3397+
int result = 0;
33973398
if (key_len == 0) {
3398-
HMAC_Init(&ctx_, "", 0, md_);
3399+
result = HMAC_Init(&ctx_, "", 0, md_);
33993400
} else {
3400-
HMAC_Init(&ctx_, key, key_len, md_);
3401+
result = HMAC_Init(&ctx_, key, key_len, md_);
3402+
}
3403+
if (!result) {
3404+
return ThrowCryptoError(env(), ERR_get_error());
34013405
}
34023406
initialised_ = true;
34033407
}
@@ -3518,7 +3522,8 @@ void Hash::New(const FunctionCallbackInfo<Value>& args) {
35183522

35193523
Hash* hash = new Hash(env, args.This());
35203524
if (!hash->HashInit(*hash_type)) {
3521-
return env->ThrowError("Digest method not supported");
3525+
return ThrowCryptoError(env, ERR_get_error(),
3526+
"Digest method not supported");
35223527
}
35233528
}
35243529

@@ -3530,6 +3535,9 @@ bool Hash::HashInit(const char* hash_type) {
35303535
return false;
35313536
EVP_MD_CTX_init(&mdctx_);
35323537
EVP_DigestInit_ex(&mdctx_, md_, nullptr);
3538+
if (0 != ERR_peek_error()) {
3539+
return false;
3540+
}
35333541
initialised_ = true;
35343542
return true;
35353543
}
@@ -4211,7 +4219,8 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
42114219

42124220
bool DiffieHellman::Init(int primeLength, int g) {
42134221
dh = DH_new();
4214-
DH_generate_parameters_ex(dh, primeLength, g, 0);
4222+
if (!DH_generate_parameters_ex(dh, primeLength, g, 0))
4223+
return false;
42154224
bool result = VerifyContext();
42164225
if (!result)
42174226
return false;
@@ -4304,7 +4313,7 @@ void DiffieHellman::New(const FunctionCallbackInfo<Value>& args) {
43044313
}
43054314

43064315
if (!initialized) {
4307-
return env->ThrowError("Initialization failed");
4316+
return ThrowCryptoError(env, ERR_get_error(), "Initialization failed");
43084317
}
43094318
}
43104319

@@ -4315,11 +4324,11 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo<Value>& args) {
43154324
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
43164325

43174326
if (!diffieHellman->initialised_) {
4318-
return env->ThrowError("Not initialized");
4327+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
43194328
}
43204329

43214330
if (!DH_generate_key(diffieHellman->dh)) {
4322-
return env->ThrowError("Key generation failed");
4331+
return ThrowCryptoError(env, ERR_get_error(), "Key generation failed");
43234332
}
43244333

43254334
int dataSize = BN_num_bytes(diffieHellman->dh->pub_key);
@@ -4338,7 +4347,7 @@ void DiffieHellman::GetPrime(const FunctionCallbackInfo<Value>& args) {
43384347
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
43394348

43404349
if (!diffieHellman->initialised_) {
4341-
return env->ThrowError("Not initialized");
4350+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
43424351
}
43434352

43444353
int dataSize = BN_num_bytes(diffieHellman->dh->p);
@@ -4356,7 +4365,7 @@ void DiffieHellman::GetGenerator(const FunctionCallbackInfo<Value>& args) {
43564365
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
43574366

43584367
if (!diffieHellman->initialised_) {
4359-
return env->ThrowError("Not initialized");
4368+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
43604369
}
43614370

43624371
int dataSize = BN_num_bytes(diffieHellman->dh->g);
@@ -4374,7 +4383,7 @@ void DiffieHellman::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
43744383
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
43754384

43764385
if (!diffieHellman->initialised_) {
4377-
return env->ThrowError("Not initialized");
4386+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
43784387
}
43794388

43804389
if (diffieHellman->dh->pub_key == nullptr) {
@@ -4397,7 +4406,7 @@ void DiffieHellman::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
43974406
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
43984407

43994408
if (!diffieHellman->initialised_) {
4400-
return env->ThrowError("Not initialized");
4409+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
44014410
}
44024411

44034412
if (diffieHellman->dh->priv_key == nullptr) {
@@ -4420,7 +4429,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
44204429
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
44214430

44224431
if (!diffieHellman->initialised_) {
4423-
return env->ThrowError("Not initialized");
4432+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
44244433
}
44254434

44264435
ClearErrorOnReturn clear_error_on_return;
@@ -4453,7 +4462,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
44534462
delete[] data;
44544463

44554464
if (!checked) {
4456-
return env->ThrowError("Invalid key");
4465+
return ThrowCryptoError(env, ERR_get_error(), "Invalid Key");
44574466
} else if (checkResult) {
44584467
if (checkResult & DH_CHECK_PUBKEY_TOO_SMALL) {
44594468
return env->ThrowError("Supplied key is too small");
@@ -4490,7 +4499,7 @@ void DiffieHellman::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
44904499
Environment* env = diffieHellman->env();
44914500

44924501
if (!diffieHellman->initialised_) {
4493-
return env->ThrowError("Not initialized");
4502+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
44944503
}
44954504

44964505
if (args.Length() == 0) {
@@ -4509,7 +4518,7 @@ void DiffieHellman::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
45094518
Environment* env = diffieHellman->env();
45104519

45114520
if (!diffieHellman->initialised_) {
4512-
return env->ThrowError("Not initialized");
4521+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
45134522
}
45144523

45154524
if (args.Length() == 0) {
@@ -4531,7 +4540,8 @@ void DiffieHellman::VerifyErrorGetter(Local<String> property,
45314540
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
45324541

45334542
if (!diffieHellman->initialised_)
4534-
return diffieHellman->env()->ThrowError("Not initialized");
4543+
return ThrowCryptoError(diffieHellman->env(), ERR_get_error(),
4544+
"Not initialized");
45354545

45364546
args.GetReturnValue().Set(diffieHellman->verifyError_);
45374547
}

0 commit comments

Comments
 (0)