checkpoint as we add PreferTunneledDirConns config option

svn:r9249

5 files changed, 129 insertions, 65 deletions

diff --git a/doc/TODO b/doc/TODO
index 4c99d18777..fc1b363520 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -35,7 +35,6 @@ Items for 0.1.2.x:
 N - enumerate events of important things that occur in tor, so vidalia can
     react.
     o Backend implementation
-R   - Clean up the spec a bit.
     - Actually list all the events (notice and warn log messages are a good
       place to look.)  Divide messages into categories, perhaps.
     - Specify general event system
@@ -64,10 +63,14 @@ R     . option to dl directory info via tor
         o Make an option like __AllDirActionsPrivate that falls back to
           non-Tor DL when not enough info present.  (TunnelDirConns).
         - Set default to 0 before release candidate.
-        - Think harder about whether TunnelDirConns should be on
-          by default.
-        - Handle case where we have no descriptors and so don't know who can
+        o Think harder about whether TunnelDirConns should be on
+          by default. No, they shouldn't, until we have much more of
+          blocking.pdf implemented.
+        o Handle case where we have no descriptors and so don't know who can
           handle BEGIN_DIR.
+        - actually cause the directory.c functions to know about or_port
+          and use it when we're supposed to.
+        - man page items for TunnelDirConns and PreferTunneledDirConns
 
 N - DNS improvements
     . Asynchronous DNS
