- tor-commits - lists.torproject.org

[tor/master] Refactor socks unit tests into a suite of their own
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit 5d43a1572094868e598541f9ce686546c910f071 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 29 17:30:55 2011 -0400 Refactor socks unit tests into a suite of their own --- src/test/test.c | 352 +++++++++++++++++++++++++++++++++---------------------- 1 files changed, 214 insertions(+), 138 deletions(-) diff --git a/src/test/test.c b/src/test/test.c index 38729cf..64916ea 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -197,14 +197,59 @@ free_pregenerated_keys(void) } } -/** Helper: Perform supported SOCKS 4 commands */ +typedef struct socks_test_data_t { + socks_request_t *req; + buf_t *buf; +} socks_test_data_t; + +static void * +socks_test_setup(const struct testcase_t *testcase) +{ + socks_test_data_t *data = tor_malloc(sizeof(socks_test_data_t)); + (void)testcase; + data->buf = buf_new_with_capacity(256); + data->req = socks_request_new(); + config_register_addressmaps(get_options()); + return data; +} +static int +socks_test_cleanup(const struct testcase_t *testcase, void *ptr) +{ + socks_test_data_t *data = ptr; + (void)testcase; + buf_free(data->buf); + socks_request_free(data->req); + tor_free(data); + return 1; +} + +const struct testcase_setup_t socks_setup = { + socks_test_setup, socks_test_cleanup +}; + +#define SOCKS_TEST_INIT() \ + socks_test_data_t *testdata = ptr; \ + buf_t *buf = testdata->buf; \ + socks_request_t *socks = testdata->req; +#define ADD_DATA(buf, s) \ + write_to_buf(s, sizeof(s)-1, buf) + +static void +socks_request_clear(socks_request_t *socks) +{ + tor_free(socks->username); + tor_free(socks->password); + memset(socks, 0, sizeof(socks_request_t)); +} + +/** Perform unsupported SOCKS 4 commands */ static void -test_buffers_socks4_unsupported_commands_helper(const char *cp, buf_t *buf, - socks_request_t *socks) +test_socks_4_unsupported_commands(void *ptr) { + SOCKS_TEST_INIT(); + /* SOCKS 4 Send BIND [02] to IP address 2.2.2.2:4369 */ - cp = "\x04\x02\x11\x11\x02\x02\x02\x02\x00"; - write_to_buf(cp, 9, buf); + ADD_DATA(buf, "\x04\x02\x11\x11\x02\x02\x02\x02\x00"); test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks) == -1); test_eq(4, socks->socks_version); @@ -214,138 +259,186 @@ test_buffers_socks4_unsupported_commands_helper(const char *cp, buf_t *buf, ; } -/** Helper: Perform supported SOCKS 4 commands */ +/** Perform supported SOCKS 4 commands */ static void -test_buffers_socks4_supported_commands_helper(const char *cp, buf_t *buf, - socks_request_t *socks) +test_socks_4_supported_commands(void *ptr) { - /* SOCKS 4 Send CONNECT [01] to IP address 2.2.2.2:4369 */ - cp = "\x04\x01\x11\x11\x02\x02\x02\x02\x00"; - write_to_buf(cp, 9, buf); + SOCKS_TEST_INIT(); + + test_eq(0, buf_datalen(buf)); + + /* SOCKS 4 Send CONNECT [01] to IP address 2.2.2.2:4370 */ + ADD_DATA(buf, "\x04\x01\x11\x12\x02\x02\x02\x03\x00"); test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks) == 1); test_eq(4, socks->socks_version); test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ - test_streq("2.2.2.2", socks->address); - test_eq(4369, socks->port); + test_eq(SOCKS_COMMAND_CONNECT, socks->command); + test_streq("2.2.2.3", socks->address); + test_eq(4370, socks->port); + test_assert(socks->got_auth == 0); + test_assert(! socks->username); + + test_eq(0, buf_datalen(buf)); + socks_request_clear(socks); /* SOCKS 4 Send CONNECT [01] to IP address 2.2.2.2:4369 with userid*/ - cp = "\x04\x01\x11\x11\x02\x02\x02\x02\x02me\x00"; - write_to_buf(cp, 12, buf); + ADD_DATA(buf, "\x04\x01\x11\x12\x02\x02\x02\x04me\x00"); test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks) == 1); test_eq(4, socks->socks_version); test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ - test_streq("2.2.2.2", socks->address); - test_eq(4369, socks->port); - - /* SOCKS 4a Send RESOLVE [F0] request for torproject.org:4369 */ - cp = "\x04\xF0\x01\x01\x00\x00\x00\x02\x02me\x00tor.org\x00"; - write_to_buf(cp, 20, buf); + test_eq(SOCKS_COMMAND_CONNECT, socks->command); + test_streq("2.2.2.4", socks->address); + test_eq(4370, socks->port); + test_assert(socks->got_auth == 1); + test_assert(socks->username); + test_eq(2, socks->usernamelen); + test_memeq("me", socks->username, 2); + + test_eq(0, buf_datalen(buf)); + socks_request_clear(socks); + + /* SOCKS 4a Send RESOLVE [F0] request for torproject.org */ + ADD_DATA(buf, "\x04\xF0\x01\x01\x00\x00\x00\x02me\x00torproject.org\x00"); test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, - get_options()->SafeSocks)); + get_options()->SafeSocks) == 1); test_eq(4, socks->socks_version); test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ - test_streq("tor.org", socks->address); + test_streq("torproject.org", socks->address); + + test_eq(0, buf_datalen(buf)); done: ; } -/** Helper: Perform supported SOCKS 5 commands */ +/** Perform unsupported SOCKS 5 commands */ static void -test_buffers_socks5_unsupported_commands_helper(const char *cp, buf_t *buf, - socks_request_t *socks) +test_socks_5_unsupported_commands(void *ptr) { + SOCKS_TEST_INIT(); + /* SOCKS 5 Send unsupported BIND [02] command */ - cp = "\x05\x02\x00\x01\x02\x02\x02\x02\x01\x01"; - write_to_buf(cp, 10, buf); - test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, - get_options()->SafeSocks) == -1); + ADD_DATA(buf, "\x05\x02\x00\x01"); + + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), 0); + test_eq(0, buf_datalen(buf)); test_eq(5, socks->socks_version); test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); - test_eq(0, socks->reply[1]); /* XXX: shouldn't tor reply 'command - not supported' [07]? */ + test_eq(0, socks->reply[1]); + ADD_DATA(buf, "\x05\x02\x00\x01\x02\x02\x02\x01\x01\x01"); + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), -1); + /* XXX: shouldn't tor reply 'command not supported' [07]? */ + + buf_clear(buf); + socks_request_clear(socks); /* SOCKS 5 Send unsupported UDP_ASSOCIATE [03] command */ - cp = "\x05\x03\x00\x01\x02\x02\x02\x02\x01\x01"; - write_to_buf(cp, 10, buf); - test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, - get_options()->SafeSocks) == -1); + ADD_DATA(buf, "\x05\x03\x00\x01\x02"); + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), 0); test_eq(5, socks->socks_version); test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); - test_eq(0, socks->reply[1]); /* XXX: shouldn't tor reply 'command - not supported' [07]? */ + test_eq(0, socks->reply[1]); + ADD_DATA(buf, "\x05\x03\x00\x01\x02\x02\x02\x01\x01\x01"); + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), -1); + /* XXX: shouldn't tor reply 'command not supported' [07]? */ done: ; } -/** Helper: Perform supported SOCKS 5 commands */ +/** Perform supported SOCKS 5 commands */ static void -test_buffers_socks5_supported_commands_helper(const char *cp, buf_t *buf, - socks_request_t *socks) +test_socks_5_supported_commands(void *ptr) { + SOCKS_TEST_INIT(); + /* SOCKS 5 Send CONNECT [01] to IP address 2.2.2.2:4369 */ - cp = "\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11"; - write_to_buf(cp, 10, buf); - test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, - get_options()->SafeSocks) == 1); + ADD_DATA(buf, "\x05\x01\x00"); + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), 0); test_eq(5, socks->socks_version); test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); + + ADD_DATA(buf, "\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11"); + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), 1); test_streq("2.2.2.2", socks->address); test_eq(4369, socks->port); + test_eq(0, buf_datalen(buf)); + socks_request_clear(socks); + /* SOCKS 5 Send CONNECT [01] to FQDN torproject.org:4369 */ - cp = "\x05\x01\x00\x03\x07tor.org\x11\x11"; - write_to_buf(cp, 14, buf); - test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, - get_options()->SafeSocks)); + ADD_DATA(buf, "\x05\x01\x00"); + ADD_DATA(buf, "\x05\x01\x00\x03\x0Etorproject.org\x11\x11"); + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), 0); + test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks), 1); + test_eq(5, socks->socks_version); test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); - test_streq("tor.org", socks->address); + test_streq("torproject.org", socks->address); test_eq(4369, socks->port); + test_eq(0, buf_datalen(buf)); + socks_request_clear(socks); + /* SOCKS 5 Send RESOLVE [F0] request for torproject.org:4369 */ - cp = "\x05\xF0\x00\x03\x07tor.org"; - write_to_buf(cp, 14, buf); + ADD_DATA(buf, "\x05\x01\x00"); + ADD_DATA(buf, "\x05\xF0\x00\x03\x0Etorproject.org\x01\x02"); test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, - get_options()->SafeSocks)); + get_options()->SafeSocks) == 0); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == 1); test_eq(5, socks->socks_version); test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); - test_streq("tor.org", socks->address); + test_streq("torproject.org", socks->address); - /* SOCKS 5 Send RESOLVE_PTR [F1] for IP address 2.2.2.2 */ - cp = "\x05\xF1\x00\x01\x02\x02\x02\x02"; - write_to_buf(cp, 10, buf); + test_eq(0, buf_datalen(buf)); + socks_request_clear(socks); + + /* SOCKS 5 Send RESOLVE_PTR [F1] for IP address 2.2.2.5 */ + ADD_DATA(buf, "\x05\x01\x00"); + ADD_DATA(buf, "\x05\xF1\x00\x01\x02\x02\x02\x05\x01\x03"); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == 0); test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks) == 1); test_eq(5, socks->socks_version); test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); - test_streq("2.2.2.2", socks->address); + test_streq("2.2.2.5", socks->address); + + test_eq(0, buf_datalen(buf)); done: ; } -/** Helper: Perform SOCKS 5 authentication */ +/** Perform SOCKS 5 authentication */ static void -test_buffers_socks5_no_authenticate_helper(const char *cp, buf_t *buf, - socks_request_t *socks) +test_socks_5_no_authenticate(void *ptr) { + SOCKS_TEST_INIT(); + /*SOCKS 5 No Authentication */ - cp = "\x05\x01\x00"; - write_to_buf(cp, 3, buf); + ADD_DATA(buf,"\x05\x01\x00"); test_assert(!fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks)); @@ -353,9 +446,10 @@ test_buffers_socks5_no_authenticate_helper(const char *cp, buf_t *buf, test_eq(5, socks->reply[0]); test_eq(SOCKS_NO_AUTH, socks->reply[1]); + test_eq(0, buf_datalen(buf)); + /*SOCKS 5 Send username/password anyway - pretend to be broken */ - cp = "\x01\x02\x01\x01\x02\x01\x01"; - write_to_buf(cp, 7, buf); + ADD_DATA(buf,"\x01\x02\x01\x01\x02\x01\x01"); test_assert(!fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks)); @@ -364,18 +458,25 @@ test_buffers_socks5_no_authenticate_helper(const char *cp, buf_t *buf, test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); + test_eq(2, socks->usernamelen); + test_eq(2, socks->passwordlen); + + test_memeq("\x01\x01", socks->username, 2); + test_memeq("\x01\x01", socks->password, 2); + done: ; } -/** Helper: Perform SOCKS 5 authentication */ +/** Perform SOCKS 5 authentication */ static void -test_buffers_socks5_authenticate_helper(const char *cp, buf_t *buf, - socks_request_t *socks) +test_socks_5_authenticate(void *ptr) { + SOCKS_TEST_INIT(); + /* SOCKS 5 Negotiate username/password authentication */ - cp = "\x05\x01\x02"; - write_to_buf(cp, 3, buf); + ADD_DATA(buf, "\x05\x01\x02"); + test_assert(!fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks)); @@ -384,9 +485,10 @@ test_buffers_socks5_authenticate_helper(const char *cp, buf_t *buf, test_eq(SOCKS_USER_PASS, socks->reply[1]); test_eq(5, socks->socks_version); + test_eq(0, buf_datalen(buf)); + /* SOCKS 5 Send username/password */ - cp = "\x01\x02me\x02me"; - write_to_buf(cp, 7, buf); + ADD_DATA(buf, "\x01\x02me\x08mypasswd"); test_assert(!fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks)); @@ -394,18 +496,26 @@ test_buffers_socks5_authenticate_helper(const char *cp, buf_t *buf, test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); + + test_eq(2, socks->usernamelen); + test_eq(8, socks->passwordlen); + + test_memeq("me", socks->username, 2); + test_memeq("mypasswd", socks->password, 8); + done: ; } -/** Helper: Perform SOCKS 5 authentication and send data all in one go */ +/** Perform SOCKS 5 authentication and send data all in one go */ static void -test_buffers_socks5_authenticate_with_data_helper(const char *cp, buf_t *buf, - socks_request_t *socks) +test_socks_5_authenticate_with_data(void *ptr) { + SOCKS_TEST_INIT(); + /* SOCKS 5 Negotiate username/password authentication */ - cp = "\x05\x01\x02"; - write_to_buf(cp, 3, buf); + ADD_DATA(buf, "\x05\x01\x02"); + test_assert(!fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks)); @@ -414,10 +524,11 @@ test_buffers_socks5_authenticate_with_data_helper(const char *cp, buf_t *buf, test_eq(SOCKS_USER_PASS, socks->reply[1]); test_eq(5, socks->socks_version); + test_eq(0, buf_datalen(buf)); + /* SOCKS 5 Send username/password */ /* SOCKS 5 Send CONNECT [01] to IP address 2.2.2.2:4369 */ - cp = "\x01\x02me\x02me\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11"; - write_to_buf(cp, 17, buf); + ADD_DATA(buf, "\x01\x02me\x02me\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11"); test_assert(!fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks)); @@ -434,18 +545,19 @@ test_buffers_socks5_authenticate_with_data_helper(const char *cp, buf_t *buf, test_eq(0, socks->reply[1]); test_streq("2.2.2.2", socks->address); test_eq(4369, socks->port); + done: ; } -/** Helper: Perform SOCKS 5 authentication before method negotiated */ +/** 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) +test_socks_5_auth_before_negotiation(void *ptr) { + SOCKS_TEST_INIT(); + /* SOCKS 5 Send username/password */ - cp = "\x01\x02me\x02me"; - write_to_buf(cp, 7, buf); + ADD_DATA(buf, "\x01\x02me\x02me"); test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks) == -1); @@ -467,7 +579,6 @@ test_buffers(void) buf_t *buf = NULL, *buf2 = NULL; const char *cp; - socks_request_t *socks; int j; size_t r; @@ -625,59 +736,6 @@ test_buffers(void) buf_free(buf); buf = NULL; - /* Test fetch_from_buf_socks() */ - buf = buf_new_with_capacity(256); - socks = socks_request_new(); - config_register_addressmaps(get_options()); - - /* Sending auth credentials before we've negotiated a method */ - test_buffers_socks5_auth_before_negotiation_helper(cp, buf, socks); - - socks_request_free(socks); - buf_free(buf); - buf = NULL; - buf = buf_new_with_capacity(256); - socks = socks_request_new(); - - /* 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); - test_buffers_socks5_unsupported_commands_helper(cp, buf, socks); - - socks_request_free(socks); - buf_free(buf); - buf = NULL; - buf = buf_new_with_capacity(256); - socks = socks_request_new(); - - /* A SOCKS 5 client that sends credentials and data in one go */ - test_buffers_socks5_authenticate_with_data_helper(cp, buf, socks); - - socks_request_free(socks); - buf_free(buf); - buf = NULL; - buf = buf_new_with_capacity(256); - socks = socks_request_new(); - - /* A SOCKS 5 client that doesn't want authentication */ - test_buffers_socks5_no_authenticate_helper(cp, buf, socks); - test_buffers_socks5_supported_commands_helper(cp, buf, socks); - test_buffers_socks5_unsupported_commands_helper(cp, buf, socks); - - socks_request_free(socks); - buf_free(buf); - buf = NULL; - buf = buf_new_with_capacity(256); - socks = socks_request_new(); - - /* A SOCKS 4(a) client */ - test_buffers_socks4_supported_commands_helper(cp, buf, socks); - test_buffers_socks4_unsupported_commands_helper(cp, buf, socks); - - socks_request_free(socks); - buf_free(buf); - buf = NULL; - done: if (buf) buf_free(buf); @@ -1564,6 +1622,23 @@ static struct testcase_t test_array[] = { END_OF_TESTCASES }; +#define SOCKSENT(name) \ + { #name, test_socks_##name, TT_FORK, &socks_setup, NULL } + +static struct testcase_t socks_tests[] = { + SOCKSENT(4_unsupported_commands), + SOCKSENT(4_supported_commands), + + SOCKSENT(5_unsupported_commands), + SOCKSENT(5_supported_commands), + SOCKSENT(5_no_authenticate), + SOCKSENT(5_auth_before_negotiation), + SOCKSENT(5_authenticate), + SOCKSENT(5_authenticate_with_data), + + END_OF_TESTCASES +}; + extern struct testcase_t addr_tests[]; extern struct testcase_t crypto_tests[]; extern struct testcase_t container_tests[]; @@ -1573,6 +1648,7 @@ extern struct testcase_t microdesc_tests[]; static struct testgroup_t testgroups[] = { { "", test_array }, + { "socks/", socks_tests }, { "addr/", addr_tests }, { "crypto/", crypto_tests }, { "container/", container_tests },

