[tor-commits] [tor/master] Give specific warnings when client-side onionskin handshakes fail

[nickm at torproject.org]

commit de9de9e7dd2f34af04c76abf3f51c72dec4bdc93
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Mar 27 14:15:53 2014 -0400

    Give specific warnings when client-side onionskin handshakes fail
    
    Fix for bug9635.
---
 changes/bug9635     |    3 +++
 src/or/onion.c      |   12 +++++++++---
 src/or/onion_fast.c |    1 +
 src/or/onion_ntor.c |   12 ++++++++++--
 src/or/onion_tap.c  |    4 +++-
 5 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/changes/bug9635 b/changes/bug9635
new file mode 100644
index 0000000..042da7b
--- /dev/null
+++ b/changes/bug9635
@@ -0,0 +1,3 @@
+  o Minor features:
+    - Give more specific warnings when we notice at the client side that
+      an onion handshake has failed. Fixes ticket 9635.
diff --git a/src/or/onion.c b/src/or/onion.c
index 30b983d..7c2c316 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -552,8 +552,10 @@ onion_skin_client_handshake(int type,
 
   switch (type) {
   case ONION_HANDSHAKE_TYPE_TAP:
-    if (reply_len != TAP_ONIONSKIN_REPLY_LEN)
+    if (reply_len != TAP_ONIONSKIN_REPLY_LEN) {
+      log_warn(LD_CIRC, "TAP reply was not of the correct length.");
       return -1;
+    }
     if (onion_skin_TAP_client_handshake(handshake_state->u.tap,
                                         (const char*)reply,
                                         (char *)keys_out, keys_out_len) < 0)
@@ -563,8 +565,10 @@ onion_skin_client_handshake(int type,
 
     return 0;
   case ONION_HANDSHAKE_TYPE_FAST:
-    if (reply_len != CREATED_FAST_LEN)
+    if (reply_len != CREATED_FAST_LEN) {
+      log_warn(LD_CIRC, "CREATED_FAST reply was not of the correct length.");
       return -1;
+    }
     if (fast_client_handshake(handshake_state->u.fast, reply,
                               keys_out, keys_out_len) < 0)
       return -1;
@@ -573,8 +577,10 @@ onion_skin_client_handshake(int type,
     return 0;
 #ifdef CURVE25519_ENABLED
   case ONION_HANDSHAKE_TYPE_NTOR:
-    if (reply_len < NTOR_REPLY_LEN)
+    if (reply_len < NTOR_REPLY_LEN) {
+      log_warn(LD_CIRC, "ntor reply was not of the correct length.");
       return -1;
+    }
     {
       size_t keys_tmp_len = keys_out_len + DIGEST_LEN;
       uint8_t *keys_tmp = tor_malloc(keys_tmp_len);
diff --git a/src/or/onion_fast.c b/src/or/onion_fast.c
index 8e778db..38b62de 100644
--- a/src/or/onion_fast.c
+++ b/src/or/onion_fast.c
@@ -104,6 +104,7 @@ fast_client_handshake(const fast_handshake_state_t *handshake_state,
   out_len = key_out_len+DIGEST_LEN;
   out = tor_malloc(out_len);
   if (crypto_expand_key_material_TAP(tmp, sizeof(tmp), out, out_len)) {
+    log_warn(LD_CIRC, "Failed to expand key material");
     goto done;
   }
   if (tor_memneq(out, handshake_reply_out+DIGEST_LEN, DIGEST_LEN)) {
diff --git a/src/or/onion_ntor.c b/src/or/onion_ntor.c
index 9cf7d5d..b91ecbe 100644
--- a/src/or/onion_ntor.c
+++ b/src/or/onion_ntor.c
@@ -256,7 +256,7 @@ onion_skin_ntor_client_handshake(
   si += CURVE25519_OUTPUT_LEN;
   curve25519_handshake(si, &handshake_state->seckey_x,
                        &handshake_state->pubkey_B);
-  bad |= safe_mem_is_zero(si, CURVE25519_OUTPUT_LEN);
+  bad |= (safe_mem_is_zero(si, CURVE25519_OUTPUT_LEN) << 1);
   si += CURVE25519_OUTPUT_LEN;
   APPEND(si, handshake_state->router_id, DIGEST_LEN);
   APPEND(si, handshake_state->pubkey_B.public_key, CURVE25519_PUBKEY_LEN);
@@ -281,7 +281,7 @@ onion_skin_ntor_client_handshake(
   /* Compute auth */
   h_tweak(s.auth, s.auth_input, sizeof(s.auth_input), T->t_mac);
 
-  bad |= tor_memneq(s.auth, auth_candidate, DIGEST256_LEN);
+  bad |= (tor_memneq(s.auth, auth_candidate, DIGEST256_LEN) << 2);
 
   crypto_expand_key_material_rfc5869_sha256(
                            s.secret_input, sizeof(s.secret_input),
@@ -290,6 +290,14 @@ onion_skin_ntor_client_handshake(
                            key_out, key_out_len);
 
   memwipe(&s, 0, sizeof(s));
+
+  if (bad & 4) {
+    log_warn(LD_PROTOCOL, "Incorrect digest from ntor circuit extension "
+             "request.");
+  } else if (bad) {
+    log_warn(LD_PROTOCOL, "Invalid result from curve25519 handshake");
+  }
+
   return bad ? -1 : 0;
 }
 
diff --git a/src/or/onion_tap.c b/src/or/onion_tap.c
index 3782e75..9a9f374 100644
--- a/src/or/onion_tap.c
+++ b/src/or/onion_tap.c
@@ -194,8 +194,10 @@ onion_skin_TAP_client_handshake(crypto_dh_t *handshake_state,
   len = crypto_dh_compute_secret(LOG_PROTOCOL_WARN, handshake_state,
                                  handshake_reply, DH_KEY_LEN, key_material,
                                  key_material_len);
-  if (len < 0)
+  if (len < 0) {
+    log_warn(LD_PROTOCOL,"DH computation failed.");
     goto err;
+  }
 
   if (tor_memneq(key_material, handshake_reply+DH_KEY_LEN, DIGEST_LEN)) {
     /* H(K) does *not* match. Something fishy. */