Cover all our DH code, and/or mark it unreachable.

This commit is contained in:
Nick Mathewson 2016-05-04 09:43:47 -04:00
parent 94b34d1be6
commit 365d0fcc6d
2 changed files with 23 additions and 6 deletions

View File

@ -2165,9 +2165,14 @@ crypto_set_tls_dh_prime(void)
int r; int r;
/* If the space is occupied, free the previous TLS DH prime */ /* 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); BN_clear_free(dh_param_p_tls);
dh_param_p_tls = NULL; dh_param_p_tls = NULL;
/* LCOV_EXCL_STOP */
} }
tls_prime = BN_new(); tls_prime = BN_new();
@ -2199,8 +2204,8 @@ init_dh_param(void)
{ {
BIGNUM *circuit_dh_prime; BIGNUM *circuit_dh_prime;
int r; int r;
if (dh_param_p && dh_param_g) if (BUG(dh_param_p && dh_param_g))
return; return; // LCOV_EXCL_LINE This function isn't supposed to be called twice.
circuit_dh_prime = BN_new(); circuit_dh_prime = BN_new();
tor_assert(circuit_dh_prime); tor_assert(circuit_dh_prime);
@ -2366,8 +2371,8 @@ tor_check_dh_key(int severity, BIGNUM *bn)
tor_assert(bn); tor_assert(bn);
x = BN_new(); x = BN_new();
tor_assert(x); tor_assert(x);
if (!dh_param_p) if (BUG(!dh_param_p))
init_dh_param(); init_dh_param(); //LCOV_EXCL_LINE we already checked whether we did this.
BN_set_word(x, 1); BN_set_word(x, 1);
if (BN_cmp(bn,x)<=0) { if (BN_cmp(bn,x)<=0) {
log_fn(severity, LD_CRYPTO, "DH key must be at least 2."); log_fn(severity, LD_CRYPTO, "DH key must be at least 2.");

View File

@ -58,7 +58,6 @@ test_crypto_dh(void *arg)
tt_int_op(s1len,OP_EQ, s2len); tt_int_op(s1len,OP_EQ, s2len);
tt_mem_op(s1,OP_EQ, s2, s1len); tt_mem_op(s1,OP_EQ, s2, s1len);
/* test dh_dup; make sure it works the same. */ /* test dh_dup; make sure it works the same. */
dh1_dup = crypto_dh_dup(dh1); dh1_dup = crypto_dh_dup(dh1);
s1len = crypto_dh_compute_secret(LOG_WARN, dh1_dup, p2, DH_BYTES, s1, 50); 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); 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: done:
crypto_dh_free(dh1); crypto_dh_free(dh1);
crypto_dh_free(dh2); crypto_dh_free(dh2);