diff --git a/src/or/config.c b/src/or/config.c
index c512dc038a..d5f7e77a93 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -208,6 +208,7 @@ static config_var_t _option_vars[] = {
   VAR("OutboundBindAddress", STRING,   OutboundBindAddress,  NULL),
   VAR("PathlenCoinWeight",   DOUBLE,   PathlenCoinWeight,    "0.3"),
   VAR("PidFile",             STRING,   PidFile,              NULL),
+  VAR("PreferTunneledDirConns", BOOL,  PreferTunneledDirConns, "1"),
   VAR("ProtocolWarnings",    BOOL,     ProtocolWarnings,     "0"),
   VAR("PublishServerDescriptor",BOOL,  PublishServerDescriptor,"1"),
   VAR("PublishHidServDescriptors",BOOL,PublishHidServDescriptors, "1"),
@@ -346,6 +347,7 @@ static config_var_description_t options_description[] = {
     "provided IP address (only usefol for multiple network interfaces)." },
   { "PIDFile", "On startup, write our PID to this file. On clean shutdown, "
     "remove the file." },
+  /* PreferTunneledDirConns */
   /* ProtocolWarnings */
   /* RephistTrackTime */
   { "RunAsDaemon", "If set, Tor forks and daemonizes to the background when "
@@ -2707,6 +2709,9 @@ options_validate(or_options_t *old_options, or_options_t *options,
   if (parse_virtual_addr_network(options->VirtualAddrNetwork, 1, NULL)<0)
     return -1;
 
+  if (options->PreferTunneledDirConns && !options->TunnelDirConns)
+    REJECT("Must set TunnelDirConns if PreferTunneledDirConns is set.");
+
   return 0;
 #undef REJECT
 #undef COMPLAIN
diff --git a/src/or/directory.c b/src/or/directory.c
index 18225aa0d5..0d8934776a 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -362,6 +362,17 @@ connection_dir_download_routerdesc_failed(dir_connection_t *conn)
   /* XXXX012 Why did the above get commented out? -NM */
 }
 
+/** Return 1 if platform can handle a BEGIN_DIR cell, and if
+ * we're willing to send one. Else return 0. */
+/* XXX we should refactor directory.c to hand status->or_port around,
+ * so we can check it here rather than platform. */
+static int
+connection_dir_supports_tunnels(or_options_t *options, const char *platform)
+{
+  return options->TunnelDirConns && platform &&
+         tor_version_as_new_as(platform, "0.1.2.2-alpha");
+}
+
 /** Helper for directory_initiate_command_(router|trusted_dir): send the
  * command to a server whose address is <b>address</b>, whose IP is
  * <b>addr</b>, whose directory port is <b>dir_port</b>, whose tor version is
@@ -376,8 +387,7 @@ directory_initiate_command(const char *address, uint32_t addr,
 {
   dir_connection_t *conn;
   or_options_t *options = get_options();
-  int want_to_tunnel = options->TunnelDirConns && platform &&
-                       tor_version_as_new_as(platform, "0.1.2.2-alpha");
+  int want_to_tunnel = connection_dir_supports_tunnels(options, platform);
 
   tor_assert(address);
   tor_assert(addr);
diff --git a/src/or/or.h b/src/or/or.h
index 23ecdad93c..6ee3f522e1 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1663,6 +1663,8 @@ typedef struct {
                                * same network zone in the same circuit. */
   int TunnelDirConns; /**< If true, use BEGIN_DIR rather than BEGIN when
                        * possible. */
+  int PreferTunneledDirConns; /**< If true, avoid dirservers that don't
+                               * support BEGIN_DIR, when possible. */
   int AllowNonRFC953Hostnames; /**< If true, we allow connections to hostnames
                                 * with weird characters. */
 } or_options_t;
@@ -2267,6 +2269,8 @@ int connection_dir_reached_eof(dir_connection_t *conn);
 int connection_dir_process_inbuf(dir_connection_t *conn);
 int connection_dir_finished_flushing(dir_connection_t *conn);
 int connection_dir_finished_connecting(dir_connection_t *conn);
+int connection_dir_supports_tunnels(or_options_t *options,
+                                    const char *platform);
 void connection_dir_request_failed(dir_connection_t *conn);
 int dir_split_resource_into_fingerprints(const char *resource,
                                     smartlist_t *fp_out, int *compresseed_out,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 92ae7da9e9..feae0778e4 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -20,9 +20,11 @@ const char routerlist_c_id[] =
 /* static function prototypes */
 static routerstatus_t *router_pick_directory_server_impl(int requireother,
                                                          int fascistfirewall,
+                                                         int prefer_tunnel,
                                                          int for_v2_directory);
 static routerstatus_t *router_pick_trusteddirserver_impl(
-                 authority_type_t type, int requireother, int fascistfirewall);
+                 authority_type_t type, int requireother,
+                 int fascistfirewall, int prefer_tunnel);
 static void mark_all_trusteddirservers_up(void);
 static int router_nickname_matches(routerinfo_t *router, const char *nickname);
 static void routerstatus_list_update_from_networkstatus(time_t now);
@@ -423,12 +425,13 @@ router_pick_directory_server(int requireother,
                              int retry_if_no_servers)
 {
   routerstatus_t *choice;
+  int prefer_tunnel = get_options()->PreferTunneledDirConns;
 
   if (!routerlist)
     return NULL;
 
   choice = router_pick_directory_server_impl(requireother, fascistfirewall,
-                                             for_v2_directory);
+                                             prefer_tunnel, for_v2_directory);
   if (choice || !retry_if_no_servers)
     return choice;
 
@@ -439,7 +442,7 @@ router_pick_directory_server(int requireother,
   mark_all_trusteddirservers_up();
   /* try again */
   choice = router_pick_directory_server_impl(requireother, fascistfirewall,
-                                             for_v2_directory);
+                                             prefer_tunnel, for_v2_directory);
   if (choice)
     return choice;
 
@@ -450,7 +453,7 @@ router_pick_directory_server(int requireother,
   }
   /* give it one last try */
   choice = router_pick_directory_server_impl(requireother, fascistfirewall,
-                                             for_v2_directory);
+                                             prefer_tunnel, for_v2_directory);
   return choice;
 }
 
@@ -486,17 +489,18 @@ router_pick_trusteddirserver(authority_type_t type,
                              int retry_if_no_servers)
 {
   routerstatus_t *choice;
+  int prefer_tunnel = get_options()->PreferTunneledDirConns;
 
-  choice = router_pick_trusteddirserver_impl(type,
-                                             requireother, fascistfirewall);
+  choice = router_pick_trusteddirserver_impl(type, requireother,
+                                fascistfirewall, prefer_tunnel);
   if (choice || !retry_if_no_servers)
     return choice;
 
   log_info(LD_DIR,
            "No trusted dirservers are reachable. Trying them all again.");
   mark_all_trusteddirservers_up();
-  return router_pick_trusteddirserver_impl(type,
-                                           requireother, fascistfirewall);
+  return router_pick_trusteddirserver_impl(type, requireother,
+                              fascistfirewall, prefer_tunnel);
 }
 
 /** How long do we avoid using a directory server after it's given us a 503? */
