Browse Source

Slight improvements to DH coverage.

Nick Mathewson 8 years ago
parent
commit
d88656ec06
2 changed files with 20 additions and 0 deletions
  1. 9 0
      src/common/crypto.c
  2. 11 0
      src/test/test_crypto.c

+ 9 - 0
src/common/crypto.c

@@ -2269,10 +2269,13 @@ crypto_dh_new(int dh_type)
 
   return res;
  err:
+  /* LCOV_EXCL_START
+   * This error condition is only reached when an allocation fails */
   crypto_log_errors(LOG_WARN, "creating DH object");
   if (res->dh) DH_free(res->dh); /* frees p and g too */
   tor_free(res);
   return NULL;
+  /* LCOV_EXCL_STOP */
 }
 
 /** Return a copy of <b>dh</b>, sharing its internal state. */
@@ -2304,10 +2307,15 @@ crypto_dh_generate_public(crypto_dh_t *dh)
 {
  again:
   if (!DH_generate_key(dh->dh)) {
+    /* LCOV_EXCL_START
+     * To test this we would need some way to tell openssl to break DH. */
     crypto_log_errors(LOG_WARN, "generating DH key");
     return -1;
+    /* LCOV_EXCL_STOP */
   }
   if (tor_check_dh_key(LOG_WARN, dh->dh->pub_key)<0) {
+    /* LCOV_EXCL_START
+     * If this happens, then openssl's DH implementation is busted. */
     log_warn(LD_CRYPTO, "Weird! Our own DH key was invalid.  I guess once-in-"
              "the-universe chances really do happen.  Trying again.");
     /* Free and clear the keys, so OpenSSL will actually try again. */
@@ -2315,6 +2323,7 @@ crypto_dh_generate_public(crypto_dh_t *dh)
     BN_clear_free(dh->dh->priv_key);
     dh->dh->pub_key = dh->dh->priv_key = NULL;
     goto again;
+    /* LCOV_EXCL_STOP */
   }
   return 0;
 }

+ 11 - 0
src/test/test_crypto.c

@@ -27,6 +27,7 @@ static void
 test_crypto_dh(void *arg)
 {
   crypto_dh_t *dh1 = crypto_dh_new(DH_TYPE_CIRCUIT);
+  crypto_dh_t *dh1_dup = NULL;
   crypto_dh_t *dh2 = crypto_dh_new(DH_TYPE_CIRCUIT);
   char p1[DH_BYTES];
   char p2[DH_BYTES];
@@ -41,6 +42,9 @@ test_crypto_dh(void *arg)
   memset(p1, 0, DH_BYTES);
   memset(p2, 0, DH_BYTES);
   tt_mem_op(p1,OP_EQ, p2, DH_BYTES);
+
+  tt_int_op(-1, OP_EQ, crypto_dh_get_public(dh1, p1, 6)); /* too short */
+
   tt_assert(! crypto_dh_get_public(dh1, p1, DH_BYTES));
   tt_mem_op(p1,OP_NE, p2, DH_BYTES);
   tt_assert(! crypto_dh_get_public(dh2, p2, DH_BYTES));
@@ -54,6 +58,12 @@ 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);
+  tt_mem_op(s1,OP_EQ, s2, s1len);
+
   {
     /* XXXX Now fabricate some bad values and make sure they get caught,
      * Check 0, 1, N-1, >= N, etc.
@@ -63,6 +73,7 @@ test_crypto_dh(void *arg)
  done:
   crypto_dh_free(dh1);
   crypto_dh_free(dh2);
+  crypto_dh_free(dh1_dup);
 }
 
 static void