commit c4a6b56cc19878de4c76e83ce8e38ad709839d92 Author: Nick Mathewson nickm@torproject.org Date: Tue Jan 3 10:11:23 2017 -0500

Fix unit test failures in response to DNS hijacking.

Some DNS NXDOMAIN hijackers hijack truly ridiculous domains, like "invalid-stuff!!" or "1.2.3.4.5". This would provoke unit test failures where we used addresses like that to force tor_addr_lookup() to fail. The fix, for testing, is to mock tor_addr_lookup() with a variant that always fails when it gets a name with a !.

Fixes bugs 20862 and 20863. --- changes/bug20862 | 6 ++++++ src/test/test_config.c | 4 ++++ src/test/test_controller.c | 5 ++++- src/test/test_helpers.c | 15 +++++++++++++++ src/test/test_helpers.h | 3 +++ src/test/test_options.c | 11 +++++++++-- 6 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/changes/bug20862 b/changes/bug20862 new file mode 100644 index 0000000..fba98c8 --- /dev/null +++ b/changes/bug20862 @@ -0,0 +1,6 @@ + o Minor bugfixes (unit tests): + - Allow the unit tests to pass even when DNS lookups of bogus + addresses do not fail as expected. Fixes bug 20862 and 20863; + bugfix on unit tests introduced in 0.2.8.1-alpha through + 0.2.9.4-alpha. + diff --git a/src/test/test_config.c b/src/test/test_config.c index a540bcc..eeda34e 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -46,6 +46,8 @@ #include "transports.h" #include "util.h"

+#include "test_helpers.h" + static void test_config_addressmap(void *arg) { @@ -4701,8 +4703,10 @@ test_config_parse_port_config__ports__ports_given(void *data) // Test failure when asked to parse an invalid address followed by auto config_free_lines(config_port_invalid); config_port_invalid = NULL; config_port_invalid = mock_config_line("DNSPort", "invalidstuff!!:auto"); + MOCK(tor_addr_lookup, mock_tor_addr_lookup__fail_on_bad_addrs); ret = parse_port_config(NULL, config_port_invalid, NULL, "DNS", 0, "127.0.0.46", 0, 0); + UNMOCK(tor_addr_lookup); tt_int_op(ret, OP_EQ, -1);

// Test success with parsing both an address and a real port diff --git a/src/test/test_controller.c b/src/test/test_controller.c index 4e65d76..d9c0a1e 100644 --- a/src/test/test_controller.c +++ b/src/test/test_controller.c @@ -10,6 +10,7 @@ #include "rendservice.h" #include "routerlist.h" #include "test.h" +#include "test_helpers.h"

static void test_add_onion_helper_keyarg(void *arg) @@ -186,8 +187,10 @@ test_rend_service_parse_port_config(void *arg) tor_free(err_msg);

/* bogus IP address */ - cfg = rend_service_parse_port_config("100 1.2.3.4.5:9000", + MOCK(tor_addr_lookup, mock_tor_addr_lookup__fail_on_bad_addrs); + cfg = rend_service_parse_port_config("100 foo!!.example.com:9000", " ", &err_msg); + UNMOCK(tor_addr_lookup); tt_assert(!cfg); tt_str_op(err_msg, OP_EQ, "Unparseable address in hidden service port " "configuration."); diff --git a/src/test/test_helpers.c b/src/test/test_helpers.c index 132af39..5b84366 100644 --- a/src/test/test_helpers.c +++ b/src/test/test_helpers.c @@ -128,3 +128,18 @@ dummy_origin_circuit_new(int n_cells) return TO_CIRCUIT(circ); }

+/** Mock-replacement. As tor_addr_lookup, but always fails on any + * address containing a !. This is necessary for running the unit tests + * on networks where DNS hijackers think it's helpful to give answers + * for things like 1.2.3.4.5 or "invalidstuff!!" + */ +int +mock_tor_addr_lookup__fail_on_bad_addrs(const char *name, + uint16_t family, tor_addr_t *out) +{ + if (name && strchr(name, '!')) { + return -1; + } + return tor_addr_lookup__real(name, family, out); +} + diff --git a/src/test/test_helpers.h b/src/test/test_helpers.h index ba93b10..c6d4d9c 100644 --- a/src/test/test_helpers.h +++ b/src/test/test_helpers.h @@ -17,6 +17,9 @@ void helper_setup_fake_routerlist(void); void connection_write_to_buf_mock(const char *string, size_t len, connection_t *conn, int zlib);

+int mock_tor_addr_lookup__fail_on_bad_addrs(const char *name, + uint16_t family, tor_addr_t *out); + extern const char TEST_DESCRIPTORS[];

#endif diff --git a/src/test/test_options.c b/src/test/test_options.c index e85e118..3fe0dc3 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -18,6 +18,7 @@ #include "sandbox.h" #include "memarea.h" #include "policies.h" +#include "test_helpers.h"

#define NS_MODULE test_options

@@ -648,18 +649,21 @@ test_options_validate__authdir(void *ignored) int ret; char *msg; setup_capture_of_logs(LOG_INFO); + // XXXX But it _can_ exist, if you're DNS-hijacked. options_test_data_t *tdata = get_options_test_data( "AuthoritativeDirectory 1\n" - "Address this.should.not_exist.example.org"); + "Address this.should.not!exist!.example.org");

sandbox_disable_getaddrinfo_cache();

+ MOCK(tor_addr_lookup, mock_tor_addr_lookup__fail_on_bad_addrs); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + UNMOCK(tor_addr_lookup); tt_int_op(ret, OP_EQ, -1); tt_str_op(msg, OP_EQ, "Failed to resolve/guess local address. See logs for" " details."); expect_log_msg("Could not resolve local Address " - "'this.should.not_exist.example.org'. Failing.\n"); + "'this.should.not!exist!.example.org'. Failing.\n"); tor_free(msg);

free_options_test_data(tdata); @@ -3037,6 +3041,7 @@ test_options_validate__proxy(void *ignored) options_test_data_t *tdata = NULL; sandbox_disable_getaddrinfo_cache(); setup_capture_of_logs(LOG_WARN); + MOCK(tor_addr_lookup, mock_tor_addr_lookup__fail_on_bad_addrs);

free_options_test_data(tdata); tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES @@ -3057,6 +3062,7 @@ test_options_validate__proxy(void *ignored) tor_free(msg);

free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "HttpProxy not_so_valid!\n" ); @@ -3357,6 +3363,7 @@ test_options_validate__proxy(void *ignored) policies_free_all(); // sandbox_free_getaddrinfo_cache(); tor_free(msg); + UNMOCK(tor_addr_lookup); }

static void