Downgrade 'invalid result from curve25519 handshake: 4' warning

Also, refactor the way we handle failed handshakes so that this
warning doesn't propagate itself to "onion_skin_client_handshake
failed" and "circuit_finish_handshake failed" and
"connection_edge_process_relay_cell (at origin) failed."

Resolves warning from 9635.

changes/bug9635

@@ -0,0 +1,3 @@
+  o Downgraded warnings:
+    - Don't warn when we've attempted to contact a relay using the wrong
+      ntor onion key. Closes ticket 9635.

src/or/circuitbuild.c

@@ -1256,8 +1256,10 @@ circuit_finish_handshake(origin_circuit_t *circ,
   crypt_path_t *hop;
   int rv;
 
-  if ((rv = pathbias_count_build_attempt(circ)) < 0)
+  if ((rv = pathbias_count_build_attempt(circ)) < 0) {
+    log_warn(LD_CIRC, "pathbias_count_build_attempt failed: %d", rv);
     return rv;
+  }
 
   if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) {
     hop = circ->cpath;
@@ -1271,12 +1273,15 @@ circuit_finish_handshake(origin_circuit_t *circ,
   tor_assert(hop->state == CPATH_STATE_AWAITING_KEYS);
 
   {
+    const char *msg = NULL;
     if (onion_skin_client_handshake(hop->handshake_state.tag,
                                     &hop->handshake_state,
                                     reply->reply, reply->handshake_len,
                                     (uint8_t*)keys, sizeof(keys),
-                                    (uint8_t*)hop->rend_circ_nonce) < 0) {
+                                    (uint8_t*)hop->rend_circ_nonce,
-      log_warn(LD_CIRC,"onion_skin_client_handshake failed.");
+                                    &msg) < 0) {
+      if (msg)
+        log_warn(LD_CIRC,"onion_skin_client_handshake failed: %s", msg);
       return -END_CIRC_REASON_TORPROTOCOL;
     }
   }
@@ -1284,6 +1289,7 @@ circuit_finish_handshake(origin_circuit_t *circ,
   onion_handshake_state_release(&hop->handshake_state);
 
   if (circuit_init_cpath_crypto(hop, keys, 0)<0) {
+    log_warn(LD_BUG, "Couldn't initialize cpath crypto");
     return -END_CIRC_REASON_TORPROTOCOL;
   }
 

src/or/command.c

@@ -398,7 +398,6 @@ command_process_created_cell(cell_t *cell, channel_t *chan)
     log_debug(LD_OR,"at OP. Finishing handshake.");
     if ((err_reason = circuit_finish_handshake(origin_circ,
                                         &extended_cell.created_cell)) < 0) {
-      log_warn(LD_OR,"circuit_finish_handshake failed.");
       circuit_mark_for_close(circ, -err_reason);
       return;
     }

src/or/onion.c

@@ -541,13 +541,15 @@ onion_skin_server_handshake(int type,
  * bytes worth of key material in <b>keys_out_len</b>, set
  * <b>rend_authenticator_out</b> to the "KH" field that can be used to
  * establish introduction points at this hop, and return 0. On failure,
- * return -1. */
+ * return -1, and set *msg_out to an error message if this is worth
+ * complaining to the usre about. */
 int
 onion_skin_client_handshake(int type,
                       const onion_handshake_state_t *handshake_state,
                       const uint8_t *reply, size_t reply_len,
                       uint8_t *keys_out, size_t keys_out_len,
-                      uint8_t *rend_authenticator_out)
+                      uint8_t *rend_authenticator_out,
+                      const char **msg_out)
 {
   if (handshake_state->tag != type)
     return -1;
@@ -555,12 +557,14 @@ onion_skin_client_handshake(int type,
   switch (type) {
   case ONION_HANDSHAKE_TYPE_TAP:
     if (reply_len != TAP_ONIONSKIN_REPLY_LEN) {
-      log_warn(LD_CIRC, "TAP reply was not of the correct length.");
+      if (msg_out)
+        *msg_out = "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)
+                                        (char *)keys_out, keys_out_len,
+                                        msg_out) < 0)
       return -1;
 
     memcpy(rend_authenticator_out, reply+DH_KEY_LEN, DIGEST_LEN);
@@ -568,11 +572,12 @@ onion_skin_client_handshake(int type,
     return 0;
   case ONION_HANDSHAKE_TYPE_FAST:
     if (reply_len != CREATED_FAST_LEN) {
-      log_warn(LD_CIRC, "CREATED_FAST reply was not of the correct length.");
+      if (msg_out)
+        *msg_out = "TAP reply was not of the correct length.";
       return -1;
     }
     if (fast_client_handshake(handshake_state->u.fast, reply,
-                              keys_out, keys_out_len) < 0)
+                              keys_out, keys_out_len, msg_out) < 0)
       return -1;
 
     memcpy(rend_authenticator_out, reply+DIGEST_LEN, DIGEST_LEN);
@@ -580,15 +585,16 @@ onion_skin_client_handshake(int type,
 #ifdef CURVE25519_ENABLED
   case ONION_HANDSHAKE_TYPE_NTOR:
     if (reply_len < NTOR_REPLY_LEN) {
-      log_warn(LD_CIRC, "ntor reply was not of the correct length.");
+      if (msg_out)
+        *msg_out = "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);
       if (onion_skin_ntor_client_handshake(handshake_state->u.ntor,
-                                           reply,
+                                        reply,
-                                           keys_tmp, keys_tmp_len) < 0) {
+                                        keys_tmp, keys_tmp_len, msg_out) < 0) {
         tor_free(keys_tmp);
         return -1;
       }

src/or/onion.h

@@ -51,7 +51,8 @@ int onion_skin_client_handshake(int type,
                       const onion_handshake_state_t *handshake_state,
                       const uint8_t *reply, size_t reply_len,
                       uint8_t *keys_out, size_t key_out_len,
-                      uint8_t *rend_authenticator_out);
+                      uint8_t *rend_authenticator_out,
+                      const char **msg_out);
 
 /** A parsed CREATE, CREATE_FAST, or CREATE2 cell. */
 typedef struct create_cell_t {

src/or/onion_fast.c

@@ -92,7 +92,8 @@ int
 fast_client_handshake(const fast_handshake_state_t *handshake_state,
                       const uint8_t *handshake_reply_out,/*DIGEST_LEN*2 bytes*/
                       uint8_t *key_out,
-                      size_t key_out_len)
+                      size_t key_out_len,
+                      const char **msg_out)
 {
   uint8_t tmp[DIGEST_LEN+DIGEST_LEN];
   uint8_t *out;
@@ -104,13 +105,14 @@ 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");
+    if (msg_out)
+      *msg_out = "Failed to expand key material";
     goto done;
   }
   if (tor_memneq(out, handshake_reply_out+DIGEST_LEN, DIGEST_LEN)) {
     /* H(K) does *not* match. Something fishy. */
-    log_warn(LD_PROTOCOL,"Digest DOES NOT MATCH on fast handshake. "
+    if (msg_out)
-             "Bug or attack.");
+      *msg_out = "Digest DOES NOT MATCH on fast handshake. Bug or attack.";
     goto done;
   }
   memcpy(key_out, out+DIGEST_LEN, key_out_len);

src/or/onion_fast.h

@@ -32,7 +32,8 @@ int fast_server_handshake(const uint8_t *message_in,
 int fast_client_handshake(const fast_handshake_state_t *handshake_state,
                           const uint8_t *handshake_reply_out,
                           uint8_t *key_out,
-                          size_t key_out_len);
+                          size_t key_out_len,
+                          const char **msg_out);
 
 #endif
 

src/or/onion_ntor.c

@@ -3,8 +3,8 @@
 
 #include "orconfig.h"
 
-#include "crypto.h"
 #define ONION_NTOR_PRIVATE
+#include "crypto.h"
 #include "onion_ntor.h"
 #include "torlog.h"
 #include "util.h"
@@ -226,7 +226,8 @@ onion_skin_ntor_client_handshake(
                              const ntor_handshake_state_t *handshake_state,
                              const uint8_t *handshake_reply,
                              uint8_t *key_out,
-                             size_t key_out_len)
+                             size_t key_out_len,
+                             const char **msg_out)
 {
   const tweakset_t *T = &proto1_tweaks;
   /* Sensitive stack-allocated material. Kept in an anonymous struct to make
@@ -291,8 +292,18 @@ onion_skin_ntor_client_handshake(
 
   memwipe(&s, 0, sizeof(s));
 
-  if (bad) {
+  if (bad && msg_out) {
-    log_warn(LD_PROTOCOL, "Invalid result from curve25519 handshake: %d", bad);
+    if (bad & 4) {
+      *msg_out = NULL; /* Don't report this one; we probably just had the
+                        * wrong onion key.*/
+      log_fn(LOG_INFO, LD_PROTOCOL,
+             "Invalid result from curve25519 handshake: %d", bad);
+    }
+    if (bad & 3) {
+      *msg_out = "Zero output from curve25519 handshake";
+      log_fn(LOG_WARN, LD_PROTOCOL,
+             "Invalid result from curve25519 handshake: %d", bad);
+    }
   }
 
   return bad ? -1 : 0;

src/or/onion_ntor.h

@@ -37,7 +37,8 @@ int onion_skin_ntor_client_handshake(
                              const ntor_handshake_state_t *handshake_state,
                              const uint8_t *handshake_reply,
                              uint8_t *key_out,
-                             size_t key_out_len);
+                             size_t key_out_len,
+                             const char **msg_out);
 
 #ifdef ONION_NTOR_PRIVATE
 

