[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[or-cvs] bugfix: don"t ask for ->next of an expired circuit

To: or-cvs@freehaven.net
Subject: [or-cvs] bugfix: don"t ask for ->next of an expired circuit
From: arma@seul.org (Roger Dingledine)
Date: Tue, 18 Nov 2003 04:53:05 -0500 (EST)
Delivered-to: archiver@seul.org
Delivered-to: or-cvs-outgoing@seul.org
Delivered-to: or-cvs@seul.org
Delivery-date: Tue, 18 Nov 2003 04:53:23 -0500
Reply-to: or-dev@freehaven.net
Sender: owner-or-cvs@freehaven.net
Update of /home/or/cvsroot/src/or
In directory moria.mit.edu:/home2/arma/work/onion/cvs/src/or

Modified Files:
	circuit.c connection_or.c main.c onion.c or.h 
Log Message:
bugfix: don't ask for ->next of an expired circuit
bugfix: keep going when a circ fails in circuit_n_conn_open
        (make circuit_enumerate_by_naddr_nport obsolete)
bugfix: make circuit_n_conn_open only look at circ's that start at us
bugfix: only try circuit_n_conn_open if we're an OP. Otherwise we
        expect connections to always already be up.
bugfix: when choosing path length, pay attention to whether the directory
        says a router is down.
bugfix: when picking good exit, skip routers which are known to be down
        (more work needs to be done on this one)


Index: circuit.c
===================================================================
RCS file: /home/or/cvsroot/src/or/circuit.c,v
retrieving revision 1.105
retrieving revision 1.106
diff -u -d -r1.105 -r1.106
--- circuit.c	18 Nov 2003 08:20:19 -0000	1.105
+++ circuit.c	18 Nov 2003 09:53:02 -0000	1.106
@@ -141,20 +141,6 @@
   return test_circ_id;
 }
 
