Expire socks connections if they spend too long waiting for the

handshake to finish. Previously we would let them sit around for
days, if the connecting application didn't close them either.

Also take this opportunity to refactor a duplicate bit of circuituse.c.

And change the semantics of SocksTimeout slightly, but I think it'll
be ok.


svn:r9350

ChangeLog

@@ -20,6 +20,9 @@ Changes in version 0.1.2.7-alpha - 2007-??-??
       indefinitely, if they came from nontrusted authorities.  Now we
       discard them if they are more than 10 days old.
     - Fix crash with "tor --list-fingerprint" (reported by seeess).
+    - Expire socks connections if they spend too long waiting for the
+      handshake to finish. Previously we would let them sit around for
+      days, if the connecting application didn't close them either.
 
 
 Changes in version 0.1.2.6-alpha - 2007-01-09

doc/tor.1.in

@@ -461,7 +461,8 @@ The policies have the same form as exit policies below.
 .LP
 .TP
 \fBSocksTimeout \fR\fINUM\fP
-Let a socks connection wait NUM seconds unattached before we fail it.
+Let a socks connection wait NUM seconds handshaking, and NUM seconds
+unattached waiting for an appropriate circuit, before we fail it.
 (Default: 2 minutes.)
 .LP
 .TP

src/or/circuituse.c

@@ -532,7 +532,7 @@ circuit_detach_stream(circuit_t *circ, edge_connection_t *conn)
 }
 
 /** Find each circuit that has been unused for too long, or dirty
- * for too long and has no streax=ms on it: mark it for close.
+ * for too long and has no streams on it: mark it for close.
  */
 static void
 circuit_expire_old_circuits(time_t now)
