commit 06031b441eb92f575f0a1c2ac264dc0daee4fbf4 Author: Roger Dingledine <arma@torproject.org> Date: Mon May 16 17:43:47 2016 -0400 touchups and refactorings on bug 18616 branch no behavior changes --- changes/bug18616 | 20 +++++++++++++------- src/or/circuitbuild.c | 2 +- src/or/circuituse.c | 5 +++-- src/or/control.c | 15 +++++++++------ src/or/dirserv.c | 4 ++-- src/or/main.c | 4 ++-- src/or/rephist.c | 9 ++++++--- src/or/router.c | 48 +++++++++++++++++++++--------------------------- src/or/router.h | 4 ++-- 9 files changed, 59 insertions(+), 52 deletions(-) diff --git a/changes/bug18616 b/changes/bug18616 index d760f15..ec59e84 100644 --- a/changes/bug18616 +++ b/changes/bug18616 @@ -1,8 +1,14 @@ o Major bugfixes (directory mirrors): - - Fix broken DirPort self-checks. Decide to advertise begindir - support the same way we decide to advertise DirPorts. - Add additional config options that might change the content of - a relay's descriptor. - Avoid checking ORPort reachability when the network is disabled. - Resolves #18616, bugfix on 0c8e042c30946faa in #12538 in - 0.2.8.1-alpha. Patch by "teor". + - Decide whether to advertise begindir support the same way we decide + whether to advertise our DirPort. These decisions being out of sync + led to surprising behavior like advertising begindir support when + our hibernation config options made us not advertise a DirPort. + Resolves bug 18616; bugfix on 0.2.8.1-alpha. Patch by teor. + + o Minor bugfixes: + - Consider more config options when relays decide whether to regenerate + their descriptor. Fixes more of bug 12538; bugfix on 0.2.8.1-alpha. + - Resolve some edge cases where we might launch an ORPort reachability + check even when DisableNetwork is set. Noticed while fixing bug + 18616; bugfix on 0.2.3.9-alpha. + diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index a5a933e..820724a 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -984,7 +984,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) } control_event_client_status(LOG_NOTICE, "CIRCUIT_ESTABLISHED"); clear_broken_connection_map(1); - if (server_mode(options) && !check_whether_orport_reachable()) { + if (server_mode(options) && !check_whether_orport_reachable(options)) { inform_testing_reachability(); consider_testing_reachability(1, 1); } diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 31003ea..f96a3b1 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1426,7 +1426,7 @@ static void circuit_testing_opened(origin_circuit_t *circ) { if (have_performed_bandwidth_test || - !check_whether_orport_reachable()) { + !check_whether_orport_reachable(get_options())) { /* either we've already done everything we want with testing circuits, * or this testing circuit became open due to a fluke, e.g. we picked * a last hop where we already had the connection open due to an @@ -1443,7 +1443,8 @@ circuit_testing_opened(origin_circuit_t *circ) static void circuit_testing_failed(origin_circuit_t *circ, int at_last_hop) { - if (server_mode(get_options()) && check_whether_orport_reachable()) + const or_options_t *options = get_options(); + if (server_mode(options) && check_whether_orport_reachable(options)) return; log_info(LD_GENERAL, diff --git a/src/or/control.c b/src/or/control.c index 655b4dd..a2ae2e9 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2143,6 +2143,7 @@ getinfo_helper_events(control_connection_t *control_conn, const char *question, char **answer, const char **errmsg) { + const or_options_t *options = get_options(); (void) control_conn; if (!strcmp(question, "circuit-status")) { smartlist_t *status = smartlist_new(); @@ -2279,17 +2280,19 @@ getinfo_helper_events(control_connection_t *control_conn, *answer = tor_strdup(directories_have_accepted_server_descriptor() ? "1" : "0"); } else if (!strcmp(question, "status/reachability-succeeded/or")) { - *answer = tor_strdup(check_whether_orport_reachable() ? "1" : "0"); + *answer = tor_strdup(check_whether_orport_reachable(options) ? + "1" : "0"); } else if (!strcmp(question, "status/reachability-succeeded/dir")) { - *answer = tor_strdup(check_whether_dirport_reachable() ? "1" : "0"); + *answer = tor_strdup(check_whether_dirport_reachable(options) ? + "1" : "0"); } else if (!strcmp(question, "status/reachability-succeeded")) { tor_asprintf(answer, "OR=%d DIR=%d", - check_whether_orport_reachable() ? 1 : 0, - check_whether_dirport_reachable() ? 1 : 0); + check_whether_orport_reachable(options) ? 1 : 0, + check_whether_dirport_reachable(options) ? 1 : 0); } else if (!strcmp(question, "status/bootstrap-phase")) { *answer = tor_strdup(last_sent_bootstrap_message); } else if (!strcmpstart(question, "status/version/")) { - int is_server = server_mode(get_options()); + int is_server = server_mode(options); networkstatus_t *c = networkstatus_get_latest_consensus(); version_status_t status; const char *recommended; @@ -2331,7 +2334,7 @@ getinfo_helper_events(control_connection_t *control_conn, } *answer = bridge_stats; } else if (!strcmp(question, "status/fresh-relay-descs")) { - if (!server_mode(get_options())) { + if (!server_mode(options)) { *errmsg = "Only relays have descriptors"; return -1; } diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 5f848bd..846951c 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1133,9 +1133,9 @@ directory_caches_unknown_auth_certs(const or_options_t *options) /** Return 1 if we want to keep descriptors, networkstatuses, etc around. * Else return 0. - * Check get_options()->DirPort_set and directory_permits_begindir_requests() + * Check options->DirPort_set and directory_permits_begindir_requests() * to see if we are willing to serve these directory documents to others via - * the DirPort and begindir over ORPort, respectively. + * the DirPort and begindir-over-ORPort, respectively. */ int directory_caches_dir_info(const or_options_t *options) diff --git a/src/or/main.c b/src/or/main.c index a2cf5b1..86fb2bd 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2094,7 +2094,7 @@ second_elapsed_callback(periodic_timer_t *timer, void *arg) TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) { /* every 20 minutes, check and complain if necessary */ const routerinfo_t *me = router_get_my_routerinfo(); - if (me && !check_whether_orport_reachable()) { + if (me && !check_whether_orport_reachable(options)) { char *address = tor_dup_ip(me->addr); log_warn(LD_CONFIG,"Your server (%s:%d) has not managed to confirm that " "its ORPort is reachable. Relays do not publish descriptors " @@ -2107,7 +2107,7 @@ second_elapsed_callback(periodic_timer_t *timer, void *arg) tor_free(address); } - if (me && !check_whether_dirport_reachable()) { + if (me && !check_whether_dirport_reachable(options)) { char *address = tor_dup_ip(me->addr); log_warn(LD_CONFIG, "Your server (%s:%d) has not managed to confirm that its " diff --git a/src/or/rephist.c b/src/or/rephist.c index fe0ca91..04ed7ae 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1867,14 +1867,17 @@ any_predicted_circuits(time_t now) int rep_hist_circbuilding_dormant(time_t now) { + const or_options_t *options = get_options(); + if (any_predicted_circuits(now)) return 0; /* see if we'll still need to build testing circuits */ - if (server_mode(get_options()) && - (!check_whether_orport_reachable() || !circuit_enough_testing_circs())) + if (server_mode(options) && + (!check_whether_orport_reachable(options) || + !circuit_enough_testing_circs())) return 0; - if (!check_whether_dirport_reachable()) + if (!check_whether_dirport_reachable(options)) return 0; return 1; diff --git a/src/or/router.c b/src/or/router.c index 8f369f8..57c0618 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1079,7 +1079,7 @@ router_reset_reachability(void) can_reach_or_port = can_reach_dir_port = 0; } -/** Return 1 if we won't do reachability checks, because: +/** Return 1 if we won't do reachability checks, because: * - AssumeReachable is set, or * - the network is disabled. * Otherwise, return 0. @@ -1100,9 +1100,8 @@ router_reachability_checks_disabled(const or_options_t *options) * - the network is disabled. */ int -check_whether_orport_reachable(void) +check_whether_orport_reachable(const or_options_t *options) { - const or_options_t *options = get_options(); int reach_checks_disabled = router_reachability_checks_disabled(options); return reach_checks_disabled || can_reach_or_port; @@ -1118,9 +1117,8 @@ check_whether_orport_reachable(void) * - the network is disabled. */ int -check_whether_dirport_reachable(void) +check_whether_dirport_reachable(const or_options_t *options) { - const or_options_t *options = get_options(); int reach_checks_disabled = router_reachability_checks_disabled(options) || !options->DirPort_set; return reach_checks_disabled || @@ -1258,18 +1256,14 @@ decide_to_advertise_dir_impl(const or_options_t *options, !router_get_advertised_or_port(options)) return 0; - /* Part two: reasons to publish or not publish that the user - * might find surprising. router_should_be_directory_server() - * considers config options that make us choose not to publish. */ + /* 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); } -/** Look at a variety of factors, and return 0 if we don't want to +/** Front-end to decide_to_advertise_dir_impl(): return 0 if we don't want to * advertise the fact that we have a DirPort open, else return the * DirPort we want to advertise. - * - * Log a helpful message if we change our mind about whether to publish - * a DirPort. */ static int decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) @@ -1278,11 +1272,8 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) return decide_to_advertise_dir_impl(options, dir_port, 0) ? dir_port : 0; } -/** Look at a variety of factors, and return 0 if we don't want to +/** Front-end to decide_to_advertise_dir_impl(): return 0 if we don't want to * advertise the fact that we support begindir requests, else return 1. - * - * Log a helpful message if we change our mind about whether to advertise - * support for begindir. */ static int decide_to_advertise_begindir(const or_options_t *options, @@ -1324,9 +1315,9 @@ void consider_testing_reachability(int test_or, int test_dir) { const routerinfo_t *me = router_get_my_routerinfo(); - int orport_reachable = check_whether_orport_reachable(); - tor_addr_t addr; const or_options_t *options = get_options(); + int orport_reachable = check_whether_orport_reachable(options); + tor_addr_t addr; if (!me) return; @@ -1359,7 +1350,7 @@ consider_testing_reachability(int test_or, int test_dir) /* XXX IPv6 self testing */ tor_addr_from_ipv4h(&addr, me->addr); - if (test_dir && !check_whether_dirport_reachable() && + 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)) { @@ -1378,18 +1369,19 @@ void router_orport_found_reachable(void) { 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); log_notice(LD_OR,"Self-testing indicates your ORPort is reachable from " "the outside. Excellent.%s", - get_options()->PublishServerDescriptor_ != NO_DIRINFO - && check_whether_dirport_reachable() ? + options->PublishServerDescriptor_ != NO_DIRINFO + && check_whether_dirport_reachable(options) ? " Publishing server descriptor." : ""); can_reach_or_port = 1; mark_my_descriptor_dirty("ORPort found reachable"); /* This is a significant enough change to upload immediately, * at least in a test network */ - if (get_options()->TestingTorNetwork == 1) { + if (options->TestingTorNetwork == 1) { reschedule_descriptor_update_check(); } control_event_server_status(LOG_NOTICE, @@ -1404,19 +1396,20 @@ void 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); log_notice(LD_DIRSERV,"Self-testing indicates your DirPort is reachable " "from the outside. Excellent.%s", - get_options()->PublishServerDescriptor_ != NO_DIRINFO - && check_whether_orport_reachable() ? + options->PublishServerDescriptor_ != NO_DIRINFO + && check_whether_orport_reachable(options) ? " Publishing server descriptor." : ""); can_reach_dir_port = 1; - if (decide_to_advertise_dirport(get_options(), me->dir_port)) { + if (decide_to_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 */ - if (get_options()->TestingTorNetwork == 1) { + if (options->TestingTorNetwork == 1) { reschedule_descriptor_update_check(); } } @@ -1633,7 +1626,8 @@ decide_if_publishable_server(void) if (!router_get_advertised_or_port(options)) return 0; - return check_whether_orport_reachable() && check_whether_dirport_reachable(); + return check_whether_orport_reachable(options) && + check_whether_dirport_reachable(options); } /** Initiate server descriptor upload as reasonable (if server is publishable, diff --git a/src/or/router.h b/src/or/router.h index 5165462..73bfea1 100644 --- a/src/or/router.h +++ b/src/or/router.h @@ -39,8 +39,8 @@ int router_initialize_tls_context(void); int init_keys(void); int init_keys_client(void); -int check_whether_orport_reachable(void); -int check_whether_dirport_reachable(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_orport_found_reachable(void);