From ece23c90df4d1844ede7cb524efb4df0fabb3f48 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Mon, 29 Oct 2018 17:09:43 +0100 Subject: crypto: Fix valgrind error for keys with passwords in Engines --- lib/crypto/c_src/crypto.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index ecb716c98c..3d16c776c0 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -4524,7 +4524,7 @@ static int get_engine_and_key_id(ErlNifEnv *env, ERL_NIF_TERM key, char ** id, E static char *get_key_password(ErlNifEnv *env, ERL_NIF_TERM key) { ERL_NIF_TERM tmp_term; ErlNifBinary pwd_bin; - char *pwd; + char *pwd = NULL; if (enif_get_map_value(env, key, atom_password, &tmp_term) && enif_inspect_binary(env, tmp_term, &pwd_bin) && zero_terminate(pwd_bin, &pwd) @@ -4549,16 +4549,17 @@ static int get_pkey_private_key(ErlNifEnv *env, ERL_NIF_TERM algorithm, ERL_NIF_ #ifdef HAS_ENGINE_SUPPORT /* Use key stored in engine */ ENGINE *e; - char *id; + char *id = NULL; char *password; if (!get_engine_and_key_id(env, key, &id, &e)) return PKEY_BADARG; password = get_key_password(env, key); *pkey = ENGINE_load_private_key(e, id, NULL, password); + if (password) enif_free(password); + enif_free(id); if (!*pkey) return PKEY_BADARG; - enif_free(id); #else return PKEY_BADARG; #endif @@ -4636,16 +4637,17 @@ static int get_pkey_public_key(ErlNifEnv *env, ERL_NIF_TERM algorithm, ERL_NIF_T #ifdef HAS_ENGINE_SUPPORT /* Use key stored in engine */ ENGINE *e; - char *id; + char *id = NULL; char *password; if (!get_engine_and_key_id(env, key, &id, &e)) return PKEY_BADARG; password = get_key_password(env, key); *pkey = ENGINE_load_public_key(e, id, NULL, password); + if (password) enif_free(password); + enif_free(id); if (!pkey) return PKEY_BADARG; - enif_free(id); #else return PKEY_BADARG; #endif -- cgit v1.2.3 From 34d377f0a557478de13b56934389144c338e367b Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 15 Nov 2018 13:08:28 +0100 Subject: crypto: Fix valgrind errors in privkey_to_pubkey --- lib/crypto/c_src/crypto.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index 3d16c776c0..b77e580391 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -5434,6 +5434,7 @@ static ERL_NIF_TERM privkey_to_pubkey_nif(ErlNifEnv* env, int argc, const ERL_NI RSA_get0_key(rsa, &n, &e, &d); result[0] = bin_from_bn(env, e); // Exponent E result[1] = bin_from_bn(env, n); // Modulus N = p*q + RSA_free(rsa); EVP_PKEY_free(pkey); return enif_make_list_from_array(env, result, 2); } @@ -5448,6 +5449,7 @@ static ERL_NIF_TERM privkey_to_pubkey_nif(ErlNifEnv* env, int argc, const ERL_NI result[1] = bin_from_bn(env, q); result[2] = bin_from_bn(env, g); result[3] = bin_from_bn(env, pub_key); + DSA_free(dsa); EVP_PKEY_free(pkey); return enif_make_list_from_array(env, result, 4); } -- cgit v1.2.3 From 71deec5cbec7bd7dcf67f30f60f2acd49ae0d0bb Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Mon, 19 Nov 2018 10:16:13 +0100 Subject: crypto: Add some crypto:engine_unload in engine test suite Got valgrind errors for error tests otherwise --- lib/crypto/test/engine_SUITE.erl | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/crypto/test/engine_SUITE.erl b/lib/crypto/test/engine_SUITE.erl index 42d8edf4d2..8a45fc9076 100644 --- a/lib/crypto/test/engine_SUITE.erl +++ b/lib/crypto/test/engine_SUITE.erl @@ -587,11 +587,11 @@ ctrl_cmd_string(Config) when is_list(Config) -> {ok, E} -> case crypto:engine_ctrl_cmd_string(E, <<"TEST">>, <<"17">>) of ok -> + ok = crypto:engine_unload(E), ct:fail(fail_ctrl_cmd_should_fail); {error,ctrl_cmd_failed} -> - ok - end, - ok = crypto:engine_unload(E); + ok = crypto:engine_unload(E) + end; {error, bad_engine_id} -> {skip, "Dynamic Engine not supported"} end @@ -617,11 +617,12 @@ ctrl_cmd_string_optional(Config) when is_list(Config) -> {ok, E} -> case crypto:engine_ctrl_cmd_string(E, <<"TEST">>, <<"17">>, true) of ok -> - ok; - _ -> + ok = crypto:engine_unload(E); + Err -> + ct:log("Error: ~p",[Err]), + ok = crypto:engine_unload(E), ct:fail(fail_ctrl_cmd_string) - end, - ok = crypto:engine_unload(E); + end; {error, bad_engine_id} -> {skip, "Dynamic Engine not supported"} end -- cgit v1.2.3 From a91182b550a3b83e67d5babb4b0675f7d9129364 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Mon, 19 Nov 2018 10:44:07 +0100 Subject: crypto: Fix valgrind errors in pkey_crypt_nif --- lib/crypto/c_src/crypto.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index b77e580391..7367a3824c 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -5122,6 +5122,15 @@ static int get_pkey_crypt_options(ErlNifEnv *env, ERL_NIF_TERM algorithm, ERL_NI return PKEY_OK; } +static size_t size_of_RSA(EVP_PKEY *pkey) { + size_t tmplen; + RSA *rsa = EVP_PKEY_get1_RSA(pkey); + if (rsa == NULL) return 0; + tmplen = RSA_size(rsa); + RSA_free(rsa); + return tmplen; +} + static ERL_NIF_TERM pkey_crypt_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {/* (Algorithm, Data, PublKey=[E,N]|[E,N,D]|[E,N,D,P1,P2,E1,E2,C], Options, IsPrivate, IsEncrypt) */ int i; @@ -5219,9 +5228,8 @@ static ERL_NIF_TERM pkey_crypt_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM #ifdef HAVE_RSA_SSLV23_PADDING if (crypt_opt.rsa_padding == RSA_SSLV23_PADDING) { if (is_encrypt) { - RSA *rsa = EVP_PKEY_get1_RSA(pkey); - if (rsa == NULL) goto badarg; - tmplen = RSA_size(rsa); + tmplen = size_of_RSA(pkey); + if (tmplen == 0) goto badarg; if (!enif_alloc_binary(tmplen, &tmp_bin)) goto badarg; if (RSA_padding_add_SSLv23(tmp_bin.data, tmplen, in_bin.data, in_bin.size) <= 0) goto badarg; @@ -5241,7 +5249,7 @@ static ERL_NIF_TERM pkey_crypt_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM if (crypt_opt.rsa_mgf1_md != NULL && EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, crypt_opt.rsa_mgf1_md) <= 0) goto badarg; if (crypt_opt.rsa_oaep_label.data != NULL && crypt_opt.rsa_oaep_label.size > 0) { - unsigned char *label_copy; + unsigned char *label_copy = NULL; label_copy = OPENSSL_malloc(crypt_opt.rsa_oaep_label.size); if (label_copy == NULL) goto badarg; memcpy((void *)(label_copy), (const void *)(crypt_opt.rsa_oaep_label.data), @@ -5353,14 +5361,11 @@ static ERL_NIF_TERM pkey_crypt_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM if ((i > 0) && argv[0] == atom_rsa && !is_encrypt) { #ifdef HAVE_RSA_SSLV23_PADDING if (crypt_opt.rsa_padding == RSA_SSLV23_PADDING) { - RSA *rsa = EVP_PKEY_get1_RSA(pkey); unsigned char *p; - if (rsa == NULL) goto badarg; - tmplen = RSA_size(rsa); - if (!enif_alloc_binary(tmplen, &tmp_bin)) { - RSA_free(rsa); + tmplen = size_of_RSA(pkey); + if (tmplen == 0) goto badarg; + if (!enif_alloc_binary(tmplen, &tmp_bin)) goto badarg; - } p = out_bin.data; p++; i = RSA_padding_check_SSLv23(tmp_bin.data, tmplen, p, out_bin.size - 1, tmplen); @@ -5371,7 +5376,6 @@ static ERL_NIF_TERM pkey_crypt_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM tmp_bin = in_bin; i = 1; } - RSA_free(rsa); } #endif } -- cgit v1.2.3 From 7cb434ca64c08269c1d4837e0133d1ef2e20bb6e Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Mon, 26 Nov 2018 11:36:46 +0100 Subject: crypto: Fix valgrind error in dh_generate/dh_compute --- lib/crypto/c_src/crypto.c | 71 +++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 30 deletions(-) (limited to 'lib') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index 7367a3824c..df607732bf 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -4234,9 +4234,9 @@ static ERL_NIF_TERM evp_compute_key_nif(ErlNifEnv* env, int argc, const ERL_NIF_ { #ifdef HAVE_ED_CURVE_DH int type; - EVP_PKEY_CTX *ctx; + EVP_PKEY_CTX *ctx = NULL; ErlNifBinary peer_bin, my_bin, key_bin; - EVP_PKEY *peer_key, *my_key; + EVP_PKEY *peer_key = NULL, *my_key = NULL; size_t max_size; if (argv[0] == atom_x25519) type = EVP_PKEY_X25519; @@ -4244,41 +4244,43 @@ static ERL_NIF_TERM evp_compute_key_nif(ErlNifEnv* env, int argc, const ERL_NIF_ else return enif_make_badarg(env); if (!enif_inspect_binary(env, argv[1], &peer_bin) || - !enif_inspect_binary(env, argv[2], &my_bin)) { - return enif_make_badarg(env); - } + !enif_inspect_binary(env, argv[2], &my_bin)) + goto return_badarg; if (!(my_key = EVP_PKEY_new_raw_private_key(type, NULL, my_bin.data, my_bin.size)) || - !(ctx = EVP_PKEY_CTX_new(my_key, NULL))) { - return enif_make_badarg(env); - } + !(ctx = EVP_PKEY_CTX_new(my_key, NULL))) + goto return_badarg; - if (!EVP_PKEY_derive_init(ctx)) { - return enif_make_badarg(env); - } + if (!EVP_PKEY_derive_init(ctx)) + goto return_badarg; if (!(peer_key = EVP_PKEY_new_raw_public_key(type, NULL, peer_bin.data, peer_bin.size)) || - !EVP_PKEY_derive_set_peer(ctx, peer_key)) { - return enif_make_badarg(env); - } + !EVP_PKEY_derive_set_peer(ctx, peer_key)) + goto return_badarg; - if (!EVP_PKEY_derive(ctx, NULL, &max_size)) { - return enif_make_badarg(env); - } + if (!EVP_PKEY_derive(ctx, NULL, &max_size)) + goto return_badarg; if (!enif_alloc_binary(max_size, &key_bin) || - !EVP_PKEY_derive(ctx, key_bin.data, &key_bin.size)) { - return enif_make_badarg(env); - } + !EVP_PKEY_derive(ctx, key_bin.data, &key_bin.size)) + goto return_badarg; if (key_bin.size < max_size) { size_t actual_size = key_bin.size; - if (!enif_realloc_binary(&key_bin, actual_size)) { - return enif_make_badarg(env); - } + if (!enif_realloc_binary(&key_bin, actual_size)) + goto return_badarg; } + EVP_PKEY_free(my_key); + EVP_PKEY_free(peer_key); + EVP_PKEY_CTX_free(ctx); return enif_make_binary(env, &key_bin); + +return_badarg: + if (my_key) EVP_PKEY_free(my_key); + if (peer_key) EVP_PKEY_free(peer_key); + if (ctx) EVP_PKEY_CTX_free(ctx); + return enif_make_badarg(env); #else return atom_notsup; #endif @@ -4289,7 +4291,7 @@ static ERL_NIF_TERM evp_generate_key_nif(ErlNifEnv* env, int argc, const ERL_NIF { #ifdef HAVE_ED_CURVE_DH int type; - EVP_PKEY_CTX *ctx; + EVP_PKEY_CTX *ctx = NULL; EVP_PKEY *pkey = NULL; ERL_NIF_TERM ret_pub, ret_prv; size_t key_len; @@ -4300,22 +4302,30 @@ static ERL_NIF_TERM evp_generate_key_nif(ErlNifEnv* env, int argc, const ERL_NIF if (!(ctx = EVP_PKEY_CTX_new_id(type, NULL))) return enif_make_badarg(env); - if (!EVP_PKEY_keygen_init(ctx)) return atom_error; - if (!EVP_PKEY_keygen(ctx, &pkey)) return atom_error; + if (!EVP_PKEY_keygen_init(ctx)) goto return_error; + if (!EVP_PKEY_keygen(ctx, &pkey)) goto return_error; - if (!EVP_PKEY_get_raw_public_key(pkey, NULL, &key_len)) return atom_error; + if (!EVP_PKEY_get_raw_public_key(pkey, NULL, &key_len)) goto return_error; if (!EVP_PKEY_get_raw_public_key(pkey, enif_make_new_binary(env, key_len, &ret_pub), &key_len)) - return atom_error; + goto return_error; - if (!EVP_PKEY_get_raw_private_key(pkey, NULL, &key_len)) return atom_error; + if (!EVP_PKEY_get_raw_private_key(pkey, NULL, &key_len)) goto return_error; if (!EVP_PKEY_get_raw_private_key(pkey, enif_make_new_binary(env, key_len, &ret_prv), &key_len)) - return atom_error; + goto return_error; + EVP_PKEY_free(pkey); + EVP_PKEY_CTX_free(ctx); return enif_make_tuple2(env, ret_pub, ret_prv); + +return_error: + if (pkey) EVP_PKEY_free(pkey); + if (ctx) EVP_PKEY_CTX_free(ctx); + return atom_error; + #else return atom_notsup; #endif @@ -4811,6 +4821,7 @@ printf("\r\n"); EVP_MD_CTX_free(mdctx); goto badarg; } + EVP_MD_CTX_free(mdctx); #else goto badarg; #endif -- cgit v1.2.3 From a277535507728c760210f9c5aaf0ba1ce9971ce4 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 29 Nov 2018 13:31:25 +0100 Subject: crypto: Engine valgrind fix Fixes memory leak when a command has wrong type. --- lib/crypto/src/crypto.erl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/crypto/src/crypto.erl b/lib/crypto/src/crypto.erl index af53a72e16..72cb9aabfd 100644 --- a/lib/crypto/src/crypto.erl +++ b/lib/crypto/src/crypto.erl @@ -1224,7 +1224,11 @@ engine_load_1(Engine, PreCmds, PostCmds, EngineMethods) -> throw:Error -> %% The engine couldn't initialise, release the structural reference ok = engine_free_nif(Engine), - throw(Error) + throw(Error); + error:badarg -> + %% For example bad argument list, release the structural reference + ok = engine_free_nif(Engine), + error(badarg) end. engine_load_2(Engine, PostCmds, EngineMethods) -> -- cgit v1.2.3