commit 4b981407335865795bb99c7297ec49d80455d693 Author: David Goulet <dgoulet@torproject.org> Date: Fri Sep 18 11:50:12 2020 -0400 relay: Use testing circuit instead of dummy descriptor fetch Tor now can learn its address from a NETINFO cell coming from an authority. Thus, instead from launching a dummy descriptor fetch to learn the address from the directory response (unauthenticated), we simply now launch a one-hop testing circuit. Related to #40071 Signed-off-by: David Goulet <dgoulet@torproject.org> --- src/core/or/circuitlist.h | 4 ++- src/feature/nodelist/routerlist.c | 69 ++++++++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/core/or/circuitlist.h b/src/core/or/circuitlist.h index fd7e22e4c0..3178e6cd0d 100644 --- a/src/core/or/circuitlist.h +++ b/src/core/or/circuitlist.h @@ -114,7 +114,9 @@ #define CIRCUIT_PURPOSE_S_HSDIR_POST 20 #define CIRCUIT_PURPOSE_S_HS_MAX_ 20 -/** A testing circuit; not meant to be used for actual traffic. */ +/** A testing circuit; not meant to be used for actual traffic. It is used for + * bandwidth measurement, reachability test and address discovery from an + * authority using the NETINFO cell. */ #define CIRCUIT_PURPOSE_TESTING 21 /** A controller made this circuit and Tor should not use it. */ #define CIRCUIT_PURPOSE_CONTROLLER 22 diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c index 3f6e31bc3a..c1b02b586d 100644 --- a/src/feature/nodelist/routerlist.c +++ b/src/feature/nodelist/routerlist.c @@ -65,6 +65,9 @@ #include "app/config/config.h" #include "core/mainloop/connection.h" #include "core/mainloop/mainloop.h" +#include "core/or/circuitlist.h" +#include "core/or/circuituse.h" +#include "core/or/extendinfo.h" #include "core/or/policies.h" #include "feature/client/bridges.h" #include "feature/control/control_events.h" @@ -137,7 +140,7 @@ static int signed_desc_digest_is_recognized(signed_descriptor_t *desc); static const char *signed_descriptor_get_body_impl( const signed_descriptor_t *desc, int with_annotations); -static void launch_dummy_descriptor_download_as_needed(time_t now, +static void launch_dummy_circuit_as_needed(time_t now, const or_options_t *options); /****************************************************************************/ @@ -2306,7 +2309,7 @@ update_all_descriptor_downloads(time_t now) return; update_router_descriptor_downloads(now); update_microdesc_downloads(now); - launch_dummy_descriptor_download_as_needed(now, get_options()); + launch_dummy_circuit_as_needed(now, get_options()); } /** Clear all our timeouts for fetching v3 directory stuff, and then @@ -2760,23 +2763,20 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote, smartlist_free(no_longer_old); } -/** How often should we launch a server/authority request to be sure of getting +/** How often should we launch a circuit to an authority to be sure of getting * a guess for our IP? */ -/*XXXX+ this info should come from netinfo cells or something, or we should - * do this only when we aren't seeing incoming data. see bug 652. */ #define DUMMY_DOWNLOAD_INTERVAL (20*60) /** As needed, launch a dummy router descriptor fetch to see if our * address has changed. */ static void -launch_dummy_descriptor_download_as_needed(time_t now, - const or_options_t *options) +launch_dummy_circuit_as_needed(time_t now, const or_options_t *options) { - static time_t last_dummy_download = 0; + static time_t last_dummy_circuit = 0; bool have_addr; tor_addr_t addr_out; - /* This dummy fetch only matter for relays. */ + /* This dummy circuit only matter for relays. */ if (!server_mode(options)) { return; } @@ -2784,27 +2784,44 @@ launch_dummy_descriptor_download_as_needed(time_t now, /* Lookup the address cache to learn if we have a good usable address. We * still force relays to have an IPv4 so that alone is enough to learn if we * need a lookup. In case we don't have one, we might want to attempt a - * dummy fetch to learn our address as a suggestion from an authority. */ + * dummy circuit to learn our address as a suggestion from an authority. */ have_addr = relay_find_addr_to_publish(options, AF_INET, RELAY_FIND_ADDR_CACHE_ONLY, &addr_out); - /* XXXX+ we could be smarter here; see notes on bug 652. */ - /* If we're a server that doesn't have an address, we rely on directory - * fetches to learn when our address changes. So if we haven't tried to get - * any routerdescs in a long time, try a dummy fetch now. */ - if (!have_addr && - last_descriptor_download_attempted + DUMMY_DOWNLOAD_INTERVAL < now && - last_dummy_download + DUMMY_DOWNLOAD_INTERVAL < now) { - last_dummy_download = now; - /* XX/teor - do we want an authority here, because they are less likely - * to give us the wrong address? (See #17782) - * I'm leaving the previous behaviour intact, because I don't like - * the idea of some relays contacting an authority every 20 minutes. */ - directory_get_from_dirserver(DIR_PURPOSE_FETCH_SERVERDESC, - ROUTER_PURPOSE_GENERAL, "authority.z", - PDS_RETRY_IF_NO_SERVERS, - DL_WANT_ANY_DIRSERVER); + /* If we're a relay or bridge for which we were unable to discover our + * public address, we rely on learning our address from a directory + * authority from the NETINFO cell. */ + if (!have_addr && last_dummy_circuit + DUMMY_DOWNLOAD_INTERVAL < now) { + last_dummy_circuit = now; + + const routerstatus_t *rs = router_pick_trusteddirserver(V3_DIRINFO, 0); + if (BUG(!rs)) { + /* We should really always have trusted directories configured at this + * stage. They are loaded early either from default list or the one + * given in the configuration file. */ + return; + } + const node_t *node = node_get_by_id(rs->identity_digest); + if (BUG(!node)) { + /* If there is a routerstatus_t, there is a node_t thus this should + * never fail. */ + return; + } + extend_info_t *ei = extend_info_from_node(node, 1); + if (BUG(!ei)) { + return; + } + + log_debug(LD_GENERAL, "Attempting dummy testing circuit to an authority " + "in order to learn our address."); + + /* Launch a one-hop testing circuit to a trusted authority so we can learn + * our address through the NETINFO cell. */ + circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei, + CIRCLAUNCH_IS_INTERNAL | + CIRCLAUNCH_ONEHOP_TUNNEL); + extend_info_free(ei); } }