Skip to content

Commit 0ec94c2

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 e2336f9 commit 0ec94c2

File tree

1 file changed

+26
-16
lines changed

1 file changed

+26
-16
lines changed

src/node_crypto.cc

+26-16
Original file line numberDiff line numberDiff line change
@@ -3233,10 +3233,14 @@ void Hmac::HmacInit(const char* hash_type, const char* key, int key_len) {
32333233
return env()->ThrowError("Unknown message digest");
32343234
}
32353235
HMAC_CTX_init(&ctx_);
3236+
int result = 0;
32363237
if (key_len == 0) {
3237-
HMAC_Init(&ctx_, "", 0, md_);
3238+
result = HMAC_Init(&ctx_, "", 0, md_);
32383239
} else {
3239-
HMAC_Init(&ctx_, key, key_len, md_);
3240+
result = HMAC_Init(&ctx_, key, key_len, md_);
3241+
}
3242+
if (!result) {
3243+
return ThrowCryptoError(env(), ERR_get_error());
32403244
}
32413245
initialised_ = true;
32423246
}
@@ -3357,7 +3361,8 @@ void Hash::New(const FunctionCallbackInfo<Value>& args) {
33573361

33583362
Hash* hash = new Hash(env, args.This());
33593363
if (!hash->HashInit(*hash_type)) {
3360-
return env->ThrowError("Digest method not supported");
3364+
return ThrowCryptoError(env, ERR_get_error(),
3365+
"Digest method not supported");
33613366
}
33623367
}
33633368

@@ -3369,6 +3374,9 @@ bool Hash::HashInit(const char* hash_type) {
33693374
return false;
33703375
EVP_MD_CTX_init(&mdctx_);
33713376
EVP_DigestInit_ex(&mdctx_, md_, nullptr);
3377+
if (0 != ERR_peek_error()) {
3378+
return false;
3379+
}
33723380
initialised_ = true;
33733381
return true;
33743382
}
@@ -4050,7 +4058,8 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
40504058

40514059
bool DiffieHellman::Init(int primeLength, int g) {
40524060
dh = DH_new();
4053-
DH_generate_parameters_ex(dh, primeLength, g, 0);
4061+
if (!DH_generate_parameters_ex(dh, primeLength, g, 0))
4062+
return false;
40544063
bool result = VerifyContext();
40554064
if (!result)
40564065
return false;
@@ -4143,7 +4152,7 @@ void DiffieHellman::New(const FunctionCallbackInfo<Value>& args) {
41434152
}
41444153

41454154
if (!initialized) {
4146-
return env->ThrowError("Initialization failed");
4155+
return ThrowCryptoError(env, ERR_get_error(), "Initialization failed");
41474156
}
41484157
}
41494158

@@ -4154,11 +4163,11 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo<Value>& args) {
41544163
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
41554164

41564165
if (!diffieHellman->initialised_) {
4157-
return env->ThrowError("Not initialized");
4166+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
41584167
}
41594168

41604169
if (!DH_generate_key(diffieHellman->dh)) {
4161-
return env->ThrowError("Key generation failed");
4170+
return ThrowCryptoError(env, ERR_get_error(), "Key generation failed");
41624171
}
41634172

41644173
int dataSize = BN_num_bytes(diffieHellman->dh->pub_key);
@@ -4177,7 +4186,7 @@ void DiffieHellman::GetPrime(const FunctionCallbackInfo<Value>& args) {
41774186
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
41784187

41794188
if (!diffieHellman->initialised_) {
4180-
return env->ThrowError("Not initialized");
4189+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
41814190
}
41824191

41834192
int dataSize = BN_num_bytes(diffieHellman->dh->p);
@@ -4195,7 +4204,7 @@ void DiffieHellman::GetGenerator(const FunctionCallbackInfo<Value>& args) {
41954204
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
41964205

41974206
if (!diffieHellman->initialised_) {
4198-
return env->ThrowError("Not initialized");
4207+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
41994208
}
42004209

42014210
int dataSize = BN_num_bytes(diffieHellman->dh->g);
@@ -4213,7 +4222,7 @@ void DiffieHellman::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
42134222
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
42144223

42154224
if (!diffieHellman->initialised_) {
4216-
return env->ThrowError("Not initialized");
4225+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
42174226
}
42184227

42194228
if (diffieHellman->dh->pub_key == nullptr) {
@@ -4236,7 +4245,7 @@ void DiffieHellman::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
42364245
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
42374246

42384247
if (!diffieHellman->initialised_) {
4239-
return env->ThrowError("Not initialized");
4248+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
42404249
}
42414250

42424251
if (diffieHellman->dh->priv_key == nullptr) {
@@ -4259,7 +4268,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
42594268
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
42604269

42614270
if (!diffieHellman->initialised_) {
4262-
return env->ThrowError("Not initialized");
4271+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
42634272
}
42644273

42654274
ClearErrorOnReturn clear_error_on_return;
@@ -4292,7 +4301,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
42924301
delete[] data;
42934302

42944303
if (!checked) {
4295-
return env->ThrowError("Invalid key");
4304+
return ThrowCryptoError(env, ERR_get_error(), "Invalid Key");
42964305
} else if (checkResult) {
42974306
if (checkResult & DH_CHECK_PUBKEY_TOO_SMALL) {
42984307
return env->ThrowError("Supplied key is too small");
@@ -4329,7 +4338,7 @@ void DiffieHellman::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
43294338
Environment* env = diffieHellman->env();
43304339

43314340
if (!diffieHellman->initialised_) {
4332-
return env->ThrowError("Not initialized");
4341+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
43334342
}
43344343

43354344
if (args.Length() == 0) {
@@ -4348,7 +4357,7 @@ void DiffieHellman::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
43484357
Environment* env = diffieHellman->env();
43494358

43504359
if (!diffieHellman->initialised_) {
4351-
return env->ThrowError("Not initialized");
4360+
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
43524361
}
43534362

43544363
if (args.Length() == 0) {
@@ -4370,7 +4379,8 @@ void DiffieHellman::VerifyErrorGetter(Local<String> property,
43704379
DiffieHellman* diffieHellman = Unwrap<DiffieHellman>(args.Holder());
43714380

43724381
if (!diffieHellman->initialised_)
4373-
return diffieHellman->env()->ThrowError("Not initialized");
4382+
return ThrowCryptoError(diffieHellman->env(), ERR_get_error(),
4383+
"Not initialized");
43744384

43754385
args.GetReturnValue().Set(diffieHellman->verifyError_);
43764386
}

0 commit comments

Comments
 (0)