-circuit_t *circuit_enumerate_by_naddr_nport(circuit_t *circ, uint32_t naddr, uint16_t nport) {
-
-  if(!circ) /* use circ if it's defined, else start from the beginning */
-    circ = global_circuitlist; 
-  else
-    circ = circ->next;
-
-  for( ; circ; circ = circ->next) {
-    if(circ->n_addr == naddr && circ->n_port == nport)
-       return circ;
-  }
-  return NULL;
-}
-
 circuit_t *circuit_get_by_circ_id_conn(circ_id_t circ_id, connection_t *conn) {
   circuit_t *circ;
   connection_t *tmpconn;
@@ -258,21 +244,23 @@
 /* close all circuits that start at us, aren't open, and were born
  * at least MIN_SECONDS_BEFORE_EXPIRING_CIRC seconds ago */
 void circuit_expire_building(void) {
-  circuit_t *circ;
   int now = time(NULL);
+  circuit_t *victim, *circ = global_circuitlist;
 
-  for(circ=global_circuitlist;circ;circ = circ->next) {
-    if(circ->cpath &&
-       circ->state != CIRCUIT_STATE_OPEN &&
-       circ->timestamp_created + MIN_SECONDS_BEFORE_EXPIRING_CIRC+1 < now) {
-      if(circ->n_conn)
+  while(circ) {
+    victim = circ;
+    circ = circ->next;
+    if(victim->cpath &&
+       victim->state != CIRCUIT_STATE_OPEN &&
+       victim->timestamp_created + MIN_SECONDS_BEFORE_EXPIRING_CIRC+1 < now) {
+      if(victim->n_conn)
         log_fn(LOG_INFO,"Abandoning circ %s:%d:%d (state %d:%s)",
-               circ->n_conn->address, circ->n_port, circ->n_circ_id,
-               circ->state, circuit_state_to_string[circ->state]);
+               victim->n_conn->address, victim->n_port, victim->n_circ_id,
+               victim->state, circuit_state_to_string[victim->state]);
       else
-        log_fn(LOG_INFO,"Abandoning circ %d (state %d:%s)", circ->n_circ_id,
-               circ->state, circuit_state_to_string[circ->state]);
-      circuit_close(circ);
+        log_fn(LOG_INFO,"Abandoning circ %d (state %d:%s)", victim->n_circ_id,
+               victim->state, circuit_state_to_string[victim->state]);
+      circuit_close(victim);
     }
   }
 }
@@ -793,21 +781,18 @@
 void circuit_n_conn_open(connection_t *or_conn) {
   circuit_t *circ;
 
-  log_fn(LOG_DEBUG,"Starting.");
-  circ = circuit_enumerate_by_naddr_nport(NULL, or_conn->addr, or_conn->port);
-  for(;;) {
-    if(!circ)
-      return;
-
-    assert(circ->state == CIRCUIT_STATE_OR_WAIT);
-    log_fn(LOG_DEBUG,"Found circ, sending onion skin.");
-    circ->n_conn = or_conn;
-    if(circuit_send_next_onion_skin(circ) < 0) {
-      log_fn(LOG_INFO,"send_next_onion_skin failed; circuit marked for closing.");
-      circuit_close(circ);
-      return; /* FIXME will want to try the other circuits too? */
+  for(circ=global_circuitlist;circ;circ = circ->next) {
+    if(circ->cpath && circ->n_addr == or_conn->addr && circ->n_port == or_conn->port) {
+      assert(circ->state == CIRCUIT_STATE_OR_WAIT);
+      log_fn(LOG_DEBUG,"Found circ %d, sending onion skin.", circ->n_circ_id);
+      circ->n_conn = or_conn;
+      if(circuit_send_next_onion_skin(circ) < 0) {
+        log_fn(LOG_INFO,"send_next_onion_skin failed; circuit marked for closing.");
+        circuit_close(circ);
+        continue;
+          /* XXX could this be bad, eg if next_onion_skin failed because conn died? */
+      }
     }
-    circ = circuit_enumerate_by_naddr_nport(circ, or_conn->addr, or_conn->port);
   }
 }
 

Index: connection_or.c
===================================================================
RCS file: /home/or/cvsroot/src/or/connection_or.c,v
retrieving revision 1.74
retrieving revision 1.75
diff -u -d -r1.74 -r1.75
--- connection_or.c	18 Nov 2003 07:25:04 -0000	1.74
+++ connection_or.c	18 Nov 2003 09:53:02 -0000	1.75
@@ -203,8 +203,8 @@
   log_fn(LOG_DEBUG, "Other side claims to be '%s'", nickname);
   pk = tor_tls_verify(conn->tls);
   if(!pk) {
-    log_fn(LOG_WARN,"Other side (%s:%d) has a cert but it's invalid. Closing.",
-           conn->address, conn->port);
+    log_fn(LOG_WARN,"Other side '%s' (%s:%d) has a cert but it's invalid. Closing.",
+           nickname, conn->address, conn->port);
     return -1;
   }
   router = router_get_by_link_pk(pk);
@@ -230,7 +230,7 @@
     }
     connection_or_init_conn_from_router(conn, router);
     crypto_free_pk_env(pk);
-  } 
+  }
   if (strcmp(conn->nickname, nickname)) {
     log_fn(LOG_WARN,"Other side claims to be '%s', but we wanted '%s'",
            nickname, conn->nickname);
@@ -239,8 +239,8 @@
   if (!options.OnionRouter) {
     /* If I'm an OP... */
     conn->receiver_bucket = conn->bandwidth = DEFAULT_BANDWIDTH_OP;
+    circuit_n_conn_open(conn); /* send the pending creates, if any. */
   }
-  circuit_n_conn_open(conn); /* send the pending creates, if any. */
   return 0;
 }
 

Index: main.c
===================================================================
RCS file: /home/or/cvsroot/src/or/main.c,v
retrieving revision 1.148
retrieving revision 1.149
diff -u -d -r1.148 -r1.149
--- main.c	18 Nov 2003 07:48:00 -0000	1.148
+++ main.c	18 Nov 2003 09:53:02 -0000	1.149
@@ -363,9 +363,9 @@
       }
       time_to_new_circuit = now + options.NewCircuitPeriod;
     }
-#define CIRCUIT_MIN_BUILDING 3
+#define CIRCUIT_MIN_BUILDING 2
     if(!circ && circuit_count_building() < CIRCUIT_MIN_BUILDING)
