From 74a4f7b390581859b798079484f4af2644d41ee2 Mon Sep 17 00:00:00 2001 From: Doug Hogan Date: Thu, 3 Jan 2019 18:41:37 -0800 Subject: Revamp get_dss_private_key() * Simplify logic by having incremental allocation and only free on error in one place. * Add error checking on all OpenSSL calls. * Make it explicit when you need to be careful with non-ref counted pointers. - set0 calls will save the pointer without reference counting. - On success, set pointers to NULL to avoid double frees since the struct is now responsible for freeing the resources. --- lib/crypto/c_src/dss.c | 71 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 20 deletions(-) (limited to 'lib/crypto/c_src/dss.c') diff --git a/lib/crypto/c_src/dss.c b/lib/crypto/c_src/dss.c index 9d39241382..934b33d87c 100644 --- a/lib/crypto/c_src/dss.c +++ b/lib/crypto/c_src/dss.c @@ -26,36 +26,67 @@ int get_dss_private_key(ErlNifEnv* env, ERL_NIF_TERM key, DSA *dsa) /* key=[P,Q,G,KEY] */ ERL_NIF_TERM head, tail; BIGNUM *dsa_p = NULL, *dsa_q = NULL, *dsa_g = NULL; - BIGNUM *dummy_pub_key, *priv_key = NULL; + BIGNUM *dummy_pub_key = NULL, *priv_key = NULL; - if (!enif_get_list_cell(env, key, &head, &tail) - || !get_bn_from_bin(env, head, &dsa_p) - || !enif_get_list_cell(env, tail, &head, &tail) - || !get_bn_from_bin(env, head, &dsa_q) - || !enif_get_list_cell(env, tail, &head, &tail) - || !get_bn_from_bin(env, head, &dsa_g) - || !enif_get_list_cell(env, tail, &head, &tail) - || !get_bn_from_bin(env, head, &priv_key) - || !enif_is_empty_list(env,tail)) { - if (dsa_p) BN_free(dsa_p); - if (dsa_q) BN_free(dsa_q); - if (dsa_g) BN_free(dsa_g); - if (priv_key) BN_free(priv_key); - return 0; - } + if (!enif_get_list_cell(env, key, &head, &tail)) + goto err; + if (!get_bn_from_bin(env, head, &dsa_p)) + goto err; + + if (!enif_get_list_cell(env, tail, &head, &tail)) + goto err; + if (!get_bn_from_bin(env, head, &dsa_q)) + goto err; + + if (!enif_get_list_cell(env, tail, &head, &tail)) + goto err; + if (!get_bn_from_bin(env, head, &dsa_g)) + goto err; + + if (!enif_get_list_cell(env, tail, &head, &tail)) + goto err; + if (!get_bn_from_bin(env, head, &priv_key)) + goto err; + + if (!enif_is_empty_list(env, tail)) + goto err; /* Note: DSA_set0_key() does not allow setting only the * private key, although DSA_sign() does not use the * public key. Work around this limitation by setting * the public key to a copy of the private key. */ - dummy_pub_key = BN_dup(priv_key); + if ((dummy_pub_key = BN_dup(priv_key)) == NULL) + goto err; + + if (!DSA_set0_pqg(dsa, dsa_p, dsa_q, dsa_g)) + goto err; + /* dsa takes ownership on success */ + dsa_p = NULL; + dsa_q = NULL; + dsa_g = NULL; + + if (!DSA_set0_key(dsa, dummy_pub_key, priv_key)) + goto err; + /* dsa takes ownership on success */ + dummy_pub_key = NULL; + priv_key = NULL; - DSA_set0_pqg(dsa, dsa_p, dsa_q, dsa_g); - DSA_set0_key(dsa, dummy_pub_key, priv_key); return 1; -} + err: + if (dsa_p) + BN_free(dsa_p); + if (dsa_q) + BN_free(dsa_q); + if (dsa_g) + BN_free(dsa_g); + if (priv_key) + BN_free(priv_key); + if (dummy_pub_key) + BN_free(dummy_pub_key); + return 0; +} int get_dss_public_key(ErlNifEnv* env, ERL_NIF_TERM key, DSA *dsa) { -- cgit v1.2.3