1 0

[tor/master] Resolve a warning from the bug1666 branch
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit e8bfe89365e86dc9ed311431d3cc87af88132f34 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 13 12:13:12 2011 -0400 Resolve a warning from the bug1666 branch --- src/or/connection_edge.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index c19e8b7..a4a77af 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2166,7 +2166,8 @@ connection_ap_handshake_process_socks(edge_connection_t *conn) if (socks->replylen) { had_reply = 1; - connection_write_to_buf(socks->reply, socks->replylen, TO_CONN(conn)); + connection_write_to_buf((const char*)socks->reply, socks->replylen, + TO_CONN(conn)); socks->replylen = 0; }

1 0

[tor/master] Add more error checks to socks parsing code
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit 16c5a62a66098274dde726c8e02110238866fe0b Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 12 10:51:31 2011 -0400 Add more error checks to socks parsing code Suggested by Linus to avoid uninitialized reads or infinite loops if it turns out our code is buggier than we had thought. --- src/or/buffers.c | 35 ++++++++++++++++++++--------------- 1 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 1310b42..b7567bf 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1544,7 +1544,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 && want_length < buf->datalen); + } while (res == 0 && buf->head && want_length < buf->datalen && + buf->datalen >= 2); return res; } @@ -1595,12 +1596,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); @@ -1612,20 +1615,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; } @@ -1652,6 +1651,12 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, 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

