[or-cvs] fix a race condition in 008pre2: don"t try to extend onto a...

[arma@seul.org (Roger Dingledine)]

Update of /home/or/cvsroot/src/or
In directory moria.mit.edu:/home2/arma/work/onion/cvs/src/or

Modified Files:
	circuitbuild.c connection.c connection_or.c or.h routerlist.c 
	routerparse.c 
Log Message:
fix a race condition in 008pre2: don't try to extend onto a connection
that's still handshaking.

for servers in clique mode, require the conn to be open before you'll
choose it for your path.


Index: circuitbuild.c
===================================================================
RCS file: /home/or/cvsroot/src/or/circuitbuild.c,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -d -r1.19 -r1.20
--- circuitbuild.c	8 Aug 2004 07:25:45 -0000	1.19
+++ circuitbuild.c	8 Aug 2004 10:32:36 -0000	1.20
@@ -443,8 +443,6 @@
   relay_header_unpack(&rh, cell->payload);
 
   if (rh.length == 4+2+ONIONSKIN_CHALLENGE_LEN) {
-    /* Once this format is no longer supported, nobody will use
-     * connection_*_get_by_addr_port. */
     old_format = 1;
   } else if (rh.length == 4+2+ONIONSKIN_CHALLENGE_LEN+DIGEST_LEN) {
     old_format = 0;
@@ -457,7 +455,7 @@
   circ->n_port = ntohs(get_uint16(cell->payload+RELAY_HEADER_SIZE+4));
 
   if (old_format) {
-    n_conn = connection_twin_get_by_addr_port(circ->n_addr,circ->n_port);
+    n_conn = connection_exact_get_by_addr_port(circ->n_addr,circ->n_port);
     onionskin = cell->payload+RELAY_HEADER_SIZE+4+2;
   } else {
     onionskin = cell->payload+RELAY_HEADER_SIZE+4+2;
@@ -465,7 +463,7 @@
     n_conn = connection_get_by_identity_digest(id_digest, CONN_TYPE_OR);
   }
 
-  if(!n_conn) { /* we should try to open a connection */
+  if(!n_conn || n_conn->state != OR_CONN_STATE_OPEN) {
      /* 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.
      */
@@ -478,7 +476,7 @@
     if (old_format) {
       router = router_get_by_addr_port(circ->n_addr, circ->n_port);
       if(!router) {
-        log_fn(LOG_INFO,"Next hop is an unknown router. Closing.");
+        log_fn(LOG_WARN,"Next hop is an unknown router. Closing.");
         return -1;
       }
       id_digest = router->identity_digest;
@@ -499,19 +497,26 @@
     /* imprint the circuit with its future n_conn->id */
     memcpy(circ->n_conn_id_digest, id_digest, DIGEST_LEN);
 
-    n_conn = connection_or_connect(circ->n_addr, circ->n_port, id_digest);
-    if(!n_conn) {
-      log_fn(LOG_INFO,"Launching n_conn failed. Closing.");
-      return -1;
+    if(n_conn) {
+      circ->n_addr = n_conn->addr;
+      circ->n_port = n_conn->port;
+    } else {
+     /* we should try to open a connection */
+      n_conn = connection_or_connect(circ->n_addr, circ->n_port, id_digest);
+      if(!n_conn) {
+        log_fn(LOG_INFO,"Launching n_conn failed. Closing.");
+        return -1;
+      }
+      log_fn(LOG_DEBUG,"connecting in progress (or finished). Good.");
     }
-    log_fn(LOG_DEBUG,"connecting in progress (or finished). Good.");
     /* return success. The onion/circuit/etc will be taken care of automatically
      * (may already have been) whenever n_conn reaches OR_CONN_STATE_OPEN.
      */
     return 0;
   }
 
-  circ->n_addr = n_conn->addr; /* these are different if we found a twin instead */
+  /* these may be different if the router connected to us from elsewhere */
+  circ->n_addr = n_conn->addr;
   circ->n_port = n_conn->port;
 
   circ->n_conn = n_conn;
@@ -1022,7 +1027,7 @@
     if(clique_mode()) {
       conn = connection_get_by_identity_digest(r->identity_digest,
                                                CONN_TYPE_OR);
-      if(!conn || conn->type != CONN_TYPE_OR || conn->state != OR_CONN_STATE_OPEN) {
+      if(!conn || conn->state != OR_CONN_STATE_OPEN) {
         log_fn(LOG_DEBUG,"Nope, %d is not connected.",i);
         goto next_i_loop;
       }

Index: connection.c
===================================================================
RCS file: /home/or/cvsroot/src/or/connection.c,v
retrieving revision 1.245
retrieving revision 1.246
diff -u -d -r1.245 -r1.246
--- connection.c	6 Aug 2004 09:56:36 -0000	1.245
+++ connection.c	8 Aug 2004 10:32:36 -0000	1.246
@@ -978,45 +978,6 @@
   return best;
 }
 
-/** Find a connection to the router described by addr and port,
- * or alternately any router with the same identity key.
- * This connection <em>must</em> be in an "open" state.
- * If not, return NULL.
- */
-/* XXX this twin thing is busted, now that we're rotating onion
- * keys. abandon/patch? */
-connection_t *connection_twin_get_by_addr_port(uint32_t addr, uint16_t port) {
-  int i, n;
-  connection_t *conn;
-  routerinfo_t *router;
-  connection_t **carray;
-
-  /* first check if it's there exactly */
-  conn = connection_exact_get_by_addr_port(addr,port);
-  if(conn && connection_state_is_open(conn)) {
-    log(LOG_DEBUG,"connection_twin_get_by_addr_port(): Found exact match.");
-    return conn;
-  }
-
-  /* now check if any of the other open connections are a twin for this one */
-
-  router = router_get_by_addr_port(addr,port);
-  if(!router)
-    return NULL;
-
-  get_connection_array(&carray,&n);
-  for(i=0;i<n;i++) {
-    conn = carray[i];
-    tor_assert(conn);
-    if(connection_state_is_open(conn) &&
-       !crypto_pk_cmp_keys(conn->identity_pkey, router->identity_pkey)) {
-      log(LOG_DEBUG,"connection_twin_get_by_addr_port(): Found twin (%s).",conn->address);
-      return conn;
-    }
-  }
-  return NULL;
-}
-
 connection_t *connection_get_by_identity_digest(const char *digest, int type)
 {
   int i, n;

Index: connection_or.c
===================================================================
RCS file: /home/or/cvsroot/src/or/connection_or.c,v
retrieving revision 1.122
retrieving revision 1.123
diff -u -d -r1.122 -r1.123
--- connection_or.c	22 Jul 2004 06:03:52 -0000	1.122
+++ connection_or.c	8 Aug 2004 10:32:36 -0000	1.123
@@ -168,8 +168,11 @@
   /* 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)
+  if(conn) {
+    tor_assert(conn->nickname);
+    log_fn(LOG_WARN,"Asked me to connect to %s, but there's already a connection.", conn->nickname);
     return conn;
+  }
 
   conn = connection_new(CONN_TYPE_OR);
 

Index: or.h
===================================================================
RCS file: /home/or/cvsroot/src/or/or.h,v
retrieving revision 1.401
retrieving revision 1.402
diff -u -d -r1.401 -r1.402
--- or.h	8 Aug 2004 07:25:45 -0000	1.401
+++ or.h	8 Aug 2004 10:32:36 -0000	1.402
@@ -1061,7 +1061,6 @@
 int connection_handle_write(connection_t *conn);
 void connection_write_to_buf(const char *string, int len, connection_t *conn);
 
-connection_t *connection_twin_get_by_addr_port(uint32_t addr, uint16_t port);
 connection_t *connection_exact_get_by_addr_port(uint32_t addr, uint16_t port);
 connection_t *connection_get_by_identity_digest(const char *digest, int type);
 

Index: routerlist.c
===================================================================
RCS file: /home/or/cvsroot/src/or/routerlist.c,v
retrieving revision 1.117
retrieving revision 1.118
diff -u -d -r1.117 -r1.118
--- routerlist.c	7 Aug 2004 09:01:56 -0000	1.117
+++ routerlist.c	8 Aug 2004 10:32:36 -0000	1.118
@@ -192,11 +192,17 @@
   for(i=0;i<smartlist_len(routerlist->routers);i++) {
     router = smartlist_get(routerlist->routers, i);
     /* XXX008 for now, only choose verified routers */
-    if(router->is_running && router->is_verified &&
-       (!clique_mode() ||
-        connection_get_by_identity_digest(router->identity_digest,
-                                          CONN_TYPE_OR)))
-      smartlist_add(sl, router);
+    if(router->is_running && router->is_verified) {
+      if(!clique_mode()) {
+        smartlist_add(sl, router);
+      } else {
+        connection_t *conn =
+          connection_get_by_identity_digest(router->identity_digest,
+                                            CONN_TYPE_OR);
+        if(conn && conn->state == OR_CONN_STATE_OPEN)
+          smartlist_add(sl, router);
+      }
+    }
   }
 }
 

Index: routerparse.c
===================================================================
RCS file: /home/or/cvsroot/src/or/routerparse.c,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -d -r1.30 -r1.31
--- routerparse.c	7 Aug 2004 09:01:56 -0000	1.30
+++ routerparse.c	8 Aug 2004 10:32:36 -0000	1.31
@@ -1308,7 +1308,7 @@
     return -1;
   }
   if (start != s && *(start-1) != '\n') {
-    log_fn(LOG_WARN, "first occurance of \"%s\" is not at the start of a line",
+    log_fn(LOG_WARN, "first occurrence of \"%s\" is not at the start of a line",
            start_str);
     return -1;
   }