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,

diff --git a/changes/feature9777 b/changes/feature9777
new file mode 100644 (file)
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 43d2ffe4db5fa39c64b5b64769739e2d601734f7..e47a2780af1fbae136bc59904132feba1485d445 100644 (file)
--- 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 b0e24a5fee80aad815d1a221fd29e11a2b11bac1..c7b15e40ba12d781f0db520078b06d27bbd5eefe 100644 (file)
--- 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 874f68cd2274944b3a949430ab5d9b71d2dd23b7..acc4b81fcd31161c0fbfd7fab00e06e33450684e 100644 (file)
--- 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);