July 2020 - tor-commits - lists.torproject.org

[tor/master] Merge branch 'ticket33238_045_01_squashed'
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit a34b9a0b6258d0f1d9373fcf7aacf1e30f8efd37 Merge: 2e98e8811 45afb31e1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 2 11:00:48 2020 -0400 Merge branch 'ticket33238_045_01_squashed' changes/ticket33238 | 5 + src/app/config/resolve_addr.c | 29 +- src/test/test_config.c | 852 +++++++++++------------------------------- 3 files changed, 228 insertions(+), 658 deletions(-)

1 0

[tor/master] addr: Prioritize interface lookup over local hostname
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 45afb31e1ce7c8572342c4e444a178b590d9645d Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jul 2 09:44:49 2020 -0400 addr: Prioritize interface lookup over local hostname The find_my_address() function now prioritize the local interface over the local hostname when guessing the IP address. See proposal 312, section 3.2.1, general case: https://gitweb.torproject.org/torspec.git/tree/proposals/312-relay-auto-ipv… The entire unit tests had to be refactored to make this possible. Instead of hot patching it, it has been rewritten to cover all possible cases and the test interface has been changed to accomodate both IPv4 and IPv6 in order for them to be tested identically. Closes #33238 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket33238 | 5 + src/app/config/resolve_addr.c | 29 +- src/test/test_config.c | 852 +++++++++++------------------------------- 3 files changed, 228 insertions(+), 658 deletions(-) diff --git a/changes/ticket33238 b/changes/ticket33238 new file mode 100644 index 000000000..2c4c3968c --- /dev/null +++ b/changes/ticket33238 @@ -0,0 +1,5 @@ + o Minor feature (address discovery): + - If no Address statements are found, relays now prioritize guessing their + address by looking at the local interface instead of the local hostname. + If the interface address can't be found, the local hostname is used. + Closes ticket 33238. diff --git a/src/app/config/resolve_addr.c b/src/app/config/resolve_addr.c index 167a4aa4d..8c08c6ec0 100644 --- a/src/app/config/resolve_addr.c +++ b/src/app/config/resolve_addr.c @@ -445,8 +445,8 @@ static fn_address_ret_t { /* These functions are in order for our find address algorithm. */ get_address_from_config, - get_address_from_hostname, get_address_from_interface, + get_address_from_hostname, }; /** Length of address table as in how many functions. */ static const size_t fn_address_table_len = ARRAY_LENGTH(fn_address_table); @@ -478,7 +478,17 @@ static const size_t fn_address_table_len = ARRAY_LENGTH(fn_address_table); * * If no given Address, fallback to the local hostname (see section 2). * - * 2. Look at the local hostname. + * 2. Look at the network interface. + * + * Attempt to find the first public usable address from the list of + * network interface returned by the OS. + * + * On failure, we attempt to look at the local hostname (3). + * + * On success, addr_out is set with it, method_out is set to "INTERFACE" + * and hostname_out is set to NULL. + * + * 3. Look at the local hostname. * * If the local hostname resolves to a non internal address, addr_out is * set with it, method_out is set to "GETHOSTNAME" and hostname_out is set @@ -489,20 +499,7 @@ static const size_t fn_address_table_len = ARRAY_LENGTH(fn_address_table); * If the local hostname resolves to an internal address, an error is * returned. * - * If the local hostname can NOT be resolved, fallback to the network - * interface (see section 3). - * - * 3. Look at the network interface. - * - * Attempt to find the first public usable address from the list of - * network interface returned by the OS. - * - * On failure, an error is returned. This error indicates that all - * attempts have failed and thus the address for the given family can not - * be found. - * - * On success, addr_out is set with it, method_out is set to "INTERFACE" - * and hostname_out is set to NULL. + * If the local hostname can NOT be resolved, an error is returned. * * @param options Global configuration options. * @param family IP address family. Only AF_INET and AF_INET6 are supported. diff --git a/src/test/test_config.c b/src/test/test_config.c index 84b18c90e..c4d6a6efc 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -53,6 +53,7 @@ #include "test/test_helpers.h" #include "test/resolve_test_helpers.h" +#include "test/log_test_helpers.h" #include "feature/dirclient/dir_server_st.h" #include "core/or/port_cfg_st.h" @@ -989,6 +990,9 @@ test_config_fix_my_family(void *arg) } static int n_hostname_01010101 = 0; +static const char *ret_addr_lookup_01010101[2] = { + "1.1.1.1", "0101::0101", +}; /** This mock function is meant to replace tor_addr_lookup(). * It answers with 1.1.1.1 as IP adddress that resulted from lookup. @@ -1002,12 +1006,13 @@ tor_addr_lookup_01010101(const char *name, uint16_t family, tor_addr_t *addr) if (family == AF_INET) { if (name && addr) { - tor_addr_from_ipv4h(addr, 0x01010101); + int ret = tor_addr_parse(addr, ret_addr_lookup_01010101[0]); + tt_int_op(ret, OP_EQ, family); } } else if (family == AF_INET6) { if (name && addr) { - int ret = tor_addr_parse(addr, "0101::0101"); - tt_int_op(ret, OP_EQ, AF_INET6); + int ret = tor_addr_parse(addr, ret_addr_lookup_01010101[1]); + tt_int_op(ret, OP_EQ, family); } } done: @@ -1154,6 +1159,9 @@ tor_gethostname_failure(char *name, size_t namelen) static int n_get_interface_address6 = 0; static sa_family_t last_address6_family; +static const char *ret_get_interface_address6_08080808[2] = { + "8.8.8.8", "0808::0808", +}; /** This mock function is meant to replace get_interface_address(). * It answers with address 8.8.8.8. This function increments @@ -1169,11 +1177,12 @@ get_interface_address6_08080808(int severity, sa_family_t family, if (family == AF_INET) { if (addr) { - tor_addr_from_ipv4h(addr, 0x08080808); + int ret = tor_addr_parse(addr, ret_get_interface_address6_08080808[0]); + tt_int_op(ret, OP_EQ, AF_INET); } } else if (family == AF_INET6) { if (addr) { - int ret = tor_addr_parse(addr, "0808::0808"); + int ret = tor_addr_parse(addr, ret_get_interface_address6_08080808[1]); tt_int_op(ret, OP_EQ, AF_INET6); } } @@ -1188,6 +1197,7 @@ get_interface_address6_08080808(int severity, sa_family_t family, * This function increments <b>n_get_interface_address6</b> by one every * time it is called. */ +#if 0 static int get_interface_address6_replacement(int severity, sa_family_t family, tor_addr_t *addr) @@ -1205,6 +1215,7 @@ get_interface_address6_replacement(int severity, sa_family_t family, return 0; } +#endif static int n_get_interface_address6_failure = 0; @@ -1389,8 +1400,40 @@ test_config_find_my_address_mixed(void *arg) UNMOCK(tor_addr_lookup); } +/** Parameters for the find_my_address() test. We test both AF_INET and + * AF_INET6 but we have one interface to do so thus we run the same exact unit + * tests for both without copying them. */ +typedef struct find_my_address_params_t { + /* Index where the mock function results are located. For intance, + * tor_addr_lookup_01010101() will have its returned value depending on the + * family in ret_addr_lookup_01010101[]. + * + * Values that can be found: + * AF_INET : index 0. + * AF_INET6: index 1. + */ + int idx; + int family; + const char *public_ip; + const char *internal_ip; +} find_my_address_params_t; + +static find_my_address_params_t addr_param_v4 = { + .idx = 0, + .family = AF_INET, + .public_ip = "128.52.128.105", + .internal_ip = "127.0.0.1", +}; + +static find_my_address_params_t addr_param_v6 = { + .idx = 1, + .family = AF_INET6, + .public_ip = "4242::4242", + .internal_ip = "::1", +}; + static void -test_config_find_my_address_v6(void *arg) +test_config_find_my_address(void *arg) { or_options_t *options; tor_addr_t resolved_addr, test_addr; @@ -1398,786 +1441,306 @@ test_config_find_my_address_v6(void *arg) char *hostname_out = NULL; bool retval; int prev_n_hostname_01010101; - int prev_n_hostname_localhost; int prev_n_hostname_failure; + int prev_n_hostname_localhost; int prev_n_gethostname_replacement; int prev_n_gethostname_failure; int prev_n_gethostname_localhost; int prev_n_get_interface_address6; int prev_n_get_interface_address6_failure; - (void)arg; + const find_my_address_params_t *p = arg; options = options_new(); - options_init(options); /* - * CASE 1: - * If options->Address is a valid IPv6 address string, we want - * the corresponding address to be parsed and returned. + * Case 1: + * 1. Address is a valid address. + * + * Expected to succeed. */ - config_line_append(&options->Address, "Address", - "2a01:4f8:fff0:4f:266:37ff:fe2c:5d19"); - tor_addr_parse(&test_addr, "2a01:4f8:fff0:4f:266:37ff:fe2c:5d19"); + config_line_append(&options->Address, "Address", p->public_ip); + tor_addr_parse(&test_addr, p->public_ip); - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(retval == true); - tt_want_str_op(method_used, OP_EQ, "CONFIGURED"); - tt_want(hostname_out == NULL); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - config_free_lines(options->Address); + VALIDATE_FOUND_ADDRESS(true, "CONFIGURED", NULL); + CLEANUP_FOUND_ADDRESS; /* - * CASE 2: - * If options->Address is a valid DNS address, we want find_my_address() - * function to ask tor_addr_lookup() for help with resolving it - * and return the address that was resolved (in host order). + * Case 2: Address is a resolvable address. Expected to succeed. */ - MOCK(tor_addr_lookup, tor_addr_lookup_01010101); config_line_append(&options->Address, "Address", "www.torproject.org"); - tor_addr_parse(&test_addr, "0101::0101"); - - prev_n_hostname_01010101 = n_hostname_01010101; - - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(retval == true); - tt_want(n_hostname_01010101 == prev_n_hostname_01010101 + 1); - tt_want_str_op(method_used, OP_EQ, "RESOLVED"); - tt_want_str_op(hostname_out, OP_EQ, "www.torproject.org"); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - UNMOCK(tor_addr_lookup); - - config_free_lines(options->Address); - tor_free(hostname_out); - - /* - * CASE 3: - * Given that options->Address is NULL, we want find_my_address() - * to try and use tor_gethostname() to get hostname AND use - * tor_addr_lookup() to get IP address. - */ - - tor_addr_make_unspec(&resolved_addr); - options->Address = NULL; - tor_addr_parse(&test_addr, "0101::0101"); - - MOCK(tor_gethostname, tor_gethostname_replacement); - MOCK(tor_addr_lookup, tor_addr_lookup_01010101); + tor_addr_parse(&test_addr, ret_addr_lookup_01010101[p->idx]); - prev_n_gethostname_replacement = n_gethostname_replacement; prev_n_hostname_01010101 = n_hostname_01010101; - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(retval == true); - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(n_hostname_01010101 == prev_n_hostname_01010101 + 1); - tt_want_str_op(method_used, OP_EQ, "GETHOSTNAME"); - tt_want_str_op(hostname_out, OP_EQ, "onionrouter!"); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - UNMOCK(tor_gethostname); - UNMOCK(tor_addr_lookup); - - tor_free(hostname_out); - - /* - * CASE 4: - * Given that options->Address is a local host address, we want - * find_my_address() function to fail. - */ - - tor_addr_make_unspec(&resolved_addr); - config_line_append(&options->Address, "Address", "::1"); - tor_addr_parse(&test_addr, "::1"); - - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(tor_addr_is_null(&resolved_addr) == 1); - tt_want(retval == false); - - config_free_lines(options->Address); - tor_free(hostname_out); - - /* - * CASE 5: - * We want find_my_address() to fail if DNS address in options->Address - * cannot be resolved. - */ - - MOCK(tor_addr_lookup, tor_addr_lookup_failure); - - prev_n_hostname_failure = n_hostname_failure; - - config_line_append(&options->Address, "Address", "www.tor-project.org"); - - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_hostname_failure == prev_n_hostname_failure + 1); - tt_want(tor_addr_is_null(&resolved_addr) == 1); - tt_want(retval == false); + tt_int_op(n_hostname_01010101, OP_EQ, ++prev_n_hostname_01010101); + VALIDATE_FOUND_ADDRESS(true, "RESOLVED", "www.torproject.org"); + CLEANUP_FOUND_ADDRESS; UNMOCK(tor_addr_lookup); - config_free_lines(options->Address); - options->Address = NULL; - tor_free(hostname_out); - /* - * CASE 6: - * If options->Address is NULL AND gettting local hostname fails, we want - * find_my_address() to fail as well. + * Case 3: Address is a local addressi (internal). Expected to fail. */ + config_line_append(&options->Address, "Address", p->internal_ip); - MOCK(tor_gethostname,tor_gethostname_failure); - - prev_n_gethostname_failure = n_gethostname_failure; + setup_full_capture_of_logs(LOG_NOTICE); - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_gethostname_failure == prev_n_gethostname_failure + 1); - tt_want(tor_addr_is_null(&resolved_addr) == 1); - tt_want(retval == false); + expect_log_msg_containing("is a private IP address. Tor relays that " + "use the default DirAuthorities must have " + "public IP addresses."); + teardown_capture_of_logs(); - UNMOCK(tor_gethostname); - tor_free(hostname_out); + VALIDATE_FOUND_ADDRESS(false, NULL, NULL); + CLEANUP_FOUND_ADDRESS; /* - * CASE 7: - * We want find_my_address() to try and get network interface address via - * get_interface_address() if hostname returned by tor_gethostname() cannot - * be resolved into IP address. + * Case 4: Address is a local address but custom authorities. Expected to + * succeed. */ + config_line_append(&options->Address, "Address", p->internal_ip); + options->DirAuthorities = tor_malloc_zero(sizeof(config_line_t)); + tor_addr_parse(&test_addr, p->internal_ip); - MOCK(tor_gethostname, tor_gethostname_replacement); - MOCK(tor_addr_lookup, tor_addr_lookup_failure); - MOCK(get_interface_address6, get_interface_address6_08080808); - - tor_addr_parse(&test_addr, "0808::0808"); - - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_get_interface_address6 = n_get_interface_address6; - - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(retval == true); - tt_want_int_op(n_gethostname_replacement, OP_EQ, - prev_n_gethostname_replacement + 1); - tt_want_int_op(n_get_interface_address6, OP_EQ, - prev_n_get_interface_address6 + 1); - tt_want_str_op(method_used, OP_EQ, "INTERFACE"); - tt_want(hostname_out == NULL); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - UNMOCK(get_interface_address); - tor_free(hostname_out); + VALIDATE_FOUND_ADDRESS(true, "CONFIGURED", NULL); + CLEANUP_FOUND_ADDRESS; /* - * CASE 8: - * Suppose options->Address is NULL AND hostname returned by - * tor_gethostname() is unresolvable. We want find_my_address to fail if - * get_interface_address() fails. + * Case 5: Multiple address in Address. Expected to fail. */ + config_line_append(&options->Address, "Address", p->public_ip); + config_line_append(&options->Address, "Address", p->public_ip); - MOCK(get_interface_address6, get_interface_address6_failure); - - prev_n_get_interface_address6_failure = n_get_interface_address6_failure; - prev_n_gethostname_replacement = n_gethostname_replacement; + setup_full_capture_of_logs(LOG_NOTICE); - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_get_interface_address6_failure == - prev_n_get_interface_address6_failure + 1); - tt_want(n_gethostname_replacement == - prev_n_gethostname_replacement + 1); - tt_want(retval == false); + expect_log_msg_containing("Found 2 Address statement of address family"); + teardown_capture_of_logs(); - UNMOCK(get_interface_address); - tor_free(hostname_out); + VALIDATE_FOUND_ADDRESS(false, NULL, NULL); + CLEANUP_FOUND_ADDRESS; /* - * CASE 9: - * Given that options->Address is NULL AND tor_addr_lookup() - * fails AND hostname returned by gethostname() resolves - * to local IP address, we want find_my_address() function to - * call get_interface_address6(.,AF_INET6,.) and return IP address - * the latter function has found. + * Case 6: Another address family is configured. Expected to fail. */ + if (p->family == AF_INET) { + config_line_append(&options->Address, "Address", "4242::4242"); + } else { + config_line_append(&options->Address, "Address", "1.1.1.1"); + } - MOCK(tor_addr_lookup, tor_addr_lookup_failure); - MOCK(tor_gethostname, tor_gethostname_replacement); - MOCK(get_interface_address6, get_interface_address6_08080808); - - tor_addr_parse(&test_addr, "0808::0808"); - - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_hostname_failure = n_hostname_failure; - prev_n_get_interface_address6 = n_get_interface_address6; + setup_full_capture_of_logs(LOG_NOTICE); - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(last_address6_family == AF_INET6); - tt_want(n_get_interface_address6 == prev_n_get_interface_address6 + 1); - tt_want(n_hostname_failure == prev_n_hostname_failure + 1); - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(retval == true); - tt_want_str_op(method_used, OP_EQ, "INTERFACE"); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - UNMOCK(tor_addr_lookup); - UNMOCK(tor_gethostname); - UNMOCK(get_interface_address6); + expect_log_msg_containing("No Address option found for family"); + teardown_capture_of_logs(); - tor_free(hostname_out); + VALIDATE_FOUND_ADDRESS(false, NULL, NULL); + CLEANUP_FOUND_ADDRESS; /* - * CASE 10: We want find_my_address() to fail if all of the following - * are true: - * 1. options->Address is not NULL - * 2. ... but it cannot be converted to struct in_addr by - * tor_inet_aton() - * 3. ... and tor_addr_lookup() fails to resolve the - * options->Address + * Case 7: Address is a non resolvable hostname. Expected to fail. */ - MOCK(tor_addr_lookup, tor_addr_lookup_failure); + config_line_append(&options->Address, "Address", "www.torproject.org"); prev_n_hostname_failure = n_hostname_failure; - config_line_append(&options->Address, "Address", "some_hostname"); + setup_full_capture_of_logs(LOG_NOTICE); - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_hostname_failure == prev_n_hostname_failure + 1); - tt_want(retval == false); + expect_log_msg_containing("Could not resolve local Address " + "'www.torproject.org'. Failing."); + teardown_capture_of_logs(); - UNMOCK(tor_gethostname); - UNMOCK(tor_addr_lookup); + tt_int_op(n_hostname_failure, OP_EQ, ++prev_n_hostname_failure); + VALIDATE_FOUND_ADDRESS(false, NULL, NULL); + CLEANUP_FOUND_ADDRESS; - tor_free(hostname_out); + UNMOCK(tor_addr_lookup); /* - * CASE 11: - * Suppose the following sequence of events: - * 1. options->Address is NULL - * 2. tor_gethostname() succeeds to get hostname of machine Tor - * if running on. - * 3. Hostname from previous step cannot be converted to - * address by using tor_inet_aton() function. - * 4. However, tor_addr_lookup() succeeds in resolving the - * hostname from step 2. - * 5. Unfortunately, tor_addr_is_internal() deems this address - * to be internal. - * 6. get_interface_address6(.,AF_INET,.) returns non-internal - * IPv4 + * Case 8: + * 1. Address is NULL + * 2. Interface address is a valid address. * - * We want resolve_my_addr() to succeed with method "INTERFACE" - * and address from step 6. + * Expected to succeed. */ - - config_free_lines(options->Address); options->Address = NULL; - tor_addr_parse(&test_addr, "0808::0808"); + tor_addr_parse(&test_addr, ret_get_interface_address6_08080808[p->idx]); - MOCK(tor_gethostname, tor_gethostname_replacement); - MOCK(tor_addr_lookup, tor_addr_lookup_localhost); MOCK(get_interface_address6, get_interface_address6_08080808); - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_hostname_localhost = n_hostname_localhost; prev_n_get_interface_address6 = n_get_interface_address6; - retval = find_my_address(options, AF_INET6, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(n_hostname_localhost == prev_n_hostname_localhost + 1); - tt_want(n_get_interface_address6 == prev_n_get_interface_address6 + 1); + tt_int_op(n_get_interface_address6, OP_EQ, ++prev_n_get_interface_address6); + VALIDATE_FOUND_ADDRESS(true, "INTERFACE", NULL); + CLEANUP_FOUND_ADDRESS; - tt_str_op(method_used, OP_EQ, "INTERFACE"); - tt_ptr_op(hostname_out, OP_EQ, NULL); - tt_want(retval == true); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); + UNMOCK(get_interface_address6); /* - * CASE 11b: - * 1-5 as above. - * 6. get_interface_address6() fails. + * Case 9: + * 1. Address is NULL + * 2. Interface address fails to be found. + * 3. Local hostname resolves to a valid address. * - * In this subcase, we want find_my_address() to fail. + * Expected to succeed. */ + options->Address = NULL; + tor_addr_parse(&test_addr, ret_addr_lookup_01010101[p->idx]); - UNMOCK(get_interface_address6); MOCK(get_interface_address6, get_interface_address6_failure); + MOCK(tor_gethostname, tor_gethostname_replacement); + MOCK(tor_addr_lookup, tor_addr_lookup_01010101); - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_hostname_localhost = n_hostname_localhost; prev_n_get_interface_address6_failure = n_get_interface_address6_failure; + prev_n_hostname_01010101 = n_hostname_01010101; + prev_n_gethostname_replacement = n_gethostname_replacement; - retval = find_my_address(options, AF_INET6, LOG_DEBUG, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(n_hostname_localhost == prev_n_hostname_localhost + 1); - tt_want(n_get_interface_address6_failure == - prev_n_get_interface_address6_failure + 1); - - tt_want(retval == false); + tt_int_op(n_get_interface_address6_failure, OP_EQ, + ++prev_n_get_interface_address6_failure); + tt_int_op(n_hostname_01010101, OP_EQ, + ++prev_n_hostname_01010101); + tt_int_op(n_gethostname_replacement, OP_EQ, + ++prev_n_gethostname_replacement); + VALIDATE_FOUND_ADDRESS(true, "GETHOSTNAME", "onionrouter!"); + CLEANUP_FOUND_ADDRESS; + UNMOCK(get_interface_address6); UNMOCK(tor_gethostname); UNMOCK(tor_addr_lookup); - UNMOCK(get_interface_address6); - /* CASE 12: - * Suppose the following happens: - * 1. options->Address is NULL AND options->DirAuthorities is non-NULL - * 2. tor_gethostname() succeeds in getting hostname of a machine ... - * 3. ... which is successfully parsed by tor_inet_aton() ... - * 4. into IPv4 address that tor_addr_is_inernal() considers to be - * internal. + /* + * Case 10: + * 1. Address is NULL + * 2. Interface address fails to be found. + * 3. Local hostname resolves to an internal address. * - * In this case, we want find_my_address() to fail. + * Expected to fail. */ - - tor_free(options->Address); options->Address = NULL; - options->DirAuthorities = tor_malloc_zero(sizeof(config_line_t)); - MOCK(tor_gethostname,tor_gethostname_localhost); + MOCK(get_interface_address6, get_interface_address6_failure); + MOCK(tor_gethostname, tor_gethostname_localhost); + MOCK(tor_addr_lookup, tor_addr_lookup_localhost); + prev_n_get_interface_address6_failure = n_get_interface_address6_failure; + prev_n_hostname_localhost = n_hostname_localhost; prev_n_gethostname_localhost = n_gethostname_localhost; - retval = find_my_address(options, AF_INET6, LOG_DEBUG, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_gethostname_localhost == prev_n_gethostname_localhost + 1); - tt_want(retval == false); - - UNMOCK(tor_gethostname); - - done: - config_free_lines(options->Address); - tor_free(options->DirAuthorities); - or_options_free(options); - tor_free(hostname_out); + tt_int_op(n_get_interface_address6_failure, OP_EQ, + ++prev_n_get_interface_address6_failure); + tt_int_op(n_hostname_localhost, OP_EQ, + ++prev_n_hostname_localhost); + tt_int_op(n_gethostname_localhost, OP_EQ, + ++prev_n_gethostname_localhost); + VALIDATE_FOUND_ADDRESS(false, NULL, NULL); + CLEANUP_FOUND_ADDRESS; - UNMOCK(tor_gethostname); - UNMOCK(tor_addr_lookup); - UNMOCK(get_interface_address); UNMOCK(get_interface_address6); UNMOCK(tor_gethostname); -} - -static void -test_config_find_my_address_v4(void *arg) -{ - or_options_t *options; - tor_addr_t resolved_addr, test_addr; - const char *method_used; - char *hostname_out = NULL; - bool retval; - int prev_n_hostname_01010101; - int prev_n_hostname_localhost; - int prev_n_hostname_failure; - int prev_n_gethostname_replacement; - int prev_n_gethostname_failure; - int prev_n_gethostname_localhost; - int prev_n_get_interface_address6; - int prev_n_get_interface_address6_failure; - - (void)arg; - - options = options_new(); - - options_init(options); - - /* - * CASE 1: - * If options->Address is a valid IPv4 address string, we want - * the corresponding address to be parsed and returned. - */ - config_line_append(&options->Address, "Address", "128.52.128.105"); - tor_addr_parse(&test_addr, "128.52.128.105"); - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(retval == true); - tt_want_str_op(method_used,OP_EQ,"CONFIGURED"); - tt_want(hostname_out == NULL); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - config_free_lines(options->Address); - -/* - * CASE 2: - * If options->Address is a valid DNS address, we want find_my_address() - * function to ask tor_addr_lookup() for help with resolving it - * and return the address that was resolved (in host order). - */ - - MOCK(tor_addr_lookup, tor_addr_lookup_01010101); - - config_line_append(&options->Address, "Address", "www.torproject.org"); - tor_addr_parse(&test_addr, "1.1.1.1"); - - prev_n_hostname_01010101 = n_hostname_01010101; - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(retval == true); - tt_want(n_hostname_01010101 == prev_n_hostname_01010101 + 1); - tt_want_str_op(method_used,OP_EQ,"RESOLVED"); - tt_want_str_op(hostname_out,OP_EQ,"www.torproject.org"); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - UNMOCK(tor_addr_lookup); - config_free_lines(options->Address); - tor_free(hostname_out); - -/* - * CASE 3: - * Given that options->Address is NULL, we want find_my_address() - * to try and use tor_gethostname() to get hostname AND use - * tor_addr_lookup() to get IP address. - */ - - tor_addr_make_unspec(&resolved_addr); - options->Address = NULL; - tor_addr_parse(&test_addr, "1.1.1.1"); - - MOCK(tor_gethostname,tor_gethostname_replacement); - MOCK(tor_addr_lookup,tor_addr_lookup_01010101); - - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_hostname_01010101 = n_hostname_01010101; - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(retval == true); - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(n_hostname_01010101 == prev_n_hostname_01010101 + 1); - tt_want_str_op(method_used,OP_EQ,"GETHOSTNAME"); - tt_want_str_op(hostname_out,OP_EQ,"onionrouter!"); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - UNMOCK(tor_gethostname); - UNMOCK(tor_addr_lookup); - - tor_free(hostname_out); - -/* - * CASE 4: - * Given that options->Address is a local host address, we want - * find_my_address() function to fail. - */ - - tor_addr_make_unspec(&resolved_addr); - config_line_append(&options->Address, "Address", "127.0.0.1"); - tor_addr_parse(&test_addr, "127.0.0.1"); - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(tor_addr_is_null(&resolved_addr) == 1); - tt_want(retval == false); - - config_free_lines(options->Address); - tor_free(hostname_out); - -/* - * CASE 5: - * We want find_my_address() to fail if DNS address in options->Address - * cannot be resolved. - */ - - MOCK(tor_addr_lookup,tor_addr_lookup_failure); - - prev_n_hostname_failure = n_hostname_failure; - - config_line_append(&options->Address, "Address", "www.tor-project.org"); - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(n_hostname_failure == prev_n_hostname_failure + 1); - tt_want(tor_addr_is_null(&resolved_addr) == 1); - tt_want(retval == false); - - UNMOCK(tor_addr_lookup); - - config_free_lines(options->Address); + /* + * Case 11: + * 1. Address is NULL + * 2. Interface address fails to be found. + * 3. Local hostname fails to be found. + * + * Expected to fail. + */ options->Address = NULL; - tor_free(hostname_out); - -/* - * CASE 6: - * If options->Address is NULL AND gettting local hostname fails, we want - * find_my_address() to fail as well. - */ - - MOCK(tor_gethostname,tor_gethostname_failure); - - prev_n_gethostname_failure = n_gethostname_failure; - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(n_gethostname_failure == prev_n_gethostname_failure + 1); - tt_want(tor_addr_is_null(&resolved_addr) == 1); - tt_want(retval == false); - - UNMOCK(tor_gethostname); - tor_free(hostname_out); - -/* - * CASE 7: - * We want find_my_address() to try and get network interface address via - * get_interface_address() if hostname returned by tor_gethostname() cannot be - * resolved into IP address. - */ - - MOCK(tor_gethostname,tor_gethostname_replacement); - MOCK(tor_addr_lookup,tor_addr_lookup_failure); - MOCK(get_interface_address6, get_interface_address6_08080808); - - tor_addr_parse(&test_addr, "8.8.8.8"); - - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_get_interface_address6 = n_get_interface_address6; - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(retval == true); - tt_want_int_op(n_gethostname_replacement, OP_EQ, - prev_n_gethostname_replacement + 1); - tt_want_int_op(n_get_interface_address6, OP_EQ, - prev_n_get_interface_address6 + 1); - tt_want_str_op(method_used,OP_EQ,"INTERFACE"); - tt_want(hostname_out == NULL); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - UNMOCK(get_interface_address); - tor_free(hostname_out); - -/* - * CASE 8: - * Suppose options->Address is NULL AND hostname returned by tor_gethostname() - * is unresolvable. We want find_my_address to fail if - * get_interface_address() fails. - */ MOCK(get_interface_address6, get_interface_address6_failure); + MOCK(tor_gethostname, tor_gethostname_failure); prev_n_get_interface_address6_failure = n_get_interface_address6_failure; - prev_n_gethostname_replacement = n_gethostname_replacement; - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(n_get_interface_address6_failure == - prev_n_get_interface_address6_failure + 1); - tt_want(n_gethostname_replacement == - prev_n_gethostname_replacement + 1); - tt_want(retval == false); - - UNMOCK(get_interface_address); - tor_free(hostname_out); - -/* - * CASE 9: - * Given that options->Address is NULL AND tor_addr_lookup() - * fails AND hostname returned by gethostname() resolves - * to local IP address, we want find_my_address() function to - * call get_interface_address6(.,AF_INET,.) and return IP address - * the latter function has found. - */ - - MOCK(tor_addr_lookup,tor_addr_lookup_failure); - MOCK(tor_gethostname,tor_gethostname_replacement); - MOCK(get_interface_address6,get_interface_address6_replacement); - - tor_addr_parse(&test_addr, "9.9.9.9"); - - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_hostname_failure = n_hostname_failure; - prev_n_get_interface_address6 = n_get_interface_address6; + prev_n_gethostname_failure = n_gethostname_failure; - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(last_address6_family == AF_INET); - tt_want(n_get_interface_address6 == prev_n_get_interface_address6 + 1); - tt_want(n_hostname_failure == prev_n_hostname_failure + 1); - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(retval == true); - tt_want_str_op(method_used,OP_EQ,"INTERFACE"); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); + tt_int_op(n_get_interface_address6_failure, OP_EQ, + ++prev_n_get_interface_address6_failure); + tt_int_op(n_gethostname_failure, OP_EQ, + ++prev_n_gethostname_failure); + VALIDATE_FOUND_ADDRESS(false, NULL, NULL); + CLEANUP_FOUND_ADDRESS; - UNMOCK(tor_addr_lookup); - UNMOCK(tor_gethostname); UNMOCK(get_interface_address6); - - tor_free(hostname_out); - - /* - * CASE 10: We want find_my_address() to fail if all of the following - * are true: - * 1. options->Address is not NULL - * 2. ... but it cannot be converted to struct in_addr by - * tor_inet_aton() - * 3. ... and tor_addr_lookup() fails to resolve the - * options->Address - */ - - MOCK(tor_addr_lookup, tor_addr_lookup_failure); - - prev_n_hostname_failure = n_hostname_failure; - - config_line_append(&options->Address, "Address", "some_hostname"); - - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, - &method_used, &hostname_out); - - tt_want(n_hostname_failure == prev_n_hostname_failure + 1); - tt_want(retval == false); - UNMOCK(tor_gethostname); - UNMOCK(tor_addr_lookup); - - tor_free(hostname_out); /* - * CASE 11: - * Suppose the following sequence of events: - * 1. options->Address is NULL - * 2. tor_gethostname() succeeds to get hostname of machine Tor - * if running on. - * 3. Hostname from previous step cannot be converted to - * address by using tor_inet_aton() function. - * 4. However, tor_addr_lookup() succeeds in resolving the - * hostname from step 2. - * 5. Unfortunately, tor_addr_is_internal() deems this address - * to be internal. - * 6. get_interface_address6(.,AF_INET,.) returns non-internal - * IPv4 + * Case 12: + * 1. Address is NULL + * 2. Interface address fails to be found. + * 3. Local hostname can't be resolved. * - * We want resolve_my_addr() to succeed with method "INTERFACE" - * and address from step 6. + * Expected to fail. */ - - config_free_lines(options->Address); options->Address = NULL; - tor_addr_parse(&test_addr, "9.9.9.9"); - MOCK(tor_gethostname,tor_gethostname_replacement); - MOCK(tor_addr_lookup,tor_addr_lookup_localhost); - MOCK(get_interface_address6,get_interface_address6_replacement); + MOCK(get_interface_address6, get_interface_address6_failure); + MOCK(tor_gethostname, tor_gethostname_replacement); + MOCK(tor_addr_lookup, tor_addr_lookup_failure); + prev_n_get_interface_address6_failure = n_get_interface_address6_failure; prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_hostname_localhost = n_hostname_localhost; - prev_n_get_interface_address6 = n_get_interface_address6; + prev_n_hostname_failure = n_hostname_failure; - retval = find_my_address(options, AF_INET, LOG_NOTICE, &resolved_addr, + retval = find_my_address(options, p->family, LOG_NOTICE, &resolved_addr, &method_used, &hostname_out); - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(n_hostname_localhost == prev_n_hostname_localhost + 1); - tt_want(n_get_interface_address6 == prev_n_get_interface_address6 + 1); - - tt_str_op(method_used,OP_EQ,"INTERFACE"); - tt_ptr_op(hostname_out, OP_EQ, NULL); - tt_want(retval == true); - tt_assert(tor_addr_eq(&resolved_addr, &test_addr)); - - /* - * CASE 11b: - * 1-5 as above. - * 6. get_interface_address6() fails. - * - * In this subcase, we want find_my_address() to fail. - */ + tt_int_op(n_get_interface_address6_failure, OP_EQ, + ++prev_n_get_interface_address6_failure); + tt_int_op(n_gethostname_replacement, OP_EQ, + ++prev_n_gethostname_replacement); + tt_int_op(n_hostname_failure, OP_EQ, + ++prev_n_hostname_failure); + VALIDATE_FOUND_ADDRESS(false, NULL, NULL); + CLEANUP_FOUND_ADDRESS; UNMOCK(get_interface_address6); - MOCK(get_interface_address6,get_interface_address6_failure); - - prev_n_gethostname_replacement = n_gethostname_replacement; - prev_n_hostname_localhost = n_hostname_localhost; - prev_n_get_interface_address6_failure = n_get_interface_address6_failure; - - retval = find_my_address(options, AF_INET, LOG_DEBUG, &resolved_addr, - &method_used, &hostname_out); - - tt_want(n_gethostname_replacement == prev_n_gethostname_replacement + 1); - tt_want(n_hostname_localhost == prev_n_hostname_localhost + 1); - tt_want(n_get_interface_address6_failure == - prev_n_get_interface_address6_failure + 1); - - tt_want(retval == false); - UNMOCK(tor_gethostname); UNMOCK(tor_addr_lookup); - UNMOCK(get_interface_address6); - - /* CASE 12: - * Suppose the following happens: - * 1. options->Address is NULL AND options->DirAuthorities is non-NULL - * 2. tor_gethostname() succeeds in getting hostname of a machine ... - * 3. ... which is successfully parsed by tor_inet_aton() ... - * 4. into IPv4 address that tor_addr_is_inernal() considers to be - * internal. - * - * In this case, we want find_my_address() to fail. - */ - - tor_free(options->Address); - options->Address = NULL; - options->DirAuthorities = tor_malloc_zero(sizeof(config_line_t)); - - MOCK(tor_gethostname,tor_gethostname_localhost); - - prev_n_gethostname_localhost = n_gethostname_localhost; - - retval = find_my_address(options, AF_INET, LOG_DEBUG, &resolved_addr, - &method_used, &hostname_out); - - tt_want(n_gethostname_localhost == prev_n_gethostname_localhost + 1); - tt_want(retval == false); - - UNMOCK(tor_gethostname); done: - config_free_lines(options->Address); - tor_free(options->DirAuthorities); or_options_free(options); - tor_free(hostname_out); UNMOCK(tor_gethostname); UNMOCK(tor_addr_lookup); - UNMOCK(get_interface_address); UNMOCK(get_interface_address6); - UNMOCK(tor_gethostname); } static void @@ -6851,6 +6414,9 @@ test_config_getinfo_config_names(void *arg) #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } +#define CONFIG_TEST_SETUP(suffix, name, flags, setup, setup_data) \ + { #name#suffix, test_config_ ## name, flags, setup, setup_data } + struct testcase_t config_tests[] = { CONFIG_TEST(adding_trusted_dir_server, TT_FORK), CONFIG_TEST(adding_fallback_dir_server, TT_FORK), @@ -6861,8 +6427,10 @@ struct testcase_t config_tests[] = { CONFIG_TEST(adding_dir_servers, TT_FORK), CONFIG_TEST(default_dir_servers, TT_FORK), CONFIG_TEST(default_fallback_dirs, 0), - CONFIG_TEST(find_my_address_v4, TT_FORK), - CONFIG_TEST(find_my_address_v6, TT_FORK), + CONFIG_TEST_SETUP(_v4, find_my_address, TT_FORK, + &passthrough_setup, &addr_param_v4), + CONFIG_TEST_SETUP(_v6, find_my_address, TT_FORK, + &passthrough_setup, &addr_param_v6), CONFIG_TEST(find_my_address_mixed, TT_FORK), CONFIG_TEST(addressmap, 0), CONFIG_TEST(parse_bridge_line, 0),

