Merge remote-tracking branch 'public/bug1666'

Conflicts:
	doc/spec/socks-extensions.txt
	src/or/buffers.c
	src/or/config.c
	src/or/connection_edge.c

7 files changed, 178 insertions, 68 deletions

diff --git a/src/or/buffers.c b/src/or/buffers.c
index 73127490c0..256b507729 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1486,6 +1486,25 @@ log_unsafe_socks_warning(int socks_protocol, const char *address,
  * actually significantly higher than the longest possible socks message. */
 #define MAX_SOCKS_MESSAGE_LEN 512
 
+/** Return a new socks_request_t. */
+socks_request_t *
+socks_request_new(void)
+{
+  return tor_malloc_zero(sizeof(socks_request_t));
+}
+
+/** Free all storage held in the socks_request_t <b>req</b>. */
+void
+socks_request_free(socks_request_t *req)
+{
+  if (!req)
+    return;
+  tor_free(req->username);
+  tor_free(req->password);
+  memset(req, 0xCC, sizeof(socks_request_t));
+  tor_free(req);
+}
+
 /** There is a (possibly incomplete) socks handshake on <b>buf</b>, of one
  * of the forms
  *  - socks4: "socksheader username\\0"
@@ -1536,9 +1555,8 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req,
     else if (n_drain > 0)
       buf_remove_from_front(buf, n_drain);
 
-  } while (res == 0 && buf->head &&
-           buf->datalen > buf->head->datalen &&
-           want_length < buf->head->datalen);
+  } while (res == 0 && buf->head && want_length < buf->datalen &&
+           buf->datalen >= 2);
 
   return res;
 }
@@ -1589,12 +1607,14 @@ fetch_from_evbuffer_socks(struct evbuffer *buf, socks_request_t *req,
    * will need more data than we currently have. */
 
   /* Loop while we have more data that we haven't given parse_socks() yet. */
-  while (evbuffer_get_length(buf) > datalen) {
+  do {
     int free_data = 0;
+    const size_t last_wanted = want_length;
     n_drain = 0;
     data = NULL;
     datalen = inspect_evbuffer(buf, &data, want_length, &free_data, NULL);
 
+    want_length = 0;
     res = parse_socks(data, datalen, req, log_sockstype,
                       safe_socks, &n_drain, &want_length);
 
@@ -1606,20 +1626,16 @@ fetch_from_evbuffer_socks(struct evbuffer *buf, socks_request_t *req,
     else if (n_drain > 0)
       evbuffer_drain(buf, n_drain);
 
-    if (res) /* If res is nonzero, parse_socks() made up its mind. */
-      return res;
-
-    /* If parse_socks says that we want less data than we actually tried to
-       give it, we've got some kind of weird situation; just exit the loop for
-       now.
-    */
-    if (want_length <= datalen)
+    if (res == 0 && n_drain == 0 && want_length <= last_wanted) {
+      /* If we drained nothing, and we didn't ask for more than last time,
+       * we're stuck in a loop. That's bad. It shouldn't be possible, but
+       * let's make sure. */
+      log_warn(LD_BUG, "We seem to be caught in a parse loop; breaking out");
       break;
-    /* Otherwise, it wants more data than we gave it.  If we can provide more
-     * data than we gave it, we'll try to do so in the next iteration of the
-     * loop. If we can't, the while loop will exit.  It's okay if it asked for
-     * more than we have total; maybe it doesn't really need so much. */
-  }
+    }
+
+    buflen = evbuffer_get_length(buf);
+  } while (res == 0 && want_length <= buflen && buflen >= 2);
 
   return res;
 }
@@ -1642,47 +1658,114 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
   tor_addr_t destaddr;
   uint32_t destip;
   uint8_t socksver;
-  enum {socks4, socks4a} socks4_prot = socks4a;
   char *next, *startaddr;
+  unsigned char usernamelen, passlen;
   struct in_addr in;
 
+  if (datalen < 2) {
+    /* We always need at least 2 bytes. */
+    *want_length_out = 2;
+    return 0;
+  }
+
+  if (req->socks_version == 5 && !req->got_auth) {
+    /* See if we have received authentication.  Strictly speaking, we should
+       also check whether we actually negotiated username/password
+       authentication.  But some broken clients will send us authentication
+       even if we negotiated SOCKS_NO_AUTH. */
+    if (*data == 1) { /* username/pass version 1 */
+      /* Format is: authversion [1 byte] == 1
+                    usernamelen [1 byte]
+                    username    [usernamelen bytes]
+                    passlen     [1 byte]
+                    password    [passlen bytes] */
+      usernamelen = (unsigned char)*(data + 1);
+      if (datalen < 2u + usernamelen + 1u) {
+        *want_length_out = 2u + usernamelen + 1u;
+        return 0;
+      }
+      passlen = (unsigned char)*(data + 2u + usernamelen);
+      if (datalen < 2u + usernamelen + 1u + passlen) {
+        *want_length_out = 2u + usernamelen + 1u + passlen;
+        return 0;
+      }
+      req->replylen = 2; /* 2 bytes of response */
+      req->reply[0] = 5;
+      req->reply[1] = 0; /* authentication successful */
+      log_debug(LD_APP,
+               "socks5: Accepted username/password without checking.");
+      if (usernamelen) {
+        req->username = tor_memdup(data+2u, usernamelen);
+        req->usernamelen = usernamelen;
+      }
+      if (passlen) {
+        req->password = tor_memdup(data+3u+usernamelen, passlen);
+        req->passwordlen = passlen;
+      }
+      *drain_out = 2u + usernamelen + 1u + passlen;
+      req->got_auth = 1;
+      *want_length_out = 7; /* Minimal socks5 sommand. */
+      return 0;
+    } else if (req->auth_type == SOCKS_USER_PASS) {
+      /* unknown version byte */
+      log_warn(LD_APP, "Socks5 username/password version %d not recognized; "
+               "rejecting.", (int)*data);
+      return -1;
+    }
+  }
+
   socksver = *data;
 
   switch (socksver) { /* which version of socks? */
-
     case 5: /* socks5 */
 
       if (req->socks_version != 5) { /* we need to negotiate a method */
         unsigned char nummethods = (unsigned char)*(data+1);
+        int r=0;
         tor_assert(!req->socks_version);
         if (datalen < 2u+nummethods) {
           *want_length_out = 2u+nummethods;
           return 0;
         }
-        if (!nummethods || !memchr(data+2, 0, nummethods)) {
+        if (!nummethods)
+          return -1;
+        req->replylen = 2; /* 2 bytes of response */
+        req->reply[0] = 5; /* socks5 reply */
+        if (memchr(data+2, SOCKS_NO_AUTH, nummethods)) {
+          req->reply[1] = SOCKS_NO_AUTH; /* tell client to use "none" auth
+                                            method */
+          req->socks_version = 5; /* remember we've already negotiated auth */
+          log_debug(LD_APP,"socks5: accepted method 0 (no authentication)");
+          r=0;
+        } else if (memchr(data+2, SOCKS_USER_PASS, nummethods)) {
+          req->auth_type = SOCKS_USER_PASS;
+          req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass"
+                                              auth method */
+          req->socks_version = 5; /* remember we've already negotiated auth */
+          log_debug(LD_APP,"socks5: accepted method 2 (username/password)");
+          r=0;
+        } else {
           log_warn(LD_APP,
-                   "socks5: offered methods don't include 'no auth'. "
-                   "Rejecting.");
-          req->replylen = 2; /* 2 bytes of response */
-          req->reply[0] = 5;
+                    "socks5: offered methods don't include 'no auth' or "
+                    "username/password. Rejecting.");
           req->reply[1] = '\xFF'; /* reject all methods */
-          return -1;
+          r=-1;
         }
-        /* remove packet from buf. also remove any other extraneous
-         * bytes, to support broken socks clients. */
-        *drain_out = -1;
+        /* Remove packet from buf. Some SOCKS clients will have sent extra
+         * junk at this point; let's hope it's an authentication message. */
+        *drain_out = 2u + nummethods;
 
-        req->replylen = 2; /* 2 bytes of response */
-        req->reply[0] = 5; /* socks5 reply */
-        req->reply[1] = 0; /* tell client to use "none" auth method */
-        req->socks_version = 5; /* remember we've already negotiated auth */
-        log_debug(LD_APP,"socks5: accepted method 0");
-        return 0;
+        return r;
+      }
+      if (req->auth_type != SOCKS_NO_AUTH && !req->got_auth) {
+        log_warn(LD_APP,
+                 "socks5: negotiated authentication, but none provided");
+        return -1;
       }
       /* we know the method; read in the request */
       log_debug(LD_APP,"socks5: checking request");
-      if (datalen < 8) {/* basic info plus >=2 for addr plus 2 for port */
-        *want_length_out = 8;
+      if (datalen < 7) {/* basic info plus >=1 for addr plus 2 for port */
+        *want_length_out = 7;
         return 0; /* not yet */
       }
       req->command = (unsigned char) *(data+1);
@@ -1771,9 +1854,11 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
           return -1;
       }
       tor_assert(0);
-    case 4: /* socks4 */
-      /* http://archive.socks.permeo.com/protocol/socks4.protocol */
-      /* http://archive.socks.permeo.com/protocol/socks4a.protocol */
+    case 4: { /* socks4 */
+      enum {socks4, socks4a} socks4_prot = socks4a;
+      const char *authstart, *authend;
+      /* http://ss5.sourceforge.net/socks4.protocol.txt */
+      /* http://ss5.sourceforge.net/socks4A.protocol.txt */
 
       req->socks_version = 4;
       if (datalen < SOCKS4_NETWORK_LEN) {/* basic info available? */
@@ -1812,7 +1897,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
         socks4_prot = socks4;
       }
 
-      next = memchr(data+SOCKS4_NETWORK_LEN, 0,
+      authstart = data + SOCKS4_NETWORK_LEN;
+      next = memchr(authstart, 0,
                     datalen-SOCKS4_NETWORK_LEN);
       if (!next) {
         if (datalen >= 1024) {
@@ -1820,9 +1906,10 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
           return -1;
         }
         log_debug(LD_APP,"socks4: Username not here yet.");
-        *want_length_out = datalen+1024; /* ???? */
+        *want_length_out = datalen+1024; /* More than we need, but safe */
         return 0;
       }
+      authend = next;
       tor_assert(next < data+datalen);
 
       startaddr = NULL;
@@ -1847,7 +1934,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
             return -1;
           }
           log_debug(LD_APP,"socks4: Destaddr not all here yet.");
-          *want_length_out = datalen + 1024;
+          *want_length_out = datalen + 1024; /* More than we need, but safe */
           return 0;
         }
         if (MAX_SOCKS_ADDR_LEN <= next-startaddr) {
@@ -1872,15 +1959,20 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
                  req->port, escaped(req->address));
         return -1;
       }
+      if (authend != authstart) {
+        req->got_auth = 1;
+        req->usernamelen = authend - authstart;
+        req->username = tor_memdup(authstart, authend - authstart);
+      }
       /* next points to the final \0 on inbuf */
       *drain_out = next - data + 1;
       return 1;
-
+    }
     case 'G': /* get */
     case 'H': /* head */
     case 'P': /* put/post */
     case 'C': /* connect */
