[or-cvs] bugfix in exit node choice: we used to find the perfect exi...

Roger Dingledine arma at seul.org
Wed Dec 3 08:06:58 UTC 2003


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

Modified Files:
	connection_edge.c onion.c or.h routers.c 
Log Message:
bugfix in exit node choice: we used to find the perfect exit node but                            then use the wrong one.
bugfix in connection_ap_can_use_exit: it was using the wrong port
bugfix: the OP now handles a port of '*' correctly when the IP is not
  yet known and it's trying to guess whether a router's exit policy
  might accept it.
we now don't ever pick exit routers which will reject *:*
attach_circuit now fails a new stream outright if it will never work.
when you get an 'end' cell that resolves an IP, now it will fail the                             circuit outright if no safe exit nodes exist for that IP.
don't try building a new circuit after an 'end' if a suitable one is
  already on the way.


Index: connection_edge.c
===================================================================
RCS file: /home/or/cvsroot/src/or/connection_edge.c,v
retrieving revision 1.68
retrieving revision 1.69
diff -u -d -r1.68 -r1.69
--- connection_edge.c	30 Nov 2003 10:10:29 -0000	1.68
+++ connection_edge.c	3 Dec 2003 08:06:55 -0000	1.69
@@ -251,7 +251,7 @@
           *(int*)conn->stream_id);
         return 0;
       }
