Recover better from TCP connections to Tor servers that are broken but

don't tell you (it happens!); and rotate TLS connections once a week.

1) If an OR conn becomes more than a week old, make it obsolete.
2) If it's obsolete and empty, kill it.
3) When an OR makes a second connection to you, allow it.
4) If we want to send a new create cell, but the best conn we've
   got is obsolete, and the router is 0.1.1.9-alpha-cvs or later, ask
   for a new conn instead.
5) When we time out on circuit building on the first hop, make that
   connection obsolete.


svn:r5429

src/or/circuitbuild.c

@@ -343,12 +343,16 @@ circuit_handle_first_hop(circuit_t *circ)
   memcpy(circ->n_conn_id_digest, firsthop->extend_info->identity_digest,
          DIGEST_LEN);
   n_conn = connection_get_by_identity_digest(
-         firsthop->extend_info->identity_digest, CONN_TYPE_OR);
-  if (!n_conn || n_conn->state != OR_CONN_STATE_OPEN) { /* not currently connected */
+         firsthop->extend_info->identity_digest);
+  if (!n_conn || n_conn->state != OR_CONN_STATE_OPEN ||
+      (n_conn->is_obsolete &&
+       router_digest_version_as_new_as(firsthop->extend_info->identity_digest,
+                                       "0.1.1.9-alpha-cvs"))) {
+    /* not currently connected */
     circ->n_addr = firsthop->extend_info->addr;
     circ->n_port = firsthop->extend_info->port;
 
-    if (!n_conn) { /* launch the connection */
+    if (!n_conn || n_conn->is_obsolete) { /* launch the connection */
       n_conn = connection_or_connect(firsthop->extend_info->addr,
                                      firsthop->extend_info->port,
                                      firsthop->extend_info->identity_digest);
@@ -633,9 +637,11 @@ circuit_extend(cell_t *cell, circuit_t *circ)
 
   onionskin = cell->payload+RELAY_HEADER_SIZE+4+2;
   id_digest = cell->payload+RELAY_HEADER_SIZE+4+2+ONIONSKIN_CHALLENGE_LEN;
-  n_conn = connection_get_by_identity_digest(id_digest, CONN_TYPE_OR);
+  n_conn = connection_get_by_identity_digest(id_digest);
 
-  if (!n_conn || n_conn->state != OR_CONN_STATE_OPEN) {
+  if (!n_conn || n_conn->state != OR_CONN_STATE_OPEN ||
+    (n_conn->is_obsolete &&
+     router_digest_version_as_new_as(id_digest,"0.1.1.9-alpha-cvs"))) {
      /* Note that this will close circuits where the onion has the same
      * router twice in a row in the path. I think that's ok.
      */
@@ -653,7 +659,7 @@ circuit_extend(cell_t *cell, circuit_t *circ)
     /* imprint the circuit with its future n_conn->id */
     memcpy(circ->n_conn_id_digest, id_digest, DIGEST_LEN);
 
-    if (n_conn) {
+    if (n_conn && !n_conn->is_obsolete) {
       circ->n_addr = n_conn->addr;
       circ->n_port = n_conn->port;
     } else {

src/or/circuituse.c

@@ -656,6 +656,16 @@ circuit_build_failed(circuit_t *circ)
       circ->cpath->prev->prev->state == CPATH_STATE_OPEN) {
     failed_at_last_hop = 1;
   }
+  if (circ->cpath &&
+      circ->cpath->state != CPATH_STATE_OPEN) {
+    /* We failed at the first hop. If there's an OR connection
+       to blame, blame it. */
+    if (circ->n_conn) {
+      info(LD_OR, "Our circuit failed to get a response from the first hop (%s:%d). I'm going to try to rotate to a better connection.",
+           circ->n_conn->address, circ->n_conn->port);
+      circ->n_conn->is_obsolete = 1;
+    }
+  }
 
   switch (circ->purpose) {
     case CIRCUIT_PURPOSE_C_GENERAL:

src/or/connection.c

@@ -1547,8 +1547,8 @@ connection_or_exact_get_by_addr_port(uint32_t addr, uint16_t port)
   return best;
 }
 
-/** Return a connection with give type, address, port, and purpose or NULL if
- * no such connection exists. */
+/** Return a connection with given type, address, port, and purpose;
+ * or NULL if no such connection exists. */
 connection_t *
 connection_get_by_type_addr_port_purpose(int type, uint32_t addr, uint16_t port,
                                          int purpose)
@@ -1570,22 +1570,42 @@ connection_get_by_type_addr_port_purpose(int type, uint32_t addr, uint16_t port,
   return NULL;
 }
 
+/** Return the best connection of type OR with the
+ * digest <b>digest</b> that we have, or NULL if we have none.
+ *
+ * 1) Don't return it if it's marked for close.
+ * 2) If there are any open conns, ignore non-open conns.
+ * 3) If there are any non-obsolete conns, ignore obsolete conns.
+ * 4) Then if there are any non-empty conns, ignore empty conns.
+ * 5) Of the remaining conns, prefer newer conns.
+ */
 connection_t *
-connection_get_by_identity_digest(const char *digest, int type)
+connection_get_by_identity_digest(const char *digest)
 {
-  int i, n;
+  int i, n, newer;
   connection_t *conn, *best=NULL;
   connection_t **carray;
 
   get_connection_array(&carray,&n);
   for (i=0;i<n;i++) {
     conn = carray[i];
-    if (conn->type != type)
+    if (conn->marked_for_close ||
+        memcmp(conn->identity_digest, digest, DIGEST_LEN))
       continue;
-    if (!memcmp(conn->identity_digest, digest, DIGEST_LEN) &&
-        !conn->marked_for_close &&
-        (!best || best->timestamp_created < conn->timestamp_created))
-      best = conn;
+    if (!best) {
+      best = conn; /* whatever it is, it's better than nothing. */
+      continue;
+    }
+    if (best->state == OR_CONN_STATE_OPEN &&
+        conn->state != OR_CONN_STATE_OPEN)
+      continue; /* avoid non-open conns if we can */
+    newer = best->timestamp_created < conn->timestamp_created;
+    if (conn->is_obsolete && (!best->is_obsolete || !newer))
+      continue; /* we have something, and it's better than this. */
+    if (circuit_get_by_conn(best) && !circuit_get_by_conn(conn))
+      continue; /* prefer conns with circuits on them */
+    if (newer)
+      best = conn; /* lastly, prefer newer conns */
   }
   return best;
 }

src/or/connection_or.c

@@ -558,12 +558,14 @@ connection_tls_finish_handshake(connection_t *conn)
     return -1;
 
   if (!connection_or_nonopen_was_started_here(conn)) {
+#if 0
     connection_t *c;
-    if ((c=connection_get_by_identity_digest(digest_rcvd, CONN_TYPE_OR))) {
+    if ((c=connection_get_by_identity_digest(digest_rcvd))) {
       debug(LD_OR,"Router '%s' is already connected on fd %d. Dropping fd %d.",
              c->nickname, c->s, conn->s);
       return -1;
     }
+#endif
     connection_or_init_conn_from_address(conn,conn->addr,conn->port,digest_rcvd);
   }
 

src/or/dirserv.c

@@ -616,8 +616,7 @@ dirserv_thinks_router_is_reachable(routerinfo_t *router, time_t now)
   connection_t *conn;
   if (router_is_me(router) && !we_are_hibernating())
     return 1;
-  conn = connection_get_by_identity_digest(router->cache_info.identity_digest,
-                                           CONN_TYPE_OR);
+  conn = connection_get_by_identity_digest(router->cache_info.identity_digest);
   if (conn && conn->state == OR_CONN_STATE_OPEN)
     return get_options()->AssumeReachable ||
            now < router->last_reachable + REACHABLE_TIMEOUT;
@@ -633,8 +632,7 @@ dirserv_thinks_router_is_blatantly_unreachable(routerinfo_t *router, time_t now)
   connection_t *conn;
   if (router->is_hibernating)
     return 0;
-  conn = connection_get_by_identity_digest(router->cache_info.identity_digest,
-                                           CONN_TYPE_OR);
+  conn = connection_get_by_identity_digest(router->cache_info.identity_digest);
   if (conn && conn->state == OR_CONN_STATE_OPEN &&
       now >= router->last_reachable + 2*REACHABLE_TIMEOUT &&
       router->testing_since &&

src/or/main.c

@@ -597,10 +597,30 @@ run_connection_housekeeping(int i, time_t now)
     return;
   }
 
+  if (!connection_speaks_cells(conn))
+    return; /* we're all done here, the rest is just for OR conns */
+
+#define TIME_BEFORE_OR_CONN_IS_OBSOLETE (60*60*24*7) /* a week */
+  if (!conn->is_obsolete &&
+      conn->timestamp_created + TIME_BEFORE_OR_CONN_IS_OBSOLETE < now) {
+    info(LD_OR, "Marking OR conn to %s:%d obsolete (fd %d, %d secs old).",
+         conn->address, conn->port, conn->s,
+         (int)(now - conn->timestamp_created));
+    conn->is_obsolete = 1;
+  }
+
+  if (conn->is_obsolete && !circuit_get_by_conn(conn)) {
+    /* no unmarked circs -- mark it now */
+    info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) [Obsolete].",
+         conn->s,conn->address, conn->port);
+    connection_mark_for_close(conn);
+    conn->hold_open_until_flushed = 1;
+    return;
+  }
+
   /* If we haven't written to an OR connection for a while, then either nuke
      the connection or send a keepalive, depending. */
-  if (connection_speaks_cells(conn) &&
-      now >= conn->timestamp_lastwritten + options->KeepalivePeriod) {
+  if (now >= conn->timestamp_lastwritten + options->KeepalivePeriod) {
     routerinfo_t *router = router_get_by_digest(conn->identity_digest);
     if (!connection_state_is_open(conn)) {
       info(LD_OR,"Expiring non-open OR connection to fd %d (%s:%d).",
@@ -614,7 +634,8 @@ run_connection_housekeeping(int i, time_t now)
       connection_mark_for_close(conn);
       conn->hold_open_until_flushed = 1;
     } else if (!clique_mode(options) && !circuit_get_by_conn(conn) &&
-               (!router || !server_mode(options) || !router_is_clique_mode(router))) {
+               (!router || !server_mode(options) ||
+                !router_is_clique_mode(router))) {
       info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) [Not in clique mode].",
              conn->s,conn->address, conn->port);
       connection_mark_for_close(conn);

src/or/or.h

@@ -606,6 +606,8 @@ struct connection_t {
   /** For control connections only. If set, we send extended info with control
    * events as appropriate. */
   unsigned int control_events_are_extended:1;
+  /** Used for OR conns that shouldn't get any new circs attached to them. */
+  unsigned int is_obsolete:1;
 
   int s; /**< Our socket; -1 if this connection is closed. */
   int poll_index; /* XXXX rename. */
@@ -1565,7 +1567,7 @@ void _connection_controller_force_write(connection_t *conn);
 void connection_write_to_buf(const char *string, size_t len, connection_t *conn);
 
 connection_t *connection_or_exact_get_by_addr_port(uint32_t addr, uint16_t port);
-connection_t *connection_get_by_identity_digest(const char *digest, int type);
+connection_t *connection_get_by_identity_digest(const char *digest);
 connection_t *connection_get_by_global_id(uint32_t id);
 
 connection_t *connection_get_by_type(int type);
@@ -2153,6 +2155,7 @@ routerinfo_t *router_get_by_nickname(const char *nickname,
 routerinfo_t *router_get_by_hexdigest(const char *hexdigest);
 routerinfo_t *router_get_by_digest(const char *digest);
 signed_descriptor_t *router_get_by_descriptor_digest(const char *digest);
+int router_digest_version_as_new_as(const char *digest, const char *cutoff);
 int router_digest_is_trusted_dir(const char *digest);
 routerlist_t *router_get_routerlist(void);
 void routerlist_reset_warnings(void);

src/or/router.c

@@ -623,8 +623,7 @@ router_retry_connections(int force)
     if (!clique_mode(options) && !router_is_clique_mode(router))
       continue;
     if (force ||
-        !connection_get_by_identity_digest(router->cache_info.identity_digest,
-                                           CONN_TYPE_OR)) {
+        !connection_get_by_identity_digest(router->cache_info.identity_digest)) {
       debug(LD_OR,"%sconnecting to %s at %s:%u.",
              clique_mode(options) ? "(forced) " : "",
              router->nickname, router->address, router->or_port);

src/or/routerlist.c

@@ -958,6 +958,18 @@ router_get_by_nickname(const char *nickname, int warn_if_unnamed)
   return NULL;
 }
 
+/** Try to find a routerinfo for <b>digest</b>. If we don't have one,
+ * return 1. If we do, ask tor_version_as_new_as() for the answer.
+ */
+int
+router_digest_version_as_new_as(const char *digest, const char *cutoff)
+{
+  routerinfo_t *router = router_get_by_digest(digest);
+  if (!router)
+    return 1;
+  return tor_version_as_new_as(router->platform, cutoff);
+}
+
 /** Return true iff <b>digest</b> is the digest of the identity key of
  * a trusted directory. */
 int
@@ -1486,7 +1498,7 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
          */
         connection_t *conn;
         while ((conn = connection_get_by_identity_digest(
-                      old_router->cache_info.identity_digest, CONN_TYPE_OR))) {
+                      old_router->cache_info.identity_digest))) {
           // And LD_OR? XXXXNM
           info(LD_DIR,"Closing conn to router '%s'; there is now a named router with that name.",
                  old_router->nickname);