From ac8f6d51f4200b27adc0da9fc1eeb44b89c8ebab Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 2 Oct 2019 21:22:53 -0400 Subject: [PATCH 1/4] Unify backend implementations for blocking hostname lookup We have a getaddrinfo() implementation that we prefer, and a gethostbyname*() implementation that we fall back on. Give them both the same interface, and let them be called by the same name. This is a preparatory step for making them both mockable. --- src/lib/net/resolve.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index 78e72fba4f..c8fe338e52 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -71,9 +71,9 @@ tor_lookup_hostname,(const char *name, uint32_t *addr)) * See tor_addr_lookup() for details. */ static int -tor_addr_lookup_host_getaddrinfo(const char *name, - uint16_t family, - tor_addr_t *addr) +tor_addr_lookup_host_impl(const char *name, + uint16_t family, + tor_addr_t *addr) { int err; struct addrinfo *res=NULL, *res_p; @@ -120,15 +120,17 @@ tor_addr_lookup_host_getaddrinfo(const char *name, #else /* !defined(HAVE_GETADDRINFO) */ -/* Host lookup helper for tor_addr_lookup(), which calls getaddrinfo(). - * Used when gethostbyname() is not available on this system. +/* Host lookup helper for tor_addr_lookup(), which calls gethostbyname(). + * Used when getaddrinfo() 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) +tor_addr_lookup_host_impl(const char *name, + uint16_t family, + tor_addr_t *addr) { + (void) family; struct hostent *ent; int err; #ifdef HAVE_GETHOSTBYNAME_R_6_ARG @@ -215,13 +217,8 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) } else { /* Clear the address after a failed tor_addr_parse(). */ memset(addr, 0, sizeof(tor_addr_t)); -#ifdef HAVE_GETADDRINFO - result = tor_addr_lookup_host_getaddrinfo(name, family, addr); + result = tor_addr_lookup_host_impl(name, family, addr); goto done; -#else /* !(defined(HAVE_GETADDRINFO)) */ - 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, From 9c24ceeb3f2082193bf76a77ad6d6d011561e239 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 3 Oct 2019 07:50:18 -0400 Subject: [PATCH 2/4] Make tor_addr_lookup_host_impl mockable. --- src/lib/net/resolve.c | 14 +++++++------- src/lib/net/resolve.h | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index c8fe338e52..442bc4a6b3 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -8,6 +8,7 @@ * \brief Use the libc DNS resolver to convert hostnames into addresses. **/ +#define RESOLVE_PRIVATE #include "lib/net/resolve.h" #include "lib/net/address.h" @@ -70,10 +71,10 @@ tor_lookup_hostname,(const char *name, uint32_t *addr)) * * See tor_addr_lookup() for details. */ -static int -tor_addr_lookup_host_impl(const char *name, +MOCK_IMPL(STATIC int, +tor_addr_lookup_host_impl,(const char *name, uint16_t family, - tor_addr_t *addr) + tor_addr_t *addr)) { int err; struct addrinfo *res=NULL, *res_p; @@ -125,10 +126,10 @@ tor_addr_lookup_host_impl(const char *name, * * See tor_addr_lookup() for details. */ -static int -tor_addr_lookup_host_impl(const char *name, +MOCK_IMPL(STATIC int, +tor_addr_lookup_host_impl,(const char *name, uint16_t family, - tor_addr_t *addr) + tor_addr_t *addr)) { (void) family; struct hostent *ent; @@ -172,7 +173,6 @@ tor_addr_lookup_host_impl(const char *name, return (err == TRY_AGAIN) ? 1 : -1; #endif } - #endif /* defined(HAVE_GETADDRINFO) */ /** Similar behavior to Unix gethostbyname: resolve name, and set diff --git a/src/lib/net/resolve.h b/src/lib/net/resolve.h index d7b60be917..b979b2fb41 100644 --- a/src/lib/net/resolve.h +++ b/src/lib/net/resolve.h @@ -24,12 +24,18 @@ struct tor_addr_t; +/* + * Primary lookup functions. + */ MOCK_DECL(int, tor_lookup_hostname,(const char *name, uint32_t *addr)); MOCK_DECL(int, tor_addr_lookup,(const char *name, uint16_t family, struct tor_addr_t *addr_out)); int tor_addr_port_lookup(const char *s, struct tor_addr_t *addr_out, uint16_t *port_out); +/* + * Sandbox helpers + */ struct addrinfo; #ifdef USE_SANDBOX_GETADDRINFO /** Pre-calls getaddrinfo in order to pre-record result. */ @@ -55,4 +61,13 @@ void tor_free_getaddrinfo_cache(void); void sandbox_disable_getaddrinfo_cache(void); void tor_make_getaddrinfo_cache_active(void); +/* + * Internal resolver wrapper; exposed for mocking. + */ +#ifdef RESOLVE_PRIVATE +MOCK_DECL(STATIC int, tor_addr_lookup_host_impl, (const char *name, + uint16_t family, + struct tor_addr_t *addr)); +#endif + #endif /* !defined(TOR_RESOLVE_H) */ From 4a0749596cc21198dd853bb0c631e9658db7fe2d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 3 Oct 2019 08:54:17 -0400 Subject: [PATCH 3/4] Add a mock replacement for blocking hostname resolution Based on examination of our tests, this mock function accepts "localhost" and "torproject.org", and rejects everything else. --- src/test/include.am | 2 + src/test/resolve_test_helpers.c | 85 +++++++++++++++++++++++++++++++++ src/test/resolve_test_helpers.h | 18 +++++++ 3 files changed, 105 insertions(+) create mode 100644 src/test/resolve_test_helpers.c create mode 100644 src/test/resolve_test_helpers.h diff --git a/src/test/include.am b/src/test/include.am index 2dd4d8c583..acd0824d0d 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -102,6 +102,7 @@ src_test_test_SOURCES += \ src/test/log_test_helpers.c \ src/test/hs_test_helpers.c \ src/test/rend_test_helpers.c \ + src/test/resolve_test_helpers.c \ src/test/rng_test_helpers.c \ src/test/test.c \ src/test/test_accounting.c \ @@ -340,6 +341,7 @@ noinst_HEADERS+= \ src/test/hs_test_helpers.h \ src/test/log_test_helpers.h \ src/test/rend_test_helpers.h \ + src/test/resolve_test_helpers.h \ src/test/rng_test_helpers.h \ src/test/test.h \ src/test/ptr_helpers.h \ diff --git a/src/test/resolve_test_helpers.c b/src/test/resolve_test_helpers.c new file mode 100644 index 0000000000..73ea730149 --- /dev/null +++ b/src/test/resolve_test_helpers.c @@ -0,0 +1,85 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * @file resolve_test_helpers.c + * @brief Helper functions for mocking libc's blocking hostname lookup + * facilities. + **/ + +#define RESOLVE_PRIVATE +#include "orconfig.h" +#include "test/resolve_test_helpers.h" +#include "lib/net/address.h" +#include "lib/net/resolve.h" +#include "test/test.h" + +#include +#include + +/** + * Mock replacement for our getaddrinfo/gethostbyname wrapper. + **/ +static int +replacement_host_lookup(const char *name, uint16_t family, tor_addr_t *addr) +{ + static const struct lookup_table_ent { + const char *name; + const char *ipv4; + const char *ipv6; + } entries[] = { + { "localhost", "127.0.0.1", "::1" }, + { "torproject.org", "198.51.100.6", "2001:DB8::700" }, + { NULL, NULL, NULL }, + }; + + int r = -1; + + for (unsigned i = 0; entries[i].name != NULL; ++i) { + if (!strcasecmp(name, entries[i].name)) { + if (family == AF_INET6) { + int s = tor_addr_parse(addr, entries[i].ipv6); + tt_int_op(s, OP_EQ, AF_INET6); + } else { + int s = tor_addr_parse(addr, entries[i].ipv4); + tt_int_op(s, OP_EQ, AF_INET); + } + r = 0; + break; + } + } + + log_debug(LD_GENERAL, "resolve(%s,%d) => %s", + name, family, r == 0 ? fmt_addr(addr) : "-1"); + + return r; + done: + return -1; +} + +/** + * Set up a mock replacement for our wrapper on libc's resolver code. + * + * According to our replacement, only "localhost" and "torproject.org" + * are real addresses; everything else doesn't exist. + * + * Use this function to avoid using the DNS resolver during unit tests; + * call unmock_hostname_resolver() when you're done. + **/ +void +mock_hostname_resolver(void) +{ + MOCK(tor_addr_lookup_host_impl, replacement_host_lookup); +} + +/** + * Unmock our wrappers for libc's blocking hostname resolver code. + **/ +void +unmock_hostname_resolver(void) +{ + UNMOCK(tor_addr_lookup_host_impl); +} diff --git a/src/test/resolve_test_helpers.h b/src/test/resolve_test_helpers.h new file mode 100644 index 0000000000..e7d2e29373 --- /dev/null +++ b/src/test/resolve_test_helpers.h @@ -0,0 +1,18 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * @file resolve_test_helpers.h + * @brief Header for test/resolve_test_helpers.c + **/ + +#ifndef TOR_TEST_RESOLVE_TEST_HELPERS_H +#define TOR_TEST_RESOLVE_TEST_HELPERS_H + +void mock_hostname_resolver(void); +void unmock_hostname_resolver(void); + +#endif /* !defined(TOR_TEST_RESOLVE_TEST_HELPERS_H) */ From fdfb4b196b7c38f45f2d37f73fcc96d746816cc3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 3 Oct 2019 08:57:14 -0400 Subject: [PATCH 4/4] Use {mock,unmock}_hostname_resolver() in relevant tests These tests all invoke the hostname resolver in one way or another, and therefore potentially block if our DNS server is missing, absent, or extremely slow. Closes ticket 31841. --- changes/ticket31841 | 5 +++++ src/test/test_addr.c | 6 +++++- src/test/test_config.c | 4 ++++ src/test/test_hs_config.c | 8 +++++--- src/test/test_options.c | 3 +++ 5 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 changes/ticket31841 diff --git a/changes/ticket31841 b/changes/ticket31841 new file mode 100644 index 0000000000..6e7fbc1da1 --- /dev/null +++ b/changes/ticket31841 @@ -0,0 +1,5 @@ + o Minor features (testing): + - When running tests that attempt to look up hostname, replace the libc + name lookup functions with ones that do not actually touch the network. + This way, the tests complete more quickly in the presence of a slow or + missing DNS resolver. Closes ticket 31841. diff --git a/src/test/test_addr.c b/src/test/test_addr.c index f99e3be8f5..c89c6e78d4 100644 --- a/src/test/test_addr.c +++ b/src/test/test_addr.c @@ -12,6 +12,7 @@ #include "test/log_test_helpers.h" #include "lib/net/resolve.h" #include "test/rng_test_helpers.h" +#include "test/resolve_test_helpers.h" #ifdef HAVE_SYS_UN_H #include @@ -1160,6 +1161,7 @@ test_addr_parse_canonical(void *arg) static void test_addr_parse(void *arg) { + int r; tor_addr_t addr; uint16_t port; @@ -1169,6 +1171,8 @@ test_addr_parse(void *arg) (void)arg; + mock_hostname_resolver(); + /* IPv6-mapped IPv4 addresses. Tor doesn't really use these. */ TEST_ADDR_V6_PARSE("11:22:33:44:55:66:1.2.3.4", 0, "11:22:33:44:55:66:102:304"); @@ -1273,7 +1277,7 @@ test_addr_parse(void *arg) "11:22::88",99); done: - ; + unmock_hostname_resolver(); } static void diff --git a/src/test/test_config.c b/src/test/test_config.c index 1c6c913078..cbb84e4dcf 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -45,6 +45,7 @@ #include "app/config/statefile.h" #include "test/test_helpers.h" +#include "test/resolve_test_helpers.h" #include "feature/dirclient/dir_server_st.h" #include "core/or/port_cfg_st.h" @@ -4068,6 +4069,8 @@ test_config_parse_port_config__ports__ports_given(void *data) slout = smartlist_new(); + mock_hostname_resolver(); + // Test error when encounters an invalid Port specification config_port_invalid = mock_config_line("DNSPort", ""); ret = parse_port_config(NULL, config_port_invalid, "DNS", 0, NULL, @@ -4764,6 +4767,7 @@ test_config_parse_port_config__ports__ports_given(void *data) #endif /* defined(_WIN32) */ done: + unmock_hostname_resolver(); if (slout) SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf)); smartlist_free(slout); diff --git a/src/test/test_hs_config.c b/src/test/test_hs_config.c index 2b3afbb6e9..71e1529216 100644 --- a/src/test/test_hs_config.c +++ b/src/test/test_hs_config.c @@ -12,6 +12,7 @@ #include "test/test.h" #include "test/test_helpers.h" #include "test/log_test_helpers.h" +#include "test/resolve_test_helpers.h" #include "app/config/config.h" #include "feature/hs/hs_common.h" @@ -272,6 +273,7 @@ test_valid_service_v2(void *arg) int ret; (void) arg; + mock_hostname_resolver(); /* Valid complex configuration. Basic client authorization. */ { @@ -314,7 +316,7 @@ test_valid_service_v2(void *arg) } done: - ; + unmock_hostname_resolver(); } static void @@ -392,6 +394,7 @@ test_valid_service_v3(void *arg) int ret; (void) arg; + mock_hostname_resolver(); /* Valid complex configuration. */ { @@ -448,7 +451,7 @@ test_valid_service_v3(void *arg) } done: - ; + unmock_hostname_resolver(); } static void @@ -623,4 +626,3 @@ struct testcase_t hs_config_tests[] = { END_OF_TESTCASES }; - diff --git a/src/test/test_options.c b/src/test/test_options.c index 2d45ecd189..d8757491fa 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -14,6 +14,7 @@ #include "feature/nodelist/routerset.h" #include "core/mainloop/mainloop.h" #include "test/log_test_helpers.h" +#include "test/resolve_test_helpers.h" #include "lib/sandbox/sandbox.h" #include "lib/memarea/memarea.h" @@ -241,6 +242,7 @@ test_options_validate(void *arg) (void)arg; setup_log_callback(); sandbox_disable_getaddrinfo_cache(); + mock_hostname_resolver(); WANT_ERR("ExtORPort 500000", "Invalid ExtORPort", PH_VALIDATE); @@ -282,6 +284,7 @@ test_options_validate(void *arg) close_temp_logs(); clear_log_messages(); + unmock_hostname_resolver(); return; }