Browse Source

Per-SOCKSPort configuration for bug 8117 fix.

This might be necessary if the bug8117 fix confuses any applications.

Also add a changes file.
Nick Mathewson 11 years ago
parent
commit
fa3c237739
6 changed files with 58 additions and 3 deletions
  1. 13 0
      changes/bug8117
  2. 13 1
      doc/tor.1.txt
  3. 5 2
      src/or/buffers.c
  4. 9 0
      src/or/config.c
  5. 6 0
      src/or/connection.c
  6. 12 0
      src/or/or.h

+ 13 - 0
changes/bug8117

@@ -0,0 +1,13 @@
+  o Major bugfixes:
+
+    - Many SOCKS5 clients, when configured to offer a username/password,
+      offer both username/password authentication and "no authentication".
+      Tor had previously preferred no authentication, but this was
+      problematic when trying to make applications get proper stream
+      isolation with IsolateSOCKSAuth. Now, on any SOCKS port with
+      IsolateSOCKSAuth turned on (which is the default), Tor selects
+      username/password authentication if it's offered. If this confuses your
+      application, you can disable it on a per-SOCKSPort basis via
+      PreferSOCKSNoAuth. Fixes bug 8117; bugfix on 0.2.3.3-alpha.
+
+

+ 13 - 1
doc/tor.1.txt

