Dirservers now do their own external reachability testing of each

Tor server, and only list them as running if they've been found to
be reachable.

Dirservers also log trouble servers, but only start complaining loudly
after they've been up for an hour, to reduce false positives. We still
need to do something about the fact that it is quite loud when there
are many trouble servers.


svn:r4829

7 files changed, 56 insertions, 40 deletions

diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index f68aed0de..58ccd4e27 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -334,15 +334,6 @@ connection_or_connect(uint32_t addr, uint16_t port, const char *id_digest)
     return NULL;
   }
 
-  /* this function should never be called if we're already connected to
-   * id_digest, but check first to be sure */
-  conn = connection_get_by_identity_digest(id_digest, CONN_TYPE_OR);
-  if (conn) {
-    tor_assert(conn->nickname);
-    log_fn(LOG_WARN,"Asked me to connect to router '%s', but there's already a connection.", conn->nickname);
-    return conn;
-  }
-
   conn = connection_new(CONN_TYPE_OR);
 
   /* set up conn so it's got all the data we need to remember */
diff --git a/src/or/directory.c b/src/or/directory.c
index 66f2d39cd..489d9adc6 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -807,7 +807,6 @@ connection_dir_client_reached_eof(connection_t *conn)
       log_fn(LOG_INFO,"updated routers.");
     }
     /* do things we've been waiting to do */
-    helper_nodes_set_status_from_directory();
     directory_has_arrived(time(NULL), conn->identity_digest);
   }
 
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 5a753af12..1dc1498fc 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -17,6 +17,8 @@ const char dirserv_c_id[] = "$Id$";
 /** How many seconds do we wait before regenerating the directory? */
 #define DIR_REGEN_SLACK_TIME 5
 
+extern long stats_n_seconds_working;
+
 /** Do we need to regenerate the directory when someone asks for it? */
 static int the_directory_is_dirty = 1;
 static int runningrouters_is_dirty = 1;
@@ -393,8 +395,9 @@ dirserv_add_descriptor(const char **desc, const char **msg)
       *desc = end;
       return verified;
     }
-    /* We don't alrady have a newer one; we'll update this one. */
+    /* We don't already have a newer one; we'll update this one. */
     log_fn(LOG_INFO,"Dirserv updating desc for server %s (nickname '%s')",hex_digest,ri->nickname);
+    ri->last_reachable = ri_old->last_reachable; /* this carries over */
     *msg = verified?"Verified server updated":"Unverified server updated. (Have you sent us your key fingerprint?)";
     routerinfo_free(ri_old);
     smartlist_del_keeporder(descriptor_list, found);
@@ -546,6 +549,9 @@ list_single_server_status(routerinfo_t *desc, int is_live)
   return tor_strdup(buf);
 }
 
+#define REACHABLE_TIMEOUT (60*60) /* an hour */
+/* Make sure this is 3 times the value of get_dir_fetch_period() */
+
 /** Based on the routerinfo_ts in <b>routers</b>, allocate the
  * contents of a router-status line, and store it in
  * *<b>router_status_out</b>.  Return 0 on success, -1 on failure.
@@ -556,6 +562,7 @@ list_server_status(smartlist_t *routers, char **router_status_out)
   /* List of entries in a router-status style: An optional !, then an optional
    * equals-suffixed nickname, then a dollar-prefixed hexdigest. */
   smartlist_t *rs_entries;
+  time_t now = time(NULL);
   int authdir_mode = get_options()->AuthoritativeDir;
   tor_assert(router_status_out);
 
