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

diff --git a/src/common/crypto.c b/src/common/crypto.c
index 24a9590cd696b1d560b1d9d49fb897e65b742393..9551435f6826acd3f9f4f63fd7c1ac7e437c05d4 100644 (file)
--- 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 dd9a3765f68793ea26f2477bb58de1d9af8bdc00..e981eac602367ac7f09bf149ec3f9b2fd4a55394 100644 (file)
--- 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);