-      if(cell->length-RELAY_HEADER_SIZE >= 5 && 
+      if(cell->length-RELAY_HEADER_SIZE >= 5 &&
          *(cell->payload+RELAY_HEADER_SIZE) == END_STREAM_REASON_EXITPOLICY) {
         /* No need to close the connection. We'll hold it open while
          * we try a new exit node.
@@ -260,9 +260,16 @@
         addr = ntohl(*(uint32_t*)(cell->payload+RELAY_HEADER_SIZE+1));
         client_dns_set_entry(conn->socks_request->address, addr);
         conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
-        if(connection_ap_handshake_attach_circuit(conn) < 0)
-          circuit_launch_new(); /* Build another circuit to handle this stream */
-        return 0;
+        switch(connection_ap_handshake_attach_circuit(conn)) {
+          case -1: /* it will never work */
+            break; /* conn will get closed below */
+          case 0: /* no useful circuits available */
+            if(!circuit_get_newest(conn, 0)) /* is one already on the way? */
+              circuit_launch_new();
+            return 0;
+          case 1: /* it succeeded, great */
+            return 0;
+        }
       }
       log_fn(LOG_INFO,"end cell (%s) for stream %d. Removing stream.",
         connection_edge_end_reason(cell->payload+RELAY_HEADER_SIZE, cell->length),
@@ -494,19 +501,29 @@
 void connection_ap_attach_pending(void)
 {
   connection_t **carray;
+  connection_t *conn;
   int n, i;
 
   get_connection_array(&carray, &n);
 
   for (i = 0; i < n; ++i) {
-    if (carray[i]->type != CONN_TYPE_AP || 
-        carray[i]->type != AP_CONN_STATE_CIRCUIT_WAIT)
+    conn = carray[i];
+    if (conn->type != CONN_TYPE_AP ||
+        conn->type != AP_CONN_STATE_CIRCUIT_WAIT)
       continue;
-    if (connection_ap_handshake_attach_circuit(carray[i])<0) {
-      if(!circuit_get_newest(carray[i], 0)) {
-        /* if there are no acceptable clean or not-very-dirty circs on the way */
-        circuit_launch_new();
-      }
+    switch(connection_ap_handshake_attach_circuit(conn)) {
+      case -1: /* it will never work */
+        conn->marked_for_close = 1;
+        conn->has_sent_end = 1;
+        break;
+      case 0: /* we need to build another circuit */
+        if(!circuit_get_newest(conn, 0)) {
+          /* if there are no acceptable clean or not-very-dirty circs on the way */
+          circuit_launch_new();
+        }
+        break;
+      case 1: /* it succeeded, great */
+        break;
     }
   }
 }
@@ -563,18 +580,28 @@
   } /* else socks handshake is done, continue processing */
 
   conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
-  if(connection_ap_handshake_attach_circuit(conn) < 0)
-    circuit_launch_new(); /* Build another circuit to handle this stream */
+  switch(connection_ap_handshake_attach_circuit(conn)) {
+    case -1: /* it will never work */
+      return -1;
+    case 0: /* no useful circuits available */
+      if(!circuit_get_newest(conn, 0)) /* is one already on the way? */
+        circuit_launch_new();
+      break;
+    case 1: /* it succeeded, great */
+      break;
+  }
   return 0;
 }
 
-/* Try to find a live circuit.  If we don't find one, tell 'conn' to
- * stop reading and return 0.  Otherwise, associate the CONN_TYPE_AP
- * connection 'conn' with a safe live circuit, start sending a
- * BEGIN cell down the circuit, and return 1.
+/* Try to find a safe live circuit for CONN_TYPE_AP connection conn. If
+ * we don't find one: if conn cannot be handled by any known nodes,
+ * warn and return -1; else tell conn to stop reading and return 0.
+ * Otherwise, associate conn with a safe live circuit, start
+ * sending a BEGIN cell down the circuit, and return 1.
  */
 static int connection_ap_handshake_attach_circuit(connection_t *conn) {
   circuit_t *circ;
+  uint32_t addr;
 
   assert(conn);
   assert(conn->type == CONN_TYPE_AP);
@@ -586,8 +613,14 @@
 
   if(!circ) {
     log_fn(LOG_INFO,"No safe circuit ready for edge connection; delaying.");
+    addr = client_dns_lookup_entry(conn->socks_request->address);
+    if(router_exit_policy_all_routers_reject(addr, conn->socks_request->port)) {
+      log_fn(LOG_WARN,"No node exists that will handle exit to %s:%d. Rejecting.",
+             conn->socks_request->address, conn->socks_request->port);
+      return -1;
+    }
     connection_stop_reading(conn); /* don't read until the connected cell arrives */
-    return -1;
+    return 0;
   }
 
   connection_start_reading(conn);
@@ -607,7 +640,7 @@
 
   connection_ap_handshake_send_begin(conn, circ);
 
-  return 0;
+  return 1;
 }
 
 /* deliver the destaddr:destport in a relay cell */
@@ -748,7 +781,7 @@
 void connection_exit_connect(connection_t *conn) {
   unsigned char connected_payload[4];
 
-  if(router_compare_to_exit_policy(conn) < 0) {
+  if(router_compare_to_my_exit_policy(conn) < 0) {
     log_fn(LOG_INFO,"%s:%d failed exit policy. Closing.", conn->address, conn->port);
     connection_edge_end(conn, END_STREAM_REASON_EXITPOLICY, NULL);
     return;
@@ -793,7 +826,7 @@
   assert(conn->socks_request);
 
   addr = client_dns_lookup_entry(conn->socks_request->address);
-  return router_supports_exit_address(addr, conn->port, exit);
+  return router_supports_exit_address(addr, conn->socks_request->port, exit);
 }
 
 /* ***** Client DNS code ***** */

Index: onion.c
===================================================================
RCS file: /home/or/cvsroot/src/or/onion.c,v
retrieving revision 1.96
retrieving revision 1.97
diff -u -d -r1.96 -r1.97
--- onion.c	20 Nov 2003 17:49:45 -0000	1.96
+++ onion.c	3 Dec 2003 08:06:55 -0000	1.97
@@ -256,15 +256,19 @@
    */
   n_supported = tor_malloc(sizeof(int)*dir->n_routers);
   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;
+  for (i = 0; i < dir->n_routers; ++i) { /* iterate over routers */
     if(!dir->routers[i]->is_running) {
       n_supported[i] = n_maybe_supported[i] = -1;
-      continue; /* skip routers which are known to be down */
+      continue; /* skip routers that are known to be down */
+    }
+    if(router_exit_policy_rejects_all(dir->routers[i])) {
+      n_supported[i] = n_maybe_supported[i] = -1;
+      continue; /* skip routers that reject all */
     }
+    n_supported[i] = n_maybe_supported[i] = 0;
     ++n_running_routers;
     for (j = 0; j < n_connections; ++j) { /* iterate over connections */
-      if (carray[j]->type != CONN_TYPE_AP || 
+      if (carray[j]->type != CONN_TYPE_AP ||
           carray[j]->state == AP_CONN_STATE_CIRCUIT_WAIT ||
           carray[j]->marked_for_close)
         continue; /* Skip everything but APs in CIRCUIT_WAIT */
@@ -308,11 +312,12 @@
      */
     for (j = best_support_idx; j < dir->n_routers; ++j) {
       if (n_supported[j] == best_support) {
-        if (i) 
+        if (i)
           --i;
         else {
           tor_free(n_supported); tor_free(n_maybe_supported);
-          return dir->routers[i];
+          log_fn(LOG_DEBUG, "Chose exit server '%s'", dir->routers[j]->nickname);
+          return dir->routers[j];
         }
       }
     }
@@ -325,11 +330,12 @@
     i = crypto_pseudo_rand_int(n_best_maybe_support);
     for (j = best_maybe_support_idx; j < dir->n_routers; ++j) {
       if (n_maybe_supported[j] == best_maybe_support) {
-        if (i) 
+        if (i)
           --i;
         else {
           tor_free(n_supported); tor_free(n_maybe_supported);
-          return dir->routers[i];
+          log_fn(LOG_DEBUG, "Chose exit server '%s'", dir->routers[j]->nickname);
+          return dir->routers[j];
         }
       }
     }
@@ -338,19 +344,19 @@
   }
   /* Either there are no pending connections, or no routers even seem to
    * possibly support any of them.  Choose a router at random. */
-  tor_free(n_supported); tor_free(n_maybe_supported);
   if (!n_running_routers) {
-    log_fn(LOG_WARN, "No routers seem to be running; can't choose an exit.");
+    log_fn(LOG_WARN, "No exit routers seem to be running; can't choose an exit.");
     return NULL;
   }
-  i = crypto_pseudo_rand_int(n_running_routers);
   /* Iterate over the routers, till we find the i'th one that has ->is_running
-   */
+   * and allows exits. */
+  i = crypto_pseudo_rand_int(n_running_routers);
   for (j = 0; j < dir->n_routers; ++j) {
-    if (dir->routers[j]->is_running) {
-      if (i) {
+    if (n_supported[j] != -1) {
+      if (i)
         --i;
-      } else {
+      else {
+        tor_free(n_supported); tor_free(n_maybe_supported);
         log_fn(LOG_DEBUG, "Chose exit server '%s'", dir->routers[j]->nickname);
         return dir->routers[j];
       }

Index: or.h
===================================================================
RCS file: /home/or/cvsroot/src/or/or.h,v
retrieving revision 1.194
retrieving revision 1.195
diff -u -d -r1.194 -r1.195
--- or.h	20 Nov 2003 17:49:45 -0000	1.194
+++ or.h	3 Dec 2003 08:06:55 -0000	1.195
@@ -756,13 +756,15 @@
 routerinfo_t *router_get_entry_from_string(char **s);
 int router_supports_exit_address(uint32_t addr, uint16_t port,
                                  routerinfo_t *router);
-int router_compare_to_exit_policy(connection_t *conn);
+int router_compare_to_my_exit_policy(connection_t *conn);
 int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
                                        struct exit_policy_t *policy);
 void routerinfo_free(routerinfo_t *router);
 int router_dump_router_to_string(char *s, int maxlen, routerinfo_t *router,
                                  crypto_pk_env_t *ident_key);
 const routerinfo_t *router_get_desc_routerinfo(void);
+int router_exit_policy_all_routers_reject(uint32_t addr, uint16_t port);
+int router_exit_policy_rejects_all(routerinfo_t *router);
 const char *router_get_my_descriptor(void);
 int router_rebuild_descriptor(void);
 int connection_ap_can_use_exit(connection_t *conn, routerinfo_t *exit);

Index: routers.c
===================================================================
RCS file: /home/or/cvsroot/src/or/routers.c,v
retrieving revision 1.97
retrieving revision 1.98
diff -u -d -r1.97 -r1.98
--- routers.c	30 Nov 2003 10:10:29 -0000	1.97
+++ routers.c	3 Dec 2003 08:06:55 -0000	1.98
@@ -197,7 +197,7 @@
     log_fn(LOG_WARN,"Failed to load routerfile %s.",routerfile);
     return -1;
   }
-  
+
   if(router_get_list_from_string(string) < 0) {
     log_fn(LOG_WARN,"The routerfile itself was corrupt.");
     free(string);
@@ -206,7 +206,7 @@
 
   free(string);
   return 0;
-} 
+}
 
 
 typedef enum {
@@ -322,7 +322,7 @@
       /* Find end of base64'd data */
       next = strstr(*s, OR_SIGNATURE_END_TAG);
       if (!next) { tok->val.error = "No signature end tag found"; return -1; }
-      
+
       signature = tor_malloc(256);
       i = base64_decode(signature, 256, *s, next-*s);
       if (i<0) {
@@ -675,8 +675,8 @@
     rarray[rarray_len++] = router;
     log_fn(LOG_DEBUG,"just added router #%d.",rarray_len);
   }
- 
-  if (*dest) 
+
+  if (*dest)
     directory_free(*dest);
   *dest = (directory_t *)tor_malloc(sizeof(directory_t));
   (*dest)->routers = rarray;
@@ -948,7 +948,7 @@
   if (router_get_next_token(&cp, &tok)) {
     log_fn(LOG_WARN, "Error reading exit policy: %s", tok.val.error);
     free(tmp);
-    return -1;                                                        
+    return -1;
   }
   if (tok.tp != K_ACCEPT && tok.tp != K_REJECT) {
     log_fn(LOG_WARN, "Expected 'accept' or 'reject'.");
@@ -1089,7 +1089,7 @@
     log_fn(LOG_DEBUG,"Considering exit policy %s", tmpe->string);
     if (!addr) {
       /* Address is unknown. */
-      if (tmpe->msk == 0 && port == tmpe->prt) {
+      if (tmpe->msk == 0 && (!tmpe || port == tmpe->prt)) {
         /* The exit policy is accept/reject *:port */
         match = 1;
       } else if ((!tmpe->prt || port == tmpe->prt) && 
@@ -1124,14 +1124,38 @@
 /* Return 0 if my exit policy says to allow connection to conn.
  * Else return -1.
  */
-int router_compare_to_exit_policy(connection_t *conn) {
+int router_compare_to_my_exit_policy(connection_t *conn) {
   assert(desc_routerinfo);
-  
+  assert(conn->addr); /* make sure it's resolved to something. this
+                         way we can't get a 'maybe' below. */
+
   if (router_compare_addr_to_exit_policy(conn->addr, conn->port,
-                                         desc_routerinfo->exit_policy))
-    return -1;
-  else 
+                                         desc_routerinfo->exit_policy) == 0)
     return 0;
+  else
+    return -1;
+}
+
+/* return 1 if all running routers will reject addr:port, return 0 if
+   any might accept it. */
+int router_exit_policy_all_routers_reject(uint32_t addr, uint16_t port) {
+  int i;
+  routerinfo_t *router;
+
+  for (i=0;i<directory->n_routers;i++) {
+    router = directory->routers[i];
+    if (router->is_running && router_compare_addr_to_exit_policy(addr,
+        port, router->exit_policy) >= 0)
+      return 0; /* this one could be ok. good enough. */
+  }
+  return 1; /* all will reject. */
+}
+
+int router_exit_policy_rejects_all(routerinfo_t *router) {
+  if (router_compare_addr_to_exit_policy(0, 0, router->exit_policy) < 0)
+    return 1; /* yes, rejects all */
+  else
+    return 0; /* no, might accept some */
 }
 
 const char *router_get_my_descriptor(void) {



More information about the tor-commits mailing list