commit 5ea993fa5a38353900f9cef3b31f695fc70679b5 Author: Fernando Fernandez Mancera <ffmancera@riseup.net> Date: Wed Jan 24 20:07:49 2018 +0100 Clarify directory and ORPort checking functions. In order to make the OR and dir checking functions in router.c less confusing we renamed some functions and splitted consider_testing_reachability() into router_should_check_reachability() and router_do_reachability_checks(). Also we improved the documentation. Fixes #18918. Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> --- changes/bug18918 | 6 +++ src/or/circuitbuild.c | 4 +- src/or/circuituse.c | 2 +- src/or/main.c | 4 +- src/or/router.c | 129 ++++++++++++++++++++++++++++---------------------- src/or/router.h | 2 +- 6 files changed, 84 insertions(+), 63 deletions(-) diff --git a/changes/bug18918 b/changes/bug18918 new file mode 100644 index 000000000..c939168f4 --- /dev/null +++ b/changes/bug18918 @@ -0,0 +1,6 @@ + o Code simplification and refactoring: + - In order to make the OR and dir checking function in router.c less + confusing we renamed some functions and consider_testing_reachability() + has been splitted into router_should_check_reachability() and + router_do_reachability_checks(). Also we improved the documentation in + some functions. Closes ticket 18918. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 9c049a24b..86d7382d6 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1095,7 +1095,7 @@ circuit_build_no_more_hops(origin_circuit_t *circ) clear_broken_connection_map(1); if (server_mode(options) && !check_whether_orport_reachable(options)) { inform_testing_reachability(); - consider_testing_reachability(1, 1); + router_do_reachability_checks(1, 1); } } @@ -1651,7 +1651,7 @@ onionskin_answer(or_circuit_t *circ, * rend_service_launch_establish_intro()) * * - We are a router testing its own reachabiity - * (CIRCUIT_PURPOSE_TESTING, via consider_testing_reachability()) + * (CIRCUIT_PURPOSE_TESTING, via router_do_reachability_checks()) * * onion_pick_cpath_exit() bypasses us (by not calling * new_route_len()) in the one-hop tunnel case, so we don't need to diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 5f9567ea1..d7d7e8233 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1630,7 +1630,7 @@ circuit_testing_opened(origin_circuit_t *circ) router_perform_bandwidth_test(NUM_PARALLEL_TESTING_CIRCS, time(NULL)); have_performed_bandwidth_test = 1; } else - consider_testing_reachability(1, 0); + router_do_reachability_checks(1, 0); } /** A testing circuit has failed to build. Take whatever stats we want. */ diff --git a/src/or/main.c b/src/or/main.c index 10e606f3a..9a4aa01ef 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1142,7 +1142,7 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) if (server_mode(options) && !net_is_disabled() && !from_cache && (have_completed_a_circuit() || !any_predicted_circuits(now))) - consider_testing_reachability(1, 1); + router_do_reachability_checks(1, 1); } /** Perform regular maintenance tasks for a single connection. This @@ -2062,7 +2062,7 @@ check_for_reachability_bw_callback(time_t now, const or_options_t *options) (have_completed_a_circuit() || !any_predicted_circuits(now)) && !net_is_disabled()) { if (stats_n_seconds_working < TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) { - consider_testing_reachability(1, dirport_reachability_count==0); + router_do_reachability_checks(1, dirport_reachability_count==0); if (++dirport_reachability_count > 5) dirport_reachability_count = 0; return 1; diff --git a/src/or/router.c b/src/or/router.c index 9c053cad4..afba8209e 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1227,7 +1227,8 @@ check_whether_dirport_reachable(const or_options_t *options) /* XXX Should this be increased? */ #define MIN_BW_TO_ADVERTISE_DIRSERVER 51200 -/** Return true iff we have enough configured bandwidth to cache directory +/** Return true iff we have enough configured bandwidth to advertise or + * automatically provide directory services from cache directory * information. */ static int router_has_bandwidth_to_be_dirserver(const or_options_t *options) @@ -1250,7 +1251,7 @@ router_has_bandwidth_to_be_dirserver(const or_options_t *options) * MIN_BW_TO_ADVERTISE_DIRSERVER, don't bother trying to serve requests. */ static int -router_should_be_directory_server(const or_options_t *options, int dir_port) +router_should_be_dirserver(const or_options_t *options, int dir_port) { static int advertising=1; /* start out assuming we will advertise */ int new_choice=1; @@ -1301,7 +1302,7 @@ router_should_be_directory_server(const or_options_t *options, int dir_port) } else { tor_assert(reason); log_notice(LD_DIR, "Not advertising Dir%s (Reason: %s)", - dir_port ? "Port" : "ectory Service support", reason); + dir_port ? "Port" : "Directory Service support", reason); } advertising = new_choice; } @@ -1355,7 +1356,7 @@ decide_to_advertise_dir_impl(const or_options_t *options, /* Part two: consider config options that could make us choose to * publish or not publish that the user might find surprising. */ - return router_should_be_directory_server(options, dir_port); + return router_should_be_dirserver(options, dir_port); } /** Front-end to decide_to_advertise_dir_impl(): return 0 if we don't want to @@ -1363,7 +1364,7 @@ decide_to_advertise_dir_impl(const or_options_t *options, * DirPort we want to advertise. */ static int -decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) +router_should_advertise_dirport(const or_options_t *options, uint16_t dir_port) { /* supports_tunnelled_dir_requests is not relevant, pass 0 */ return decide_to_advertise_dir_impl(options, dir_port, 0) ? dir_port : 0; @@ -1373,7 +1374,7 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) * advertise the fact that we support begindir requests, else return 1. */ static int -decide_to_advertise_begindir(const or_options_t *options, +router_should_advertise_begindir(const or_options_t *options, int supports_tunnelled_dir_requests) { /* dir_port is not relevant, pass 0 */ @@ -1406,26 +1407,17 @@ extend_info_from_router(const routerinfo_t *r) &ap.addr, ap.port); } -/** Some time has passed, or we just got new directory information. - * See if we currently believe our ORPort or DirPort to be - * unreachable. If so, launch a new test for it. - * - * For ORPort, we simply try making a circuit that ends at ourselves. - * Success is noticed in onionskin_answer(). - * - * For DirPort, we make a connection via Tor to our DirPort and ask - * for our own server descriptor. - * Success is noticed in connection_dir_client_reached_eof(). +/**See if we currently believe our ORPort or DirPort to be + * unreachable. If so, return 1 else return 0. */ -void -consider_testing_reachability(int test_or, int test_dir) +static int +router_should_check_reachability(int test_or, int test_dir) { const routerinfo_t *me = router_get_my_routerinfo(); const or_options_t *options = get_options(); - int orport_reachable = check_whether_orport_reachable(options); - tor_addr_t addr; + if (!me) - return; + return 0; if (routerset_contains_router(options->ExcludeNodes, me, -1) && options->StrictNodes) { @@ -1440,43 +1432,66 @@ consider_testing_reachability(int test_or, int test_dir) "We cannot learn whether we are usable, and will not " "be able to advertise ourself."); } - return; + return 0; } + return 1; +} + +/** Some time has passed, or we just got new directory information. + * See if we currently believe our ORPort or DirPort to be + * unreachable. If so, launch a new test for it. + * + * For ORPort, we simply try making a circuit that ends at ourselves. + * Success is noticed in onionskin_answer(). + * + * For DirPort, we make a connection via Tor to our DirPort and ask + * for our own server descriptor. + * Success is noticed in connection_dir_client_reached_eof(). + */ +void +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 = check_whether_orport_reachable(options); + tor_addr_t addr; + + if (router_should_check_reachability(test_or, test_dir)) { + if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) { + extend_info_t *ei = extend_info_from_router(me); + /* XXX IPv6 self testing */ + log_info(LD_CIRC, "Testing %s of my ORPort: %s:%d.", + !orport_reachable ? "reachability" : "bandwidth", + fmt_addr32(me->addr), me->or_port); + circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei, + CIRCLAUNCH_NEED_CAPACITY|CIRCLAUNCH_IS_INTERNAL); + extend_info_free(ei); + } - if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) { - extend_info_t *ei = extend_info_from_router(me); /* XXX IPv6 self testing */ - log_info(LD_CIRC, "Testing %s of my ORPort: %s:%d.", - !orport_reachable ? "reachability" : "bandwidth", - fmt_addr32(me->addr), me->or_port); - circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei, - CIRCLAUNCH_NEED_CAPACITY|CIRCLAUNCH_IS_INTERNAL); - extend_info_free(ei); - } - - /* XXX IPv6 self testing */ - tor_addr_from_ipv4h(&addr, me->addr); - if (test_dir && !check_whether_dirport_reachable(options) && - !connection_get_by_type_addr_port_purpose( - CONN_TYPE_DIR, &addr, me->dir_port, - DIR_PURPOSE_FETCH_SERVERDESC)) { - tor_addr_port_t my_orport, my_dirport; - memcpy(&my_orport.addr, &addr, sizeof(addr)); - memcpy(&my_dirport.addr, &addr, sizeof(addr)); - my_orport.port = me->or_port; - my_dirport.port = me->dir_port; - /* ask myself, via tor, for my server descriptor. */ - directory_request_t *req = - directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC); - directory_request_set_or_addr_port(req, &my_orport); - directory_request_set_dir_addr_port(req, &my_dirport); - directory_request_set_directory_id_digest(req, + tor_addr_from_ipv4h(&addr, me->addr); + if (test_dir && !check_whether_dirport_reachable(options) && + !connection_get_by_type_addr_port_purpose( + CONN_TYPE_DIR, &addr, me->dir_port, + DIR_PURPOSE_FETCH_SERVERDESC)) { + tor_addr_port_t my_orport, my_dirport; + memcpy(&my_orport.addr, &addr, sizeof(addr)); + memcpy(&my_dirport.addr, &addr, sizeof(addr)); + my_orport.port = me->or_port; + my_dirport.port = me->dir_port; + /* ask myself, via tor, for my server descriptor. */ + directory_request_t *req = + directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC); + directory_request_set_or_addr_port(req, &my_orport); + directory_request_set_dir_addr_port(req, &my_dirport); + directory_request_set_directory_id_digest(req, me->cache_info.identity_digest); - // ask via an anon circuit, connecting to our dirport. - directory_request_set_indirection(req, DIRIND_ANON_DIRPORT); - directory_request_set_resource(req, "authority.z"); - directory_initiate_request(req); - directory_request_free(req); + // ask via an anon circuit, connecting to our dirport. + directory_request_set_indirection(req, DIRIND_ANON_DIRPORT); + directory_request_set_resource(req, "authority.z"); + directory_initiate_request(req); + directory_request_free(req); + } } } @@ -1521,7 +1536,7 @@ router_dirport_found_reachable(void) && check_whether_orport_reachable(options) ? " Publishing server descriptor." : ""); can_reach_dir_port = 1; - if (decide_to_advertise_dirport(options, me->dir_port)) { + 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, * at least in a test network */ @@ -2915,7 +2930,7 @@ router_dump_router_to_string(routerinfo_t *router, router->nickname, address, router->or_port, - decide_to_advertise_dirport(options, router->dir_port), + router_should_advertise_dirport(options, router->dir_port), ed_cert_line ? ed_cert_line : "", extra_or_address ? extra_or_address : "", router->platform, @@ -2989,7 +3004,7 @@ router_dump_router_to_string(routerinfo_t *router, tor_free(p6); } - if (decide_to_advertise_begindir(options, + if (router_should_advertise_begindir(options, router->supports_tunnelled_dir_requests)) { smartlist_add_strdup(chunks, "tunnelled-dir-server\n"); } diff --git a/src/or/router.h b/src/or/router.h index 696e98366..7f5030895 100644 --- a/src/or/router.h +++ b/src/or/router.h @@ -47,7 +47,7 @@ int init_keys_client(void); int check_whether_orport_reachable(const or_options_t *options); int check_whether_dirport_reachable(const or_options_t *options); int dir_server_mode(const or_options_t *options); -void consider_testing_reachability(int test_or, int test_dir); +void router_do_reachability_checks(int test_or, int test_dir); void router_orport_found_reachable(void); void router_dirport_found_reachable(void); void router_perform_bandwidth_test(int num_circs, time_t now);