Browse Source

Merge remote-tracking branch 'origin/maint-0.2.3'

Nick Mathewson 12 years ago
parent
commit
aa584fd3a3
2 changed files with 33 additions and 5 deletions
  1. 14 0
      changes/pathsel-BUGGY-a
  2. 19 5
      src/or/routerlist.c

+ 14 - 0
changes/pathsel-BUGGY-a

@@ -0,0 +1,14 @@
+  o Security fixes:
+
+    - Try to leak less information about what relays a client is
+      choosing to a side-channel attacker.  Previously, a Tor client
+      would stop iterating through the list of available relays as
+      soon as it had chosen one, thus finishing a little earlier
+      when it picked a router earlier in the list.  If an attacker
+      can recover this timing information (nontrivial but not
+      proven to be impossible), they could learn some coarse-
+      grained information about which relays a client was picking
+      (middle nodes in particular are likelier to be affected than
+      exits).  The timing attack might be mitigated by other factors
+      (see bug #6537 for some discussion), but it's best not to
+      take chances.  Fixes bug 6537; bugfix on 0.0.8rc1.

+ 19 - 5
src/or/routerlist.c

@@ -1711,6 +1711,8 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
   double *bandwidths;
   double tmp = 0;
   unsigned int i;
+  unsigned int i_chosen;
+  unsigned int i_has_been_chosen;
   int have_unknown = 0; /* true iff sl contains element not in consensus. */
 
   /* Can't choose exit and guard at same time */
@@ -1873,12 +1875,17 @@ smartlist_choose_node_by_bandwidth_weights(smartlist_t *sl,
               * from 1 below. See bug 1203 for details. */
 
   /* Last, count through sl until we get to the element we picked */
+  i_chosen = (unsigned)smartlist_len(sl);
+  i_has_been_chosen = 0;
   tmp = 0.0;
   for (i=0; i < (unsigned)smartlist_len(sl); i++) {
     tmp += bandwidths[i];
-    if (tmp >= rand_bw)
-      break;
+    if (tmp >= rand_bw && !i_has_been_chosen) {
+      i_chosen = i;
+      i_has_been_chosen = 1;
+    }
   }
+  i = i_chosen;
 
   if (i == (unsigned)smartlist_len(sl)) {
     /* This was once possible due to round-off error, but shouldn't be able
@@ -1911,7 +1918,9 @@ static const node_t *
 smartlist_choose_node_by_bandwidth(smartlist_t *sl,
                                    bandwidth_weight_rule_t rule)
 {
-  unsigned i;
+  unsigned int i;
+  unsigned int i_chosen;
+  unsigned int i_has_been_chosen;
   int32_t *bandwidths;
   int is_exit;
   int is_guard;
@@ -2111,6 +2120,8 @@ smartlist_choose_node_by_bandwidth(smartlist_t *sl,
 
   /* Last, count through sl until we get to the element we picked */
   tmp = 0;
+  i_chosen = (unsigned)smartlist_len(sl);
+  i_has_been_chosen = 0;
   for (i=0; i < (unsigned)smartlist_len(sl); i++) {
     is_exit = bitarray_is_set(exit_bits, i);
     is_guard = bitarray_is_set(guard_bits, i);
@@ -2125,9 +2136,12 @@ smartlist_choose_node_by_bandwidth(smartlist_t *sl,
     else
       tmp += bandwidths[i];
 
-    if (tmp >= rand_bw)
-      break;
+    if (tmp >= rand_bw && !i_has_been_chosen) {
+      i_chosen = i;
+      i_has_been_chosen = 1;
+    }
   }
+  i = i_chosen;
   if (i == (unsigned)smartlist_len(sl)) {
     /* This was once possible due to round-off error, but shouldn't be able
      * to occur any longer. */