1 0

[tor/master] Merge remote-tracking branch 'public/bug1666'
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit 1aab5b6b391ca179d9acded9465ee4941ec4700b Merge: 9a6642f 16c5a62 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 13 12:12:16 2011 -0400 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 changes/bug1666 | 4 + src/or/buffers.c | 182 ++++++++++++++++------ src/or/buffers.h | 2 + src/or/config.c | 3 +- src/or/config.h | 2 + src/or/connection.c | 8 +- src/or/connection_edge.c | 23 ++-- src/or/or.h | 26 +++- src/test/test.c | 384 ++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 566 insertions(+), 68 deletions(-) diff --cc src/or/buffers.c index 7312749,b7567bf..256b507 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@@ -1482,10 -1475,25 +1482,29 @@@ log_unsafe_socks_warning(int socks_prot socks_protocol, address, (int)port); } +/** Do not attempt to parse socks messages longer than this. This value is + * 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 req. */ + 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 buf, of one * of the forms * - socks4: "socksheader username\\0" diff --cc src/or/config.c index 4a9d2c9,d2dbdaa..cdf0122 --- a/src/or/config.c +++ b/src/or/config.c @@@ -547,44 -521,38 +547,43 @@@ static char *get_windows_conf_root(void #endif static void config_line_append(config_line_t **lst, const char *key, const char *val); -static void option_clear(config_format_t *fmt, or_options_t *options, - config_var_t *var); -static void option_reset(config_format_t *fmt, or_options_t *options, - config_var_t *var, int use_defaults); -static void config_free(config_format_t *fmt, void *options); +static void option_clear(const config_format_t *fmt, or_options_t *options, + const config_var_t *var); +static void option_reset(const config_format_t *fmt, or_options_t *options, + const config_var_t *var, int use_defaults); +static void config_free(const config_format_t *fmt, void *options); static int config_lines_eq(config_line_t *a, config_line_t *b); -static int option_is_same(config_format_t *fmt, - or_options_t *o1, or_options_t *o2, +static int option_is_same(const config_format_t *fmt, + const or_options_t *o1, const or_options_t *o2, const char *name); -static or_options_t *options_dup(config_format_t *fmt, or_options_t *old); -static int options_validate(or_options_t *old_options, or_options_t *options, +static or_options_t *options_dup(const config_format_t *fmt, + const or_options_t *old); +static int options_validate(or_options_t *old_options, + or_options_t *options, int from_setconf, char **msg); -static int options_act_reversible(or_options_t *old_options, char **msg); -static int options_act(or_options_t *old_options); -static int options_transition_allowed(or_options_t *old, or_options_t *new, +static int options_act_reversible(const or_options_t *old_options, char **msg); +static int options_act(const or_options_t *old_options); +static int options_transition_allowed(const or_options_t *old, + const or_options_t *new, char **msg); -static int options_transition_affects_workers(or_options_t *old_options, - or_options_t *new_options); -static int options_transition_affects_descriptor(or_options_t *old_options, - or_options_t *new_options); +static int options_transition_affects_workers( + const or_options_t *old_options, const or_options_t *new_options); +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); static int parse_dir_server_line(const char *line, - authority_type_t required_type, + dirinfo_type_t required_type, int validate_only); static int validate_data_directory(or_options_t *options); -static int write_configuration_file(const char *fname, or_options_t *options); -static config_line_t *get_assigned_option(config_format_t *fmt, - void *options, const char *key, - int escape_val); -static void config_init(config_format_t *fmt, void *options); +static int write_configuration_file(const char *fname, + const or_options_t *options); +static config_line_t *get_assigned_option(const config_format_t *fmt, + const void *options, const char *key, + int escape_val); +static void config_init(const config_format_t *fmt, void *options); static int or_state_validate(or_state_t *old_options, or_state_t *options, int from_setconf, char **msg); static int or_state_load(void); @@@ -4430,8 -4313,8 +4429,8 @@@ get_torrc_fname(void /** Adjust the address map based on the MapAddress elements in the * configuration options */ - static void + void -config_register_addressmaps(or_options_t *options) +config_register_addressmaps(const or_options_t *options) { smartlist_t *elts; config_line_t *opt; diff --cc src/or/config.h index bc71191,db871d4..8a06f44 --- a/src/or/config.h +++ b/src/or/config.h @@@ -79,5 -76,7 +79,7 @@@ uint32_t get_effective_bwburst(const or or_options_t *options_new(void); #endif -void config_register_addressmaps(or_options_t *options); ++void config_register_addressmaps(const or_options_t *options); + #endif diff --cc src/or/connection_edge.c index 7516716,bff73d4..c19e8b7 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@@ -2144,7 -1882,8 +2144,8 @@@ connection_ap_handshake_process_socks(e { socks_request_t *socks; int sockshere; - or_options_t *options = get_options(); + const or_options_t *options = get_options(); + int had_reply = 0; tor_assert(conn); tor_assert(conn->_base.type == CONN_TYPE_AP);

1 0

[tor/master] bug1666 - Pass-through support for SOCKS5 authentication
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit bf136b94de39de65638ce3daaf2e87731cd0a44a Author: Robert Hogan <robert(a)roberthogan.net> Date: Tue Aug 3 22:28:55 2010 +0100 bug1666 - Pass-through support for SOCKS5 authentication If a SOCKS5 client insists on authentication, allow it to negotiate a connection with Tor's SOCKS server successfully. Any credentials the client provides are ignored. This allows Tor to work with SOCKS5 clients that can only support 'authenticated' connections. Also add a bunch of basic unit tests for SOCKS4/4a/5 support in buffers.c. --- doc/spec/socks-extensions.txt | 5 +- src/or/buffers.c | 62 ++++++++--- src/or/config.c | 3 +- src/or/config.h | 2 + src/or/or.h | 2 + src/test/test.c | 237 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 293 insertions(+), 18 deletions(-) diff --git a/doc/spec/socks-extensions.txt b/doc/spec/socks-extensions.txt index 62d86ac..52d6834 100644 --- a/doc/spec/socks-extensions.txt +++ b/doc/spec/socks-extensions.txt @@ -31,8 +31,9 @@ Tor's extensions to the SOCKS protocol SOCKS5: - The (SOCKS5) "UDP ASSOCIATE" command is not supported. - IPv6 is not supported in CONNECT commands. - - Only the "NO AUTHENTICATION" (SOCKS5) authentication method [00] is - supported. + - The "NO AUTHENTICATION" (SOCKS5) authentication method [00] and the + "USERNAME/PASSWORD" (SOCKS5) authentication method [02] are both + supported. Any credentials passed in the latter are ignored. 2. Name lookup diff --git a/src/or/buffers.c b/src/or/buffers.c index 11c4fc8..534f31c 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1633,40 +1633,74 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, uint8_t socksver; enum {socks4, socks4a} socks4_prot = socks4a; char *next, *startaddr; + unsigned char usernamelen, passlen; struct in_addr in; socksver = *data; switch (socksver) { /* which version of socks? */ + case 1: /* socks5: username/password authentication request */ + + usernamelen = (unsigned char)*(buf->head->data + 1); + if (buf->datalen < 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) + return 0; + if (buf->datalen > 2u + usernamelen + 1u + passlen) { + log_warn(LD_APP, + "socks5: Malformed username/password. Rejecting."); + return -1; + } + 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."); + return 0; + 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)) { + 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)) { + 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)) { + 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; - 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; } /* we know the method; read in the request */ log_debug(LD_APP,"socks5: checking request"); @@ -1761,8 +1795,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, } tor_assert(0); case 4: /* socks4 */ - /* http://archive.socks.permeo.com/protocol/socks4.protocol */ - /* http://archive.socks.permeo.com/protocol/socks4a.protocol */ + /* 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? */ diff --git a/src/or/config.c b/src/or/config.c index cd6b0b0..d2dbdaa 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -542,7 +542,6 @@ static int options_transition_affects_workers(or_options_t *old_options, static int options_transition_affects_descriptor(or_options_t *old_options, or_options_t *new_options); static int check_nickname_list(const char *lst, const char *name, char **msg); -static void config_register_addressmaps(or_options_t *options); static int parse_bridge_line(const char *line, int validate_only); static int parse_dir_server_line(const char *line, @@ -4314,7 +4313,7 @@ get_torrc_fname(void) /** Adjust the address map based on the MapAddress elements in the * configuration options */ -static void +void config_register_addressmaps(or_options_t *options) { smartlist_t *elts; diff --git a/src/or/config.h b/src/or/config.h index bd5827b..db871d4 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -76,5 +76,7 @@ uint32_t get_effective_bwburst(or_options_t *options); or_options_t *options_new(void); #endif +void config_register_addressmaps(or_options_t *options); + #endif diff --git a/src/or/or.h b/src/or/or.h index c5349ae..6ae62e8 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3067,6 +3067,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 diff --git a/src/test/test.c b/src/test/test.c index 9b0251b..68f1aeb 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -197,6 +197,207 @@ free_pregenerated_keys(void) } } +/** Helper: Perform supported SOCKS 5 commands */ +static void +test_buffers_socks4_unsupported_commands_helper(const char *cp, buf_t *buf, + socks_request_t *socks) +{ + /* SOCKS 4 Send BIND [02] to IP address 2.2.2.2:4369 */ + cp = "\x04\x02\x11\x11\x02\x02\x02\x02\x00"; + write_to_buf(cp, 9, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == -1); + test_eq(4, socks->socks_version); + test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ + +done: + ; +} + +/** Helper: Perform supported SOCKS 5 commands */ +static void +test_buffers_socks4_supported_commands_helper(const char *cp, buf_t *buf, + socks_request_t *socks) +{ + /* SOCKS 4 Send CONNECT [01] to IP address 2.2.2.2:4369 */ + cp = "\x04\x01\x11\x11\x02\x02\x02\x02\x00"; + write_to_buf(cp, 9, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == 1); + test_eq(4, socks->socks_version); + test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ + test_streq("2.2.2.2", socks->address); + test_eq(4369, socks->port); + + /* SOCKS 4 Send CONNECT [01] to IP address 2.2.2.2:4369 with userid*/ + cp = "\x04\x01\x11\x11\x02\x02\x02\x02\x02me\x00"; + write_to_buf(cp, 12, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == 1); + test_eq(4, socks->socks_version); + test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ + test_streq("2.2.2.2", socks->address); + test_eq(4369, socks->port); + + /* SOCKS 4a Send RESOLVE [F0] request for torproject.org:4369 */ + cp = "\x04\xF0\x01\x01\x00\x00\x00\x02\x02me\x00tor.org\x00"; + write_to_buf(cp, 20, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(4, socks->socks_version); + test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ + test_streq("tor.org", socks->address); + +done: + ; +} + +/** Helper: Perform supported SOCKS 5 commands */ +static void +test_buffers_socks5_unsupported_commands_helper(const char *cp, buf_t *buf, + socks_request_t *socks) +{ + /* SOCKS 5 Send unsupported BIND [02] command */ + cp = "\x05\x02\x00\x01\x02\x02\x02\x02\x01\x01"; + write_to_buf(cp, 10, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == -1); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); /* XXX: shouldn't tor reply 'command + not supported' [07]? */ + + /* SOCKS 5 Send unsupported UDP_ASSOCIATE [03] command */ + cp = "\x05\x03\x00\x01\x02\x02\x02\x02\x01\x01"; + write_to_buf(cp, 10, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == -1); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); /* XXX: shouldn't tor reply 'command + not supported' [07]? */ + +done: + ; +} + +/** Helper: Perform supported SOCKS 5 commands */ +static void +test_buffers_socks5_supported_commands_helper(const char *cp, buf_t *buf, + socks_request_t *socks) +{ + /* SOCKS 5 Send CONNECT [01] to IP address 2.2.2.2:4369 */ + cp = "\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11"; + write_to_buf(cp, 10, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == 1); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); + test_streq("2.2.2.2", socks->address); + test_eq(4369, socks->port); + + /* SOCKS 5 Send CONNECT [01] to FQDN torproject.org:4369 */ + cp = "\x05\x01\x00\x03\x07tor.org\x11\x11"; + write_to_buf(cp, 14, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); + test_streq("tor.org", socks->address); + test_eq(4369, socks->port); + + /* SOCKS 5 Send RESOLVE [F0] request for torproject.org:4369 */ + cp = "\x05\xF0\x00\x03\x07tor.org"; + write_to_buf(cp, 14, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); + test_streq("tor.org", socks->address); + + /* SOCKS 5 Send RESOLVE_PTR [F1] for IP address 2.2.2.2 */ + cp = "\x05\xF1\x00\x01\x02\x02\x02\x02"; + write_to_buf(cp, 10, buf); + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == 1); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); + test_streq("2.2.2.2", socks->address); + +done: + ; +} + +/** Helper: Perform SOCKS 5 authentication */ +static void +test_buffers_socks5_no_authenticate_helper(const char *cp, buf_t *buf, + socks_request_t *socks) +{ + /*SOCKS 5 No Authentication */ + cp = "\x05\x01\x00"; + write_to_buf(cp, 3, buf); + test_assert(!fetch_from_buf_socks(buf, socks, + get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(SOCKS_NO_AUTH, socks->reply[1]); + + /*SOCKS 5 Send username/password anyway - pretend to be broken */ + cp = "\x01\x02\x01\x01\x02\x01\x01"; + write_to_buf(cp, 7, buf); + test_assert(!fetch_from_buf_socks(buf, socks, + get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); + +done: + ; +} + +/** Helper: Perform SOCKS 5 authentication */ +static void +test_buffers_socks5_authenticate_helper(const char *cp, buf_t *buf, + socks_request_t *socks) +{ + /* SOCKS 5 Negotiate username/password authentication */ + cp = "\x05\x01\x02"; + write_to_buf(cp, 3, buf); + test_assert(!fetch_from_buf_socks(buf, socks, + get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(SOCKS_USER_PASS, socks->reply[1]); + test_eq(5, socks->socks_version); + + /* 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)); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); +done: + ; +} + /** Run unit tests for buffers.c */ static void test_buffers(void) @@ -206,6 +407,7 @@ test_buffers(void) buf_t *buf = NULL, *buf2 = NULL; const char *cp; + socks_request_t *socks; int j; size_t r; @@ -363,6 +565,41 @@ test_buffers(void) buf_free(buf); buf = NULL; + /* Test fetch_from_buf_socks() */ + buf = buf_new_with_capacity(256); + socks = tor_malloc_zero(sizeof(socks_request_t));; + config_register_addressmaps(get_options()); + + /* 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); + test_buffers_socks5_unsupported_commands_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 doesn't want authentication */ + test_buffers_socks5_no_authenticate_helper(cp, buf, socks); + test_buffers_socks5_supported_commands_helper(cp, buf, socks); + test_buffers_socks5_unsupported_commands_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 4(a) client */ + test_buffers_socks4_supported_commands_helper(cp, buf, socks); + test_buffers_socks4_unsupported_commands_helper(cp, buf, socks); + + tor_free(socks); + buf_free(buf); + buf = NULL; + done: if (buf) buf_free(buf);

1 0

[tor/master] bug1666 - Pass-through support for SOCKS5 authentication (2)
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit f85f52808cd29e50874b233e55b0a0fe98e425f3 Author: Robert Hogan <robert(a)roberthogan.net> Date: Sun Oct 17 14:14:51 2010 +0100 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 --- src/or/buffers.c | 28 ++++++++++++++++++---------- src/test/test.c | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 534f31c..5cd76fa 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -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 */ diff --git a/src/test/test.c b/src/test/test.c index 68f1aeb..8155c3f 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -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);

1 0

[tor/master] bug1666 - Pass-through support for SOCKS5 authentication(3)
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit 8a2f7ef66ac3bee0d6885ec3ca16937fed0703bb Author: Robert Hogan <robert(a)roberthogan.net> Date: Sun Oct 17 14:22:24 2010 +0100 bug1666 - Pass-through support for SOCKS5 authentication(3) Add changes file. --- changes/bug1666 | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/changes/bug1666 b/changes/bug1666 new file mode 100644 index 0000000..9fd790d --- /dev/null +++ b/changes/bug1666 @@ -0,0 +1,4 @@ + o Minor features: + - Accept attempts to include a password authenticator in the handshake, as + supported by SOCKS5. This handles SOCKS clients that don't know how to + omit the password when authenticating. Resolves bug 1666.

1 0

[tor/master] bug1666 - Pass-through support for SOCKS5 authentication(4)
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit 02c2d9a4aa2a7ce339e87be9c0c0dc23a6881c14 Author: Robert Hogan <robert(a)roberthogan.net> Date: Tue Dec 14 19:59:42 2010 +0000 bug1666 - Pass-through support for SOCKS5 authentication(4) Implement nickm's suggestion that we tolerate SOCKS5 clients that send authentication credentials and SOCKS commands all in one go. --- src/or/buffers.c | 5 ----- src/test/test.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 5cd76fa..445376f 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1658,11 +1658,6 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, *want_length_out = 2u+usernamelen; return 0; } - if (datalen > 2u + usernamelen + 1u + passlen) { - log_warn(LD_APP, - "socks5: Malformed username/password. Rejecting."); - return -1; - } req->replylen = 2; /* 2 bytes of response */ req->reply[0] = 5; req->reply[1] = 0; /* authentication successful */ diff --git a/src/test/test.c b/src/test/test.c index 8155c3f..bac73f2 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -398,6 +398,46 @@ done: ; } +/** Helper: Perform SOCKS 5 authentication and send data all in one go */ +static void +test_buffers_socks5_authenticate_with_data_helper(const char *cp, buf_t *buf, + socks_request_t *socks) +{ + /* SOCKS 5 Negotiate username/password authentication */ + cp = "\x05\x01\x02"; + write_to_buf(cp, 3, buf); + test_assert(!fetch_from_buf_socks(buf, socks, + get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(SOCKS_USER_PASS, socks->reply[1]); + test_eq(5, socks->socks_version); + + /* SOCKS 5 Send username/password */ + /* SOCKS 5 Send CONNECT [01] to IP address 2.2.2.2:4369 */ + cp = "\x01\x02me\x02me\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11"; + write_to_buf(cp, 17, buf); + test_assert(!fetch_from_buf_socks(buf, socks, + get_options()->TestSocks, + get_options()->SafeSocks)); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); + + test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, + get_options()->SafeSocks) == 1); + test_eq(5, socks->socks_version); + test_eq(2, socks->replylen); + test_eq(5, socks->reply[0]); + test_eq(0, socks->reply[1]); + test_streq("2.2.2.2", socks->address); + test_eq(4369, socks->port); +done: + ; +} + /** Helper: Perform SOCKS 5 authentication before method negotiated */ static void test_buffers_socks5_auth_before_negotiation_helper(const char *cp, buf_t *buf, @@ -610,6 +650,15 @@ test_buffers(void) buf = buf_new_with_capacity(256); socks = tor_malloc_zero(sizeof(socks_request_t));; + /* A SOCKS 5 client that sends credentials and data in one go */ + test_buffers_socks5_authenticate_with_data_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 doesn't want authentication */ test_buffers_socks5_no_authenticate_helper(cp, buf, socks); test_buffers_socks5_supported_commands_helper(cp, buf, socks);

1 0

[tor/master] Correct byte-counting in socks auth parsing code
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit 1ed615ded7db0765e8355687bda8b00fdc643e3e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 29 11:45:15 2011 -0400 Correct byte-counting in socks auth parsing code --- src/or/buffers.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 445376f..4b8532a 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1648,14 +1648,19 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, "authentication negotiated. Rejecting."); return -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) { - *want_length_out = 2u+usernamelen; + 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; + *want_length_out = 2u + usernamelen + 1u + passlen; return 0; } req->replylen = 2; /* 2 bytes of response */