@@ -506,60 +510,81 @@ router_pick_trusteddirserver(authority_type_t type,
  * routerlist.  Don't pick an authority if any non-authorities are viable.
  * If <b>fascistfirewall</b>,
  * make sure the router we pick is allowed by our firewall options.
- * If <b>requireother</b>, it cannot be us.  If <b>for_v2_directory</b>,
+ * If <b>requireother</b>, it cannot be us. If <b>for_v2_directory</b>,
  * choose a directory server new enough to support the v2 directory
  * functionality.
+ * If <b>prefer_tunnel</b>, choose a directory server that is reachable
+ * and supports BEGIN_DIR cells, if possible.
+ * Try to avoid using servers that are overloaded (have returned 503
+ * recently).
  */
 static routerstatus_t *
 router_pick_directory_server_impl(int requireother, int fascistfirewall,
-                                  int for_v2_directory)
+                                  int prefer_tunnel, int for_v2_directory)
 {
   routerstatus_t *result;
-  smartlist_t *sl;
-  smartlist_t *overloaded;
-  smartlist_t *trusted;
+  smartlist_t *direct, *tunnel;
+  smartlist_t *trusted_direct, *trusted_tunnel;
+  smartlist_t *overloaded_direct, *overloaded_tunnel;
   time_t now = time(NULL);
 
   if (!routerstatus_list)
     return NULL;
 
+  direct = smartlist_create();
+  tunnel = smartlist_create();
+  trusted_direct = smartlist_create();
+  trusted_tunnel = smartlist_create();
+  overloaded_direct = smartlist_create();
+  overloaded_tunnel = smartlist_create();
+
   /* Find all the running dirservers we know about. */
-  sl = smartlist_create();
-  overloaded = smartlist_create();
-  trusted = smartlist_create();
   SMARTLIST_FOREACH(routerstatus_list, local_routerstatus_t *, _local_status,
   {
     routerstatus_t *status = &(_local_status->status);
     int is_trusted;
+    int is_overloaded = _local_status->last_dir_503_at + DIR_503_TIMEOUT > now;
     if (!status->is_running || !status->dir_port || !status->is_valid)
       continue;
     if (requireother && router_digest_is_me(status->identity_digest))
       continue;
-    if (fascistfirewall) {
-      if (!fascist_firewall_allows_address_dir(status->addr, status->dir_port))
-        continue;
-    }
     is_trusted = router_digest_is_trusted_dir(status->identity_digest);
     if (for_v2_directory && !(status->is_v2_dir || is_trusted))
       continue;
-    if (is_trusted) {
-      smartlist_add(trusted, status);
-    } else if (_local_status->last_dir_503_at + DIR_503_TIMEOUT > now) {
-      smartlist_add(overloaded, status);
-    } else {
-      smartlist_add(sl, status);
-    }
+    if (fascistfirewall &&
+        prefer_tunnel &&
+        status->version_supports_begindir &&
+        fascist_firewall_allows_address_or(status->addr, status->or_port))
+      smartlist_add(is_trusted ? trusted_tunnel :
+                      is_overloaded ? overloaded_tunnel : tunnel, status);
+    else if (!fascistfirewall || (fascistfirewall &&
+             fascist_firewall_allows_address_dir(status->addr,
+                                                 status->dir_port)))
+      smartlist_add(is_trusted ? trusted_direct :
+                      is_overloaded ? overloaded_direct : direct, status);
   });
 
-  if (smartlist_len(sl))
-    result = smartlist_choose(sl);
-  else if (smartlist_len(overloaded))
-    result = smartlist_choose(overloaded);
-  else
-    result = smartlist_choose(trusted);
-  smartlist_free(sl);
-  smartlist_free(overloaded);
-  smartlist_free(trusted);
+  if (smartlist_len(tunnel)) {
+    result = smartlist_choose(tunnel);
+  } else if (smartlist_len(overloaded_tunnel)) {
+    result = smartlist_choose(overloaded_tunnel);
+  } else if (trusted_tunnel) {
+    /* FFFF We don't distinguish between trusteds and overloaded trusteds
+     * yet. Maybe one day we should. */
+    result = smartlist_choose(trusted_tunnel);
+  } else if (smartlist_len(direct)) {
+    result = smartlist_choose(direct);
+  } else if (smartlist_len(overloaded_direct)) {
+    result = smartlist_choose(overloaded_direct);
+  } else {
+    result = smartlist_choose(trusted_direct);
+  }
+  smartlist_free(direct);
+  smartlist_free(tunnel);
+  smartlist_free(trusted_direct);
+  smartlist_free(trusted_tunnel);
+  smartlist_free(overloaded_direct);
+  smartlist_free(overloaded_tunnel);
   return result;
 }
 
