November 2014 - tor-commits - lists.torproject.org

[tor/master] Introducing helper function to validate DNS name strings.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 1ea9a6fd72b66ec634446cbd2119641a5ed1e703 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sun Oct 12 19:33:08 2014 +0300 Introducing helper function to validate DNS name strings. --- src/common/util.c | 36 ++++++++++++++++++++++++++++++++++++ src/common/util.h | 1 + src/test/test_util.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/src/common/util.c b/src/common/util.c index f4d293c..606b9e1 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -957,6 +957,42 @@ string_is_key_value(int severity, const char *string) return 1; } +/** Return true iff string is valid DNS name, as defined in + * RFC 1035 Section 2.3.1. + */ +int +string_is_valid_hostname(const char *string) +{ + int result = 1; + smartlist_t *components; + + components = smartlist_new(); + + smartlist_split_string(components,string,".",0,0); + + SMARTLIST_FOREACH_BEGIN(components, char *, c) { + if (c[0] == '-') { + result = 0; + break; + } + + do { + if ((*c >= 'a' && *c <= 'z') || + (*c >= 'A' && *c <= 'Z') || + (*c >= '0' && *c <= '9') || + (*c == '-')) + c++; + else + result = 0; + } while (result && *c); + + } SMARTLIST_FOREACH_END(c); + + smartlist_free(components); + + return result; +} + /** Return true iff the DIGEST256_LEN bytes in digest are all zero. */ int tor_digest256_is_zero(const char *digest) diff --git a/src/common/util.h b/src/common/util.h index 61bb05f..2634adc 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -227,6 +227,7 @@ const char *find_str_at_start_of_line(const char *haystack, const char *needle); int string_is_C_identifier(const char *string); int string_is_key_value(int severity, const char *string); +int string_is_valid_hostname(const char *string); int tor_mem_is_zero(const char *mem, size_t len); int tor_digest_is_zero(const char *digest); diff --git a/src/test/test_util.c b/src/test/test_util.c index c67aa71..fb3ce7d 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4122,6 +4122,36 @@ test_util_max_mem(void *arg) ; } +static void +test_util_hostname_validation(void *arg) +{ + (void)arg; + + // Lets try valid hostnames first. + tt_assert(string_is_valid_hostname("torproject.org")); + tt_assert(string_is_valid_hostname("ocw.mit.edu")); + tt_assert(string_is_valid_hostname("i.4cdn.org")); + tt_assert(string_is_valid_hostname("stanford.edu")); + tt_assert(string_is_valid_hostname("multiple-words-with-hypens.jp")); + + // Subdomain name cannot start with '-'. + tt_assert(!string_is_valid_hostname("-torproject.org")); + tt_assert(!string_is_valid_hostname("subdomain.-domain.org")); + tt_assert(!string_is_valid_hostname("-subdomain.domain.org")); + + // Hostnames cannot contain non-alphanumeric characters. + tt_assert(!string_is_valid_hostname("%%domain.\\org.")); + tt_assert(!string_is_valid_hostname("***x.net")); + tt_assert(!string_is_valid_hostname("___abc.org")); + tt_assert(!string_is_valid_hostname("\xff\xffxyz.org")); + tt_assert(!string_is_valid_hostname("word1 word2.net")); + + // XXX: do we allow single-label DNS names? + + done: + return; +} + struct testcase_t util_tests[] = { UTIL_LEGACY(time), UTIL_TEST(parse_http_time, 0), @@ -4194,6 +4224,7 @@ struct testcase_t util_tests[] = { { "socketpair_ersatz", test_util_socketpair, TT_FORK, &socketpair_setup, (void*)"1" }, UTIL_TEST(max_mem, 0), + UTIL_TEST(hostname_validation, 0), END_OF_TESTCASES };

1 0

[tor/master] Introducing helper function to validate IPv4 address strings.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit e8e45ff13ed86d8851bab77d65d899d0ca6e3b89 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sun Oct 12 20:39:00 2014 +0300 Introducing helper function to validate IPv4 address strings. --- src/common/util.c | 19 +++++++++++++++++-- src/common/util.h | 1 + src/test/test_util.c | 21 +++++++++++++++++++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 606b9e1..ba9d78a 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -957,8 +957,19 @@ string_is_key_value(int severity, const char *string) return 1; } -/** Return true iff string is valid DNS name, as defined in - * RFC 1035 Section 2.3.1. +/** Return true if string represents a valid IPv4 adddress in + * 'a.b.c.d' form. + */ +int +string_is_valid_ipv4_address(const char *string) +{ + struct sockaddr_in sockaddr; + + return (tor_inet_pton(AF_INET,string,&sockaddr) == 1); +} + +/** Return true iff string matches a pattern of DNS names + * that we allow Tor clients to connect to. */ int string_is_valid_hostname(const char *string) @@ -988,6 +999,10 @@ string_is_valid_hostname(const char *string) } SMARTLIST_FOREACH_END(c); + SMARTLIST_FOREACH_BEGIN(components, char *, c) { + tor_free(c); + } SMARTLIST_FOREACH_END(c); + smartlist_free(components); return result; diff --git a/src/common/util.h b/src/common/util.h index 2634adc..dcb54f0 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -228,6 +228,7 @@ const char *find_str_at_start_of_line(const char *haystack, int string_is_C_identifier(const char *string); int string_is_key_value(int severity, const char *string); int string_is_valid_hostname(const char *string); +int string_is_valid_ipv4_address(const char *string); int tor_mem_is_zero(const char *mem, size_t len); int tor_digest_is_zero(const char *digest); diff --git a/src/test/test_util.c b/src/test/test_util.c index fb3ce7d..fba90da 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4138,9 +4138,9 @@ test_util_hostname_validation(void *arg) tt_assert(!string_is_valid_hostname("-torproject.org")); tt_assert(!string_is_valid_hostname("subdomain.-domain.org")); tt_assert(!string_is_valid_hostname("-subdomain.domain.org")); - + // Hostnames cannot contain non-alphanumeric characters. - tt_assert(!string_is_valid_hostname("%%domain.\\org.")); + tt_assert(!string_is_valid_hostname("%%domain.\\org.")); tt_assert(!string_is_valid_hostname("***x.net")); tt_assert(!string_is_valid_hostname("___abc.org")); tt_assert(!string_is_valid_hostname("\xff\xffxyz.org")); @@ -4152,6 +4152,22 @@ test_util_hostname_validation(void *arg) return; } +static void +test_util_ipv4_validation(void *arg) +{ + (void)arg; + + tt_assert(string_is_valid_ipv4_address("192.168.0.1")); + tt_assert(string_is_valid_ipv4_address("8.8.8.8")); + + tt_assert(!string_is_valid_ipv4_address("abcd")); + tt_assert(!string_is_valid_ipv4_address("300.300.300.300")); + tt_assert(!string_is_valid_ipv4_address("8.8.")); + + done: + return; +} + struct testcase_t util_tests[] = { UTIL_LEGACY(time), UTIL_TEST(parse_http_time, 0), @@ -4225,6 +4241,7 @@ struct testcase_t util_tests[] = { &socketpair_setup, (void*)"1" }, UTIL_TEST(max_mem, 0), UTIL_TEST(hostname_validation, 0), + UTIL_TEST(ipv4_validation, 0), END_OF_TESTCASES };

1 0

[tor/master] Validating SOCKS5 hostname more correctly.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 2862b769deaaaa40347ffe808349c4e139e7eb45 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sun Oct 12 21:04:15 2014 +0300 Validating SOCKS5 hostname more correctly. --- src/or/buffers.c | 10 +++++++++- src/test/test_socks.c | 11 +++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index d174f81..e98f569 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -2048,7 +2048,15 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, req->address[len] = 0; req->port = ntohs(get_uint16(data+5+len)); *drain_out = 5+len+2; - if (!tor_strisprint(req->address) || strchr(req->address,'\"')) { + + if (string_is_valid_ipv4_address(req->address)) { + log_unsafe_socks_warning(5,req->address,req->port,safe_socks); + + if (safe_socks) + return -1; + } + + if (!string_is_valid_hostname(req->address)) { log_warn(LD_PROTOCOL, "Your application (using socks5 to port %d) gave Tor " "a malformed hostname: %s. Rejecting the connection.", diff --git a/src/test/test_socks.c b/src/test/test_socks.c index 2b8f824..b9520b5 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -229,6 +229,17 @@ test_socks_5_supported_commands(void *ptr) tt_int_op(0,==, buf_datalen(buf)); socks_request_clear(socks); + /* SOCKS 5 Should reject RESOLVE [F0] request for IPv4 address + * string if SafeSocks is enabled. */ + + ADD_DATA(buf, "\x05\x01\x00"); + ADD_DATA(buf, "\x05\xF0\x00\x03\x07"); + ADD_DATA(buf, "8.8.8.8"); + ADD_DATA(buf, "\x01\x02"); + tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1) + == -1); + 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");

1 0

[tor/master] Adding helper function that checks if string is a valid IPv6 address.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 2f1068e68a95a2ad9ba058297b54622323b216c6 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Tue Oct 14 21:53:48 2014 +0300 Adding helper function that checks if string is a valid IPv6 address. --- src/common/util.c | 11 +++++++++++ src/common/util.h | 1 + 2 files changed, 12 insertions(+) diff --git a/src/common/util.c b/src/common/util.c index ba9d78a..e9ee437 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -968,6 +968,17 @@ string_is_valid_ipv4_address(const char *string) return (tor_inet_pton(AF_INET,string,&sockaddr) == 1); } +/** Return true if string represents a valid IPv6 address in + * a form that inet_pton() can parse. + */ +int +string_is_valid_ipv6_address(const char *string) +{ + struct sockaddr_in sockaddr_dummy; + + return (inet_pton(AF_INET6,string,&sockaddr_dummy) == 1); +} + /** Return true iff string matches a pattern of DNS names * that we allow Tor clients to connect to. */ diff --git a/src/common/util.h b/src/common/util.h index dcb54f0..5eecfad 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -229,6 +229,7 @@ int string_is_C_identifier(const char *string); int string_is_key_value(int severity, const char *string); int string_is_valid_hostname(const char *string); int string_is_valid_ipv4_address(const char *string); +int string_is_valid_ipv6_address(const char *string); int tor_mem_is_zero(const char *mem, size_t len); int tor_digest_is_zero(const char *digest);

1 0

[tor/master] Checking if FQDN is actually IPv6 address string and handling that case.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 0da4ddda4f8f1c3c931349e45acbdcae2b7cc750 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Tue Oct 14 21:56:04 2014 +0300 Checking if FQDN is actually IPv6 address string and handling that case. --- src/common/util.c | 2 +- src/or/buffers.c | 3 ++- src/test/test_socks.c | 11 +++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index e9ee437..c292c79 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -976,7 +976,7 @@ string_is_valid_ipv6_address(const char *string) { struct sockaddr_in sockaddr_dummy; - return (inet_pton(AF_INET6,string,&sockaddr_dummy) == 1); + return (tor_inet_pton(AF_INET6,string,&sockaddr_dummy) == 1); } /** Return true iff string matches a pattern of DNS names diff --git a/src/or/buffers.c b/src/or/buffers.c index e98f569..354bec6 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -2049,7 +2049,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, req->port = ntohs(get_uint16(data+5+len)); *drain_out = 5+len+2; - if (string_is_valid_ipv4_address(req->address)) { + if (string_is_valid_ipv4_address(req->address) || + string_is_valid_ipv6_address(req->address)) { log_unsafe_socks_warning(5,req->address,req->port,safe_socks); if (safe_socks) diff --git a/src/test/test_socks.c b/src/test/test_socks.c index b9520b5..ba6b9a9 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -240,6 +240,17 @@ test_socks_5_supported_commands(void *ptr) == -1); socks_request_clear(socks); + /* SOCKS 5 should reject RESOLVE [F0] reject for IPv6 address + * string if SafeSocks is enabled. */ + + ADD_DATA(buf, "\x05\x01\x00"); + ADD_DATA(buf, "\x05\xF0\x00\x03\x27"); + ADD_DATA(buf, "2001:0db8:85a3:0000:0000:8a2e:0370:7334"); + ADD_DATA(buf, "\x01\x02"); + tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1) + == -1); + 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");

