From 86eba14ac5495cb031669b7e495d5e85653535f2 Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Wed, 18 Nov 2015 23:25:21 +1100 Subject: [PATCH 1/5] Fix unit tests on systems without IPv4 or localhost addresses Make unit tests pass on IPv6-only systems, and systems without localhost addresses (like some FreeBSD jails). Fixes: * get_if_addrs_ifaddrs: systems without localhost * get_if_addrs_ioctl: only works on IPv4 systems * socket: check IPv4 and IPv6, skip on EPROTONOSUPPORT * socketpair_ersatz: uses IPv4, skip on EPROTONOSUPPORT Fixes bug #17632; bugfix on unit tests in 0.2.7.3-rc. c464a367728d was a partial fix for this issue in #17255; it was released in unit tests in 0.2.7.4-rc. Patch by "teor". --- changes/bug17632-no-ipv4-no-localhost | 7 +++ src/test/test_address.c | 65 ++++++++++++++++++++++++--- src/test/test_util.c | 40 +++++++++++++---- 3 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 changes/bug17632-no-ipv4-no-localhost diff --git a/changes/bug17632-no-ipv4-no-localhost b/changes/bug17632-no-ipv4-no-localhost new file mode 100644 index 0000000000..04622079d3 --- /dev/null +++ b/changes/bug17632-no-ipv4-no-localhost @@ -0,0 +1,7 @@ + o Minor bugfix (unit tests): + - Make unit tests pass on IPv6-only systems, and systems without + localhost addresses (like some FreeBSD jails). + Fixes bug #17632; bugfix on unit tests in 0.2.7.3-rc. + c464a367728d was a partial fix for this issue in #17255; + it was released in unit tests in 0.2.7.4-rc. + Patch by "teor". diff --git a/src/test/test_address.c b/src/test/test_address.c index 7368b05035..9f3d81c92c 100644 --- a/src/test/test_address.c +++ b/src/test/test_address.c @@ -118,6 +118,21 @@ smartlist_contains_internal_tor_addr(smartlist_t *smartlist) return 0; } +/** Return 1 iff smartlist contains a tor_addr_t structure + * that is NULL or the null tor_addr_t. Otherwise, return 0. + */ +static int +smartlist_contains_null_tor_addr(smartlist_t *smartlist) +{ + SMARTLIST_FOREACH_BEGIN(smartlist, tor_addr_t *, tor_addr) { + if (tor_addr == NULL || tor_addr_is_null(tor_addr)) { + return 1; + } + } SMARTLIST_FOREACH_END(tor_addr); + + return 0; +} + /** Return 1 iff smartlist contains a tor_addr_t structure * that is an IPv4 address. Otherwise, return 0. */ @@ -268,8 +283,18 @@ test_address_get_if_addrs_ifaddrs(void *arg) results = get_interface_addresses_ifaddrs(LOG_ERR); - tt_int_op(smartlist_len(results),>=,1); - tt_assert(smartlist_contains_localhost_tor_addr(results)); + tt_assert(results); + /* Some FreeBSD jails don't have localhost IP address. Instead, they only + * have the address assigned to the jail (whatever that may be). + * And a jail without a network connection might not have any addresses at + * all. */ + tt_assert(!smartlist_contains_null_tor_addr(results)); + + /* If there are addresses, they must be IPv4 or IPv6 */ + if (smartlist_len(results) > 0) { + tt_assert(smartlist_contains_ipv4_tor_addr(results) + || smartlist_contains_ipv6_tor_addr(results)); + } done: SMARTLIST_FOREACH(results, tor_addr_t *, t, tor_free(t)); @@ -293,6 +318,13 @@ test_address_get_if_addrs_win32(void *arg) tt_int_op(smartlist_len(results),>=,1); tt_assert(smartlist_contains_localhost_tor_addr(results)); + tt_assert(!smartlist_contains_null_tor_addr(results)); + + /* If there are addresses, they must be IPv4 or IPv6 */ + if (smartlist_len(results) > 0) { + tt_assert(smartlist_contains_ipv4_tor_addr(results) + || smartlist_contains_ipv6_tor_addr(results)); + } done: SMARTLIST_FOREACH(results, tor_addr_t *, t, tor_free(t)); @@ -481,10 +513,24 @@ test_address_get_if_addrs_ioctl(void *arg) result = get_interface_addresses_ioctl(LOG_ERR); + /* On an IPv6-only system, this will fail and return NULL tt_assert(result); - tt_int_op(smartlist_len(result),>=,1); + */ - tt_assert(smartlist_contains_localhost_tor_addr(result)); + /* Some FreeBSD jails don't have localhost IP address. Instead, they only + * have the address assigned to the jail (whatever that may be). + * And a jail without a network connection might not have any addresses at + * all. */ + if (result) { + tt_assert(!smartlist_contains_null_tor_addr(result)); + + /* If there are addresses, they must be IPv4 or IPv6. + * (AIX supports IPv6 from SIOCGIFCONF.) */ + if (smartlist_len(result) > 0) { + tt_assert(smartlist_contains_ipv4_tor_addr(result) + || smartlist_contains_ipv6_tor_addr(result)); + } + } done: if (result) { @@ -696,12 +742,13 @@ test_address_get_if_addrs_list_internal(void *arg) tt_assert(!smartlist_contains_localhost_tor_addr(results)); tt_assert(!smartlist_contains_multicast_tor_addr(results)); /* The list may or may not contain internal addresses */ + tt_assert(!smartlist_contains_null_tor_addr(results)); - /* Allow unit tests to pass on IPv6-only machines */ + /* if there are any addresses, they must be IPv4 */ if (smartlist_len(results) > 0) { - tt_assert(smartlist_contains_ipv4_tor_addr(results) - || smartlist_contains_ipv6_tor_addr(results)); + tt_assert(smartlist_contains_ipv4_tor_addr(results)); } + tt_assert(!smartlist_contains_ipv6_tor_addr(results)); done: free_interface_address_list(results); @@ -724,6 +771,7 @@ test_address_get_if_addrs_list_no_internal(void *arg) tt_assert(!smartlist_contains_localhost_tor_addr(results)); tt_assert(!smartlist_contains_multicast_tor_addr(results)); tt_assert(!smartlist_contains_internal_tor_addr(results)); + tt_assert(!smartlist_contains_null_tor_addr(results)); /* if there are any addresses, they must be IPv4 */ if (smartlist_len(results) > 0) { @@ -752,6 +800,7 @@ test_address_get_if_addrs6_list_internal(void *arg) tt_assert(!smartlist_contains_localhost_tor_addr(results)); tt_assert(!smartlist_contains_multicast_tor_addr(results)); /* The list may or may not contain internal addresses */ + tt_assert(!smartlist_contains_null_tor_addr(results)); /* if there are any addresses, they must be IPv6 */ tt_assert(!smartlist_contains_ipv4_tor_addr(results)); @@ -780,7 +829,9 @@ test_address_get_if_addrs6_list_no_internal(void *arg) tt_assert(!smartlist_contains_localhost_tor_addr(results)); tt_assert(!smartlist_contains_multicast_tor_addr(results)); tt_assert(!smartlist_contains_internal_tor_addr(results)); + tt_assert(!smartlist_contains_null_tor_addr(results)); + /* if there are any addresses, they must be IPv6 */ tt_assert(!smartlist_contains_ipv4_tor_addr(results)); if (smartlist_len(results) > 0) { tt_assert(smartlist_contains_ipv6_tor_addr(results)); diff --git a/src/test/test_util.c b/src/test/test_util.c index 208186c7d5..9a1084640a 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4342,9 +4342,17 @@ fd_is_nonblocking(tor_socket_t fd) } #endif +#ifdef EPROTONOSUPPORT +#define SOCKET_EPROTO(s) (s == EPROTONOSUPPORT) +#else +#define SOCKET_EPROTO(s) (0) +#endif + +/* Test for tor_open_socket*, using IPv4 or IPv6 depending on arg. */ static void test_util_socket(void *arg) { + const int domain = !strcmp(arg, "4") ? AF_INET : AF_INET6; tor_socket_t fd1 = TOR_INVALID_SOCKET; tor_socket_t fd2 = TOR_INVALID_SOCKET; tor_socket_t fd3 = TOR_INVALID_SOCKET; @@ -4355,15 +4363,19 @@ test_util_socket(void *arg) (void)arg; - fd1 = tor_open_socket_with_extensions(AF_INET, SOCK_STREAM, 0, 0, 0); - fd2 = tor_open_socket_with_extensions(AF_INET, SOCK_STREAM, 0, 0, 1); + fd1 = tor_open_socket_with_extensions(domain, SOCK_STREAM, 0, 0, 0); + if (SOCKET_EPROTO(fd1)) { + /* Assume we're on an IPv4-only or IPv6-only system, and give up now. */ + goto done; + } + fd2 = tor_open_socket_with_extensions(domain, SOCK_STREAM, 0, 0, 1); tt_assert(SOCKET_OK(fd1)); tt_assert(SOCKET_OK(fd2)); tt_int_op(get_n_open_sockets(), OP_EQ, n + 2); - //fd3 = tor_open_socket_with_extensions(AF_INET, SOCK_STREAM, 0, 1, 0); - //fd4 = tor_open_socket_with_extensions(AF_INET, SOCK_STREAM, 0, 1, 1); - fd3 = tor_open_socket(AF_INET, SOCK_STREAM, 0); - fd4 = tor_open_socket_nonblocking(AF_INET, SOCK_STREAM, 0); + //fd3 = tor_open_socket_with_extensions(domain, SOCK_STREAM, 0, 1, 0); + //fd4 = tor_open_socket_with_extensions(domain, SOCK_STREAM, 0, 1, 1); + fd3 = tor_open_socket(domain, SOCK_STREAM, 0); + fd4 = tor_open_socket_nonblocking(domain, SOCK_STREAM, 0); tt_assert(SOCKET_OK(fd3)); tt_assert(SOCKET_OK(fd4)); tt_int_op(get_n_open_sockets(), OP_EQ, n + 4); @@ -4412,8 +4424,15 @@ test_util_socketpair(void *arg) int n = get_n_open_sockets(); tor_socket_t fds[2] = {TOR_INVALID_SOCKET, TOR_INVALID_SOCKET}; const int family = AF_UNIX; + int socketpair_result = 0; - tt_int_op(0, OP_EQ, tor_socketpair_fn(family, SOCK_STREAM, 0, fds)); + socketpair_result = tor_socketpair_fn(family, SOCK_STREAM, 0, fds); + if (ersatz && SOCKET_EPROTO(socketpair_result)) { + /* Assume we're on an IPv6-only system, and give up now. + * (tor_ersatz_socketpair uses IPv4.) */ + goto done; + } + tt_int_op(0, OP_EQ, socketpair_result); tt_assert(SOCKET_OK(fds[0])); tt_assert(SOCKET_OK(fds[1])); tt_int_op(get_n_open_sockets(), OP_EQ, n + 2); @@ -4433,6 +4452,8 @@ test_util_socketpair(void *arg) tor_close_socket(fds[1]); } +#undef SOCKET_EPROTO + static void test_util_max_mem(void *arg) { @@ -4636,7 +4657,10 @@ struct testcase_t util_tests[] = { UTIL_TEST(write_chunks_to_file, 0), UTIL_TEST(mathlog, 0), UTIL_TEST(weak_random, 0), - UTIL_TEST(socket, TT_FORK), + { "socket_ipv4", test_util_socket, TT_FORK, &passthrough_setup, + (void*)"4" }, + { "socket_ipv6", test_util_socket, TT_FORK, + &passthrough_setup, (void*)"6" }, { "socketpair", test_util_socketpair, TT_FORK, &passthrough_setup, (void*)"0" }, { "socketpair_ersatz", test_util_socketpair, TT_FORK, From 878b5738c25bf378f7e9a801eca623af2abde2b1 Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Wed, 18 Nov 2015 23:30:25 +1100 Subject: [PATCH 2/5] Update comments in get_interface_addresses_ioctl Comment-only change noting platforms that can return IPv6 addresses from SIOCGIFCONF (or SIOCGLIFCONF). --- src/common/address.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/address.c b/src/common/address.c index cfa8fd1dca..aef229b02c 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -1469,7 +1469,8 @@ get_interface_addresses_ioctl(int severity) int fd; smartlist_t *result = NULL; - /* This interface, AFAICT, only supports AF_INET addresses */ + /* This interface, AFAICT, only supports AF_INET addresses, + * except on AIX. For Solaris, we could use SIOCGLIFCONF. */ fd = socket(AF_INET, SOCK_DGRAM, 0); if (fd < 0) { tor_log(severity, LD_NET, "socket failed: %s", strerror(errno)); From 3351f69c757b23e89f3785a3d1215f7152c1836e Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Wed, 18 Nov 2015 23:47:12 +1100 Subject: [PATCH 3/5] Fixup 86eba14ac549: add errno.h for EPROTONOSUPPORT --- src/test/test_util.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/test_util.c b/src/test/test_util.c index 9a1084640a..1a749735b6 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -20,6 +20,7 @@ #include #include #include +#include /* XXXX this is a minimal wrapper to make the unit tests compile with the * changed tor_timegm interface. */ From eed86892ddac634be61ddf5bcb8deb23d86512c4 Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Wed, 18 Nov 2015 23:54:26 +1100 Subject: [PATCH 4/5] Really Fixup 86eba14ac549: error return values are negative --- src/test/test_util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index 1a749735b6..15eb2cf3b0 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -20,7 +20,6 @@ #include #include #include -#include /* XXXX this is a minimal wrapper to make the unit tests compile with the * changed tor_timegm interface. */ @@ -4344,7 +4343,7 @@ fd_is_nonblocking(tor_socket_t fd) #endif #ifdef EPROTONOSUPPORT -#define SOCKET_EPROTO(s) (s == EPROTONOSUPPORT) +#define SOCKET_EPROTO(s) (s == -EPROTONOSUPPORT) #else #define SOCKET_EPROTO(s) (0) #endif From a1ce111d32a105ce09e17974800e6cc3e239e60f Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Thu, 19 Nov 2015 00:13:58 +1100 Subject: [PATCH 5/5] Really Really Fixup 86eba14ac549: Windows support, error return values --- src/test/test_util.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index 15eb2cf3b0..187cb23125 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4342,11 +4342,8 @@ fd_is_nonblocking(tor_socket_t fd) } #endif -#ifdef EPROTONOSUPPORT -#define SOCKET_EPROTO(s) (s == -EPROTONOSUPPORT) -#else -#define SOCKET_EPROTO(s) (0) -#endif +#define ERRNO_IS_EPROTO(e) (e == SOCK_ERRNO(EPROTONOSUPPORT)) +#define SOCK_ERR_IS_EPROTO(s) ERRNO_IS_EPROTO(tor_socket_errno(s)) /* Test for tor_open_socket*, using IPv4 or IPv6 depending on arg. */ static void @@ -4364,7 +4361,7 @@ test_util_socket(void *arg) (void)arg; fd1 = tor_open_socket_with_extensions(domain, SOCK_STREAM, 0, 0, 0); - if (SOCKET_EPROTO(fd1)) { + if (SOCK_ERR_IS_EPROTO(fd1)) { /* Assume we're on an IPv4-only or IPv6-only system, and give up now. */ goto done; } @@ -4427,7 +4424,7 @@ test_util_socketpair(void *arg) int socketpair_result = 0; socketpair_result = tor_socketpair_fn(family, SOCK_STREAM, 0, fds); - if (ersatz && SOCKET_EPROTO(socketpair_result)) { + if (ersatz && ERRNO_IS_EPROTO(-socketpair_result)) { /* Assume we're on an IPv6-only system, and give up now. * (tor_ersatz_socketpair uses IPv4.) */ goto done;