March 2018 - tor-commits - lists.torproject.org

[tor/release-0.3.3] Allow IPv6 address strings to be used as hostnames in SOCKS5 requests
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 0e453929d21b030832b0c48fceac0c5688657e15 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sun Feb 11 15:22:41 2018 +0100 Allow IPv6 address strings to be used as hostnames in SOCKS5 requests --- src/common/util.c | 11 +++++++++++ src/common/util.h | 1 + src/or/proto_socks.c | 4 ++-- src/test/test_socks.c | 9 +++++---- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 90204befc..1818b4f19 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1071,6 +1071,17 @@ string_is_valid_ipv6_address(const char *string) return (tor_inet_pton(AF_INET6,string,&addr) == 1); } +/** Return true iff string is a valid destination address, + * i.e. either a DNS hostname or IPv4/IPv6 address string. + */ +int +string_is_valid_dest(const char *string) +{ + return string_is_valid_ipv4_address(string) || + string_is_valid_ipv6_address(string) || + string_is_valid_hostname(string); +} + /** 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 2ee0ea28c..d6bda8036 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -233,6 +233,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_dest(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); diff --git a/src/or/proto_socks.c b/src/or/proto_socks.c index 91633d02a..8700fe126 100644 --- a/src/or/proto_socks.c +++ b/src/or/proto_socks.c @@ -393,7 +393,7 @@ 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_hostname(req->address)) { + if (!string_is_valid_dest(req->address)) { socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR); log_warn(LD_PROTOCOL, @@ -518,7 +518,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, log_debug(LD_APP,"socks4: Everything is here. Success."); strlcpy(req->address, startaddr ? startaddr : tmpbuf, sizeof(req->address)); - if (!string_is_valid_hostname(req->address)) { + if (!string_is_valid_dest(req->address)) { log_warn(LD_PROTOCOL, "Your application (using socks4 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 9ae7530e2..70509e43e 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -347,17 +347,18 @@ test_socks_5_supported_commands(void *ptr) socks_request_clear(socks); - /* SOCKS 5 should NOT reject RESOLVE [F0] reject for IPv6 address + /* SOCKS 5 should NOT reject RESOLVE [F0] request 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, "\x05\xF0\x00\x03\x29"); + ADD_DATA(buf, "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]"); ADD_DATA(buf, "\x01\x02"); tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, 1), OP_EQ, -1); - tt_str_op("2001:0db8:85a3:0000:0000:8a2e:0370:7334", OP_EQ, socks->address); + tt_str_op("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", OP_EQ, + socks->address); tt_int_op(258, OP_EQ, socks->port); tt_int_op(0, OP_EQ, buf_datalen(buf));

1 0

[tor/master] Rename string_is_valid_hostname -> string_is_valid_nonrfc_hostname
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit b504c854d34a938943f68c3036840f10a84fcea4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 07:42:27 2018 -0400 Rename string_is_valid_hostname -> string_is_valid_nonrfc_hostname Per discussion on 25055. --- src/common/util.c | 4 ++-- src/common/util.h | 2 +- src/test/test_util.c | 55 ++++++++++++++++++++++++++-------------------------- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 5ae4d0408..90aaf0ebe 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1094,7 +1094,7 @@ string_is_valid_dest(const char *string) retval = string_is_valid_ipv4_address(string) || string_is_valid_ipv6_address(string) || - string_is_valid_hostname(string); + string_is_valid_nonrfc_hostname(string); tor_free(tmp); @@ -1108,7 +1108,7 @@ string_is_valid_dest(const char *string) * with misconfigured zones that have been encountered in the wild. */ int -string_is_valid_hostname(const char *string) +string_is_valid_nonrfc_hostname(const char *string) { int result = 1; int has_trailing_dot; diff --git a/src/common/util.h b/src/common/util.h index d6bda8036..938078912 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -234,7 +234,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_dest(const char *string); -int string_is_valid_hostname(const char *string); +int string_is_valid_nonrfc_hostname(const char *string); int string_is_valid_ipv4_address(const char *string); int string_is_valid_ipv6_address(const char *string); diff --git a/src/test/test_util.c b/src/test/test_util.c index c734426a5..036f739b8 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5559,56 +5559,57 @@ 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")); + tt_assert(string_is_valid_nonrfc_hostname("torproject.org")); + tt_assert(string_is_valid_nonrfc_hostname("ocw.mit.edu")); + tt_assert(string_is_valid_nonrfc_hostname("i.4cdn.org")); + tt_assert(string_is_valid_nonrfc_hostname("stanford.edu")); + tt_assert(string_is_valid_nonrfc_hostname("multiple-words-with-hypens.jp")); // Subdomain name cannot start with '-' or '_'. - 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")); - tt_assert(!string_is_valid_hostname("___abc.org")); + tt_assert(!string_is_valid_nonrfc_hostname("-torproject.org")); + tt_assert(!string_is_valid_nonrfc_hostname("subdomain.-domain.org")); + tt_assert(!string_is_valid_nonrfc_hostname("-subdomain.domain.org")); + tt_assert(!string_is_valid_nonrfc_hostname("___abc.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("\xff\xffxyz.org")); - tt_assert(!string_is_valid_hostname("word1 word2.net")); + tt_assert(!string_is_valid_nonrfc_hostname("%%domain.\\org.")); + tt_assert(!string_is_valid_nonrfc_hostname("***x.net")); + tt_assert(!string_is_valid_nonrfc_hostname("\xff\xffxyz.org")); + tt_assert(!string_is_valid_nonrfc_hostname("word1 word2.net")); // Test workaround for nytimes.com stupidity, technically invalid, // but we allow it since they are big, even though they are failing to // comply with a ~30 year old standard. - tt_assert(string_is_valid_hostname("core3_euw1.fabrik.nytimes.com")); + tt_assert(string_is_valid_nonrfc_hostname("core3_euw1.fabrik.nytimes.com")); // Firefox passes FQDNs with trailing '.'s directly to the SOCKS proxy, // which is redundant since the spec states DOMAINNAME addresses are fully // qualified. While unusual, this should be tollerated. - tt_assert(string_is_valid_hostname("core9_euw1.fabrik.nytimes.com.")); - tt_assert(!string_is_valid_hostname("..washingtonpost.is.better.com")); - tt_assert(!string_is_valid_hostname("so.is..ft.com")); - tt_assert(!string_is_valid_hostname("...")); + tt_assert(string_is_valid_nonrfc_hostname("core9_euw1.fabrik.nytimes.com.")); + tt_assert(!string_is_valid_nonrfc_hostname( + "..washingtonpost.is.better.com")); + tt_assert(!string_is_valid_nonrfc_hostname("so.is..ft.com")); + tt_assert(!string_is_valid_nonrfc_hostname("...")); // XXX: do we allow single-label DNS names? // We shouldn't for SOCKS (spec says "contains a fully-qualified domain name" // but only test pathologically malformed traling '.' cases for now. - tt_assert(!string_is_valid_hostname(".")); - tt_assert(!string_is_valid_hostname("..")); + tt_assert(!string_is_valid_nonrfc_hostname(".")); + tt_assert(!string_is_valid_nonrfc_hostname("..")); // IP address strings are not hostnames. - tt_assert(!string_is_valid_hostname("8.8.8.8")); - tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]")); - tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e")); + tt_assert(!string_is_valid_nonrfc_hostname("8.8.8.8")); + tt_assert(!string_is_valid_nonrfc_hostname("[2a00:1450:401b:800::200e]")); + tt_assert(!string_is_valid_nonrfc_hostname("2a00:1450:401b:800::200e")); // We allow alphanumeric TLDs. For discussion, see ticket #25055. - tt_assert(string_is_valid_hostname("lucky.13")); - tt_assert(string_is_valid_hostname("luck.y13")); - tt_assert(string_is_valid_hostname("luck.y13.")); + tt_assert(string_is_valid_nonrfc_hostname("lucky.13")); + tt_assert(string_is_valid_nonrfc_hostname("luck.y13")); + tt_assert(string_is_valid_nonrfc_hostname("luck.y13.")); // We allow punycode TLDs. For examples, see // http://data.iana.org/TLD/tlds-alpha-by-domain.txt - tt_assert(string_is_valid_hostname("example.xn--l1acc")); + tt_assert(string_is_valid_nonrfc_hostname("example.xn--l1acc")); done: return;