1 0

[tor/maint-0.4.4] Store errors from TLS write and handshake in the tls_error field.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit d486c6cf74db6bce80689c17c7427a6e3a055f93 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 10:45:42 2020 -0400 Store errors from TLS write and handshake in the tls_error field. Previously, we would only update this field when the error happened during a read. This will improves our reporting for our bootstrap status, and help to address #32622. The problem is not completely solved by this patch, however: too many errors are still lumped into "MISC". --- src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index a8417e46d..792cdbf68 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -4205,6 +4205,7 @@ connection_handle_write_impl(connection_t *conn, int force) switch (result) { CASE_TOR_TLS_ERROR_ANY: case TOR_TLS_CLOSE: + or_conn->tls_error = result; log_info(LD_NET, result != TOR_TLS_CLOSE ? "tls error. breaking.":"TLS connection closed on flush"); /* Don't flush; connection is dead. */ diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 5d71b363f..4cb83f45a 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -1692,7 +1692,8 @@ connection_tls_continue_handshake(or_connection_t *conn) switch (result) { CASE_TOR_TLS_ERROR_ANY: - log_info(LD_OR,"tls error [%s]. breaking connection.", + conn->tls_error = result; + log_info(LD_OR,"tls error [%s]. breaking connection.", tor_tls_err_to_string(result)); return -1; case TOR_TLS_DONE: @@ -1724,6 +1725,7 @@ connection_tls_continue_handshake(or_connection_t *conn) log_debug(LD_OR,"wanted read"); return 0; case TOR_TLS_CLOSE: + conn->tls_error = result; log_info(LD_OR,"tls closed. breaking connection."); return -1; }

