June 2019 - tor-commits - lists.torproject.org

[tor/release-0.4.1] Bug 30649: Changes file.
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 9bef75a29a009f057c69c730b6bf3eccfc0c4b45 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Thu May 30 15:18:16 2019 -0700 Bug 30649: Changes file. --- changes/bug30649 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changes/bug30649 b/changes/bug30649 new file mode 100644 index 000000000..3f5768bcb --- /dev/null +++ b/changes/bug30649 @@ -0,0 +1,4 @@ + o Minor bugfixes (circuit padding): + - On relays, properly check that a padding machine is absent before + logging a warn about it being absent. Fixes bug 30649; + bugfix on 0.4.1.1-alpha.

1 0

[tor/release-0.4.1] circpad: some more logging changes.
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit a15ec8bf84db2dcadcc292b9672fbd6d35ffdf77 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Jun 11 14:28:38 2019 +0300 circpad: some more logging changes. - Add an info log when receiving a STOP command. - Keep warning if we receive padding from a wrong hop. --- src/core/or/circuitpadding.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/or/circuitpadding.c b/src/core/or/circuitpadding.c index ebad6518b..7521fd339 100644 --- a/src/core/or/circuitpadding.c +++ b/src/core/or/circuitpadding.c @@ -2821,6 +2821,8 @@ circpad_handle_padding_negotiate(circuit_t *circ, cell_t *cell) /* Free the machine corresponding to this machine type */ if (free_circ_machineinfos_with_machine_num(circ, negotiate->machine_type)) { + log_info(LD_CIRC, "Received STOP command for machine %u", + negotiate->machine_type); goto done; } log_fn(LOG_PROTOCOL_WARN, LD_CIRC, @@ -2871,7 +2873,7 @@ circpad_handle_padding_negotiated(circuit_t *circ, cell_t *cell, /* Verify this came from the expected hop */ if (!circpad_padding_is_from_expected_hop(circ, layer_hint)) { - log_fn(LOG_PROTOCOL_WARN, LD_CIRC, + log_fn(LOG_WARN, LD_CIRC, "Padding negotiated cell from wrong hop!"); return -1; }

1 0

[tor/release-0.4.1] More LOG_PROTOCOL_WARN.
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit e54ce03b4f439d7cdf9d0d0da3be11b72a1a83fd Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Wed Jun 5 12:33:39 2019 -0700 More LOG_PROTOCOL_WARN. Make origin-side messages about padding negotiation failure into LOG_PROTOCOL_WARN. I'm not sure I like this either.. But the negotiation refusal case might happen naturally due to consensus drift, and is functionally no different than a corrupted cell. --- src/core/or/circuitpadding.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/or/circuitpadding.c b/src/core/or/circuitpadding.c index 84650a5a9..ebad6518b 100644 --- a/src/core/or/circuitpadding.c +++ b/src/core/or/circuitpadding.c @@ -2805,7 +2805,7 @@ circpad_handle_padding_negotiate(circuit_t *circ, cell_t *cell) circpad_negotiate_t *negotiate; if (CIRCUIT_IS_ORIGIN(circ)) { - log_fn(LOG_WARN, LD_CIRC, + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, "Padding negotiate cell unsupported at origin."); return -1; } @@ -2871,7 +2871,7 @@ circpad_handle_padding_negotiated(circuit_t *circ, cell_t *cell, /* Verify this came from the expected hop */ if (!circpad_padding_is_from_expected_hop(circ, layer_hint)) { - log_fn(LOG_WARN, LD_CIRC, + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, "Padding negotiated cell from wrong hop!"); return -1; } @@ -2898,7 +2898,7 @@ circpad_handle_padding_negotiated(circuit_t *circ, cell_t *cell, // and be sad free_circ_machineinfos_with_machine_num(circ, negotiated->machine_type); TO_ORIGIN_CIRCUIT(circ)->padding_negotiation_failed = 1; - log_fn(LOG_INFO, LD_CIRC, + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, "Middle node did not accept our padding request."); }

1 0

[tor/release-0.4.1] Make some warns into protocol warns
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 6263755728f62b2535535f68960e601dccaead9d Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Thu May 30 16:20:56 2019 -0700 Make some warns into protocol warns I'm not sure I agree with this option. --- src/core/or/circuitpadding.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/or/circuitpadding.c b/src/core/or/circuitpadding.c index ae32713bf..84650a5a9 100644 --- a/src/core/or/circuitpadding.c +++ b/src/core/or/circuitpadding.c @@ -2805,14 +2805,14 @@ circpad_handle_padding_negotiate(circuit_t *circ, cell_t *cell) circpad_negotiate_t *negotiate; if (CIRCUIT_IS_ORIGIN(circ)) { - log_fn(LOG_WARN, LD_PROTOCOL, + log_fn(LOG_WARN, LD_CIRC, "Padding negotiate cell unsupported at origin."); return -1; } if (circpad_negotiate_parse(&negotiate, cell->payload+RELAY_HEADER_SIZE, CELL_PAYLOAD_SIZE-RELAY_HEADER_SIZE) < 0) { - log_fn(LOG_WARN, LD_CIRC, + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, "Received malformed PADDING_NEGOTIATE cell; dropping."); return -1; } @@ -2823,7 +2823,7 @@ circpad_handle_padding_negotiate(circuit_t *circ, cell_t *cell) negotiate->machine_type)) { goto done; } - log_fn(LOG_WARN, LD_CIRC, + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, "Received circuit padding stop command for unknown machine."); goto err; } else if (negotiate->command == CIRCPAD_COMMAND_START) { @@ -2864,21 +2864,21 @@ circpad_handle_padding_negotiated(circuit_t *circ, cell_t *cell, circpad_negotiated_t *negotiated; if (!CIRCUIT_IS_ORIGIN(circ)) { - log_fn(LOG_WARN, LD_PROTOCOL, + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, "Padding negotiated cell unsupported at non-origin."); return -1; } /* Verify this came from the expected hop */ if (!circpad_padding_is_from_expected_hop(circ, layer_hint)) { - log_fn(LOG_WARN, LD_PROTOCOL, + log_fn(LOG_WARN, LD_CIRC, "Padding negotiated cell from wrong hop!"); return -1; } if (circpad_negotiated_parse(&negotiated, cell->payload+RELAY_HEADER_SIZE, CELL_PAYLOAD_SIZE-RELAY_HEADER_SIZE) < 0) { - log_fn(LOG_WARN, LD_CIRC, + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, "Received malformed PADDING_NEGOTIATED cell; " "dropping."); return -1;

1 0

[tor/release-0.4.1] Merge remote-tracking branch 'tor-github/pr/1101' into maint-0.4.1
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 3ccf91027b9fa39182372060e3e92841403f4b5b Merge: 648d5df62 a15ec8bf8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 26 10:05:51 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1101' into maint-0.4.1 changes/bug30649 | 4 ++++ src/core/or/circuitpadding.c | 33 +++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 10 deletions(-)

1 0

[tor/release-0.4.1] Merge branch 'maint-0.4.1' into release-0.4.1
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 4ff08309866e62cd6ace83813843c95177f60433 Merge: 89ca35ff5 3ccf91027 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 26 10:06:02 2019 -0400 Merge branch 'maint-0.4.1' into release-0.4.1 changes/bug30649 | 4 ++++ src/core/or/circuitpadding.c | 33 +++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 10 deletions(-)

1 0

[tor/master] resolve: consistently parse IP addresses in square brackets
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit cd1de99468aa4d7b48c9f3bcb7c7d4fb5b3bfe9a Author: teor <teor(a)torproject.org> Date: Sun Jun 2 17:46:58 2019 +1000 resolve: consistently parse IP addresses in square brackets When parsing addreses via Tor's internal DNS lookup API: * reject IPv4 addresses in square brackets (with or without a port), * accept IPv6 addresses in square brackets (with or without a port), and * accept IPv6 addresses without square brackets, as long as they have no port. This change completes the work started in 23082, making address parsing consistent between tor's internal DNS lookup and address parsing APIs. Fixes bug 30721; bugfix on 0.2.1.5-alpha. --- changes/bug30721 | 6 +++++ src/lib/net/address.c | 31 +++++++++++++++++---- src/lib/net/resolve.c | 75 ++++++++++++++++++++++++++------------------------- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/changes/bug30721 b/changes/bug30721 new file mode 100644 index 000000000..5e0365300 --- /dev/null +++ b/changes/bug30721 @@ -0,0 +1,6 @@ + o Minor bugfixes (networking): + - When parsing addreses via Tor's internal DNS lookup API, reject IPv4 + addresses in square brackets, and accept IPv6 addresses in square + brackets. This change completes the work started in 23082, making + address parsing consistent between tor's internal DNS lookup and address + parsing APIs. Fixes bug 30721; bugfix on 0.2.1.5-alpha. diff --git a/src/lib/net/address.c b/src/lib/net/address.c index 546af800a..4989e4ab2 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -373,7 +373,8 @@ tor_addr_to_str(char *dest, const tor_addr_t *addr, size_t len, int decorate) * * If accept_regular is set and the address is in neither recognized * reverse lookup hostname format, try parsing the address as a regular - * IPv4 or IPv6 address too. + * IPv4 or IPv6 address too. This mode will accept IPv6 addresses with or + * without square brackets. */ int tor_addr_parse_PTR_name(tor_addr_t *result, const char *address, @@ -1204,6 +1205,10 @@ tor_addr_parse(tor_addr_t *addr, const char *src) tor_assert(addr && src); + /* Clear the address before starting, to avoid returning uninitialised data. + */ + memset(addr, 0, sizeof(tor_addr_t)); + size_t len = strlen(src); if (len && src[0] == '[' && src[len - 1] == ']') { @@ -1718,6 +1723,11 @@ get_interface_address6_list,(int severity, * form "ip" or "ip:0". Otherwise, accept those forms, and set * *port_out to default_port. * + * This function accepts: + * - IPv6 address and port, when the IPv6 address is in square brackets, + * - IPv6 address with square brackets, + * - IPv6 address without square brackets. + * * Return 0 on success, -1 on failure. */ int tor_addr_port_parse(int severity, const char *addrport, @@ -1755,9 +1765,17 @@ tor_addr_port_parse(int severity, const char *addrport, } /** Given an address of the form "host[:port]", try to divide it into its host - * and port portions, setting *address_out to a newly allocated string - * holding the address portion and *port_out to the port (or 0 if no - * port is given). Return 0 on success, -1 on failure. */ + * and port portions. + * + * Like tor_addr_port_parse(), this function accepts: + * - IPv6 address and port, when the IPv6 address is in square brackets, + * - IPv6 address with square brackets, + * - IPv6 address without square brackets. + * + * Sets *address_out to a newly allocated string holding the address + * portion, and *port_out to the port (or 0 if no port is given). + * + * Return 0 on success, -1 on failure. */ int tor_addr_port_split(int severity, const char *addrport, char **address_out, uint16_t *port_out) @@ -1766,8 +1784,11 @@ tor_addr_port_split(int severity, const char *addrport, tor_assert(addrport); tor_assert(address_out); tor_assert(port_out); + /* We need to check for IPv6 manually because the logic below doesn't - * do a good job on IPv6 addresses that lack a port. */ + * do a good job on IPv6 addresses that lack a port. + * If an IPv6 address without square brackets is ambiguous, it gets parsed + * here as an address, rather than address:port. */ if (tor_addr_parse(&a_tmp, addrport) == AF_INET6) { *port_out = 0; *address_out = tor_strdup(addrport); diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index 49c263faa..71c033fe7 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -35,6 +35,8 @@ * *addr to the proper IP address, in host byte order. Returns 0 * on success, -1 on failure; 1 on transient failure. * + * This function only accepts IPv4 addresses. + * * (This function exists because standard windows gethostbyname * doesn't treat raw IP addresses properly.) */ @@ -62,6 +64,9 @@ tor_lookup_hostname,(const char *name, uint32_t *addr)) * preferred family, though another one may be returned if only one * family is implemented for this address. * + * Like tor_addr_parse(), this function accepts IPv6 addresses with or without + * square brackets. + * * Return 0 on success, -1 on failure; 1 on transient failure. */ MOCK_IMPL(int, @@ -70,26 +75,39 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) /* Perhaps eventually this should be replaced by a tor_getaddrinfo or * something. */ - struct in_addr iaddr; - struct in6_addr iaddr6; + int parsed_family = 0; + tor_assert(name); tor_assert(addr); tor_assert(family == AF_INET || family == AF_INET6 || family == AF_UNSPEC); + + /* Clear address before starting, to avoid returning uninitialised data */ + memset(addr, 0, sizeof(tor_addr_t)); + if (!*name) { /* Empty address is an error. */ return -1; - } else if (tor_inet_pton(AF_INET, name, &iaddr)) { + } + + /* Is it an IP address? */ + parsed_family = tor_addr_parse(addr, name); + + if (parsed_family == AF_INET) { /* It's an IPv4 IP. */ - if (family == AF_INET6) + if (family == AF_INET6) { + memset(addr, 0, sizeof(tor_addr_t)); return -1; - tor_addr_from_in(addr, &iaddr); + } return 0; - } else if (tor_inet_pton(AF_INET6, name, &iaddr6)) { - if (family == AF_INET) + } else if (parsed_family == AF_INET6) { + if (family == AF_INET) { + memset(addr, 0, sizeof(tor_addr_t)); return -1; - tor_addr_from_in6(addr, &iaddr6); + } return 0; } else { + /* Clear the address after a failed tor_addr_parse(). */ + memset(addr, 0, sizeof(tor_addr_t)); #ifdef HAVE_GETADDRINFO int err; struct addrinfo *res=NULL, *res_p; @@ -179,52 +197,35 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) /** Parse an address or address-port combination from s, resolve the * address as needed, and put the result in addr_out and (optionally) - * port_out. Return 0 on success, negative on failure. */ + * port_out. + * + * Like tor_addr_port_parse(), this function accepts: + * - IPv6 address and port, when the IPv6 address is in square brackets, + * - IPv6 address with square brackets, + * - IPv6 address without square brackets. + * + * Return 0 on success, negative on failure. */ int tor_addr_port_lookup(const char *s, tor_addr_t *addr_out, uint16_t *port_out) { - const char *port; tor_addr_t addr; uint16_t portval; char *tmp = NULL; + int rv = 0; tor_assert(s); tor_assert(addr_out); s = eat_whitespace(s); - if (*s == '[') { - port = strstr(s, "]"); - if (!port) - goto err; - tmp = tor_strndup(s+1, port-(s+1)); - port = port+1; - if (*port == ':') - port++; - else - port = NULL; - } else { - port = strchr(s, ':'); - if (port) - tmp = tor_strndup(s, port-s); - else - tmp = tor_strdup(s); - if (port) - ++port; - } + rv = tor_addr_port_split(LOG_WARN, s, &tmp, &portval); + if (rv < 0) + goto err; if (tor_addr_lookup(tmp, AF_UNSPEC, &addr) != 0) goto err; tor_free(tmp); - if (port) { - portval = (int) tor_parse_long(port, 10, 1, 65535, NULL, NULL); - if (!portval) - goto err; - } else { - portval = 0; - } - if (port_out) *port_out = portval; tor_addr_copy(addr_out, &addr);

1 0

[tor/master] resolve: split sub-functions out of tor_addr_lookup()
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit fb93646c1c2e97a978045019403d9cfb43f55cbc Author: teor <teor(a)torproject.org> Date: Sun Jun 2 18:42:01 2019 +1000 resolve: split sub-functions out of tor_addr_lookup() And remove the practracker exception for tor_addr_lookup(). Cleanup after 30721. --- scripts/maint/practracker/exceptions.txt | 1 - src/lib/net/resolve.c | 221 +++++++++++++++++++------------ 2 files changed, 134 insertions(+), 88 deletions(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 3a32e97be..4c1ae1ba5 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -272,7 +272,6 @@ problem function-size /src/lib/math/prob_distr.c:sample_uniform_interval() 145 problem function-size /src/lib/net/address.c:tor_addr_parse_mask_ports() 198 problem function-size /src/lib/net/address.c:tor_addr_compare_masked() 111 problem function-size /src/lib/net/inaddr.c:tor_inet_pton() 107 -problem function-size /src/lib/net/resolve.c:tor_addr_lookup() 110 problem function-size /src/lib/net/socketpair.c:tor_ersatz_socketpair() 102 problem function-size /src/lib/osinfo/uname.c:get_uname() 116 problem function-size /src/lib/process/process_unix.c:process_unix_exec() 220 diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index 1b68a0b33..a70fb0a82 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -58,6 +58,116 @@ tor_lookup_hostname,(const char *name, uint32_t *addr)) return -1; } +#ifdef HAVE_GETADDRINFO + +/* Host lookup helper for tor_addr_lookup(), when getaddrinfo() is + * available on this system. + * + * See tor_addr_lookup() for details. + */ +static int +tor_addr_lookup_host_getaddrinfo(const char *name, + uint16_t family, + tor_addr_t *addr) +{ + int err; + struct addrinfo *res=NULL, *res_p; + struct addrinfo *best=NULL; + struct addrinfo hints; + int result = -1; + memset(&hints, 0, sizeof(hints)); + hints.ai_family = family; + hints.ai_socktype = SOCK_STREAM; + err = tor_getaddrinfo(name, NULL, &hints, &res); + /* The check for 'res' here shouldn't be necessary, but it makes static + * analysis tools happy. */ + if (!err && res) { + best = NULL; + for (res_p = res; res_p; res_p = res_p->ai_next) { + if (family == AF_UNSPEC) { + if (res_p->ai_family == AF_INET) { + best = res_p; + break; + } else if (res_p->ai_family == AF_INET6 && !best) { + best = res_p; + } + } else if (family == res_p->ai_family) { + best = res_p; + break; + } + } + if (!best) + best = res; + if (best->ai_family == AF_INET) { + tor_addr_from_in(addr, + &((struct sockaddr_in*)best->ai_addr)->sin_addr); + result = 0; + } else if (best->ai_family == AF_INET6) { + tor_addr_from_in6(addr, + &((struct sockaddr_in6*)best->ai_addr)->sin6_addr); + result = 0; + } + tor_freeaddrinfo(res); + return result; + } + return (err == EAI_AGAIN) ? 1 : -1; +} + +#else /* !(defined(HAVE_GETADDRINFO)) */ + +/* Host lookup helper for tor_addr_lookup(), which calls getaddrinfo(). + * Used when gethostbyname() is not available on this system. + * + * See tor_addr_lookup() for details. + */ +static int +tor_addr_lookup_host_gethostbyname(const char *name, + tor_addr_t *addr) +{ + struct hostent *ent; + int err; +#ifdef HAVE_GETHOSTBYNAME_R_6_ARG + char buf[2048]; + struct hostent hostent; + int r; + r = gethostbyname_r(name, &hostent, buf, sizeof(buf), &ent, &err); +#elif defined(HAVE_GETHOSTBYNAME_R_5_ARG) + char buf[2048]; + struct hostent hostent; + ent = gethostbyname_r(name, &hostent, buf, sizeof(buf), &err); +#elif defined(HAVE_GETHOSTBYNAME_R_3_ARG) + struct hostent_data data; + struct hostent hent; + memset(&data, 0, sizeof(data)); + err = gethostbyname_r(name, &hent, &data); + ent = err ? NULL : &hent; +#else + ent = gethostbyname(name); +#ifdef _WIN32 + err = WSAGetLastError(); +#else + err = h_errno; +#endif /* defined(_WIN32) */ +#endif /* defined(HAVE_GETHOSTBYNAME_R_6_ARG) || ... */ + if (ent) { + if (ent->h_addrtype == AF_INET) { + tor_addr_from_in(addr, (struct in_addr*) ent->h_addr); + } else if (ent->h_addrtype == AF_INET6) { + tor_addr_from_in6(addr, (struct in6_addr*) ent->h_addr); + } else { + tor_assert(0); // LCOV_EXCL_LINE: gethostbyname() returned bizarre type + } + return 0; + } +#ifdef _WIN32 + return (err == WSATRY_AGAIN) ? 1 : -1; +#else + return (err == TRY_AGAIN) ? 1 : -1; +#endif +} + +#endif /* defined(HAVE_GETADDRINFO) */ + /** Similar behavior to Unix gethostbyname: resolve name, and set * *addr to the proper IP address and family. The family * argument (which must be AF_INET, AF_INET6, or AF_UNSPEC) declares a @@ -76,6 +186,7 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) * something. */ int parsed_family = 0; + int result = -1; tor_assert(name); tor_assert(addr); @@ -83,8 +194,7 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) if (!*name) { /* Empty address is an error. */ - memset(addr, 0, sizeof(tor_addr_t)); - return -1; + goto permfail; } /* Is it an IP address? */ @@ -93,100 +203,37 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) if (parsed_family >= 0) { /* If the IP address family matches, or was unspecified */ if (parsed_family == family || family == AF_UNSPEC) { - return 0; + goto success; } else { - /* Clear the address before returning an error. */ - memset(addr, 0, sizeof(tor_addr_t)); - return -1; + goto permfail; } } else { /* Clear the address after a failed tor_addr_parse(). */ memset(addr, 0, sizeof(tor_addr_t)); #ifdef HAVE_GETADDRINFO - int err; - struct addrinfo *res=NULL, *res_p; - struct addrinfo *best=NULL; - struct addrinfo hints; - int result = -1; - memset(&hints, 0, sizeof(hints)); - hints.ai_family = family; - hints.ai_socktype = SOCK_STREAM; - err = tor_getaddrinfo(name, NULL, &hints, &res); - /* The check for 'res' here shouldn't be necessary, but it makes static - * analysis tools happy. */ - if (!err && res) { - best = NULL; - for (res_p = res; res_p; res_p = res_p->ai_next) { - if (family == AF_UNSPEC) { - if (res_p->ai_family == AF_INET) { - best = res_p; - break; - } else if (res_p->ai_family == AF_INET6 && !best) { - best = res_p; - } - } else if (family == res_p->ai_family) { - best = res_p; - break; - } - } - if (!best) - best = res; - if (best->ai_family == AF_INET) { - tor_addr_from_in(addr, - &((struct sockaddr_in*)best->ai_addr)->sin_addr); - result = 0; - } else if (best->ai_family == AF_INET6) { - tor_addr_from_in6(addr, - &((struct sockaddr_in6*)best->ai_addr)->sin6_addr); - result = 0; - } - tor_freeaddrinfo(res); - return result; - } - return (err == EAI_AGAIN) ? 1 : -1; + result = tor_addr_lookup_host_getaddrinfo(name, family, addr); + goto done; #else /* !(defined(HAVE_GETADDRINFO)) */ - struct hostent *ent; - int err; -#ifdef HAVE_GETHOSTBYNAME_R_6_ARG - char buf[2048]; - struct hostent hostent; - int r; - r = gethostbyname_r(name, &hostent, buf, sizeof(buf), &ent, &err); -#elif defined(HAVE_GETHOSTBYNAME_R_5_ARG) - char buf[2048]; - struct hostent hostent; - ent = gethostbyname_r(name, &hostent, buf, sizeof(buf), &err); -#elif defined(HAVE_GETHOSTBYNAME_R_3_ARG) - struct hostent_data data; - struct hostent hent; - memset(&data, 0, sizeof(data)); - err = gethostbyname_r(name, &hent, &data); - ent = err ? NULL : &hent; -#else - ent = gethostbyname(name); -#ifdef _WIN32 - err = WSAGetLastError(); -#else - err = h_errno; -#endif -#endif /* defined(HAVE_GETHOSTBYNAME_R_6_ARG) || ... */ - if (ent) { - if (ent->h_addrtype == AF_INET) { - tor_addr_from_in(addr, (struct in_addr*) ent->h_addr); - } else if (ent->h_addrtype == AF_INET6) { - tor_addr_from_in6(addr, (struct in6_addr*) ent->h_addr); - } else { - tor_assert(0); // LCOV_EXCL_LINE: gethostbyname() returned bizarre type - } - return 0; - } -#ifdef _WIN32 - return (err == WSATRY_AGAIN) ? 1 : -1; -#else - return (err == TRY_AGAIN) ? 1 : -1; -#endif + result = tor_addr_lookup_host_gethostbyname(name, addr); + goto done; #endif /* defined(HAVE_GETADDRINFO) */ } + + /* If we weren't successful, and haven't already set the result, + * assume it's a permanent failure */ + permfail: + result = -1; + goto done; + success: + result = 0; + + /* We have set the result, now it's time to clean up */ + done: + if (result) { + /* Clear the address on error */ + memset(addr, 0, sizeof(tor_addr_t)); + } + return result; } /** Parse an address or address-port combination from s, resolve the

1 0

[tor/master] address/resolve: Require square brackets on IPv6 address:ports
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit adb8538e7b6a7a2531233797990f6d11be86a718 Author: teor <teor(a)torproject.org> Date: Thu Jun 13 21:21:19 2019 +1000 address/resolve: Require square brackets on IPv6 address:ports When parsing addreses via Tor's internal address:port parsing and DNS lookup APIs, require IPv6 addresses with ports to have square brackets. But allow IPv6 addresses without ports, whether or not they have square brackets. Fixes bug 30721; bugfix on 0.2.1.5-alpha. --- changes/bug30721 | 6 +++- src/lib/net/address.c | 77 ++++++++++++++++++++++++++++++++++++--------------- src/lib/net/resolve.c | 40 ++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 27 deletions(-) diff --git a/changes/bug30721 b/changes/bug30721 index 5e0365300..5ea4a1462 100644 --- a/changes/bug30721 +++ b/changes/bug30721 @@ -1,6 +1,10 @@ - o Minor bugfixes (networking): + o Minor bugfixes (networking, IP addresses): - When parsing addreses via Tor's internal DNS lookup API, reject IPv4 addresses in square brackets, and accept IPv6 addresses in square brackets. This change completes the work started in 23082, making address parsing consistent between tor's internal DNS lookup and address parsing APIs. Fixes bug 30721; bugfix on 0.2.1.5-alpha. + - When parsing addreses via Tor's internal address:port parsing and + DNS lookup APIs, require IPv6 addresses with ports to have square + brackets. But allow IPv6 addresses without ports, whether or not they + have square brackets. Fixes bug 30721; bugfix on 0.2.1.5-alpha. diff --git a/src/lib/net/address.c b/src/lib/net/address.c index 75322ad88..0a2c84caf 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -1188,27 +1188,28 @@ fmt_addr32(uint32_t addr) } /** Convert the string in src to a tor_addr_t addr. The string - * may be an IPv4 address, an IPv6 address, or an IPv6 address surrounded by - * square brackets. + * may be an IPv4 address, or an IPv6 address surrounded by square brackets. * - * Return an address family on success, or -1 if an invalid address string is - * provided. */ -int -tor_addr_parse(tor_addr_t *addr, const char *src) + * If allow_ipv6_without_brackets is true, also allow IPv6 addresses + * without brackets. + * + * Always rejects IPv4 addresses with brackets. + * + * Returns an address family on success, or -1 if an invalid address string is + * provided. */ +static int +tor_addr_parse_impl(tor_addr_t *addr, const char *src, + bool allow_ipv6_without_brackets) { /* Holds substring of IPv6 address after removing square brackets */ char *tmp = NULL; - int result; + int result = -1; struct in_addr in_tmp; struct in6_addr in6_tmp; int brackets_detected = 0; tor_assert(addr && src); - /* Clear the address before starting, to avoid returning uninitialised data. - */ - memset(addr, 0, sizeof(tor_addr_t)); - size_t len = strlen(src); if (len && src[0] == '[' && src[len - 1] == ']') { @@ -1216,21 +1217,46 @@ tor_addr_parse(tor_addr_t *addr, const char *src) src = tmp = tor_strndup(src+1, strlen(src)-2); } - if (tor_inet_pton(AF_INET6, src, &in6_tmp) > 0) { - result = AF_INET6; - tor_addr_from_in6(addr, &in6_tmp); - } else if (!brackets_detected && - tor_inet_pton(AF_INET, src, &in_tmp) > 0) { - result = AF_INET; - tor_addr_from_in(addr, &in_tmp); - } else { - result = -1; + /* Try to parse an IPv6 address if it has brackets, or if IPv6 addresses + * without brackets are allowed */ + if (brackets_detected || allow_ipv6_without_brackets) { + if (tor_inet_pton(AF_INET6, src, &in6_tmp) > 0) { + result = AF_INET6; + tor_addr_from_in6(addr, &in6_tmp); + } + } + + /* Try to parse an IPv4 address without brackets */ + if (!brackets_detected) { + if (tor_inet_pton(AF_INET, src, &in_tmp) > 0) { + result = AF_INET; + tor_addr_from_in(addr, &in_tmp); + } + } + + /* Clear the address on error, to avoid returning uninitialised or partly + * parsed data. + */ + if (result == -1) { + memset(addr, 0, sizeof(tor_addr_t)); } tor_free(tmp); return result; } +/** Convert the string in src to a tor_addr_t addr. The string + * may be an IPv4 address, an IPv6 address, or an IPv6 address surrounded by + * square brackets. + * + * Returns an address family on success, or -1 if an invalid address string is + * provided. */ +int +tor_addr_parse(tor_addr_t *addr, const char *src) +{ + return tor_addr_parse_impl(addr, src, 1); +} + #ifdef HAVE_IFADDRS_TO_SMARTLIST /* * Convert a linked list consisting of ifaddrs structures @@ -1737,6 +1763,7 @@ tor_addr_port_parse(int severity, const char *addrport, int retval = -1; int r; char *addr_tmp = NULL; + bool has_port; tor_assert(addrport); tor_assert(address_out); @@ -1746,15 +1773,19 @@ tor_addr_port_parse(int severity, const char *addrport, if (r < 0) goto done; - if (!*port_out) { + has_port = !! *port_out; + /* If there's no port, use the default port, or fail if there is no default + */ + if (!has_port) { if (default_port >= 0) *port_out = default_port; else goto done; } - /* make sure that address_out is an IP address */ - if (tor_addr_parse(address_out, addr_tmp) < 0) + /* Make sure that address_out is an IP address. + * If there is no port in addrport, allow IPv6 addresses without brackets. */ + if (tor_addr_parse_impl(address_out, addr_tmp, !has_port) < 0) goto done; retval = 0; diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index 955572088..1097e36bd 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -258,32 +258,66 @@ tor_addr_port_lookup(const char *s, tor_addr_t *addr_out, uint16_t *port_out) uint16_t portval = 0; char *tmp = NULL; int rv = 0; + int result; tor_assert(s); tor_assert(addr_out); s = eat_whitespace(s); + /* Try parsing s as an address:port first, so we don't have to duplicate + * the logic that rejects IPv6:Port with no square brackets. */ + rv = tor_addr_port_parse(LOG_WARN, s, &addr, &portval, 0); + /* That was easy, no DNS required. */ + if (rv == 0) + goto success; + + /* Now let's check for malformed IPv6 addresses and ports: + * tor_addr_port_parse() requires squared brackes if there is a port, + * and we want tor_addr_port_lookup() to have the same requirement. + * But we strip the port using tor_addr_port_split(), so tor_addr_lookup() + * only sees the address, and will accept it without square brackets. */ + int family = tor_addr_parse(&addr, s); + /* If tor_addr_parse() succeeds where tor_addr_port_parse() failed, we need + * to reject this address as malformed. */ + if (family >= 0) { + /* Double-check it's an IPv6 address. If not, we have a parsing bug. + */ + tor_assertf_nonfatal(family == AF_INET6, + "Wrong family: %d (should be IPv6: %d) which " + "failed IP:port parsing, but passed IP parsing. " + "input string: '%s'; parsed address: '%s'.", + family, AF_INET6, s, fmt_addr(&addr)); + goto err; + } + + /* Now we have a hostname. Let's split off the port, if any. */ rv = tor_addr_port_split(LOG_WARN, s, &tmp, &portval); if (rv < 0) goto err; + /* And feed the hostname to the lookup function. */ if (tor_addr_lookup(tmp, AF_UNSPEC, &addr) != 0) goto err; - tor_free(tmp); + success: if (port_out) *port_out = portval; tor_addr_copy(addr_out, &addr); + result = 0; + goto done; - return 0; err: /* Clear the address and port on error */ memset(addr_out, 0, sizeof(tor_addr_t)); if (port_out) *port_out = 0; + result = -1; + + /* We have set the result, now it's time to clean up */ + done: tor_free(tmp); - return -1; + return result; } #ifdef USE_SANDBOX_GETADDRINFO

1 0

[tor/master] address/resolve: try harder to avoid returning uninitialised data
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 308d3002130cfb5b9ad2100dab25b0c4c83162f6 Author: teor <teor(a)torproject.org> Date: Thu Jun 13 21:14:36 2019 +1000 address/resolve: try harder to avoid returning uninitialised data Cleanup after 30721. --- src/lib/net/address.c | 10 ++++++++-- src/lib/net/resolve.c | 11 ++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/lib/net/address.c b/src/lib/net/address.c index 4989e4ab2..75322ad88 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -1760,6 +1760,13 @@ tor_addr_port_parse(int severity, const char *addrport, retval = 0; done: + /* Clear the address and port on error, to avoid returning uninitialised or + * partly parsed data. + */ + if (retval == -1) { + memset(address_out, 0, sizeof(tor_addr_t)); + *port_out = 0; + } tor_free(addr_tmp); return retval; } @@ -1828,8 +1835,7 @@ tor_addr_port_split(int severity, const char *addrport, tor_free(address_); } - if (port_out) - *port_out = ok ? ((uint16_t) port_) : 0; + *port_out = ok ? ((uint16_t) port_) : 0; return ok ? 0 : -1; } diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index a70fb0a82..955572088 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -47,6 +47,11 @@ tor_lookup_hostname,(const char *name, uint32_t *addr)) tor_addr_t myaddr; int ret; + if (BUG(!addr)) + return -1; + + *addr = 0; + if ((ret = tor_addr_lookup(name, AF_INET, &myaddr))) return ret; @@ -250,7 +255,7 @@ int tor_addr_port_lookup(const char *s, tor_addr_t *addr_out, uint16_t *port_out) { tor_addr_t addr; - uint16_t portval; + uint16_t portval = 0; char *tmp = NULL; int rv = 0; @@ -273,6 +278,10 @@ tor_addr_port_lookup(const char *s, tor_addr_t *addr_out, uint16_t *port_out) return 0; err: + /* Clear the address and port on error */ + memset(addr_out, 0, sizeof(tor_addr_t)); + if (port_out) + *port_out = 0; tor_free(tmp); return -1; }

1 0