1 0

[tor/master] Add a paranoia check in string_is_valid_nonrfc_hostname()
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit d4bf1f6c8eb08c39def69c839515afe475bf0a6b Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 07:48:18 2018 -0400 Add a paranoia check in string_is_valid_nonrfc_hostname() The earlier checks in this function should ensure that components is always nonempty. But in case somebody messes with them in the future, let's add an extra check to make sure we aren't crashing. --- src/common/util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/common/util.c b/src/common/util.c index 90aaf0ebe..a68fd30d0 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1125,6 +1125,9 @@ string_is_valid_nonrfc_hostname(const char *string) smartlist_split_string(components,string,".",0,0); + if (BUG(smartlist_len(components) == 0)) + return 0; // LCOV_EXCL_LINE should be impossible given the earlier checks. + /* Allow a single terminating '.' used rarely to indicate domains * are FQDNs rather than relative. */ last_label = (char *)smartlist_get(components,

1 0

[tor/master] Merge branch 'maint-0.3.3'
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 6317aa2cc03a3b7a6f9fd4a4bdd99447f7914493 Merge: fa6eaab83 d416e208e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 07:50:47 2018 -0400 Merge branch 'maint-0.3.3' changes/bugs_25036_25055 | 7 +++++ src/common/util.c | 72 ++++++++++++++++++++++++++++++++++++++---------- src/common/util.h | 3 +- src/or/proto_socks.c | 4 +-- src/test/test_socks.c | 24 ++++++++++++++-- src/test/test_util.c | 68 +++++++++++++++++++++++++++++++-------------- 6 files changed, 138 insertions(+), 40 deletions(-)