1 0

[tor/maint-0.4.4] Merge branch 'ticket32622_044_squashed' into maint-0.4.4
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 18fa53fdf4e2b35ceee54c317e989bdbc31183d5 Merge: 8da10e823 65b05b12a Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 2 10:14:55 2020 -0400 Merge branch 'ticket32622_044_squashed' into maint-0.4.4 changes/ticket32622 | 5 +++++ src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 18 +++++++++++++----- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 9 files changed, 62 insertions(+), 6 deletions(-)

1 0

[tor/maint-0.4.4] Changes file for #32622.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 65b05b12ab009aad6f7a2702e5217337d2f67f50 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 11:21:53 2020 -0400 Changes file for #32622. --- changes/ticket32622 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/ticket32622 b/changes/ticket32622 new file mode 100644 index 000000000..1c663567f --- /dev/null +++ b/changes/ticket32622 @@ -0,0 +1,5 @@ + o Minor features (bootstrap reporting): + - Report more detailed reasons for bootstrap failure when the failure + happens due to a TLS error. Previously we would just call these errors + "MISC" when they happened during read, and "DONE" when they + happened during any other TLS operation. Closes ticket 32622.

1 0

[tor/maint-0.4.4] Add a TLS_ERROR bootstrap failure reason.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit e429ceb2667787750af070929d0e64cbc7a6dda4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 10:59:59 2020 -0400 Add a TLS_ERROR bootstrap failure reason. If our TLS connection fails for a "misc" reason, we don't need to say that the reason is "misc" -- we can at least localize it to the TLS module. Part of a fix for #32622. --- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/or/or.h b/src/core/or/or.h index 5b35cbe7f..8758a2ec6 100644 --- a/src/core/or/or.h +++ b/src/core/or/or.h @@ -220,7 +220,8 @@ struct curve25519_public_key_t; #define END_OR_CONN_REASON_IO_ERROR 7 /* read/write error */ #define END_OR_CONN_REASON_RESOURCE_LIMIT 8 /* sockets, buffers, etc */ #define END_OR_CONN_REASON_PT_MISSING 9 /* PT failed or not available */ -#define END_OR_CONN_REASON_MISC 10 +#define END_OR_CONN_REASON_TLS_ERROR 10 /* Problem in TLS protocol */ +#define END_OR_CONN_REASON_MISC 11 /* Reasons why we (or a remote OR) might close a stream. See tor-spec.txt for * documentation of these. The values must match. */ diff --git a/src/core/or/reasons.c b/src/core/or/reasons.c index 7da7843ca..708f43a68 100644 --- a/src/core/or/reasons.c +++ b/src/core/or/reasons.c @@ -244,6 +244,8 @@ orconn_end_reason_to_control_string(int r) return "IOERROR"; case END_OR_CONN_REASON_RESOURCE_LIMIT: return "RESOURCELIMIT"; + case END_OR_CONN_REASON_TLS_ERROR: + return "TLS_ERROR"; case END_OR_CONN_REASON_MISC: return "MISC"; case END_OR_CONN_REASON_PT_MISSING: @@ -276,6 +278,8 @@ tls_error_to_orconn_end_reason(int e) case TOR_TLS_CLOSE: case TOR_TLS_DONE: return END_OR_CONN_REASON_DONE; + case TOR_TLS_ERROR_MISC: + return END_OR_CONN_REASON_TLS_ERROR; default: return END_OR_CONN_REASON_MISC; }