1 0

[tor/master] Use correct argument types for inet_pton.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 254ab5a8deb9ed5fb571016f402ffd7243208d73 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 4 00:45:14 2014 -0500 Use correct argument types for inet_pton. (I blame whoever decided that using a void* for a union was a good idea.) --- src/common/util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index c292c79..8300299 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -963,9 +963,9 @@ string_is_key_value(int severity, const char *string) int string_is_valid_ipv4_address(const char *string) { - struct sockaddr_in sockaddr; + struct in_addr addr; - return (tor_inet_pton(AF_INET,string,&sockaddr) == 1); + return (tor_inet_pton(AF_INET,string,&addr) == 1); } /** Return true if string represents a valid IPv6 address in @@ -974,9 +974,9 @@ string_is_valid_ipv4_address(const char *string) int string_is_valid_ipv6_address(const char *string) { - struct sockaddr_in sockaddr_dummy; + struct in6_addr addr; - return (tor_inet_pton(AF_INET6,string,&sockaddr_dummy) == 1); + return (tor_inet_pton(AF_INET6,string,&addr) == 1); } /** Return true iff string matches a pattern of DNS names

1 0

[tor/master] Adding changes file for 13315.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit fc9591da727817923e1084d1a73d5dd8a52e6948 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Tue Oct 14 22:09:44 2014 +0300 Adding changes file for 13315. --- changes/bug13315 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/bug13315 b/changes/bug13315 new file mode 100644 index 0000000..c2ae5ff --- /dev/null +++ b/changes/bug13315 @@ -0,0 +1,5 @@ + o Minor features: + - Validate hostnames in SOCKS5 requests more strictly. If SafeSocks + is enabled, reject requests with IP addresses as hostnames. Resolves + ticket 13315. +

1 0

[tor/master] Sending 'Not allowed' error message before closing the connection.
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 51e247361824fa64f4322fb59e9d2cffd9d72cba Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Tue Oct 21 20:50:32 2014 +0300 Sending 'Not allowed' error message before closing the connection. --- src/or/buffers.c | 4 +++- src/test/test_socks.c | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 354bec6..691845e 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -2053,8 +2053,10 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, string_is_valid_ipv6_address(req->address)) { log_unsafe_socks_warning(5,req->address,req->port,safe_socks); - if (safe_socks) + if (safe_socks) { + socks_request_set_socks5_error(req, SOCKS5_NOT_ALLOWED); return -1; + } } if (!string_is_valid_hostname(req->address)) { diff --git a/src/test/test_socks.c b/src/test/test_socks.c index ba6b9a9..a3fe07f 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -238,6 +238,13 @@ test_socks_5_supported_commands(void *ptr) ADD_DATA(buf, "\x01\x02"); tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1) == -1); + + tt_int_op(5,==,socks->socks_version); + tt_int_op(10,==,socks->replylen); + tt_int_op(5,==,socks->reply[0]); + tt_int_op(SOCKS5_NOT_ALLOWED,==,socks->reply[1]); + tt_int_op(1,==,socks->reply[3]); + socks_request_clear(socks); /* SOCKS 5 should reject RESOLVE [F0] reject for IPv6 address @@ -249,6 +256,13 @@ test_socks_5_supported_commands(void *ptr) ADD_DATA(buf, "\x01\x02"); tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1) == -1); + + tt_int_op(5,==,socks->socks_version); + tt_int_op(10,==,socks->replylen); + tt_int_op(5,==,socks->reply[0]); + tt_int_op(SOCKS5_NOT_ALLOWED,==,socks->reply[1]); + tt_int_op(1,==,socks->reply[3]); + socks_request_clear(socks); /* SOCKS 5 Send RESOLVE_PTR [F1] for IP address 2.2.2.5 */

