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.


svn:r874

src/or/connection_edge.c

@@ -251,7 +251,7 @@ int connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, connection
           *(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 @@ int connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, connection
         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 @@ repeat_connection_edge_package_raw_inbuf:
 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 @@ static int connection_ap_handshake_process_socks(connection_t *conn) {
   } /* 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 @@ static int connection_ap_handshake_attach_circuit(connection_t *conn) {
 
   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 @@ static int connection_ap_handshake_attach_circuit(connection_t *conn) {
 
   connection_ap_handshake_send_begin(conn, circ);
 
-  return 0;
+  return 1;
 }
 
 /* deliver the destaddr:destport in a relay cell */
@@ -748,7 +781,7 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
 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 @@ int connection_ap_can_use_exit(connection_t *conn, routerinfo_t *exit)
   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 ***** */

src/or/onion.c

@@ -256,15 +256,19 @@ static routerinfo_t *choose_good_exit_server(directory_t *dir)
    */
   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 @@ static routerinfo_t *choose_good_exit_server(directory_t *dir)
      */
     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 @@ static routerinfo_t *choose_good_exit_server(directory_t *dir)
     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 @@ static routerinfo_t *choose_good_exit_server(directory_t *dir)
   }
   /* 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];
       }

src/or/or.h

@@ -756,13 +756,15 @@ int router_get_dir_from_string_impl(char *s, directory_t **dest,
 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);

src/or/routers.c

@@ -197,7 +197,7 @@ int router_get_list_from_file(char *routerfile)
     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 @@ int router_get_list_from_file(char *routerfile)
 
   free(string);
   return 0;
-} 
+}
 
 
 typedef enum {
@@ -322,7 +322,7 @@ _router_get_next_token(char **s, directory_token_t *tok) {
       /* 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 @@ int router_get_list_from_string_impl(char **s, directory_t **dest,
     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 @@ router_add_exit_policy_from_string(routerinfo_t *router,
   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 @@ int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
     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 @@ int router_compare_addr_to_exit_policy(uint32_t addr, uint16_t port,
 /* 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) {