commit 365d0fcc6db6a628c0ff118f2d5b03cebdcd4734 Author: Nick Mathewson nickm@torproject.org Date: Wed May 4 09:43:47 2016 -0400
Cover all our DH code, and/or mark it unreachable. --- src/common/crypto.c | 15 ++++++++++----- src/test/test_crypto.c | 14 +++++++++++++- 2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/common/crypto.c b/src/common/crypto.c index 24a9590..9551435 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -2165,9 +2165,14 @@ crypto_set_tls_dh_prime(void) int r;
/* If the space is occupied, free the previous TLS DH prime */ - if (dh_param_p_tls) { + if (BUG(dh_param_p_tls)) { + /* LCOV_EXCL_START + * + * We shouldn't be calling this twice. + */ BN_clear_free(dh_param_p_tls); dh_param_p_tls = NULL; + /* LCOV_EXCL_STOP */ }
tls_prime = BN_new(); @@ -2199,8 +2204,8 @@ init_dh_param(void) { BIGNUM *circuit_dh_prime; int r; - if (dh_param_p && dh_param_g) - return; + if (BUG(dh_param_p && dh_param_g)) + return; // LCOV_EXCL_LINE This function isn't supposed to be called twice.
circuit_dh_prime = BN_new(); tor_assert(circuit_dh_prime); @@ -2366,8 +2371,8 @@ tor_check_dh_key(int severity, BIGNUM *bn) tor_assert(bn); x = BN_new(); tor_assert(x); - if (!dh_param_p) - init_dh_param(); + if (BUG(!dh_param_p)) + init_dh_param(); //LCOV_EXCL_LINE we already checked whether we did this. BN_set_word(x, 1); if (BN_cmp(bn,x)<=0) { log_fn(severity, LD_CRYPTO, "DH key must be at least 2."); diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index dd9a376..e981eac 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -58,7 +58,6 @@ test_crypto_dh(void *arg) tt_int_op(s1len,OP_EQ, s2len); tt_mem_op(s1,OP_EQ, s2, s1len);
- /* test dh_dup; make sure it works the same. */ dh1_dup = crypto_dh_dup(dh1); s1len = crypto_dh_compute_secret(LOG_WARN, dh1_dup, p2, DH_BYTES, s1, 50); @@ -132,6 +131,19 @@ test_crypto_dh(void *arg) tt_int_op(-1, OP_EQ, s1len); }
+ { + /* provoke an error in the openssl DH_compute_key function; make sure we + * survive. */ + tt_assert(! crypto_dh_get_public(dh1, p1, DH_BYTES)); + + crypto_dh_free(dh2); + dh2= crypto_dh_new(DH_TYPE_CIRCUIT); /* no private key set */ + s1len = crypto_dh_compute_secret(LOG_WARN, dh2, + p1, DH_BYTES, + s1, 50); + tt_int_op(s1len, OP_EQ, -1); + } + done: crypto_dh_free(dh1); crypto_dh_free(dh2);