[tor-commits] [tor/master] Improve test coverage a little on onion*.c

nickm at torproject.org nickm at torproject.org
Wed Jul 6 16:37:58 UTC 2016


commit ae22c249c3423acfb7938a36c2c8c5b2cac7fb29
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Jul 6 12:37:52 2016 -0400

    Improve test coverage a little on onion*.c
---
 src/or/onion.c      | 15 +++++++++++++++
 src/or/onion_ntor.c |  5 +++++
 src/or/onion_tap.c  | 12 ++++++++++++
 src/test/test.c     | 21 ++++++++++++++++++---
 4 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/or/onion.c b/src/or/onion.c
index 7c7f97f..5495074 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -130,9 +130,12 @@ onion_pending_add(or_circuit_t *circ, create_cell_t *onionskin)
   time_t now = time(NULL);
 
   if (onionskin->handshake_type > MAX_ONION_HANDSHAKE_TYPE) {
+    /* LCOV_EXCL_START
+     * We should have rejected this far before this point */
     log_warn(LD_BUG, "Handshake %d out of range! Dropping.",
              onionskin->handshake_type);
     return -1;
+    /* LCOV_EXCL_STOP */
   }
 
   tmp = tor_malloc_zero(sizeof(onion_queue_t));
@@ -305,10 +308,13 @@ static void
 onion_queue_entry_remove(onion_queue_t *victim)
 {
   if (victim->handshake_type > MAX_ONION_HANDSHAKE_TYPE) {
+    /* LCOV_EXCL_START
+     * We should have rejected this far before this point */
     log_warn(LD_BUG, "Handshake %d out of range! Dropping.",
              victim->handshake_type);
     /* XXX leaks */
     return;
+    /* LCOV_EXCL_STOP */
   }
 
   TOR_TAILQ_REMOVE(&ol_list[victim->handshake_type], victim, next);
@@ -391,9 +397,12 @@ onion_handshake_state_release(onion_handshake_state_t *state)
     state->u.ntor = NULL;
     break;
   default:
+    /* LCOV_EXCL_START
+     * This state should not even exist. */
     log_warn(LD_BUG, "called with unknown handshake state type %d",
              (int)state->tag);
     tor_fragile_assert();
+    /* LCOV_EXCL_STOP */
   }
 }
 
@@ -441,9 +450,12 @@ onion_skin_create(int type,
     r = NTOR_ONIONSKIN_LEN;
     break;
   default:
+    /* LCOV_EXCL_START
+     * We should never try to create an impossible handshake type. */
     log_warn(LD_BUG, "called with unknown handshake state type %d", type);
     tor_fragile_assert();
     r = -1;
+    /* LCOV_EXCL_STOP */
   }
 
   if (r > 0)
@@ -512,9 +524,12 @@ onion_skin_server_handshake(int type,
     }
     break;
   default:
+    /* LCOV_EXCL_START
+     * We should have rejected this far before this point */
     log_warn(LD_BUG, "called with unknown handshake state type %d", type);
     tor_fragile_assert();
     return -1;
+    /* LCOV_EXCL_STOP */
   }
 
   return r;
diff --git a/src/or/onion_ntor.c b/src/or/onion_ntor.c
index 33afc27..d1a268f 100644
--- a/src/or/onion_ntor.c
+++ b/src/or/onion_ntor.c
@@ -85,8 +85,13 @@ onion_skin_ntor_create(const uint8_t *router_id,
   memcpy(state->router_id, router_id, DIGEST_LEN);
   memcpy(&state->pubkey_B, router_key, sizeof(curve25519_public_key_t));
   if (curve25519_secret_key_generate(&state->seckey_x, 0) < 0) {
+    /* LCOV_EXCL_START
+     * Secret key generation should be unable to fail when the key isn't
+     * marked as "extra-strong" */
+    tor_assert_nonfatal_unreached();
     tor_free(state);
     return -1;
+    /* LCOV_EXCL_STOP */
   }
   curve25519_public_key_generate(&state->pubkey_X, &state->seckey_x);
 
diff --git a/src/or/onion_tap.c b/src/or/onion_tap.c
index bfd4723..abe7793 100644
--- a/src/or/onion_tap.c
+++ b/src/or/onion_tap.c
@@ -74,9 +74,13 @@ onion_skin_TAP_create(crypto_pk_t *dest_router_key,
 
   return 0;
  err:
+  /* LCOV_EXCL_START
+   * We only get here if RSA encryption fails or DH keygen fails. Those
+   * shouldn't be possible. */
   memwipe(challenge, 0, sizeof(challenge));
   if (dh) crypto_dh_free(dh);
   return -1;
+  /* LCOV_EXCL_STOP */
 }
 
 /** Given an encrypted DH public key as generated by onion_skin_create,
@@ -130,12 +134,20 @@ onion_skin_TAP_server_handshake(
 
   dh = crypto_dh_new(DH_TYPE_CIRCUIT);
   if (!dh) {
+    /* LCOV_EXCL_START
+     * Failure to allocate a DH key should be impossible.
+     */
     log_warn(LD_BUG, "Couldn't allocate DH key");
     goto err;
+    /* LCOV_EXCL_STOP */
   }
   if (crypto_dh_get_public(dh, handshake_reply_out, DH_KEY_LEN)) {
+    /* LCOV_EXCL_START
+     * This can only fail if the length of the key we just allocated is too
+     * big. That should be impossible. */
     log_info(LD_GENERAL, "crypto_dh_get_public failed.");
     goto err;
+    /* LCOV_EXCP_STOP */
   }
 
   key_material_len = DIGEST_LEN+key_out_len;
diff --git a/src/test/test.c b/src/test/test.c
index 3a1054d..2f18346 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -178,20 +178,26 @@ test_bad_onion_handshake(void *arg)
                                             s_buf, s_keys, 40));
 
   /* Client: Case 1: The server sent back junk. */
+  const char *msg = NULL;
   s_buf[64] ^= 33;
   tt_int_op(-1, OP_EQ,
-            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, NULL));
+            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, &msg));
   s_buf[64] ^= 33;
+  tt_str_op(msg, OP_EQ, "Digest DOES NOT MATCH on onion handshake. "
+            "Bug or attack.");
 
   /* Let the client finish; make sure it can. */
+  msg = NULL;
   tt_int_op(0, OP_EQ,
-            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, NULL));
+            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, &msg));
   tt_mem_op(s_keys,OP_EQ, c_keys, 40);
+  tt_ptr_op(msg, OP_EQ, NULL);
 
   /* Client: Case 2: The server sent back a degenerate DH. */
   memset(s_buf, 0, sizeof(s_buf));
   tt_int_op(-1, OP_EQ,
-            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, NULL));
+            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, &msg));
+  tt_str_op(msg, OP_EQ, "DH computation failed.");
 
  done:
   crypto_dh_free(c_dh);
@@ -246,6 +252,15 @@ test_ntor_handshake(void *arg)
   memset(s_buf, 0, 40);
   tt_mem_op(c_keys,OP_NE, s_buf, 40);
 
+  /* Now try with a bogus server response. Zero input should trigger
+   * All The Problems. */
+  memset(c_keys, 0, 400);
+  memset(s_buf, 0, NTOR_REPLY_LEN);
+  const char *msg = NULL;
+  tt_int_op(-1, OP_EQ, onion_skin_ntor_client_handshake(c_state, s_buf,
+                                                        c_keys, 400, &msg));
+  tt_str_op(msg, OP_EQ, "Zero output from curve25519 handshake");
+
  done:
   ntor_handshake_state_free(c_state);
   dimap_free(s_keymap, NULL);



More information about the tor-commits mailing list