-      strlcpy(req->reply,
+      strlcpy((char*)req->reply,
 "HTTP/1.0 501 Tor is not an HTTP Proxy\r\n"
 "Content-Type: text/html; charset=iso-8859-1\r\n\r\n"
 "<html>\n"
@@ -1906,7 +1998,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
 "</body>\n"
 "</html>\n"
              , MAX_SOCKS_REPLY_LEN);
-      req->replylen = strlen(req->reply)+1;
+      req->replylen = strlen((char*)req->reply)+1;
       /* fall through */
     default: /* version is not socks4 or socks5 */
       log_warn(LD_APP,
diff --git a/src/or/buffers.h b/src/or/buffers.h
index 613001066c..b0161b9c2c 100644
--- a/src/or/buffers.h
+++ b/src/or/buffers.h
@@ -41,6 +41,8 @@ int fetch_from_buf_http(buf_t *buf,
                         char **headers_out, size_t max_headerlen,
                         char **body_out, size_t *body_used, size_t max_bodylen,
                         int force_complete);
+socks_request_t *socks_request_new(void);
+void socks_request_free(socks_request_t *req);
 int fetch_from_buf_socks(buf_t *buf, socks_request_t *req,
                          int log_sockstype, int safe_socks);
 int fetch_from_buf_socks_client(buf_t *buf, int state, char **reason);
diff --git a/src/or/config.c b/src/or/config.c
index 4a9d2c9fbd..cdf0122f75 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -571,7 +571,6 @@ static int options_transition_affects_workers(
 static int options_transition_affects_descriptor(
       const or_options_t *old_options, const or_options_t *new_options);
 static int check_nickname_list(const char *lst, const char *name, char **msg);
-static void config_register_addressmaps(const or_options_t *options);
 
 static int parse_bridge_line(const char *line, int validate_only);
 static int parse_client_transport_line(const char *line, int validate_only);
@@ -4430,7 +4429,7 @@ get_torrc_fname(void)
 /** Adjust the address map based on the MapAddress elements in the
  * configuration <b>options</b>
  */
-static void
+void
 config_register_addressmaps(const or_options_t *options)
 {
   smartlist_t *elts;
diff --git a/src/or/config.h b/src/or/config.h
index bc71191439..8a06f4443a 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -79,5 +79,7 @@ uint32_t get_effective_bwburst(const or_options_t *options);
 or_options_t *options_new(void);
 #endif
 
+void config_register_addressmaps(const or_options_t *options);
+
 #endif
 
diff --git a/src/or/connection.c b/src/or/connection.c
index d82a66ed7d..8b9fb126d3 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -248,7 +248,7 @@ edge_connection_new(int type, int socket_family)
   tor_assert(type == CONN_TYPE_EXIT || type == CONN_TYPE_AP);
   connection_init(time(NULL), TO_CONN(edge_conn), type, socket_family);
   if (type == CONN_TYPE_AP)
-    edge_conn->socks_request = tor_malloc_zero(sizeof(socks_request_t));
+    edge_conn->socks_request = socks_request_new();
   return edge_conn;
 }
 
@@ -442,10 +442,8 @@ _connection_free(connection_t *conn)
   if (CONN_IS_EDGE(conn)) {
     edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
     tor_free(edge_conn->chosen_exit_name);
-    if (edge_conn->socks_request) {
-      memset(edge_conn->socks_request, 0xcc, sizeof(socks_request_t));
-      tor_free(edge_conn->socks_request);
-    }
+    if (edge_conn->socks_request)
+      socks_request_free(edge_conn->socks_request);
 
     rend_data_free(edge_conn->rend_data);
   }
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 7516716fb1..c19e8b74d6 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2145,6 +2145,7 @@ connection_ap_handshake_process_socks(edge_connection_t *conn)
   socks_request_t *socks;
   int sockshere;
   const or_options_t *options = get_options();
+  int had_reply = 0;
 
   tor_assert(conn);
   tor_assert(conn->_base.type == CONN_TYPE_AP);
@@ -2162,22 +2163,18 @@ connection_ap_handshake_process_socks(edge_connection_t *conn)
     sockshere = fetch_from_buf_socks(conn->_base.inbuf, socks,
                                      options->TestSocks, options->SafeSocks);
   };
+
+  if (socks->replylen) {
+    had_reply = 1;
+    connection_write_to_buf(socks->reply, socks->replylen, TO_CONN(conn));
+    socks->replylen = 0;
+  }
+
   if (sockshere == 0) {
-    if (socks->replylen) {
-      connection_write_to_buf(socks->reply, socks->replylen, TO_CONN(conn));
-      /* zero it out so we can do another round of negotiation */
-      socks->replylen = 0;
-    } else {
-      log_debug(LD_APP,"socks handshake not all here yet.");
-    }
+    log_debug(LD_APP,"socks handshake not all here yet.");
     return 0;
   } else if (sockshere == -1) {
-    if (socks->replylen) { /* we should send reply back */
-      log_debug(LD_APP,"reply is already set for us. Using it.");
-      connection_ap_handshake_socks_reply(conn, socks->reply, socks->replylen,
-                                          END_STREAM_REASON_SOCKSPROTOCOL);
-
-    } else {
+    if (!had_reply) {
       log_warn(LD_APP,"Fetching socks handshake failed. Closing.");
       connection_ap_handshake_socks_reply(conn, NULL, 0,
                                           END_STREAM_REASON_SOCKSPROTOCOL);
diff --git a/src/or/or.h b/src/or/or.h
index 2f960cae17..7669efb665 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3182,6 +3182,8 @@ static INLINE void or_state_mark_dirty(or_state_t *state, time_t when)
 
 #define MAX_SOCKS_REPLY_LEN 1024
 #define MAX_SOCKS_ADDR_LEN 256
+#define SOCKS_NO_AUTH 0x00
+#define SOCKS_USER_PASS 0x02
 
 /** Please open a TCP connection to this addr:port. */
 #define SOCKS_COMMAND_CONNECT       0x01
@@ -3201,10 +3203,15 @@ struct socks_request_t {
   /** Which version of SOCKS did the client use? One of "0, 4, 5" -- where
    * 0 means that no socks handshake ever took place, and this is just a
    * stub connection (e.g. see connection_ap_make_link()). */
-  char socks_version;
-  int command; /**< What is this stream's goal? One from the above list. */
+  uint8_t socks_version;
+  /** If using socks5 authentication, which authentication type did we
+   * negotiate?  currently we support 0 (no authentication) and 2
+   * (username/password). */
+  uint8_t auth_type;
+  /** What is this stream's goal? One of the SOCKS_COMMAND_* values */
+  uint8_t command;
   size_t replylen; /**< Length of <b>reply</b>. */
-  char reply[MAX_SOCKS_REPLY_LEN]; /**< Write an entry into this string if
+  uint8_t reply[MAX_SOCKS_REPLY_LEN]; /**< Write an entry into this string if
                                     * we want to specify our own socks reply,
                                     * rather than using the default socks4 or
                                     * socks5 socks reply. We use this for the
@@ -3216,6 +3223,19 @@ struct socks_request_t {
   unsigned int has_finished : 1; /**< Has the SOCKS handshake finished? Used to
                               * make sure we send back a socks reply for
                               * every connection. */
+  unsigned int got_auth : 1; /**< Have we received any authentication data? */
+
+  /** Number of bytes in username; 0 if username is NULL */
+  uint8_t usernamelen;
+  /** Number of bytes in password; 0 if password is NULL */
+  uint8_t passwordlen;
+  /** The negotiated username value if any (for socks5), or the entire
+   * authentication string (for socks4).  This value is NOT nul-terminated;
+   * see usernamelen for its length. */
+  char *username;
+  /** The negotiated password value if any (for socks5). This value is NOT
+   * nul-terminated; see passwordlen for its length. */
+  char *password;
 };
 
 /********************************* circuitbuild.c **********************/