@@ -571,22 +596,27 @@ router_pick_directory_server_impl(int requireother, int fascistfirewall,
  */
 static routerstatus_t *
 router_pick_trusteddirserver_impl(authority_type_t type,
-                                  int requireother, int fascistfirewall)
+                                  int requireother, int fascistfirewall,
+                                  int prefer_tunnel)
 {
-  smartlist_t *sl;
-  smartlist_t *overloaded;
-  routerinfo_t *me;
-  routerstatus_t *rs;
+  smartlist_t *direct, *tunnel;
+  smartlist_t *overloaded_direct, *overloaded_tunnel;
+  routerinfo_t *me = router_get_my_routerinfo();
+  routerstatus_t *result;
   time_t now = time(NULL);
-  sl = smartlist_create();
-  overloaded = smartlist_create();
-  me = router_get_my_routerinfo();
+
+  direct = smartlist_create();
+  tunnel = smartlist_create();
+  overloaded_direct = smartlist_create();
+  overloaded_tunnel = smartlist_create();
 
   if (!trusted_dir_servers)
     return NULL;
 
   SMARTLIST_FOREACH(trusted_dir_servers, trusted_dir_server_t *, d,
     {
+      int is_overloaded =
+          d->fake_status.last_dir_503_at + DIR_503_TIMEOUT > now;
       if (!d->is_running) continue;
       if (type == V1_AUTHORITY && !d->is_v1_authority)
         continue;
@@ -596,23 +626,35 @@ router_pick_trusteddirserver_impl(authority_type_t type,
         continue;
       if (requireother && me && router_digest_is_me(d->digest))
           continue;
-      if (fascistfirewall) {
-        if (!fascist_firewall_allows_address_dir(d->addr, d->dir_port))
-          continue;
-      }
-      if (d->fake_status.last_dir_503_at + DIR_503_TIMEOUT > now)
-        smartlist_add(overloaded, &d->fake_status.status);
-      else
-        smartlist_add(sl, &d->fake_status.status);
+
+      if (fascistfirewall &&
+          prefer_tunnel &&
+          d->or_port &&
+          fascist_firewall_allows_address_or(d->addr, d->or_port))
+        smartlist_add(is_overloaded ? overloaded_tunnel : tunnel,
+                      &d->fake_status.status);
+      else if (!fascistfirewall || (fascistfirewall &&
+               fascist_firewall_allows_address_dir(d->addr,
+                                                   d->dir_port)))
+        smartlist_add(is_overloaded ? overloaded_direct : direct,
+                      &d->fake_status.status);
     });
 
-  if (smartlist_len(sl))
-    rs = smartlist_choose(sl);
-  else
-    rs = smartlist_choose(overloaded);
-  smartlist_free(sl);
-  smartlist_free(overloaded);
-  return rs;
+  if (smartlist_len(tunnel)) {
+    result = smartlist_choose(tunnel);
+  } else if (smartlist_len(overloaded_tunnel)) {
+    result = smartlist_choose(overloaded_tunnel);
+  } else if (smartlist_len(direct)) {
+    result = smartlist_choose(direct);
+  } else {
+    result = smartlist_choose(overloaded_direct);
+  }
+
+  smartlist_free(direct);
+  smartlist_free(tunnel);
+  smartlist_free(overloaded_direct);
+  smartlist_free(overloaded_tunnel);
+  return result;
 }
 
 /** Go through and mark the authoritative dirservers as up. */