1 0

[tor/master] Merge branch 'bug13315_squashed'
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 60c86a3b79d542543f191a78f5c7ea2c77c0d3f0 Merge: 593909e 74cbd8d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 4 00:48:25 2014 -0500 Merge branch 'bug13315_squashed' Conflicts: src/or/buffers.c changes/bug13315 | 5 ++++ src/common/util.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ src/common/util.h | 3 +++ src/or/buffers.c | 14 ++++++++++- src/test/test_socks.c | 36 ++++++++++++++++++++++++++++ src/test/test_util.c | 48 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 167 insertions(+), 1 deletion(-) diff --cc src/or/buffers.c index 080d8fb,691845e..bd33fe4 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@@ -2054,8 -2048,18 +2054,20 @@@ parse_socks(const char *data, size_t da req->address[len] = 0; req->port = ntohs(get_uint16(data+5+len)); *drain_out = 5+len+2; - if (!tor_strisprint(req->address) || strchr(req->address,'\"')) { + + if (string_is_valid_ipv4_address(req->address) || + string_is_valid_ipv6_address(req->address)) { + log_unsafe_socks_warning(5,req->address,req->port,safe_socks); + + if (safe_socks) { + socks_request_set_socks5_error(req, SOCKS5_NOT_ALLOWED); + return -1; + } + } + + if (!string_is_valid_hostname(req->address)) { + socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR); ++ log_warn(LD_PROTOCOL, "Your application (using socks5 to port %d) gave Tor " "a malformed hostname: %s. Rejecting the connection.",

1 0

[tor/master] fix indentation
by nickm＠torproject.org 04 Nov '14

04 Nov '14

commit 74cbd8d55953969c15c60f025889b1e07a185a79 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 4 00:46:32 2014 -0500 fix indentation --- src/common/util.c | 56 ++++++++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 8300299..c6a3989 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -963,9 +963,9 @@ string_is_key_value(int severity, const char *string) int string_is_valid_ipv4_address(const char *string) { - struct in_addr addr; + struct in_addr addr; - return (tor_inet_pton(AF_INET,string,&addr) == 1); + return (tor_inet_pton(AF_INET,string,&addr) == 1); } /** Return true if string represents a valid IPv6 address in @@ -974,9 +974,9 @@ string_is_valid_ipv4_address(const char *string) int string_is_valid_ipv6_address(const char *string) { - struct in6_addr addr; + struct in6_addr addr; - return (tor_inet_pton(AF_INET6,string,&addr) == 1); + return (tor_inet_pton(AF_INET6,string,&addr) == 1); } /** Return true iff string matches a pattern of DNS names @@ -985,38 +985,38 @@ string_is_valid_ipv6_address(const char *string) int string_is_valid_hostname(const char *string) { - int result = 1; - smartlist_t *components; + int result = 1; + smartlist_t *components; - components = smartlist_new(); + components = smartlist_new(); - smartlist_split_string(components,string,".",0,0); + smartlist_split_string(components,string,".",0,0); - SMARTLIST_FOREACH_BEGIN(components, char *, c) { - if (c[0] == '-') { - result = 0; - break; - } + SMARTLIST_FOREACH_BEGIN(components, char *, c) { + if (c[0] == '-') { + result = 0; + break; + } - do { - if ((*c >= 'a' && *c <= 'z') || - (*c >= 'A' && *c <= 'Z') || - (*c >= '0' && *c <= '9') || - (*c == '-')) - c++; - else - result = 0; - } while (result && *c); + do { + if ((*c >= 'a' && *c <= 'z') || + (*c >= 'A' && *c <= 'Z') || + (*c >= '0' && *c <= '9') || + (*c == '-')) + c++; + else + result = 0; + } while (result && *c); - } SMARTLIST_FOREACH_END(c); + } SMARTLIST_FOREACH_END(c); - SMARTLIST_FOREACH_BEGIN(components, char *, c) { - tor_free(c); - } SMARTLIST_FOREACH_END(c); + SMARTLIST_FOREACH_BEGIN(components, char *, c) { + tor_free(c); + } SMARTLIST_FOREACH_END(c); - smartlist_free(components); + smartlist_free(components); - return result; + return result; } /** Return true iff the DIGEST256_LEN bytes in digest are all zero. */

1 0