1 0

[tor/maint-0.4.4] Carry TLS error strings forward to controller when reporting them.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 7b5f58a1c93f99ef3eaa840b4443ca0a4120a512 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 11:18:38 2020 -0400 Carry TLS error strings forward to controller when reporting them. Now instead of saying "DONE, DONE" or "MISC, MISC" or "TLS_ERROR, TLS_ERROR", we can finally give a nice sensible "TLS_ERROR, wrong version number" which should help debug a great deal. Closes ticket 32622. --- src/core/or/connection_or.c | 14 ++++++++++---- src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 4cb83f45a..b88d1b6af 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -745,10 +745,16 @@ connection_or_about_to_close(or_connection_t *or_conn) int reason = tls_error_to_orconn_end_reason(or_conn->tls_error); connection_or_event_status(or_conn, OR_CONN_EVENT_FAILED, reason); - if (!authdir_mode_tests_reachability(options)) - control_event_bootstrap_prob_or( - orconn_end_reason_to_control_string(reason), - reason, or_conn); + if (!authdir_mode_tests_reachability(options)) { + const char *warning = NULL; + if (reason == END_OR_CONN_REASON_TLS_ERROR && or_conn->tls) { + warning = tor_tls_get_last_error_msg(or_conn->tls); + } + if (warning == NULL) { + warning = orconn_end_reason_to_control_string(reason); + } + control_event_bootstrap_prob_or(warning, reason, or_conn); + } } } } else if (conn->hold_open_until_flushed) { diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h index e8dbbf527..517cdc17d 100644 --- a/src/lib/tls/tortls.h +++ b/src/lib/tls/tortls.h @@ -81,6 +81,7 @@ void tor_tls_free_all(void); void tor_tls_init(void); void tls_log_errors(tor_tls_t *tls, int severity, int domain, const char *doing); +const char *tor_tls_get_last_error_msg(const tor_tls_t *tls); int tor_tls_context_init(unsigned flags, crypto_pk_t *client_identity, crypto_pk_t *server_identity, diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index 62e826211..a9ec731c0 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -369,6 +369,8 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, (void)tls; PRErrorCode code = PORT_GetError(); + if (tls) + tls->last_error = code; const char *addr = tls ? tls->address : NULL; const char *string = PORT_ErrorToString(code); @@ -391,6 +393,17 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, with, addr); } } +const char * +tor_tls_get_last_error_msg(const tor_tls_t *tls) +{ + IF_BUG_ONCE(!tls) { + return NULL; + } + if (tls->last_error == 0) { + return NULL; + } + return PORT_ErrorToString((PRErrorCode)tls->last_error); +} tor_tls_t * tor_tls_new(tor_socket_t sock, int is_server) diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index 68d6e2aa5..226971414 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -245,10 +245,30 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, const char *doing) unsigned long err; while ((err = ERR_get_error()) != 0) { + if (tls) + tls->last_error = err; tor_tls_log_one_error(tls, err, severity, domain, doing); } } +/** + * Return a string representing more detail about the last error received + * on TLS. + * + * May return null if no error was found. + **/ +const char * +tor_tls_get_last_error_msg(const tor_tls_t *tls) +{ + IF_BUG_ONCE(!tls) { + return NULL; + } + if (tls->last_error == 0) { + return NULL; + } + return (const char*)ERR_reason_error_string(tls->last_error); +} + #define CATCH_SYSCALL 1 #define CATCH_ZERO 2 diff --git a/src/lib/tls/tortls_st.h b/src/lib/tls/tortls_st.h index 925896d49..34abe52ee 100644 --- a/src/lib/tls/tortls_st.h +++ b/src/lib/tls/tortls_st.h @@ -67,6 +67,8 @@ struct tor_tls_t { */ unsigned long last_write_count; unsigned long last_read_count; + /** Most recent error value from ERR_get_error(). */ + unsigned long last_error; /** If set, a callback to invoke whenever the client tries to renegotiate * the handshake. */ void (*negotiated_callback)(tor_tls_t *tls, void *arg); @@ -77,6 +79,7 @@ struct tor_tls_t { /** Last values retried from tor_get_prfiledesc_byte_counts(). */ uint64_t last_write_count; uint64_t last_read_count; + long last_error; #endif };