@@ -860,7 +860,7 @@ The following options are useful only for clients (that is, if
     the same circuit. Currently, two addresses are "too close" if they lie in
     the same circuit. Currently, two addresses are "too close" if they lie in
     the same /16 range. (Default: 1)
     the same /16 range. (Default: 1)
 
 
-**SOCKSPort** \['address':]__port__|**auto** [_isolation flags_]::
+**SOCKSPort** \['address':]__port__|**auto** [_flags_] [_isolation flags_]::
     Open this port to listen for connections from SOCKS-speaking
     Open this port to listen for connections from SOCKS-speaking
     applications. Set this to 0 if you don't want to allow application
     applications. Set this to 0 if you don't want to allow application
     connections via SOCKS. Set it to "auto" to have Tor pick a port for
     connections via SOCKS. Set it to "auto" to have Tor pick a port for
@@ -894,6 +894,18 @@ The following options are useful only for clients (that is, if
         port with the same session group.  (By default, streams received
         port with the same session group.  (By default, streams received
         on different SOCKSPorts, TransPorts, etc are always isolated from one
         on different SOCKSPorts, TransPorts, etc are always isolated from one
         another. This option overrides that behavior.)
         another. This option overrides that behavior.)
++
+    Other recognized _flags_ for a SOCKSPort are:
+    **PreferSOCKSNoAuth**;;
+        Ordinarily, when an application offers both "username/password
+        authentication" and "no authentication" to Tor via SOCKS5, Tor
+        selects username/password authentication so that IsolateSOCKSAuth can
+        work.  This can confuse some applications, if they offer a
+        username/password combination then get confused when asked for
+        one. You can disable this behavior, so that Tor will select "No
+        authentication" when IsolateSOCKSAuth is disabled, or when this
+        option is set.
+
 
 
 **SOCKSListenAddress** __IP__[:__PORT__]::
 **SOCKSListenAddress** __IP__[:__PORT__]::
     Bind to this address to listen for connections from Socks-speaking
     Bind to this address to listen for connections from Socks-speaking

+ 5 - 2
src/or/buffers.c

@@ -1773,6 +1773,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
 
 
       if (req->socks_version != 5) { /* we need to negotiate a method */
       if (req->socks_version != 5) { /* we need to negotiate a method */
         unsigned char nummethods = (unsigned char)*(data+1);
         unsigned char nummethods = (unsigned char)*(data+1);
+        int have_user_pass, have_no_auth;
         int r=0;
         int r=0;
         tor_assert(!req->socks_version);
         tor_assert(!req->socks_version);
         if (datalen < 2u+nummethods) {
         if (datalen < 2u+nummethods) {
@@ -1783,14 +1784,16 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
           return -1;
           return -1;
         req->replylen = 2; /* 2 bytes of response */
         req->replylen = 2; /* 2 bytes of response */
         req->reply[0] = 5; /* socks5 reply */
         req->reply[0] = 5; /* socks5 reply */
-        if (memchr(data+2, SOCKS_USER_PASS, nummethods)) {
+        have_user_pass = (memchr(data+2, SOCKS_USER_PASS, nummethods) !=NULL);
+        have_no_auth   = (memchr(data+2, SOCKS_NO_AUTH,   nummethods) !=NULL);
+        if (have_user_pass && !(have_no_auth && req->socks_prefer_no_auth)) {
           req->auth_type = SOCKS_USER_PASS;
           req->auth_type = SOCKS_USER_PASS;
           req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass"
           req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass"
                                               auth method */
                                               auth method */
           req->socks_version = 5; /* remember we've already negotiated auth */
           req->socks_version = 5; /* remember we've already negotiated auth */
           log_debug(LD_APP,"socks5: accepted method 2 (username/password)");
           log_debug(LD_APP,"socks5: accepted method 2 (username/password)");
           r=0;
           r=0;
-        } else if (memchr(data+2, SOCKS_NO_AUTH, nummethods)) {
+        } else if (have_no_auth) {
           req->reply[1] = SOCKS_NO_AUTH; /* tell client to use "none" auth
           req->reply[1] = SOCKS_NO_AUTH; /* tell client to use "none" auth
                                             method */
                                             method */
           req->socks_version = 5; /* remember we've already negotiated auth */
           req->socks_version = 5; /* remember we've already negotiated auth */

+ 9 - 0
src/or/config.c

@@ -5787,6 +5787,7 @@ parse_port_config(smartlist_t *out,
     int port;
     int port;
     int sessiongroup = SESSION_GROUP_UNSET;
     int sessiongroup = SESSION_GROUP_UNSET;
     unsigned isolation = ISO_DEFAULT;
     unsigned isolation = ISO_DEFAULT;
+    int prefer_no_auth = 0;
 
 
     char *addrport;
     char *addrport;
     uint16_t ptmp=0;
     uint16_t ptmp=0;
@@ -5916,6 +5917,11 @@ parse_port_config(smartlist_t *out,
           no = 1;
           no = 1;
           elt += 2;
           elt += 2;
         }
         }
+        if (!strcasecmp(elt, "PreferSOCKSNoAuth")) {
+          prefer_no_auth = ! no;
+          continue;
+        }
+
         if (!strcasecmpend(elt, "s"))
         if (!strcasecmpend(elt, "s"))
           elt[strlen(elt)-1] = '\0'; /* kill plurals. */
           elt[strlen(elt)-1] = '\0'; /* kill plurals. */
 
 
@@ -5959,6 +5965,9 @@ parse_port_config(smartlist_t *out,
       cfg->all_addrs = all_addrs;
       cfg->all_addrs = all_addrs;
       cfg->ipv4_only = ipv4_only;
       cfg->ipv4_only = ipv4_only;
       cfg->ipv6_only = ipv6_only;
       cfg->ipv6_only = ipv6_only;
+      cfg->socks_prefer_no_auth = prefer_no_auth;
+      if (! (isolation & ISO_SOCKSAUTH))
+        cfg->socks_prefer_no_auth = 1;
 
 
       smartlist_add(out, cfg);
       smartlist_add(out, cfg);
     }
     }

+ 6 - 0
src/or/connection.c

@@ -1056,6 +1056,7 @@ connection_listener_new(const struct sockaddr *listensockaddr,
       lis_conn->session_group = global_next_session_group--;
       lis_conn->session_group = global_next_session_group--;
     }
     }
   }
   }
+  lis_conn->socks_prefer_no_auth = port_cfg->socks_prefer_no_auth;
 
 
   if (connection_add(conn) < 0) { /* no space, forget it */
   if (connection_add(conn) < 0) { /* no space, forget it */
     log_warn(LD_NET,"connection_add for listener failed. Giving up.");
     log_warn(LD_NET,"connection_add for listener failed. Giving up.");
@@ -1238,6 +1239,11 @@ connection_handle_listener_read(connection_t *conn, int new_type)
     newconn->port = port;
     newconn->port = port;
     newconn->address = tor_dup_addr(&addr);
     newconn->address = tor_dup_addr(&addr);
 
 
+    if (new_type == CONN_TYPE_AP) {
+      TO_ENTRY_CONN(newconn)->socks_request->socks_prefer_no_auth =
+        TO_LISTENER_CONN(conn)->socks_prefer_no_auth;
+    }
+
   } else if (conn->socket_family == AF_UNIX) {
   } else if (conn->socket_family == AF_UNIX) {
     /* For now only control ports can be Unix domain sockets
     /* For now only control ports can be Unix domain sockets
      * and listeners at the same time */
      * and listeners at the same time */

+ 12 - 0
src/or/or.h

@@ -1085,6 +1085,10 @@ typedef struct listener_connection_t {
   /** One or more ISO_ flags to describe how to isolate streams. */
   /** One or more ISO_ flags to describe how to isolate streams. */
   uint8_t isolation_flags;
   uint8_t isolation_flags;
   /**@}*/
   /**@}*/
+  /** For SOCKS connections only: If this is set, we will choose "no
+   * authentication" instead of "username/password" authentication if both
+   * are offered. Used as input to parse_socks. */
+  unsigned int socks_prefer_no_auth : 1;
 
 
 } listener_connection_t;
 } listener_connection_t;
 
 
@@ -2910,6 +2914,10 @@ typedef struct port_cfg_t {
   uint8_t isolation_flags; /**< Zero or more isolation flags */
   uint8_t isolation_flags; /**< Zero or more isolation flags */
   int session_group; /**< A session group, or -1 if this port is not in a
   int session_group; /**< A session group, or -1 if this port is not in a
                       * session group. */
                       * session group. */
+  /* Socks only: */
+  /** When both no-auth and user/pass are advertised by a SOCKS client, select
+   * no-auth. */
+  unsigned int socks_prefer_no_auth : 1;
 
 
   /* Server port types (or, dir) only: */
   /* Server port types (or, dir) only: */
   unsigned int no_advertise : 1;
   unsigned int no_advertise : 1;
@@ -3729,6 +3737,10 @@ struct socks_request_t {
                               * make sure we send back a socks reply for
                               * make sure we send back a socks reply for
                               * every connection. */
                               * every connection. */
   unsigned int got_auth : 1; /**< Have we received any authentication data? */
   unsigned int got_auth : 1; /**< Have we received any authentication data? */
+  /** If this is set, we will choose "no authentication" instead of
+   * "username/password" authentication if both are offered. Used as input to
+   * parse_socks. */
+  unsigned int socks_prefer_no_auth : 1;
 
 
   /** Number of bytes in username; 0 if username is NULL */
   /** Number of bytes in username; 0 if username is NULL */
   size_t usernamelen;
   size_t usernamelen;