leave the socks handshake on the inbuf until it's complete

this paves the way for supporting socks5 and other handshakes
it also removes those pesky AP-only variables from connection_t

also hacked a fix for a bug where some streams weren't ending properly --
maybe because marked connections weren't flushing properly?


svn:r472

8 files changed, 149 insertions, 150 deletions

diff --git a/src/or/buffers.c b/src/or/buffers.c
index eba43e30ec..ad00234c49 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -126,8 +126,8 @@ int flush_buf(int s, char **buf, int *buflen, int *buf_flushlen, int *buf_datale
     *buf_datalen -= write_result;
     *buf_flushlen -= write_result;
     memmove(*buf, *buf+write_result, *buf_datalen);
-//    log_fn(LOG_DEBUG,"flushed %d bytes, %d ready to flush, %d remain.",
-//       write_result,*buf_flushlen,*buf_datalen);
+    log_fn(LOG_DEBUG,"%d: flushed %d bytes, %d ready to flush, %d remain.",
+      s,write_result,*buf_flushlen,*buf_datalen);
     return *buf_flushlen;
     /* XXX USE_TLS should change to return write_result like any sane function would */
   }
@@ -257,6 +257,86 @@ int fetch_from_buf_http(char *buf, int *buf_datalen,
   return 1;
 }
 
