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

src/or/config.c

@@ -594,6 +594,7 @@ static void option_reset(const config_format_t *fmt, or_options_t *options,
                          const config_var_t *var, int use_defaults);
 static void config_free(const config_format_t *fmt, void *options);
 static int config_lines_eq(config_line_t *a, config_line_t *b);
+static int config_count_key(const config_line_t *a, const char *key);
 static int option_is_same(const config_format_t *fmt,
                           const or_options_t *o1, const or_options_t *o2,
                           const char *name);
@@ -3066,6 +3067,20 @@ config_lines_eq(config_line_t *a, config_line_t *b)
   return 1;
 }
 
+/** Return the number of lines in <b>a</b> whose key is <b>key</b>. */
+static int
+config_count_key(const config_line_t *a, const char *key)
+{
+  int n = 0;
+  while (a) {
+    if (!strcasecmp(a->key, key)) {
+      ++n;
+    }
+    a = a->next;
+  }
+  return n;
+}
+
 /** Return true iff the option <b>name</b> has the same value in <b>o1</b>
  * and <b>o2</b>.  Must not be called for LINELIST_S or OBSOLETE options.
  */
@@ -3848,6 +3863,20 @@ options_validate(or_options_t *old_options, or_options_t *options,
   if (accounting_parse_options(options, 1)<0)
     REJECT("Failed to parse accounting options. See logs for details.");
 
+  if (options->AccountingMax) {
+    if (options->RendConfigLines && server_mode(options)) {
+      log_warn(LD_CONFIG, "Using accounting with a hidden service and an "
+               "ORPort is risky: your hidden service(s) and your public "
+               "address will all turn off at the same time, which may alert "
+               "observers that they are being run by the same party.");
+    } else if (config_count_key(options->RendConfigLines,
+                                "HiddenServiceDir") > 1) {
+      log_warn(LD_CONFIG, "Using accounting with multiple hidden services is "
+               "risky: they will all turn off at the same time, which may "
+               "alert observers that they are being run by the same party.");
+    }
+  }
+
   if (options->HTTPProxy) { /* parse it now */
     if (tor_addr_port_lookup(options->HTTPProxy,
                         &options->HTTPProxyAddr, &options->HTTPProxyPort) < 0)

src/or/dns.c

@@ -168,7 +168,8 @@ static void add_wildcarded_test_address(const char *address);
 static int configure_nameservers(int force);
 static int answer_is_wildcarded(const char *ip);
 static int dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
-                            or_circuit_t *oncirc, char **resolved_to_hostname);
+                            or_circuit_t *oncirc, char **resolved_to_hostname,
+                            int *made_connection_pending_out);
 #ifdef DEBUG_DNS_CACHE
 static void _assert_cache_ok(void);
 #define assert_cache_ok() _assert_cache_ok()
@@ -596,10 +597,12 @@ dns_resolve(edge_connection_t *exitconn)
 {
   or_circuit_t *oncirc = TO_OR_CIRCUIT(exitconn->on_circuit);
   int is_resolve, r;
+  int made_connection_pending = 0;
   char *hostname = NULL;
   is_resolve = exitconn->_base.purpose == EXIT_PURPOSE_RESOLVE;
 
-  r = dns_resolve_impl(exitconn, is_resolve, oncirc, &hostname);
+  r = dns_resolve_impl(exitconn, is_resolve, oncirc, &hostname,
+                       &made_connection_pending);
 
   switch (r) {
     case 1:
@@ -639,16 +642,11 @@ dns_resolve(edge_connection_t *exitconn)
 
       dns_cancel_pending_resolve(exitconn->_base.address);
 
-      if (!exitconn->_base.marked_for_close) {
+      if (!made_connection_pending && !exitconn->_base.marked_for_close) {
+        /* If we made the connection pending, then we freed it already in
+         * dns_cancel_pending_resolve().  If we marked it for close, it'll
+         * get freed from the main loop.  Otherwise, can free it now. */
         connection_free(TO_CONN(exitconn));
-        // XXX ... and we just leak exitconn otherwise? -RD
-        // If it's marked for close, it's on closeable_connection_lst in
-        // main.c.  If it's on the closeable list, it will get freed from
-        // main.c. -NM
-        // "<armadev> If that's true, there are other bugs around, where we
-        //  don't check if it's marked, and will end up double-freeing."
-        // On the other hand, I don't know of any actual bugs here, so this
-        // shouldn't be holding up the rc. -RD
       }
       break;
     default:
@@ -667,10 +665,15 @@ dns_resolve(edge_connection_t *exitconn)
  * Return -2 on a transient error. If it's a reverse resolve and it's
  * successful, sets *<b>hostname_out</b> to a newly allocated string
  * holding the cached reverse DNS value.
+ *
+ * Set *<b>made_connection_pending_out</b> to true if we have placed
+ * <b>exitconn</b> on the list of pending connections for some resolve; set it
+ * to false otherwise.
  */
 static int
 dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
-                 or_circuit_t *oncirc, char **hostname_out)
+                 or_circuit_t *oncirc, char **hostname_out,
+                 int *made_connection_pending_out)
 {
   cached_resolve_t *resolve;
   cached_resolve_t search;
@@ -684,6 +687,7 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
   tor_assert(!SOCKET_OK(exitconn->_base.s));
   assert_cache_ok();
   tor_assert(oncirc);
+  *made_connection_pending_out = 0;
 
   /* first check if exitconn->_base.address is an IP. If so, we already
    * know the answer. */
@@ -757,6 +761,7 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
         pending_connection->conn = exitconn;
         pending_connection->next = resolve->pending_connections;
         resolve->pending_connections = pending_connection;
+        *made_connection_pending_out = 1;
         log_debug(LD_EXIT,"Connection (fd %d) waiting for pending DNS "
                   "resolve of %s", exitconn->_base.s,
                   escaped_safe_str(exitconn->_base.address));
@@ -797,6 +802,7 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
   pending_connection = tor_malloc_zero(sizeof(pending_connection_t));
   pending_connection->conn = exitconn;
   resolve->pending_connections = pending_connection;
+  *made_connection_pending_out = 1;
 
   /* Add this resolve to the cache and priority queue. */
   HT_INSERT(cache_map, &cache_root, resolve);

src/or/routerlist.c

@@ -1709,6 +1709,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 */
@@ -1871,12 +1873,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
@@ -1909,7 +1916,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;
@@ -1983,9 +1992,10 @@ smartlist_choose_node_by_bandwidth(smartlist_t *sl,
     if (is_guard)
       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
+      bandwidths[i] = (int32_t) this_bw;
+      /* Casting this_bw to int32_t is safe because both kb_to_bytes
+         and bridge_get_advertised_bandwidth_bounded limit it to below
+         INT32_MAX. */
       tor_assert(bandwidths[i] >= 0);
       if (is_guard)
         total_guard_bw += this_bw;
@@ -2108,6 +2118,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);
@@ -2122,9 +2134,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. */

src/or/routerparse.c

@@ -2853,7 +2853,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out,
     int flavor = networkstatus_parse_flavor_name(tok->args[1]);
     if (flavor < 0) {
       log_warn(LD_DIR, "Can't parse document with unknown flavor %s",
-               escaped(tok->args[2]));
+               escaped(tok->args[1]));
       goto err;
     }
     ns->flavor = flav = flavor;