1 0

[tor/master] Carry TLS error strings forward to controller when reporting them.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 7b5f58a1c93f99ef3eaa840b4443ca0a4120a512 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 11:18:38 2020 -0400 Carry TLS error strings forward to controller when reporting them. Now instead of saying "DONE, DONE" or "MISC, MISC" or "TLS_ERROR, TLS_ERROR", we can finally give a nice sensible "TLS_ERROR, wrong version number" which should help debug a great deal. Closes ticket 32622. --- src/core/or/connection_or.c | 14 ++++++++++---- src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 4cb83f45a..b88d1b6af 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -745,10 +745,16 @@ connection_or_about_to_close(or_connection_t *or_conn) int reason = tls_error_to_orconn_end_reason(or_conn->tls_error); connection_or_event_status(or_conn, OR_CONN_EVENT_FAILED, reason); - if (!authdir_mode_tests_reachability(options)) - control_event_bootstrap_prob_or( - orconn_end_reason_to_control_string(reason), - reason, or_conn); + if (!authdir_mode_tests_reachability(options)) { + const char *warning = NULL; + if (reason == END_OR_CONN_REASON_TLS_ERROR && or_conn->tls) { + warning = tor_tls_get_last_error_msg(or_conn->tls); + } + if (warning == NULL) { + warning = orconn_end_reason_to_control_string(reason); + } + control_event_bootstrap_prob_or(warning, reason, or_conn); + } } } } else if (conn->hold_open_until_flushed) { diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h index e8dbbf527..517cdc17d 100644 --- a/src/lib/tls/tortls.h +++ b/src/lib/tls/tortls.h @@ -81,6 +81,7 @@ void tor_tls_free_all(void); void tor_tls_init(void); void tls_log_errors(tor_tls_t *tls, int severity, int domain, const char *doing); +const char *tor_tls_get_last_error_msg(const tor_tls_t *tls); int tor_tls_context_init(unsigned flags, crypto_pk_t *client_identity, crypto_pk_t *server_identity, diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index 62e826211..a9ec731c0 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -369,6 +369,8 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, (void)tls; PRErrorCode code = PORT_GetError(); + if (tls) + tls->last_error = code; const char *addr = tls ? tls->address : NULL; const char *string = PORT_ErrorToString(code); @@ -391,6 +393,17 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, with, addr); } } +const char * +tor_tls_get_last_error_msg(const tor_tls_t *tls) +{ + IF_BUG_ONCE(!tls) { + return NULL; + } + if (tls->last_error == 0) { + return NULL; + } + return PORT_ErrorToString((PRErrorCode)tls->last_error); +} tor_tls_t * tor_tls_new(tor_socket_t sock, int is_server) diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index 68d6e2aa5..226971414 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -245,10 +245,30 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, const char *doing) unsigned long err; while ((err = ERR_get_error()) != 0) { + if (tls) + tls->last_error = err; tor_tls_log_one_error(tls, err, severity, domain, doing); } } +/** + * Return a string representing more detail about the last error received + * on TLS. + * + * May return null if no error was found. + **/ +const char * +tor_tls_get_last_error_msg(const tor_tls_t *tls) +{ + IF_BUG_ONCE(!tls) { + return NULL; + } + if (tls->last_error == 0) { + return NULL; + } + return (const char*)ERR_reason_error_string(tls->last_error); +} + #define CATCH_SYSCALL 1 #define CATCH_ZERO 2 diff --git a/src/lib/tls/tortls_st.h b/src/lib/tls/tortls_st.h index 925896d49..34abe52ee 100644 --- a/src/lib/tls/tortls_st.h +++ b/src/lib/tls/tortls_st.h @@ -67,6 +67,8 @@ struct tor_tls_t { */ unsigned long last_write_count; unsigned long last_read_count; + /** Most recent error value from ERR_get_error(). */ + unsigned long last_error; /** If set, a callback to invoke whenever the client tries to renegotiate * the handshake. */ void (*negotiated_callback)(tor_tls_t *tls, void *arg); @@ -77,6 +79,7 @@ struct tor_tls_t { /** Last values retried from tor_get_prfiledesc_byte_counts(). */ uint64_t last_write_count; uint64_t last_read_count; + long last_error; #endif };

1 0

[tor/master] Changes file for #32622.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 65b05b12ab009aad6f7a2702e5217337d2f67f50 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 11:21:53 2020 -0400 Changes file for #32622. --- changes/ticket32622 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/ticket32622 b/changes/ticket32622 new file mode 100644 index 000000000..1c663567f --- /dev/null +++ b/changes/ticket32622 @@ -0,0 +1,5 @@ + o Minor features (bootstrap reporting): + - Report more detailed reasons for bootstrap failure when the failure + happens due to a TLS error. Previously we would just call these errors + "MISC" when they happened during read, and "DONE" when they + happened during any other TLS operation. Closes ticket 32622.

1 0

[tor/master] Merge branch 'maint-0.4.4'
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 2e98e881178470201ed23ef381b0e9ede4f4f613 Merge: 20d1a1cdb 18fa53fdf Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 2 10:15:04 2020 -0400 Merge branch 'maint-0.4.4' changes/ticket32622 | 5 +++++ src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 18 +++++++++++++----- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 9 files changed, 62 insertions(+), 6 deletions(-)

1 0