src/or/onion_tap.c

@@ -183,7 +183,8 @@ int
 onion_skin_TAP_client_handshake(crypto_dh_t *handshake_state,
             const char *handshake_reply, /* TAP_ONIONSKIN_REPLY_LEN bytes */
             char *key_out,
-            size_t key_out_len)
+            size_t key_out_len,
+            const char **msg_out)
 {
   ssize_t len;
   char *key_material=NULL;
@@ -196,14 +197,15 @@ onion_skin_TAP_client_handshake(crypto_dh_t *handshake_state,
                                  handshake_reply, DH_KEY_LEN, key_material,
                                  key_material_len);
   if (len < 0) {
-    log_warn(LD_PROTOCOL,"DH computation failed.");
+    if (msg_out)
+      *msg_out = "DH computation failed.";
     goto err;
   }
 
   if (tor_memneq(key_material, handshake_reply+DH_KEY_LEN, DIGEST_LEN)) {
     /* H(K) does *not* match. Something fishy. */
-    log_warn(LD_PROTOCOL,"Digest DOES NOT MATCH on onion handshake. "
+    if (msg_out)
-             "Bug or attack.");
+      *msg_out = "Digest DOES NOT MATCH on onion handshake. Bug or attack.";
     goto err;
   }
 

src/or/onion_tap.h

@@ -31,7 +31,8 @@ int onion_skin_TAP_server_handshake(const char *onion_skin,
 int onion_skin_TAP_client_handshake(crypto_dh_t *handshake_state,
                                 const char *handshake_reply,
                                 char *key_out,
-                                size_t key_out_len);
+                                size_t key_out_len,
+                                const char **msg_out);
 
 #endif
 

src/or/relay.c

@@ -1643,8 +1643,9 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
         }
         if ((reason = circuit_finish_handshake(TO_ORIGIN_CIRCUIT(circ),
                                          &extended_cell.created_cell)) < 0) {
-          log_warn(domain,"circuit_finish_handshake failed.");
+          circuit_mark_for_close(circ, -reason);
-          return reason;
+          return 0; /* We don't want to cause a warning, so we mark the circuit
+                     * here. */
         }
       }
       if ((reason=circuit_send_next_onion_skin(TO_ORIGIN_CIRCUIT(circ)))<0) {

src/test/bench.c

@@ -162,7 +162,8 @@ bench_onion_TAP(void)
     char key_out[CPATH_KEY_MATERIAL_LEN];
     int s;
     dh = crypto_dh_dup(dh_out);
-    s = onion_skin_TAP_client_handshake(dh, or, key_out, sizeof(key_out));
+    s = onion_skin_TAP_client_handshake(dh, or, key_out, sizeof(key_out),
+                                        NULL);
     crypto_dh_free(dh);
     tor_assert(s == 0);
   }
