Merge remote branch 'origin/maint-0.2.2'

3 files changed, 37 insertions, 2 deletions

diff --git a/changes/bug1912 b/changes/bug1912
new file mode 100644
index 000000000..5a7af756d
--- /dev/null
+++ b/changes/bug1912
@@ -0,0 +1,9 @@
+  o Major bugfixes:
+    - When weighting bridges, we used to trust the bandwidths they provided
+      in their descriptor, only capping them at 10MB/s. This turned out to be
+      problematic for two reasons: Bridges could claim to handle a lot more
+      traffic then they actually would, thus making more clients pick them and
+      have a pretty effective DoS attack. The other issue is that new bridges
+      that might not have a good estimate for their bw capacity yet would not
+      get used at all unless no other bridges are available to a client.
+      This fixes bug 1912; bugfix on 0.2.2.7-alpha.
diff --git a/src/common/util.c b/src/common/util.c
index b4f3052e1..dafe12071 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2877,3 +2877,4 @@ load_windows_system_library(const TCHAR *library_name)
   return LoadLibrary(path);
 }
 #endif
+
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 0b271f1aa..43be8346c 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1579,6 +1579,29 @@ router_get_advertised_bandwidth_capped(routerinfo_t *router)
   return result;
 }
 
+/** When weighting bridges, enforce these values as lower and upper
+ * bound for believable bandwidth, because there is no way for us
+ * to verify a bridge's bandwidth currently. */
+#define BRIDGE_MIN_BELIEVABLE_BANDWIDTH 20000  /* 20 kB/sec */
+#define BRIDGE_MAX_BELIEVABLE_BANDWIDTH 100000 /* 100 kB/sec */
+
+/** Return the smaller of the router's configured BandwidthRate
+ * and its advertised capacity, making sure to stay within the
+ * interval between bridge-min-believe-bw and
+ * bridge-max-believe-bw. */
+static uint32_t
+bridge_get_advertised_bandwidth_bounded(routerinfo_t *router)
+{
+  uint32_t result = router->bandwidthcapacity;
+  if (result > router->bandwidthrate)
+    result = router->bandwidthrate;
+  if (result > BRIDGE_MAX_BELIEVABLE_BANDWIDTH)
+    result = BRIDGE_MAX_BELIEVABLE_BANDWIDTH;
+  else if (result < BRIDGE_MIN_BELIEVABLE_BANDWIDTH)
+    result = BRIDGE_MIN_BELIEVABLE_BANDWIDTH;
+  return result;
+}
+
 /** Return bw*1000, unless bw*1000 would overflow, in which case return
  * INT32_MAX. */
 static INLINE int32_t
@@ -1733,7 +1756,7 @@ smartlist_choose_by_bandwidth_weights(smartlist_t *sl,
       if (rs && rs->has_bandwidth) {
         this_bw = kb_to_bytes(rs->bandwidth);
       } else { /* bridge or other descriptor not in our consensus */
-        this_bw = router_get_advertised_bandwidth_capped(router);
+        this_bw = bridge_get_advertised_bandwidth_bounded(router);
         have_unknown = 1;
       }
       if (router_digest_is_me(router->cache_info.identity_digest))
@@ -1904,7 +1927,7 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule,
         flags |= is_exit ? 2 : 0;
         flags |= is_guard ? 4 : 0;
       } else /* bridge or other descriptor not in our consensus */
-        this_bw = router_get_advertised_bandwidth_capped(router);
+        this_bw = bridge_get_advertised_bandwidth_bounded(router);
     }
     if (is_exit)
       bitarray_set(exit_bits, i);
@@ -1912,6 +1935,8 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule,
       bitarray_set(guard_bits, i);
     if (is_known) {
       bandwidths[i] = (int32_t) this_bw; // safe since MAX_BELIEVABLE<INT32_MAX
+      // XXX this is no longer true! We don't always cap the bw anymore. Can
+      // a consensus make us overflow?-sh
       tor_assert(bandwidths[i] >= 0);
       if (is_guard)
         total_guard_bw += this_bw;