@@ -563,16 +570,25 @@ list_server_status(smartlist_t *routers, char **router_status_out)
 
   SMARTLIST_FOREACH(routers, routerinfo_t *, ri,
   {
-    int is_live;
+    int is_live = 0;
     connection_t *conn;
     conn = connection_get_by_identity_digest(
                     ri->identity_digest, CONN_TYPE_OR);
     if (authdir_mode) {
       /* Treat a router as alive if
        *    - It's me, and I'm not hibernating.
-       * or - we're connected to it. */
-      is_live = (router_is_me(ri) && !we_are_hibernating()) ||
-        (conn && conn->state == OR_CONN_STATE_OPEN);
+       * or - we're connected to it and we've found it reachable recently. */
+      if (router_is_me(ri) && !we_are_hibernating()) {
+        is_live = 1;
+      } else if (conn && conn->state == OR_CONN_STATE_OPEN) {
+        if (now < ri->last_reachable + REACHABLE_TIMEOUT) {
+          is_live = 1;
+        } else {
+          log_fn(stats_n_seconds_working>REACHABLE_TIMEOUT ? LOG_NOTICE : LOG_INFO,
+                 "Router %s (%s:%d) is connected to us but not reachable by us.",
+                 ri->nickname, ri->address, ri->or_port);
+        }
+      }
     } else {
       is_live = ri->is_running;
     }
@@ -982,7 +998,7 @@ dirserv_orconn_tls_done(const char *address,
                         uint16_t or_port,
                         const char *digest_rcvd,
                         const char *nickname_rcvd,
-                        int as_advertised) //XXXRD
+                        int as_advertised)
 {
   int i;
   tor_assert(address);
@@ -995,10 +1011,9 @@ dirserv_orconn_tls_done(const char *address,
   for (i = 0; i < smartlist_len(descriptor_list); ++i) {
     routerinfo_t *ri = smartlist_get(descriptor_list, i);
     int drop = 0;
-    if (ri->is_verified)
+    if (strcasecmp(address, ri->address) || or_port != ri->or_port)
       continue;
-    if (!strcasecmp(address, ri->address) &&
-        or_port == ri->or_port) {
+    if (!ri->is_verified) {
       /* We have a router at the same address! */
       if (strcasecmp(ri->nickname, nickname_rcvd)) {
         log_fn(LOG_NOTICE, "Dropping descriptor: nickname '%s' does not match nickname '%s' in cert from %s:%d",
@@ -1009,11 +1024,14 @@ dirserv_orconn_tls_done(const char *address,
                address, or_port);
         drop = 1;
       }
-      if (drop) {
-        routerinfo_free(ri);
-        smartlist_del(descriptor_list, i--);
-        directory_set_dirty();
-      }
+    }
+    if (drop) {
+      routerinfo_free(ri);
+      smartlist_del(descriptor_list, i--);
+      directory_set_dirty();
+    } else { /* correct nickname and digest. mark this router reachable! */
+      log_fn(LOG_INFO,"Found router %s to be reachable. Yay.", ri->nickname);
+      ri->last_reachable = time(NULL);
     }
   }
 }
diff --git a/src/or/main.c b/src/or/main.c
index 227a5599c..3b22c2728 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -546,6 +546,9 @@ directory_has_arrived(time_t now, char *identity_digest)
   if (!time_to_fetch_running_routers)
     time_to_fetch_running_routers = now + get_status_fetch_period(options);
 
+  if (identity_digest) /* if it's fresh */
+    helper_nodes_set_status_from_directory();
+
   if (server_mode(options) && identity_digest) {
     /* if this is us, then our dirport is reachable */
     if (router_digest_is_me(identity_digest))
@@ -554,7 +557,8 @@ directory_has_arrived(time_t now, char *identity_digest)
 
   if (server_mode(options) &&
       !we_are_hibernating()) { /* connect to the appropriate routers */
-    router_retry_connections();
+    if (!authdir_mode(options))
+      router_retry_connections(0);
     if (identity_digest) /* we got a fresh directory */
       consider_testing_reachability();
   }
@@ -688,13 +692,11 @@ run_scheduled_events(time_t now)
     routerlist_remove_old_routers(ROUTER_MAX_AGE);
 
     if (authdir_mode(options)) {
-      /* We're a directory; dump any old descriptors. */
+      /* Dump any old descriptors. */
       dirserv_remove_old_servers(ROUTER_MAX_AGE);
-    }
-    if (server_mode(options) && !we_are_hibernating()) {
-      /* dirservers try to reconnect, in case connections have failed;
-       * and normal servers try to reconnect to dirservers */
-      router_retry_connections();
+      if (!we_are_hibernating()) { /* try to determine reachability */
+        router_retry_connections(1);
+      }
     }
 
     directory_get_from_dirserver(DIR_PURPOSE_FETCH_DIR, NULL, 1);
@@ -975,7 +977,7 @@ do_main_loop(void)
 
   if (authdir_mode(get_options())) {
     /* the directory is already here, run startup things */
-    router_retry_connections();
+    router_retry_connections(1);
   }
 
   if (server_mode(get_options())) {
diff --git a/src/or/or.h b/src/or/or.h
index 550d5aa54..4e35ba865 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -737,6 +737,7 @@ typedef struct {
   /* local info */
   int is_running; /**< As far as we know, is this OR currently running? */
   time_t status_set_at; /**< When did we last update is_running? */
+  time_t last_reachable; /**< When was the last time we could reach this OR? */
   int is_verified; /**< Has a trusted dirserver validated this OR? */
 
   smartlist_t *declared_family; /**< Nicknames of router which this router
@@ -1901,7 +1902,7 @@ int server_mode(or_options_t *options);
 int advertised_server_mode(void);
 int proxy_mode(or_options_t *options);
 
-void router_retry_connections(void);
+void router_retry_connections(int force);
 int router_is_clique_mode(routerinfo_t *router);
 void router_upload_dir_desc_to_dirservers(int force);
 void mark_my_descriptor_dirty_if_older_than(time_t when);
diff --git a/src/or/router.c b/src/or/router.c
index 9bb4716b4..bbb43db67 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -568,10 +568,13 @@ consider_publishable_server(time_t now, int force)
 
 /** OR only: if in clique mode, try to open connections to all of the
  * other ORs we know about. Otherwise, open connections to those we
- * think are in clique mode.
+ * think are in clique mode.o
+ *
+ * If <b>force</b> is zero, only open the connection if we don't already
+ * have one.
  */
 void
-router_retry_connections(void)
+router_retry_connections(int force)
 {
   int i;
   routerinfo_t *router;
@@ -588,10 +591,12 @@ router_retry_connections(void)
       continue;
     if (!clique_mode(options) && !router_is_clique_mode(router))
       continue;
-    if (!connection_get_by_identity_digest(router->identity_digest,
+    if (force ||
+        !connection_get_by_identity_digest(router->identity_digest,
                                            CONN_TYPE_OR)) {
-      /* not in the list */
-      log_fn(LOG_DEBUG,"connecting to OR at %s:%u.",router->address,router->or_port);
+      log_fn(LOG_INFO,"%sconnecting to %s at %s:%u.",
+             clique_mode(options) ? "(forced) " : "",
+             router->nickname, router->address, router->or_port);
       connection_or_connect(router->addr, router->or_port, router->identity_digest);
     }
   }
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index b370f245f..73e2ca8f9 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -738,7 +738,7 @@ check_directory_signature(const char *digest,
     return -1;
   }
   log_fn(LOG_DEBUG,"Signed directory hash starts %s", hex_str(signed_digest,4));
-  if (memcmp(digest, signed_digest, 20)) {
+  if (memcmp(digest, signed_digest, DIGEST_LEN)) {
     log_fn(LOG_WARN, "Error reading directory: signature does not match.");
     return -1;
   }
@@ -992,7 +992,7 @@ router_parse_entry_from_string(const char *s, const char *end)
     log_fn(LOG_WARN, "Invalid signature %d",t);
     goto err;
   }
-  if (memcmp(digest, signed_digest, 20)) {
+  if (memcmp(digest, signed_digest, DIGEST_LEN)) {
     log_fn(LOG_WARN, "Mismatched signature");
     goto err;
   }