commit de9de9e7dd2f34af04c76abf3f51c72dec4bdc93 Author: Nick Mathewson nickm@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. */