@@ -222,7 +223,8 @@ bench_onion_ntor(void)
   for (i = 0; i < iters; ++i) {
     uint8_t key_out[CPATH_KEY_MATERIAL_LEN];
     int s;
-    s = onion_skin_ntor_client_handshake(state, or, key_out, sizeof(key_out));
+    s = onion_skin_ntor_client_handshake(state, or, key_out, sizeof(key_out),
+                                         NULL);
     tor_assert(s == 0);
   }
   end = perftime();

src/test/test.c

@@ -264,7 +264,8 @@ test_onion_handshake(void)
 
     /* client handshake 2 */
     memset(c_keys, 0, 40);
-    test_assert(! onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40));
+    test_assert(! onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40,
+                                                  NULL));
 
     test_memeq(c_keys, s_keys, 40);
     memset(s_buf, 0, 40);
@@ -337,18 +338,18 @@ test_bad_onion_handshake(void *arg)
   /* Client: Case 1: The server sent back junk. */
   s_buf[64] ^= 33;
   tt_int_op(-1, ==,
-            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40));
+            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, NULL));
   s_buf[64] ^= 33;
 
   /* Let the client finish; make sure it can. */
   tt_int_op(0, ==,
-            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40));
+            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, NULL));
   test_memeq(s_keys, c_keys, 40);
 
   /* Client: Case 2: The server sent back a degenerate DH. */
   memset(s_buf, 0, sizeof(s_buf));
   tt_int_op(-1, ==,
-            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40));
+            onion_skin_TAP_client_handshake(c_dh, s_buf, c_keys, 40, NULL));
 
  done:
   crypto_dh_free(c_dh);
@@ -398,7 +399,7 @@ test_ntor_handshake(void *arg)
   /* client handshake 2 */
   memset(c_keys, 0, 40);
   tt_int_op(0, ==, onion_skin_ntor_client_handshake(c_state, s_buf,
-                                                    c_keys, 400));
+                                                    c_keys, 400, NULL));
 
   test_memeq(c_keys, s_keys, 400);
   memset(s_buf, 0, 40);

src/test/test_ntor_cl.c

@@ -130,7 +130,7 @@ client2(int argc, char **argv)
 
   keys = tor_malloc(keybytes);
   hexkeys = tor_malloc(keybytes*2+1);
-  if (onion_skin_ntor_client_handshake(&state, msg, keys, keybytes)<0) {
+  if (onion_skin_ntor_client_handshake(&state, msg, keys, keybytes, NULL)<0) {
     fprintf(stderr, "handshake failed");
     result = 2;
     goto done;