@@ -1180,21 +1180,12 @@ connection_ap_handshake_attach_circuit(edge_connection_t *conn)
 {
   int retval;
   int conn_age;
-  int severity;
 
   tor_assert(conn);
   tor_assert(conn->_base.state == AP_CONN_STATE_CIRCUIT_WAIT);
   tor_assert(conn->socks_request);
 
   conn_age = time(NULL) - conn->_base.timestamp_created;
-  severity = (!conn->_base.addr && !conn->_base.port) ? LOG_INFO : LOG_NOTICE;
-  if (conn_age > get_options()->SocksTimeout) {
-    log_fn(severity, LD_APP,
-           "Tried for %d seconds to get a connection to %s:%d. Giving up.",
-           conn_age, safe_str(conn->socks_request->address),
-           conn->socks_request->port);
-    return -1;
-  }
 
   if (!connection_edge_is_rendezvous_stream(conn)) { /* we're a general conn */
     origin_circuit_t *circ=NULL;

src/or/connection_edge.c

@@ -353,6 +353,7 @@ connection_ap_expire_beginning(void)
   or_options_t *options = get_options();
   int severity;
   int cutoff;
+  int seconds_idle;
 
   get_connection_array(&carray, &n);
 
@@ -363,19 +364,28 @@ connection_ap_expire_beginning(void)
     /* if it's an internal bridge connection, don't yell its status. */
     severity = (!conn->_base.addr && !conn->_base.port)
       ? LOG_INFO : LOG_NOTICE;
-    if (conn->_base.state == AP_CONN_STATE_CONTROLLER_WAIT) {
+    seconds_idle = now - conn->_base.timestamp_lastread;
-      if (now - conn->_base.timestamp_lastread >= options->SocksTimeout) {
+
-        log_fn(severity, LD_APP, "Closing unattached stream.");
+    if (AP_CONN_STATE_IS_UNATTACHED(conn->_base.state)) {
+      if (seconds_idle >= options->SocksTimeout) {
+        if (conn->_base.state == AP_CONN_STATE_CIRCUIT_WAIT) {
+          log_fn(severity, LD_APP,
+            "Tried for %d seconds to get a connection to %s:%d. Giving up.",
+            seconds_idle, safe_str(conn->socks_request->address),
+            conn->socks_request->port);
+        } else {
+          log_fn(severity, LD_APP, "Closing unattached stream.");
+        }
         connection_mark_unattached_ap(conn, END_STREAM_REASON_TIMEOUT);
       }
       continue;
     }
 
-    if (conn->_base.state != AP_CONN_STATE_RESOLVE_WAIT &&
+    if (conn->_base.state == AP_CONN_STATE_OPEN)
-        conn->_base.state != AP_CONN_STATE_CONNECT_WAIT)
       continue;
+
     cutoff = compute_socks_timeout(conn);
-    if (now - conn->_base.timestamp_lastread < cutoff)
+    if (seconds_idle < cutoff)
       continue;
     circ = circuit_get_by_edge_conn(conn);
     if (!circ) { /* it's vanished? */
@@ -385,11 +395,11 @@ connection_ap_expire_beginning(void)
       continue;
     }
     if (circ->purpose == CIRCUIT_PURPOSE_C_REND_JOINED) {
-      if (now - conn->_base.timestamp_lastread > options->SocksTimeout) {
+      if (seconds_idle > options->SocksTimeout) {
         log_fn(severity, LD_REND,
                "Rend stream is %d seconds late. Giving up on address"
                " '%s.onion'.",
-               (int)(now - conn->_base.timestamp_lastread),
+               seconds_idle,
                safe_str(conn->socks_request->address));
         connection_edge_end(conn, END_STREAM_REASON_TIMEOUT,
                             conn->cpath_layer);
@@ -403,8 +413,7 @@ connection_ap_expire_beginning(void)
     log_fn(cutoff < 15 ? LOG_INFO : severity, LD_APP,
            "We tried for %d seconds to connect to '%s' using exit '%s'."
            " Retrying on a new circuit.",
-           (int)(now - conn->_base.timestamp_lastread),
+           seconds_idle, safe_str(conn->socks_request->address),
-           safe_str(conn->socks_request->address),
            nickname ? nickname : "*unnamed*");
     /* send an end down the circuit */
     connection_edge_end(conn, END_STREAM_REASON_TIMEOUT, conn->cpath_layer);

src/or/or.h

@@ -294,7 +294,7 @@ typedef enum {
 #define AP_CONN_STATE_CIRCUIT_WAIT 8
 /** State for a SOCKS connection: sent BEGIN, waiting for CONNECTED. */
 #define AP_CONN_STATE_CONNECT_WAIT 9
-/** State for a SOCKS connection: send RESOLVE, waiting for RESOLVED. */
+/** State for a SOCKS connection: sent RESOLVE, waiting for RESOLVED. */
 #define AP_CONN_STATE_RESOLVE_WAIT 10
 /** State for a SOCKS connection: ready to send and receive. */
 #define AP_CONN_STATE_OPEN 11
@@ -303,6 +303,9 @@ typedef enum {
 #define AP_CONN_STATE_NATD_WAIT 12
 #define _AP_CONN_STATE_MAX 12
 
+#define AP_CONN_STATE_IS_UNATTACHED(s) \
+  ((s) <= AP_CONN_STATE_CIRCUIT_WAIT || (s) == AP_CONN_STATE_NATD_WAIT)
+
 #define _DIR_CONN_STATE_MIN 1
 /** State for connection to directory server: waiting for connect(). */
 #define DIR_CONN_STATE_CONNECTING 1
@@ -501,8 +504,8 @@ typedef enum {
 #define END_STREAM_REASON_CANT_FETCH_ORIG_DEST 260
 #define END_STREAM_REASON_INVALID_NATD_DEST 261
 
-/* OR this with the argument to control_event_stream_status to indicate that
+/** Bitwise-or this with the argument to control_event_stream_status
- * the reason came from an END cell. */
+ * to indicate that the reason came from an END cell. */
 #define END_STREAM_REASON_FLAG_REMOTE     512
 
 #define RESOLVED_TYPE_HOSTNAME 0