-      /* if there's no open circ, and less than 3 are on the way,
+      /* if there's no open circ, and less than 2 are on the way,
        * go ahead and try another.
        */
       circuit_launch_new(1);

Index: onion.c
===================================================================
RCS file: /home/or/cvsroot/src/or/onion.c,v
retrieving revision 1.90
retrieving revision 1.91
diff -u -d -r1.90 -r1.91
--- onion.c	18 Nov 2003 08:20:19 -0000	1.90
+++ onion.c	18 Nov 2003 09:53:02 -0000	1.91
@@ -254,15 +254,17 @@
   n_maybe_supported = tor_malloc(sizeof(int)*dir->n_routers);
   for (i = 0; i < dir->n_routers; ++i) {   /* iterate over routers */
     n_supported[i] = n_maybe_supported[i] = 0;
+    if(!dir->routers[i]->is_running)
+      continue; /* skip routers which are known to be down */
     for (j = 0; j < n_connections; ++j) { /* iterate over connections */
       if (carray[j]->type != CONN_TYPE_AP || 
           carray[j]->state == AP_CONN_STATE_CIRCUIT_WAIT ||
           carray[j]->marked_for_close)
-        continue; /* Skip everything by open APs in CIRCUIT_WAIT */
+        continue; /* Skip everything but APs in CIRCUIT_WAIT */
       switch (connection_ap_can_use_exit(carray[j], dir->routers[i])) 
         {
         case -1:
-          break;
+          break; /* would be rejected; try next connection */
         case 0:
           ++n_supported[i];
           ; /* Fall through: If it is supported, it is also maybe supported. */
@@ -308,6 +310,7 @@
       }
     }
     /* This point should never be reached. */
+    assert(0);
   }
   /* If any routers _maybe_ support pending connections, choose one at
    * random, as above.  */
@@ -324,9 +327,11 @@
       }
     }
     /* This point should never be reached. */
+    assert(0);
   }
   /* Either there are no pending connections, or no routers even seem to
    * possibly support any of them.  Choose a router at random. */
+  /* XXX should change to not choose non-running routers */
   tor_free(n_supported); tor_free(n_maybe_supported);
   i = crypto_pseudo_rand_int(dir->n_routers);
   log_fn(LOG_DEBUG, "Chose exit server '%s'", dir->routers[i]->nickname);
@@ -355,6 +360,10 @@
 
   for(i=0;i<rarray_len;i++) {
     log_fn(LOG_DEBUG,"Contemplating whether router %d is a new option...",i);
+    if(rarray[i]->is_running == 0) {
+      log_fn(LOG_DEBUG,"Nope, the directory says %d is not running.",i);
+      goto next_i_loop;
+    }
     if(options.OnionRouter) {
       conn = connection_exact_get_by_addr_port(rarray[i]->addr, rarray[i]->or_port);
       if(!conn || conn->type != CONN_TYPE_OR || conn->state != OR_CONN_STATE_OPEN) {

Index: or.h
===================================================================
RCS file: /home/or/cvsroot/src/or/or.h,v
retrieving revision 1.191
retrieving revision 1.192
diff -u -d -r1.191 -r1.192
--- or.h	18 Nov 2003 07:48:00 -0000	1.191
+++ or.h	18 Nov 2003 09:53:03 -0000	1.192
@@ -512,7 +512,6 @@
 void circuit_free(circuit_t *circ);
 void circuit_free_cpath(crypt_path_t *cpath);
 
-circuit_t *circuit_enumerate_by_naddr_nport(circuit_t *start, uint32_t naddr, uint16_t nport);
 circuit_t *circuit_get_by_circ_id_conn(circ_id_t circ_id, connection_t *conn);
 circuit_t *circuit_get_by_conn(connection_t *conn);
 circuit_t *circuit_get_newest(connection_t *conn, int must_be_open);
Prev by Author: [or-cvs] use the tor_malloc_zero wrapper
Next by Author: [or-cvs] make more sure we can"t end up with two connections to the ...
Previous by thread: [or-cvs] use the tor_malloc_zero wrapper
Next by thread: [or-cvs] make more sure we can"t end up with two connections to the ...
Index(es):
- Author
- Thread