[tor-commits] [tor/release-0.2.4] Discard circuit paths on which nobody supports ntor

[arma at torproject.org]

commit 1068e50aecefac8469991884afc08b6ecb24e740
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Feb 6 17:08:50 2014 -0500

    Discard circuit paths on which nobody supports ntor
    
    Right now this accounts for about 1% of circuits over all, but if you
    pick a guard that's running 0.2.3, it will be about 6% of the circuits
    running through that guard.
    
    Making sure that every circuit has at least one ntor link means that
    we're getting plausibly good forward secrecy on every circuit.
    
    This implements ticket 9777,
---
 changes/feature9777   |    3 ++
 src/or/circuitbuild.c |   77 +++++++++++++++++++++++++++++++++++++++++++------
 src/or/circuitlist.c  |   10 ++++++-
 src/or/circuitlist.h  |    1 +
 4 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/changes/feature9777 b/changes/feature9777
new file mode 100644
index 0000000..312b5e0
--- /dev/null
+++ b/changes/feature9777
@@ -0,0 +1,3 @@
+  o Minor features:
+    - Avoid using circuit paths if no node in the path supports the ntor
+      circuit extension handshake. Implements ticket 9777.
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 43d2ffe..e47a278 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -72,6 +72,9 @@ static void pathbias_count_use_failed(origin_circuit_t *circ);
 static void pathbias_measure_use_rate(entry_guard_t *guard);
 static void pathbias_measure_close_rate(entry_guard_t *guard);
 static void pathbias_scale_use_rates(entry_guard_t *guard);
+#ifdef CURVE25519_ENABLED
+static int circuits_can_use_ntor(void);
+#endif
 
 /** This function tries to get a channel to the specified endpoint,
  * and then calls command_setup_channel() to give it the right
@@ -284,21 +287,74 @@ circuit_rep_hist_note_result(origin_circuit_t *circ)
   } while (hop!=circ->cpath);
 }
 
+#ifdef CURVE25519_ENABLED
+/** Return 1 iff at least one node in circ's cpath supports ntor. */
+static int
+circuit_cpath_supports_ntor(const origin_circuit_t *circ)
+{
+  crypt_path_t *head = circ->cpath, *cpath = circ->cpath;
+
+  cpath = head;
+  do {
+    if (cpath->extend_info &&
+        !tor_mem_is_zero(
+            (const char*)cpath->extend_info->curve25519_onion_key.public_key,
+            CURVE25519_PUBKEY_LEN))
+      return 1;
+
+    cpath = cpath->next;
+  } while (cpath != head);
+
+  return 0;
+}
+#else
+#define circuit_cpath_supports_ntor(circ) 0
+#endif
+
 /** Pick all the entries in our cpath. Stop and return 0 when we're
  * happy, or return -1 if an error occurs. */
 static int
 onion_populate_cpath(origin_circuit_t *circ)
 {
-  int r;
- again:
-  r = onion_extend_cpath(circ);
-  if (r < 0) {
-    log_info(LD_CIRC,"Generating cpath hop failed.");
-    return -1;
+  int n_tries = 0;
+#ifdef CURVE25519_ENABLED
+  const int using_ntor = circuits_can_use_ntor();
+#else
+  const int using_ntor = 0;
+#endif
+
+#define MAX_POPULATE_ATTEMPTS 32
+
+  while (1) {
+    int r = onion_extend_cpath(circ);
+    if (r < 0) {
+      log_info(LD_CIRC,"Generating cpath hop failed.");
+      return -1;
+    }
+    if (r == 1) {
+      /* This circuit doesn't need/shouldn't be forced to have an ntor hop */
+      if (circ->build_state->desired_path_len <= 1 || ! using_ntor)
+        return 0;
+
+      /* This circuit has an ntor hop. great! */
+      if (circuit_cpath_supports_ntor(circ))
+        return 0;
+
+      /* No node in the circuit supports ntor.  Have we already tried too many
+       * times? */
+      if (++n_tries >= MAX_POPULATE_ATTEMPTS)
+        break;
+
+      /* Clear the path and retry */
+      circuit_clear_cpath(circ);
+    }
   }
-  if (r == 0)
-    goto again;
-  return 0; /* if r == 1 */
+  log_warn(LD_CIRC, "I tried for %d times, but I couldn't build a %d-hop "
+           "circuit with at least one node that supports ntor.",
+           MAX_POPULATE_ATTEMPTS,
+           circ->build_state->desired_path_len);
+
+  return -1;
 }
 
 /** Create and return a new origin circuit. Initialize its purpose and
@@ -3475,6 +3531,9 @@ onion_next_hop_in_cpath(crypt_path_t *cpath)
 
 /** Choose a suitable next hop in the cpath <b>head_ptr</b>,
  * based on <b>state</b>. Append the hop info to head_ptr.
+ *
+ * Return 1 if the path is complete, 0 if we successfully added a hop,
+ * and -1 on error.
  */
 static int
 onion_extend_cpath(origin_circuit_t *circ)
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index b0e24a5..c7b15e4 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -709,7 +709,7 @@ circuit_free_cpath(crypt_path_t *cpath)
   if (!cpath)
     return;
 
-  /* it's a doubly linked list, so we have to notice when we've
+  /* it's a circular list, so we have to notice when we've
    * gone through it once. */
   while (cpath->next && cpath->next != head) {
     victim = cpath;
@@ -720,6 +720,14 @@ circuit_free_cpath(crypt_path_t *cpath)
   circuit_free_cpath_node(cpath);
 }
 
+/** Remove all the items in the cpath on <b>circ</b>.*/
+void
+circuit_clear_cpath(origin_circuit_t *circ)
+{
+  circuit_free_cpath(circ->cpath);
+  circ->cpath = NULL;
+}
+
 /** Release all storage held by circuits. */
 void
 circuit_free_all(void)
diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h
index 874f68c..acc4b81 100644
--- a/src/or/circuitlist.h
+++ b/src/or/circuitlist.h
@@ -50,6 +50,7 @@ void circuit_mark_all_dirty_circs_as_unusable(void);
 void circuit_mark_for_close_(circuit_t *circ, int reason,
                              int line, const char *file);
 int circuit_get_cpath_len(origin_circuit_t *circ);
+void circuit_clear_cpath(origin_circuit_t *circ);
 crypt_path_t *circuit_get_cpath_hop(origin_circuit_t *circ, int hopnum);
 void circuit_get_all_pending_on_channel(smartlist_t *out,
                                         channel_t *chan);