Commit f85f5280 authored by Robert Hogan's avatar Robert Hogan
Browse files

bug1666 - Pass-through support for SOCKS5 authentication (2) 

Address Nick's comments:
- Refactor against changes in buffers.c
- Ensure we have negotiated a method before accepting
  authentication credentials
parent bf136b94

src/or/buffers.c

+18 −10
Original line number Diff line number Diff line
@@ -1642,14 +1642,23 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, 


    case 1: /* socks5: username/password authentication request */



      usernamelen = (unsigned char)*(buf->head->data + 1);

      if (buf->datalen < 2u + usernamelen)

      if (req->socks_version != 5) {

        log_warn(LD_APP,

                 "socks5: Received authentication attempt before "

                 "authentication negotiated. Rejecting.");

        return -1;

      }

      usernamelen = (unsigned char)*(data + 1);

      if (datalen < 2u + usernamelen) {

        *want_length_out = 2u+usernamelen;

        return 0;

      buf_pullup(buf, 2u + usernamelen + 1, 0);

      passlen = (unsigned char)*(buf->head->data + 2u + usernamelen);

      if (buf->datalen < 2u + usernamelen + 1u + passlen)

      }

      passlen = (unsigned char)*(data + 2u + usernamelen);

      if (datalen < 2u + usernamelen + 1u + passlen) {

        *want_length_out = 2u+usernamelen;

        return 0;

      if (buf->datalen > 2u + usernamelen + 1u + passlen) {

      }

      if (datalen > 2u + usernamelen + 1u + passlen) {

        log_warn(LD_APP,

                 "socks5: Malformed username/password. Rejecting.");

        return -1;
@@ -1657,9 +1666,9 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, 
      req->replylen = 2; /* 2 bytes of response */

      req->reply[0] = 5;

      req->reply[1] = 0; /* authentication successful */

      buf_clear(buf);

      log_debug(LD_APP,

               "socks5: Accepted username/password without checking.");

      *drain_out = 2u + usernamelen + 1u + passlen;

      return 0;



    case 5: /* socks5 */
@@ -1672,18 +1681,17 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, 
          *want_length_out = 2u+nummethods;

          return 0;

        }

        buf_pullup(buf, 2u+nummethods, 0);

        if (!nummethods)

          return -1;

        req->replylen = 2; /* 2 bytes of response */

        req->reply[0] = 5; /* socks5 reply */

        if (memchr(buf->head->data+2, SOCKS_NO_AUTH, nummethods)) {

        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(buf->head->data+2, SOCKS_USER_PASS,nummethods)) {

        }else if (memchr(data+2, SOCKS_USER_PASS,nummethods)) {

          req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass"

                                              auth method */

          req->socks_version = 5; /* remember we've already negotiated auth */

src/test/test.c

+31 −2
Original line number Diff line number Diff line
@@ -197,7 +197,7 @@ free_pregenerated_keys(void) 
  }

}



/** Helper: Perform supported SOCKS 5 commands */

/** Helper: Perform supported SOCKS 4 commands */

static void

test_buffers_socks4_unsupported_commands_helper(const char *cp, buf_t *buf,

                                                socks_request_t *socks)
@@ -214,7 +214,7 @@ done: 
  ;

}



/** Helper: Perform supported SOCKS 5 commands */

/** Helper: Perform supported SOCKS 4 commands */

static void

test_buffers_socks4_supported_commands_helper(const char *cp, buf_t *buf,

                                              socks_request_t *socks)
@@ -398,6 +398,26 @@ done: 
  ;

}



/** Helper: Perform SOCKS 5 authentication before method negotiated */

static void

test_buffers_socks5_auth_before_negotiation_helper(const char *cp, buf_t *buf,

                                        socks_request_t *socks)

{

  /* SOCKS 5 Send username/password */

  cp = "\x01\x02me\x02me";

  write_to_buf(cp, 7, buf);

  test_assert(fetch_from_buf_socks(buf, socks,

                                   get_options()->TestSocks,

                                   get_options()->SafeSocks) == -1);

  test_eq(0, socks->socks_version);

  test_eq(0, socks->replylen);

  test_eq(0, socks->reply[0]);

  test_eq(0, socks->reply[1]);



done:

  ;

}



/** Run unit tests for buffers.c */

static void

test_buffers(void)
@@ -570,6 +590,15 @@ test_buffers(void) 
  socks = tor_malloc_zero(sizeof(socks_request_t));;

  config_register_addressmaps(get_options());



  /* Sending auth credentials before we've negotiated a method */

  test_buffers_socks5_auth_before_negotiation_helper(cp, buf, socks);



  tor_free(socks);

  buf_free(buf);

  buf = NULL;

  buf = buf_new_with_capacity(256);

  socks = tor_malloc_zero(sizeof(socks_request_t));;



  /* A SOCKS 5 client that only supports authentication  */

  test_buffers_socks5_authenticate_helper(cp, buf, socks);

  test_buffers_socks5_supported_commands_helper(cp, buf, socks);