commit bc9979a670026004bf0a516bd38a25251ca6d9bb Author: Nick Mathewson <nickm@torproject.org> Date: Thu Jun 18 16:05:16 2020 -0400 Split "can reach ipv4 orport" from "can reach ipv6 orport". I've managed to keep this change mainly contained to our self-testing module. The changes here are: * There are two different variables for tracking "is our orport reachable". * We have a new function that says whether we can skip a single family's orport reachability test; the old function for this now tells whether we can skip _all_ orport reachability testing. (The name, router_should_skip_orport_reachability_test, is not so good. I will rename it later if I can think of a good replacement.) * The function that launches orport reachability tests now only launches the ones that haven't completed. * The function that notes that we're reachable on an ORPort now takes a family. * Various log messages are cleaned up. --- changes/ticket34067 | 4 + src/feature/relay/circuitbuild_relay.c | 6 +- src/feature/relay/relay_periodic.c | 44 +++++++--- src/feature/relay/selftest.c | 141 +++++++++++++++++++++++++-------- src/feature/relay/selftest.h | 13 ++- 5 files changed, 163 insertions(+), 45 deletions(-) diff --git a/changes/ticket34067 b/changes/ticket34067 new file mode 100644 index 000000000..b67ccf608 --- /dev/null +++ b/changes/ticket34067 @@ -0,0 +1,4 @@ + o Major features (relay self-testing, IPv6): + - Relays now track their IPv6 ORPort separately from the reachability of + their IPv4 ORPort. They will not publish a descriptor unless _both_ + ports appear to be externally reachable. Closes ticket 34067. diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index b89866b47..4a7f639ca 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -542,7 +542,11 @@ onionskin_answer(struct or_circuit_t *circ, /* record that we could process create cells from a non-local conn * that we didn't initiate; presumably this means that create cells * can reach us too. */ - router_orport_found_reachable(); + tor_addr_t remote_addr; + if (channel_get_addr_if_possible(circ->p_chan, &remote_addr)) { + int family = tor_addr_family(&remote_addr); + router_orport_found_reachable(family); + } } return 0; diff --git a/src/feature/relay/relay_periodic.c b/src/feature/relay/relay_periodic.c index 6a92f49d2..3171dffa5 100644 --- a/src/feature/relay/relay_periodic.c +++ b/src/feature/relay/relay_periodic.c @@ -202,19 +202,45 @@ reachability_warnings_callback(time_t now, const or_options_t *options) /* every 20 minutes, check and complain if necessary */ const routerinfo_t *me = router_get_my_routerinfo(); if (me && !router_should_skip_orport_reachability_check(options)) { - char *address = tor_dup_ip(me->addr); - if (address) { + /* We need to warn that one or more of our ORPorts isn't reachable. + * Determine which, and give a reasonable warning. */ + char *address4 = tor_dup_ip(me->addr); + char *address6 = tor_addr_to_str_dup(&me->ipv6_addr); + bool v4_ok = + router_should_skip_orport_reachability_check_family(options,AF_INET); + bool v6_ok = + router_should_skip_orport_reachability_check_family(options,AF_INET6); + if (address4 || address6) { + char *where4=NULL, *where6=NULL; + if (!v4_ok) + tor_asprintf(&where4, "%s:%d", address4, me->or_port); + if (!v6_ok) + tor_asprintf(&where6, "[%s]:%d", address6, me->or_port); + const char *opt_and = (!v4_ok && !v6_ok) ? "and" : ""; + log_warn(LD_CONFIG, - "Your server (%s:%d) has not managed to confirm that " - "its ORPort is reachable. Relays do not publish descriptors " + "Your server has not managed to confirm reachability for " + "its ORPort(s) at %s%s%s. Relays do not publish descriptors " "until their ORPort and DirPort are reachable. Please check " "your firewalls, ports, address, /etc/hosts file, etc.", - address, me->or_port); - control_event_server_status(LOG_WARN, - "REACHABILITY_FAILED ORADDRESS=%s:%d", - address, me->or_port); - tor_free(address); + where4?where4:"", + opt_and, + where6?where6:""); + tor_free(where4); + tor_free(where6); + if (!v4_ok) { + control_event_server_status(LOG_WARN, + "REACHABILITY_FAILED ORADDRESS=%s:%d", + address4, me->or_port); + } + if (!v6_ok) { + control_event_server_status(LOG_WARN, + "REACHABILITY_FAILED ORADDRESS=[%s]:%d", + address6, me->ipv6_orport); + } } + tor_free(address4); + tor_free(address6); } if (me && !router_should_skip_dirport_reachability_check(options)) { diff --git a/src/feature/relay/selftest.c b/src/feature/relay/selftest.c index 834e8cf0a..7dc579c19 100644 --- a/src/feature/relay/selftest.c +++ b/src/feature/relay/selftest.c @@ -44,8 +44,12 @@ #include "feature/relay/router.h" #include "feature/relay/selftest.h" -/** Whether we can reach our ORPort from the outside. */ -static bool can_reach_or_port = false; +static bool have_orport_for_family(int family); + +/** Whether we can reach our IPv4 ORPort from the outside. */ +static bool can_reach_or_port_ipv4 = false; +/** Whether we can reach our IPv6 ORPort from the outside. */ +static bool can_reach_or_port_ipv6 = false; /** Whether we can reach our DirPort from the outside. */ static bool can_reach_dir_port = false; @@ -53,7 +57,7 @@ static bool can_reach_dir_port = false; void router_reset_reachability(void) { - can_reach_or_port = can_reach_dir_port = false; + can_reach_or_port_ipv4 = can_reach_or_port_ipv6 = can_reach_dir_port = false; } /** Return 1 if we won't do reachability checks, because: @@ -75,13 +79,35 @@ router_reachability_checks_disabled(const or_options_t *options) * - we've seen a successful reachability check, or * - AssumeReachable is set, or * - the network is disabled. + + * If `family'`is AF_INET or AF_INET6, return true only when we should skip + * the given family's orport check (Because it's been checked, or because we + * aren't checking it.) If `family` is 0, return true if we can skip _all_ + * orport checks. */ int -router_should_skip_orport_reachability_check(const or_options_t *options) +router_should_skip_orport_reachability_check_family( + const or_options_t *options, + int family) { + tor_assert_nonfatal(family == AF_INET || family == AF_INET6 || family == 0); int reach_checks_disabled = router_reachability_checks_disabled(options); - return reach_checks_disabled || - can_reach_or_port; + if (reach_checks_disabled) { + return true; + } + + if (family != AF_INET6) { + if (have_orport_for_family(AF_INET) && !can_reach_or_port_ipv4) { + return false; + } + } + if (family != AF_INET) { + if (have_orport_for_family(AF_INET6) && !can_reach_or_port_ipv6) { + return false; + } + } + + return true; } /** Return 0 if we need to do a DirPort reachability check, because: @@ -133,6 +159,28 @@ router_should_check_reachability(int test_or, int test_dir) return 1; } +/** + * Return true if we have configured an ORPort for the given family that + * we would like to advertise. + * + * Like other self-testing functions, this function looks at our most + * recently built descriptor. + **/ +static bool +have_orport_for_family(int family) +{ + const routerinfo_t *me = router_get_my_routerinfo(); + + if (!me) + return false; + + tor_addr_port_t ap; + if (router_get_orport(me, &ap, family) < 0) { + return false; + } + return true; +} + /** Allocate and return a new extend_info_t that can be used to build * a circuit to or through the router <b>r</b>, using an address from * <b>family</b> (if available). @@ -255,16 +303,22 @@ router_do_reachability_checks(int test_or, int test_dir) { const routerinfo_t *me = router_get_my_routerinfo(); const or_options_t *options = get_options(); - int orport_reachable = router_should_skip_orport_reachability_check(options); + int orport_reachable_v4 = + router_should_skip_orport_reachability_check_family(options, AF_INET); + int orport_reachable_v6 = + router_should_skip_orport_reachability_check_family(options, AF_INET6); if (router_should_check_reachability(test_or, test_dir)) { - if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) { - /* At the moment, tor relays believe that they are reachable when they - * receive any create cell on an inbound connection. We'll do separate - * IPv4 and IPv6 reachability checks in #34067, and make them more - * precise. */ - router_do_orport_reachability_checks(me, AF_INET, orport_reachable); - router_do_orport_reachability_checks(me, AF_INET6, orport_reachable); + bool need_testing = !circuit_enough_testing_circs(); + /* At the moment, tor relays believe that they are reachable when they + * receive any create cell on an inbound connection, if the address + * family is correct. + */ + if (test_or && (!orport_reachable_v4 || need_testing)) { + router_do_orport_reachability_checks(me, AF_INET, orport_reachable_v4); + } + if (test_or && (!orport_reachable_v6 || need_testing)) { + router_do_orport_reachability_checks(me, AF_INET6, orport_reachable_v6); } if (test_dir && !router_should_skip_dirport_reachability_check(options)) { @@ -341,34 +395,58 @@ inform_testing_reachability(void) return 1; } -/** Annotate that we found our ORPort reachable. */ +/** + * Return true if this module knows of no reason why we shouldn't publish + * a server descriptor. + **/ +static bool +ready_to_publish(const or_options_t *options) +{ + return options->PublishServerDescriptor_ != NO_DIRINFO && + router_should_skip_dirport_reachability_check(options) && + router_should_skip_orport_reachability_check(options); +} + +/** Annotate that we found our ORPort reachable with a given address + * family. */ void -router_orport_found_reachable(void) +router_orport_found_reachable(int family) { const routerinfo_t *me = router_get_my_routerinfo(); const or_options_t *options = get_options(); - if (!can_reach_or_port && me) { - char *address = tor_dup_ip(me->addr); - - if (!address) + bool *can_reach_ptr; + if (family == AF_INET) { + can_reach_ptr = &can_reach_or_port_ipv4; + } else if (family == AF_INET6) { + can_reach_ptr = &can_reach_or_port_ipv6; + } else { + tor_assert_nonfatal_unreached(); + return; + } + if (!*can_reach_ptr && me) { + tor_addr_port_t ap; + if (router_get_orport(me, &ap, family) < 0) { return; + } + char *address = tor_strdup(fmt_addrport_ap(&ap)); + + *can_reach_ptr = true; - log_notice(LD_OR,"Self-testing indicates your ORPort is reachable from " + log_notice(LD_OR,"Self-testing indicates your ORPort %s is reachable from " "the outside. Excellent.%s", - options->PublishServerDescriptor_ != NO_DIRINFO - && router_should_skip_dirport_reachability_check(options) ? - " Publishing server descriptor." : ""); - can_reach_or_port = true; + address, + ready_to_publish(options) ? + " Publishing server descriptor." : ""); + mark_my_descriptor_dirty("ORPort found reachable"); /* This is a significant enough change to upload immediately, * at least in a test network */ if (options->TestingTorNetwork == 1) { reschedule_descriptor_update_check(); } - /* We'll add an IPv6 event in #34068. */ control_event_server_status(LOG_NOTICE, - "REACHABILITY_SUCCEEDED ORADDRESS=%s:%d", - address, me->or_port); + "REACHABILITY_SUCCEEDED ORADDRESS=%s", + address); tor_free(address); } } @@ -379,18 +457,19 @@ router_dirport_found_reachable(void) { const routerinfo_t *me = router_get_my_routerinfo(); const or_options_t *options = get_options(); + if (!can_reach_dir_port && me) { char *address = tor_dup_ip(me->addr); if (!address) return; + can_reach_dir_port = true; log_notice(LD_DIRSERV,"Self-testing indicates your DirPort is reachable " "from the outside. Excellent.%s", - options->PublishServerDescriptor_ != NO_DIRINFO - && router_should_skip_orport_reachability_check(options) ? + ready_to_publish(options) ? " Publishing server descriptor." : ""); - can_reach_dir_port = true; + if (router_should_advertise_dirport(options, me->dir_port)) { mark_my_descriptor_dirty("DirPort found reachable"); /* This is a significant enough change to upload immediately, diff --git a/src/feature/relay/selftest.h b/src/feature/relay/selftest.h index 5799a6ca3..d65edf56f 100644 --- a/src/feature/relay/selftest.h +++ b/src/feature/relay/selftest.h @@ -15,8 +15,11 @@ #ifdef HAVE_MODULE_RELAY struct or_options_t; -int router_should_skip_orport_reachability_check( - const struct or_options_t *options); +#define router_should_skip_orport_reachability_check(opts) \ + router_should_skip_orport_reachability_check_family((opts),0) +int router_should_skip_orport_reachability_check_family( + const struct or_options_t *options, + int family); int router_should_skip_dirport_reachability_check( const struct or_options_t *options); @@ -24,15 +27,17 @@ void router_do_reachability_checks(int test_or, int test_dir); void router_perform_bandwidth_test(int num_circs, time_t now); int inform_testing_reachability(void); -void router_orport_found_reachable(void); +void router_orport_found_reachable(int family); void router_dirport_found_reachable(void); void router_reset_reachability(void); #else /* !defined(HAVE_MODULE_RELAY) */ -#define router_should_skip_orport_reachability_check(opts) \ +#define router_should_skip_orport_reachability_check(opts) \ ((void)(opts), 0) +#define router_should_skip_orport_reachability_check_family(opts, fam) \ + ((void)(opts), (void)(fam), 0) #define router_should_skip_dirport_reachability_check(opts) \ ((void)(opts), 0)