1 0

[tor/master] Fix 'make check-spaces'
by nickm＠torproject.org 13 Jul '11

13 Jul '11

commit 9b3f48c0744bd72cb8b25ac7857476181b29481a Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 29 11:47:35 2011 -0400 Fix 'make check-spaces' --- src/or/buffers.c | 2 +- src/test/test.c | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 4b8532a..3490d6d 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1691,7 +1691,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, 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)) { + } 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 */ diff --git a/src/test/test.c b/src/test/test.c index bac73f2..d0e24d1 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -210,7 +210,7 @@ test_buffers_socks4_unsupported_commands_helper(const char *cp, buf_t *buf, test_eq(4, socks->socks_version); test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ -done: + done: ; } @@ -248,7 +248,7 @@ test_buffers_socks4_supported_commands_helper(const char *cp, buf_t *buf, test_eq(0, socks->replylen); /* XXX: shouldn't tor reply? */ test_streq("tor.org", socks->address); -done: + done: ; } @@ -279,7 +279,7 @@ test_buffers_socks5_unsupported_commands_helper(const char *cp, buf_t *buf, test_eq(0, socks->reply[1]); /* XXX: shouldn't tor reply 'command not supported' [07]? */ -done: + done: ; } @@ -334,7 +334,7 @@ test_buffers_socks5_supported_commands_helper(const char *cp, buf_t *buf, test_eq(0, socks->reply[1]); test_streq("2.2.2.2", socks->address); -done: + done: ; } @@ -364,7 +364,7 @@ test_buffers_socks5_no_authenticate_helper(const char *cp, buf_t *buf, test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); -done: + done: ; } @@ -394,7 +394,7 @@ test_buffers_socks5_authenticate_helper(const char *cp, buf_t *buf, test_eq(2, socks->replylen); test_eq(5, socks->reply[0]); test_eq(0, socks->reply[1]); -done: + done: ; } @@ -434,7 +434,7 @@ test_buffers_socks5_authenticate_with_data_helper(const char *cp, buf_t *buf, test_eq(0, socks->reply[1]); test_streq("2.2.2.2", socks->address); test_eq(4369, socks->port); -done: + done: ; } @@ -454,7 +454,7 @@ test_buffers_socks5_auth_before_negotiation_helper(const char *cp, buf_t *buf, test_eq(0, socks->reply[0]); test_eq(0, socks->reply[1]); -done: + done: ; }

1 0