1 0

[tor/master] Merge branch 'bugs25036_25055_clean_033' into maint-0.3.3
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit d416e208e4e94c2b4ccd4e11013151c72180faff Merge: ddee28a3c d4bf1f6c8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 07:49:34 2018 -0400 Merge branch 'bugs25036_25055_clean_033' into maint-0.3.3 changes/bugs_25036_25055 | 7 +++++ src/common/util.c | 72 ++++++++++++++++++++++++++++++++++++++---------- src/common/util.h | 3 +- src/or/proto_socks.c | 4 +-- src/test/test_socks.c | 24 ++++++++++++++-- src/test/test_util.c | 68 +++++++++++++++++++++++++++++++-------------- 6 files changed, 138 insertions(+), 40 deletions(-)

1 0

[tor/release-0.3.3] Adding changes file
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit df529c60936ef290c917d09d51820680cd31cc8b Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sun Feb 11 18:16:51 2018 +0100 Adding changes file --- changes/bugs_25036_25055 | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/changes/bugs_25036_25055 b/changes/bugs_25036_25055 new file mode 100644 index 000000000..daa46321c --- /dev/null +++ b/changes/bugs_25036_25055 @@ -0,0 +1,7 @@ + o Minor bugfixes (networking): + - Tor will not reject IPv6 address strings from TorBrowser when they + are passed as hostnames in SOCKS5 requests. Fixes bug 25036, + bugfix on Tor 0.3.1.2. + - string_is_valid_hostname() will not consider IP strings to be valid + hostnames. Fixes bug 25055; bugfix on Tor 0.2.5.5. +

1 0