+/* There is a (possibly incomplete) socks handshake on *buf, of the
+ * forms
+ *   socks4: "socksheader || username\0".
+ *   socks4a: "socksheader || username\0 || destaddr\0".
+ * If it's a complete and valid handshake, and destaddr fits in addr_out,
+ *   then pull the handshake off the buf, assign to addr_out and port_out,
+ *   and return 1.
+ * If it's invalid or too big, return -1.
+ * Else it's not all there yet, change nothing return 0.
+ */
+int fetch_from_buf_socks(char *buf, int *buf_datalen,
+                         char *addr_out, int max_addrlen,
+                         uint16_t *port_out) {
+  socks4_t *socks4_info;
+  char tmpbuf[512];
+  uint16_t port;
+  enum {socks4, socks4a } socks_prot = socks4a;
+  char *next, *startaddr;
+
+  if(*buf_datalen < sizeof(socks4_t)) /* basic info available? */
+    return 0; /* not yet */
+
+  socks4_info = (socks4_t *)buf;
+
+  if(socks4_info->version != 4) {
+    log_fn(LOG_NOTICE,"Unrecognized version %d.",socks4_info->version);
+    return -1;
+  }
+
+  if(socks4_info->command != 1) { /* not a connect? we don't support it. */
+    log_fn(LOG_NOTICE,"command %d not '1'.",socks4_info->command);
+    return -1;
+  }
+
+  port = ntohs(*(uint16_t*)&socks4_info->destport);
+  if(!port) {
+    log_fn(LOG_NOTICE,"Port is zero.");
+    return -1;
+  }
+
+  if(socks4_info->destip[0] || socks4_info->destip[1] ||
+     socks4_info->destip[2] || !socks4_info->destip[3]) { /* not 0.0.0.x */
+    log_fn(LOG_NOTICE,"destip not in form 0.0.0.x.");
+    sprintf(tmpbuf, "%d.%d.%d.%d", socks4_info->destip[0],
+      socks4_info->destip[1], socks4_info->destip[2], socks4_info->destip[3]);
+    if(max_addrlen <= strlen(tmpbuf)) {
+      log_fn(LOG_DEBUG,"socks4-addr too long.");
+      return -1;
+    }
+    log_fn(LOG_DEBUG,"Successfully read destip (%s)", tmpbuf);
+    socks_prot = socks4;
+  }
+
+  next = memchr(buf+sizeof(socks4_t), 0, *buf_datalen);
+  if(!next) {
+    log_fn(LOG_DEBUG,"Username not here yet.");
+    return 0;
+  }
+
+  startaddr = next+1;
+  if(socks_prot == socks4a) {
+    next = memchr(startaddr, 0, buf+*buf_datalen-startaddr);
+    if(!next) {
+      log_fn(LOG_DEBUG,"Destaddr not here yet.");
+      return 0;
+    }
+    if(max_addrlen <= next-startaddr) {
+      log_fn(LOG_DEBUG,"Destaddr not here yet.");
+      return -1;
+    }
+  }
+  log_fn(LOG_DEBUG,"Everything is here. Success.");
+  *port_out = port; 
+  strcpy(addr_out, socks_prot == socks4 ? tmpbuf : startaddr);
+  *buf_datalen -= (next-buf+1); /* next points to the final \0 on inbuf */
+  memmove(buf, next+1, *buf_datalen);
+//  log_fn(LOG_DEBUG,"buf_datalen is now %d:'%s'",*buf_datalen,buf);
+  return 1;
+}
+
 int find_on_inbuf(char *string, int string_len,
                   char *buf, int buf_datalen) {
   /* find first instance of needle 'string' on haystack 'buf'. return how
diff --git a/src/or/circuit.c b/src/or/circuit.c
index 50d0591345..98953738bf 100644
--- a/src/or/circuit.c
+++ b/src/or/circuit.c
@@ -807,8 +807,9 @@ int circuit_send_next_onion_skin(circuit_t *circ) {
   return 0;
 }
 
-/* take the 'extend' cell, pull out addr/port plus the onion skin. Connect
- * to the next hop, and pass it the onion skin in a create cell.
+/* take the 'extend' cell, pull out addr/port plus the onion skin. Make
+ * sure we're connected to the next hop, and pass it the onion skin in
+ * a create cell.
  */
 int circuit_extend(cell_t *cell, circuit_t *circ) {
   connection_t *n_conn;
diff --git a/src/or/connection.c b/src/or/connection.c
index 5dead3f567..64355e1aad 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -98,8 +98,6 @@ void connection_free(connection_t *conn) {
   buf_free(conn->outbuf);
   if(conn->address)
     free(conn->address);
-  if(conn->dest_addr)
-    free(conn->dest_addr);
 
   if(connection_speaks_cells(conn)) {
     directory_set_dirty();
@@ -527,13 +525,6 @@ int connection_fetch_from_buf(char *string, int len, connection_t *conn) {
   return fetch_from_buf(string, len, &conn->inbuf, &conn->inbuflen, &conn->inbuf_datalen);
 }
 
-int connection_fetch_from_buf_http(connection_t *conn,
-                                   char *headers_out, int max_headerlen,
-                                   char *body_out, int max_bodylen) {
-  return fetch_from_buf_http(conn->inbuf,&conn->inbuf_datalen,
-                             headers_out, max_headerlen, body_out, max_bodylen);
-}
-
 int connection_find_on_inbuf(char *string, int len, connection_t *conn) {
   return find_on_inbuf(string, len, conn->inbuf, conn->inbuf_datalen);
 }
@@ -782,13 +773,6 @@ void assert_connection_ok(connection_t *conn, time_t now)
     /* XXX unchecked, package window, deliver window. */
   }
 
-  if (conn->type != CONN_TYPE_AP) {
-    assert(!conn->socks_version);
-    assert(!conn->read_username);
-    assert(!conn->dest_addr);
-    assert(!conn->dest_port);
-  }
-
   switch(conn->type) 
     {
     case CONN_TYPE_OR_LISTENER:
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index cb53b8efce..1657e645a8 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -7,7 +7,8 @@
 extern or_options_t options; /* command-line and config-file options */
 
 static int connection_ap_handshake_process_socks(connection_t *conn);
-static int connection_ap_handshake_send_begin(connection_t *ap_conn, circuit_t *circ);
+static int connection_ap_handshake_send_begin(connection_t *ap_conn, circuit_t *circ,
+                                              char *destaddr, uint16_t destport);
 static int connection_ap_handshake_socks_reply(connection_t *conn, char result);
 
 static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ);
@@ -17,16 +18,6 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ);
 #define SOCKS4_REQUEST_IDENT_FAILED     92
 #define SOCKS4_REQUEST_IDENT_CONFLICT   93
 
-/* structure of a socks client operation */
-typedef struct {
-   unsigned char version;     /* socks version number */
-   unsigned char command;     /* command code */
-   unsigned char destport[2]; /* destination port, network order */
-   unsigned char destip[4];   /* destination address */
-   /* userid follows, terminated by a NULL */
-   /* dest host follows, terminated by a NULL */
-} socks4_t;
-
 int connection_edge_process_inbuf(connection_t *conn) {
 
   assert(conn);
@@ -441,86 +432,24 @@ int connection_consider_sending_sendme(connection_t *conn, int edge_type) {
 }
 
 static int connection_ap_handshake_process_socks(connection_t *conn) {
-  socks4_t socks4_info; 
   circuit_t *circ;
-  char tmpbuf[512];
-  int amt;
+  char destaddr[200];
+  uint16_t destport;
 
   assert(conn);
 
   log_fn(LOG_DEBUG,"entered.");
 
-  if(!conn->socks_version) { /* try to pull it in */
-
-    if(conn->inbuf_datalen < sizeof(socks4_t)) /* basic info available? */
-      return 0; /* not yet */
-
-    connection_fetch_from_buf((char *)&socks4_info,sizeof(socks4_t),conn);
-
-    log_fn(LOG_DEBUG,"Successfully read socks info.");
-
-    if(socks4_info.version != 4) {
-      log_fn(LOG_NOTICE,"Unrecognized version %d.",socks4_info.version);
-      connection_ap_handshake_socks_reply(conn, SOCKS4_REQUEST_REJECT);
-      return -1;
-    }
-    conn->socks_version = socks4_info.version;
-
-    if(socks4_info.command != 1) { /* not a connect? we don't support it. */
-      log_fn(LOG_NOTICE,"command %d not '1'.",socks4_info.command);
-      connection_ap_handshake_socks_reply(conn, SOCKS4_REQUEST_REJECT);
-      return -1;
-    }
-
-    conn->dest_port = ntohs(*(uint16_t*)&socks4_info.destport);
-    if(!conn->dest_port) {
-      log_fn(LOG_NOTICE,"Port is zero.");
-      connection_ap_handshake_socks_reply(conn, SOCKS4_REQUEST_REJECT);
-      return -1;
-    }
-    log_fn(LOG_NOTICE,"Dest port is %d.",conn->dest_port);
-
-    if(socks4_info.destip[0] || 
-       socks4_info.destip[1] ||
-       socks4_info.destip[2] ||
-       !socks4_info.destip[3]) { /* not 0.0.0.x */
-      log_fn(LOG_NOTICE,"destip not in form 0.0.0.x.");
-      sprintf(tmpbuf, "%d.%d.%d.%d", socks4_info.destip[0],
-        socks4_info.destip[1], socks4_info.destip[2], socks4_info.destip[3]);
-      conn->dest_addr = strdup(tmpbuf);
-      log_fn(LOG_DEBUG,"Successfully read destip (%s)", conn->dest_addr);
-    }
-
-  }
-
-  if(!conn->read_username) { /* the socks spec says we've got to read stuff until we get a null */
-    amt = connection_find_on_inbuf("\0", 1, conn);
-    if(amt < 0) /* not there yet */
-      return 0;
-    if(amt > 500) {
-      log_fn(LOG_NOTICE,"username too long.");    
+  switch(fetch_from_buf_socks(conn->inbuf,&conn->inbuf_datalen,
+                              destaddr, sizeof(destaddr), &destport)) {
+    case -1:
+      log_fn(LOG_DEBUG,"Fetching socks handshake failed. Closing.");
       connection_ap_handshake_socks_reply(conn, SOCKS4_REQUEST_REJECT);
       return -1;
-    }
-    connection_fetch_from_buf(tmpbuf,amt,conn);
-    conn->read_username = 1;
-    log_fn(LOG_DEBUG,"Successfully read username.");
-  }
-
-  if(!conn->dest_addr) { /* no dest_addr found yet */
-    amt = connection_find_on_inbuf("\0", 1, conn);
-    if(amt < 0) /* not there yet */
+    case 0:
+      log_fn(LOG_DEBUG,"Fetching socks handshake, not all here yet. Ignoring.");
       return 0;
-    if(amt > 500) {
-      log_fn(LOG_NOTICE,"dest_addr too long.");    
-      connection_ap_handshake_socks_reply(conn, SOCKS4_REQUEST_REJECT);
-      return -1;
-    }
-    connection_fetch_from_buf(tmpbuf,amt,conn);
-
-    conn->dest_addr = strdup(tmpbuf);
-    log_fn(LOG_NOTICE,"successfully read dest addr '%s'",
-      conn->dest_addr);
+    /* case 1, fall through */
   }
 
   /* find the circuit that we should use, if there is one. */
@@ -542,7 +471,7 @@ static int connection_ap_handshake_process_socks(connection_t *conn) {
   assert(circ->cpath->prev->state == CPATH_STATE_OPEN);
   conn->cpath_layer = circ->cpath->prev;
 
-  if(connection_ap_handshake_send_begin(conn, circ) < 0) {
+  if(connection_ap_handshake_send_begin(conn, circ, destaddr, destport) < 0) {
     circuit_close(circ);
     return -1;
   }
@@ -550,11 +479,12 @@ static int connection_ap_handshake_process_socks(connection_t *conn) {
   return 0;
 }
 
-static int connection_ap_handshake_send_begin(connection_t *ap_conn, circuit_t *circ) {
+/* deliver the destaddr:destport in a relay cell */
+static int connection_ap_handshake_send_begin(connection_t *ap_conn, circuit_t *circ,
+                                              char *destaddr, uint16_t destport) {
   cell_t cell;
-
   memset(&cell, 0, sizeof(cell_t));
-  /* deliver the dest_addr in a relay cell */
+
   cell.command = CELL_RELAY;
   cell.aci = circ->n_aci;
   SET_CELL_RELAY_COMMAND(cell, RELAY_COMMAND_BEGIN);
@@ -566,7 +496,7 @@ static int connection_ap_handshake_send_begin(connection_t *ap_conn, circuit_t *
   memcpy(cell.payload+RELAY_HEADER_SIZE, ap_conn->stream_id, STREAM_ID_SIZE);
   cell.length = 
     snprintf(cell.payload+RELAY_HEADER_SIZE+STREAM_ID_SIZE, CELL_PAYLOAD_SIZE-RELAY_HEADER_SIZE-STREAM_ID_SIZE,
-             "%s:%d", ap_conn->dest_addr, ap_conn->dest_port) + 
+             "%s:%d", destaddr, destport) + 
     1 + STREAM_ID_SIZE + RELAY_HEADER_SIZE;
   log_fn(LOG_DEBUG,"Sending relay cell (id %d) to begin stream %d.", *(int *)(cell.payload+1),*(int *)ap_conn->stream_id);
   if(circuit_deliver_relay_cell(&cell, circ, CELL_DIRECTION_OUT, ap_conn->cpath_layer) < 0) {
diff --git a/src/or/directory.c b/src/or/directory.c
index ac70ba08dc..293da09ddd 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -141,7 +141,8 @@ int connection_dir_process_inbuf(connection_t *conn) {
     switch(conn->state) {
       case DIR_CONN_STATE_CLIENT_READING_GET:
         /* kill it, but first process the_directory and learn about new routers. */
-        switch(connection_fetch_from_buf_http(conn, NULL, 0, the_directory, MAX_DIR_SIZE)) {
+        switch(fetch_from_buf_http(conn->inbuf,&conn->inbuf_datalen,
+                                   NULL, 0, the_directory, MAX_DIR_SIZE)) {
           case -1: /* overflow */
             log_fn(LOG_DEBUG,"'get' response too large. Failing.");
             return -1;
@@ -191,7 +192,8 @@ static int directory_handle_command(connection_t *conn) {
 
   assert(conn && conn->type == CONN_TYPE_DIR);
 
-  switch(connection_fetch_from_buf_http(conn, headers, sizeof(headers), body, sizeof(body))) {
+  switch(fetch_from_buf_http(conn->inbuf,&conn->inbuf_datalen,
+                             headers, sizeof(headers), body, sizeof(body))) {
     case -1: /* overflow */
       log_fn(LOG_DEBUG,"input too large. Failing.");
       return -1;
diff --git a/src/or/main.c b/src/or/main.c
index 7d9be0f061..6fc3c1c5cc 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -301,9 +301,11 @@ static void check_conn_marked(int i) {
   assert(conn);
   if(conn->marked_for_close) {
     log_fn(LOG_DEBUG,"Cleaning up connection.");
-    if(conn->s >= 0) { /* might be an incomplete exit connection */
+    if(conn->s >= 0) { /* might be an incomplete edge connection */
       /* FIXME there's got to be a better way to check for this -- and make other checks? */
-      connection_flush_buf(conn); /* flush it first */
+      connection_handle_write(conn); /* flush it first */
+      if(connection_wants_to_flush(conn)) /* not done flushing */
+        log_fn(LOG_WARNING,"Conn (socket %d) still wants to flush. Losing %d bytes!",conn->s, conn->inbuf_datalen);
     }
     connection_remove(conn);
     connection_free(conn);
diff --git a/src/or/or.h b/src/or/or.h
index dbd27d0fa6..915c6deffd 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -215,6 +215,16 @@
 /* legal characters in a filename */
 #define CONFIG_LEGAL_FILENAME_CHARACTERS "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_/"
 
+/* structure of a socks client operation */
+typedef struct {
+   unsigned char version;     /* socks version number */
+   unsigned char command;     /* command code */
+   unsigned char destport[2]; /* destination port, network order */
+   unsigned char destip[4];   /* destination address */
+   /* userid follows, terminated by a NULL */
+   /* dest host follows, terminated by a NULL */
+} socks4_t;
+
 typedef uint16_t aci_t;
 
 /* cell definition */
@@ -293,12 +303,6 @@ struct connection_t {
 
   int done_sending; /* for half-open connections; not used currently */
   int done_receiving;
-
-/* Used only by AP connections: */
-  char socks_version; /* what socks version are they speaking at me? */
-  char read_username; /* have i read the username yet? */
-  char *dest_addr; /* what address and port are this stream's destination? */
-  uint16_t dest_port; /* host order */
 };
 
 typedef struct connection_t connection_t;
@@ -443,6 +447,9 @@ int fetch_from_buf(char *string, int string_len, char **buf, int *buflen, int *b
 int fetch_from_buf_http(char *buf, int *buf_datalen,
                         char *headers_out, int max_headerlen,
                         char *body_out, int max_bodylen);
+int fetch_from_buf_socks(char *buf, int *buf_datalen,
+                         char *addr_out, int max_addrlen,
+                         uint16_t *port_out);
 int find_on_inbuf(char *string, int string_len, char *buf, int buf_datalen);
 
 /********************************* circuit.c ***************************/
@@ -510,9 +517,6 @@ int connection_handle_read(connection_t *conn);
 int connection_read_to_buf(connection_t *conn);
 
 int connection_fetch_from_buf(char *string, int len, connection_t *conn);
-int connection_fetch_from_buf_http(connection_t *conn,
-                                   char *headers_out, int max_headerlen,
-                                   char *body_out, int max_bodylen);
 int connection_find_on_inbuf(char *string, int len, connection_t *conn);
 
 int connection_wants_to_flush(connection_t *conn);
diff --git a/src/or/routers.c b/src/or/routers.c
index ca8f567e3d..6341d335f1 100644
--- a/src/or/routers.c
+++ b/src/or/routers.c
@@ -42,39 +42,35 @@ router_resolve_directory(directory_t *dir);
 int learn_my_address(struct sockaddr_in *me) {
   /* local host information */
   char localhostname[512];
-  static struct hostent *localhost;
+  struct hostent *localhost;
+  static struct sockaddr_in answer;
   static int already_learned=0;
 
-  if(already_learned) {
-    memset(me,0,sizeof(struct sockaddr_in));
-    me->sin_family = AF_INET;
-    memcpy((void *)&me->sin_addr,(void *)localhost->h_addr,sizeof(struct in_addr));
-    me->sin_port = htons((uint16_t) options.ORPort);
-    return 0;
-  }
-
-  /* obtain local host information */
-  if(gethostname(localhostname,512) < 0) {
-    log_fn(LOG_ERR,"Error obtaining local hostname");
-    return -1;
-  }
-  log_fn(LOG_DEBUG,"localhostname is '%s'.",localhostname);
-  localhost = gethostbyname(localhostname);
-  if (!localhost) {
-    log_fn(LOG_ERR,"Error obtaining local host info.");
-    return -1;
+  if(!already_learned) {
+    /* obtain local host information */
+    if(gethostname(localhostname,512) < 0) {
+      log_fn(LOG_ERR,"Error obtaining local hostname");
+      return -1;
+    }
+    log_fn(LOG_DEBUG,"localhostname is '%s'.",localhostname);
+    localhost = gethostbyname(localhostname);
+    if (!localhost) {
+      log_fn(LOG_ERR,"Error obtaining local host info.");
+      return -1;
+    }
+    memset(&answer,0,sizeof(struct sockaddr_in));
+    answer.sin_family = AF_INET;
+    memcpy((void *)&answer.sin_addr,(void *)localhost->h_addr,sizeof(struct in_addr));
+    answer.sin_port = htons((uint16_t) options.ORPort);
+    log_fn(LOG_DEBUG,"chose address as '%s'.",inet_ntoa(answer.sin_addr));
+    if (!strncmp("127.",inet_ntoa(answer.sin_addr), 4) &&
+        strcasecmp(localhostname, "localhost")) {
+      /* We're a loopback IP but we're not called localhost.  Uh oh! */
+      log_fn(LOG_WARNING, "Got a loopback address: /etc/hosts may be wrong");
+    }
+    already_learned = 1;
   }
-  memset(me,0,sizeof(struct sockaddr_in));
-  me->sin_family = AF_INET;
-  memcpy((void *)&me->sin_addr,(void *)localhost->h_addr,sizeof(struct in_addr));
-  me->sin_port = htons((uint16_t) options.ORPort);
-  log_fn(LOG_DEBUG,"chose address as '%s'.",inet_ntoa(me->sin_addr));
-  if (!strncmp("127.",inet_ntoa(me->sin_addr), 4) &&
-      strcasecmp(localhostname, "localhost")) {
-    /* We're a loopback IP but we're not called localhost.  Uh oh! */
-    log_fn(LOG_WARNING, "Got a loopback address: /etc/hosts may be wrong");
-  }
-  already_learned=1;
+  memcpy(me,&answer,sizeof(struct sockaddr_in));
   return 0;
 }