[tor/release-0.3.3] Also test bracket-less IPv6 string validation
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 12afd8bfed96c57cd47b18644c2030673496a74f Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 12 21:16:38 2018 +0100 Also test bracket-less IPv6 string validation --- src/test/test_socks.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/test_socks.c b/src/test/test_socks.c index 3f9cc887b..8da7191e8 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -365,6 +365,23 @@ test_socks_5_supported_commands(void *ptr) socks_request_clear(socks); + /* Also allow bracket-less form. */ + + 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_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, 1), + OP_EQ, 1); + + tt_str_op("2001:0db8:85a3:0000:0000:8a2e:0370:7334", OP_EQ, + socks->address); + tt_int_op(258, OP_EQ, socks->port); + + tt_int_op(0, OP_EQ, 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");

1 0

[tor/release-0.3.3] Fix bracketed IPv6 string validation
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit b0ba4aa7e98af030e0e1be19a58ab7a6f00fa423 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 12 19:52:47 2018 +0100 Fix bracketed IPv6 string validation --- src/common/util.c | 15 ++++++++++++++- src/test/test_socks.c | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 7c715fb3c..ea0ec3dae 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1079,9 +1079,22 @@ string_is_valid_ipv6_address(const char *string) int string_is_valid_dest(const char *string) { - return string_is_valid_ipv4_address(string) || + char *tmp = NULL; + int retval; + + tor_assert(string); + tor_assert(strlen(string) > 0); + + if (string[0] == '[' && string[strlen(string) - 1] == ']') + string = tmp = tor_strndup(string + 1, strlen(string) - 2); + + retval = string_is_valid_ipv4_address(string) || string_is_valid_ipv6_address(string) || string_is_valid_hostname(string); + + tor_free(tmp); + + return retval; } /** Return true iff string matches a pattern of DNS names diff --git a/src/test/test_socks.c b/src/test/test_socks.c index 70509e43e..3f9cc887b 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -355,7 +355,7 @@ test_socks_5_supported_commands(void *ptr) ADD_DATA(buf, "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]"); ADD_DATA(buf, "\x01\x02"); tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, 1), - OP_EQ, -1); + OP_EQ, 1); tt_str_op("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", OP_EQ, socks->address);

1 0

[tor/release-0.3.3] Do not consider IP strings valid DNS names. Fixes #25055
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 1af016e96e133718546b55c8b7fafd3345aaeeb8 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sun Feb 11 16:39:23 2018 +0100 Do not consider IP strings valid DNS names. Fixes #25055 --- src/common/util.c | 23 +++++++++++++++-------- src/test/test_util.c | 5 +++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 1818b4f19..7c715fb3c 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -100,6 +100,8 @@ #undef MALLOC_ZERO_WORKS #endif +#include <ctype.h> + /* ===== * Memory management * ===== */ @@ -1110,16 +1112,21 @@ string_is_valid_hostname(const char *string) continue; } - do { - if ((*c >= 'a' && *c <= 'z') || - (*c >= 'A' && *c <= 'Z') || - (*c >= '0' && *c <= '9') || - (*c == '-') || (*c == '_')) + if (c_sl_idx == c_sl_len - 1) { + do { + result = isalpha(*c); c++; - else - result = 0; - } while (result && *c); + } while (result && *c); + } else { + do { + result = (isalnum(*c) || (*c == '-') || (*c == '_')); + c++; + } while (result > 0 && *c); + } + if (result == 0) { + break; + } } SMARTLIST_FOREACH_END(c); SMARTLIST_FOREACH_BEGIN(components, char *, c) { diff --git a/src/test/test_util.c b/src/test/test_util.c index b67fad58e..2fa03e5bc 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5584,6 +5584,11 @@ test_util_hostname_validation(void *arg) tt_assert(!string_is_valid_hostname(".")); tt_assert(!string_is_valid_hostname("..")); + // IP address strings are not hostnames. + tt_assert(!string_is_valid_hostname("8.8.8.8")); + tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]")); + tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e")); + done: return; }

1 0

[tor/release-0.3.3] Call strlen() once
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 5986589b48de6addf99436df1feeea1362767acb Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Mon Feb 12 21:08:17 2018 +0100 Call strlen() once --- src/common/util.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index ea0ec3dae..d8891c6a5 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1081,12 +1081,13 @@ string_is_valid_dest(const char *string) { char *tmp = NULL; int retval; + size_t len = strlen(string); tor_assert(string); - tor_assert(strlen(string) > 0); + tor_assert(len > 0); - if (string[0] == '[' && string[strlen(string) - 1] == ']') - string = tmp = tor_strndup(string + 1, strlen(string) - 2); + if (string[0] == '[' && string[len - 1] == ']') + string = tmp = tor_strndup(string + 1, len - 2); retval = string_is_valid_ipv4_address(string) || string_is_valid_ipv6_address(string) ||

1 0