diff options
author | Nick Mathewson <nickm@torproject.org> | 2016-05-17 11:09:54 -0400 |
---|---|---|
committer | Nick Mathewson <nickm@torproject.org> | 2016-05-17 11:09:54 -0400 |
commit | d6a2fec05ebcc39773a2f6666e1378410c64f047 (patch) | |
tree | c962faa2f33eede043d67419e0cfbba80a3b8653 | |
parent | ff5eb7fc625374c29944b9ab7160b86559ce90a4 (diff) | |
parent | 548d14247e3c8a48e2efb2ca62ee7a05fca843ba (diff) | |
download | tor-d6a2fec05ebcc39773a2f6666e1378410c64f047.tar.gz tor-d6a2fec05ebcc39773a2f6666e1378410c64f047.zip |
Merge branch 'bug18616-v4-merged_028' into maint-0.2.8
-rw-r--r-- | changes/bug18616 | 14 | ||||
-rw-r--r-- | src/or/circuitbuild.c | 2 | ||||
-rw-r--r-- | src/or/circuituse.c | 5 | ||||
-rw-r--r-- | src/or/config.c | 9 | ||||
-rw-r--r-- | src/or/control.c | 15 | ||||
-rw-r--r-- | src/or/dirserv.c | 7 | ||||
-rw-r--r-- | src/or/main.c | 4 | ||||
-rw-r--r-- | src/or/rephist.c | 9 | ||||
-rw-r--r-- | src/or/router.c | 140 | ||||
-rw-r--r-- | src/or/router.h | 4 | ||||
-rw-r--r-- | src/test/test_dir.c | 46 | ||||
-rw-r--r-- | src/test/test_policy.c | 3 |
12 files changed, 193 insertions, 65 deletions
diff --git a/changes/bug18616 b/changes/bug18616 new file mode 100644 index 0000000000..ec59e846ed --- /dev/null +++ b/changes/bug18616 @@ -0,0 +1,14 @@ + o Major bugfixes (directory mirrors): + - 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 a5a933e6b0..820724adea 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 a4b580104f..e43b24ca7d 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/config.c b/src/or/config.c index 5d938d101a..0850013d33 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4343,8 +4343,10 @@ options_transition_affects_descriptor(const or_options_t *old_options, !opt_streq(old_options->MyFamily, new_options->MyFamily) || !opt_streq(old_options->AccountingStart, new_options->AccountingStart) || old_options->AccountingMax != new_options->AccountingMax || + old_options->AccountingRule != new_options->AccountingRule || public_server_mode(old_options) != public_server_mode(new_options) || - old_options->DirCache != new_options->DirCache) + old_options->DirCache != new_options->DirCache || + old_options->AssumeReachable != new_options->AssumeReachable) return 1; return 0; @@ -7005,9 +7007,8 @@ get_first_listener_addrport_string(int listener_type) int get_first_advertised_port_by_type_af(int listener_type, int address_family) { - if (!configured_ports) - return 0; - SMARTLIST_FOREACH_BEGIN(configured_ports, const port_cfg_t *, cfg) { + const smartlist_t *conf_ports = get_configured_ports(); + SMARTLIST_FOREACH_BEGIN(conf_ports, const port_cfg_t *, cfg) { if (cfg->type == listener_type && !cfg->server_cfg.no_advertise && (tor_addr_family(&cfg->addr) == address_family || diff --git a/src/or/control.c b/src/or/control.c index e06d7d28a2..e2ad8cc6dc 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2148,6 +2148,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(); @@ -2284,17 +2285,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; @@ -2336,7 +2339,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 3e1f48062c..d38a024e14 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1131,8 +1131,11 @@ directory_caches_unknown_auth_certs(const or_options_t *options) return dir_server_mode(options) || options->BridgeRelay; } -/** Return 1 if we want to keep descriptors, networkstatuses, etc around - * and we're willing to serve them to others. Else return 0. +/** Return 1 if we want to keep descriptors, networkstatuses, etc around. + * Else return 0. + * 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. */ int directory_caches_dir_info(const or_options_t *options) diff --git a/src/or/main.c b/src/or/main.c index a2cf5b1101..86fb2bd46d 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 fe0ca91c25..04ed7aef0f 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 3f94703a26..d70756fdbf 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1079,23 +1079,49 @@ router_reset_reachability(void) can_reach_or_port = can_reach_dir_port = 0; } -/** Return 1 if ORPort is known reachable; else return 0. */ -int -check_whether_orport_reachable(void) +/** Return 1 if we won't do reachability checks, because: + * - AssumeReachable is set, or + * - the network is disabled. + * Otherwise, return 0. + */ +static int +router_reachability_checks_disabled(const or_options_t *options) { - const or_options_t *options = get_options(); return options->AssumeReachable || + net_is_disabled(); +} + +/** Return 0 if we need to do an ORPort reachability check, because: + * - no reachability check has been done yet, or + * - we've initiated reachability checks, but none have succeeded. + * Return 1 if we don't need to do an ORPort reachability check, because: + * - we've seen a successful reachability check, or + * - AssumeReachable is set, or + * - the network is disabled. + */ +int +check_whether_orport_reachable(const or_options_t *options) +{ + int reach_checks_disabled = router_reachability_checks_disabled(options); + return reach_checks_disabled || can_reach_or_port; } -/** Return 1 if we don't have a dirport configured, or if it's reachable. */ +/** Return 0 if we need to do a DirPort reachability check, because: + * - no reachability check has been done yet, or + * - we've initiated reachability checks, but none have succeeded. + * Return 1 if we don't need to do a DirPort reachability check, because: + * - we've seen a successful reachability check, or + * - there is no DirPort set, or + * - AssumeReachable is set, or + * - 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(); - return !options->DirPort_set || - options->AssumeReachable || - net_is_disabled() || + int reach_checks_disabled = router_reachability_checks_disabled(options) || + !options->DirPort_set; + return reach_checks_disabled || can_reach_dir_port; } @@ -1148,10 +1174,11 @@ router_should_be_directory_server(const or_options_t *options, int dir_port) "seconds long. Raising to 1."); interval_length = 1; } - log_info(LD_GENERAL, "Calculating whether to disable dirport: effective " + log_info(LD_GENERAL, "Calculating whether to advertise %s: effective " "bwrate: %u, AccountingMax: "U64_FORMAT", " - "accounting interval length %d", effective_bw, - U64_PRINTF_ARG(options->AccountingMax), + "accounting interval length %d", + dir_port ? "dirport" : "begindir", + effective_bw, U64_PRINTF_ARG(options->AccountingMax), interval_length); acc_bytes = options->AccountingMax; @@ -1199,34 +1226,62 @@ dir_server_mode(const or_options_t *options) } /** Look at a variety of factors, and 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. + * advertise the fact that we have a DirPort open or begindir support, else + * return 1. * - * Log a helpful message if we change our mind about whether to publish - * a DirPort. + * Where dir_port or supports_tunnelled_dir_requests are not relevant, they + * must be 0. + * + * Log a helpful message if we change our mind about whether to publish. */ static int -decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) +decide_to_advertise_dir_impl(const or_options_t *options, + uint16_t dir_port, + int supports_tunnelled_dir_requests) { /* Part one: reasons to publish or not publish that aren't * worth mentioning to the user, either because they're obvious * or because they're normal behavior. */ - if (!dir_port) /* short circuit the rest of the function */ + /* short circuit the rest of the function */ + if (!dir_port && !supports_tunnelled_dir_requests) return 0; if (authdir_mode(options)) /* always publish */ - return dir_port; + return 1; if (net_is_disabled()) return 0; - if (!check_whether_dirport_reachable()) + if (dir_port && !router_get_advertised_dir_port(options, dir_port)) return 0; - if (!router_get_advertised_dir_port(options, dir_port)) + if (supports_tunnelled_dir_requests && + !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. */ - return router_should_be_directory_server(options, dir_port) ? dir_port : 0; + /* 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); +} + +/** 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. + */ +static int +decide_to_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; +} + +/** 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. + */ +static int +decide_to_advertise_begindir(const or_options_t *options, + int supports_tunnelled_dir_requests) +{ + /* dir_port is not relevant, pass 0 */ + return decide_to_advertise_dir_impl(options, 0, + supports_tunnelled_dir_requests); } /** Allocate and return a new extend_info_t that can be used to build @@ -1260,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; @@ -1295,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)) { @@ -1314,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, @@ -1340,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(); } } @@ -1570,14 +1627,14 @@ decide_if_publishable_server(void) return 1; if (!router_get_advertised_or_port(options)) return 0; - if (!check_whether_orport_reachable()) + if (!check_whether_orport_reachable(options)) return 0; if (router_have_consensus_path() == CONSENSUS_PATH_INTERNAL) { /* All set: there are no exits in the consensus (maybe this is a tiny * test network), so we can't check our DirPort reachability. */ return 1; } else { - return check_whether_dirport_reachable(); + return check_whether_dirport_reachable(options); } } @@ -1933,8 +1990,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) ri->addr = addr; ri->or_port = router_get_advertised_or_port(options); ri->dir_port = router_get_advertised_dir_port(options, 0); - ri->supports_tunnelled_dir_requests = dir_server_mode(options) && - router_should_be_directory_server(options, ri->dir_port); + ri->supports_tunnelled_dir_requests = + directory_permits_begindir_requests(options); ri->cache_info.published_on = time(NULL); ri->onion_pkey = crypto_pk_dup_key(get_onion_key()); /* must invoke from * main thread */ @@ -2715,7 +2772,8 @@ router_dump_router_to_string(routerinfo_t *router, tor_free(p6); } - if (router->supports_tunnelled_dir_requests) { + if (decide_to_advertise_begindir(options, + router->supports_tunnelled_dir_requests)) { smartlist_add(chunks, tor_strdup("tunnelled-dir-server\n")); } diff --git a/src/or/router.h b/src/or/router.h index 5165462a13..73bfea1faa 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); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index ea179fb02c..401c7b299c 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -85,6 +85,15 @@ test_dir_nicknames(void *arg) ; } +static smartlist_t *mocked_configured_ports = NULL; + +/** Returns mocked_configured_ports */ +static const smartlist_t * +mock_get_configured_ports(void) +{ + return mocked_configured_ports; +} + /** Run unit tests for router descriptor generation logic. */ static void test_dir_formats(void *arg) @@ -104,6 +113,7 @@ test_dir_formats(void *arg) or_options_t *options = get_options_mutable(); const addr_policy_t *p; time_t now = time(NULL); + port_cfg_t orport, dirport; (void)arg; pk1 = pk_generate(0); @@ -185,9 +195,31 @@ test_dir_formats(void *arg) /* XXXX025 router_dump_to_string should really take this from ri.*/ options->ContactInfo = tor_strdup("Magri White " "<magri@elsewhere.example.com>"); + /* Skip reachability checks for DirPort and tunnelled-dir-server */ + options->AssumeReachable = 1; + + /* Fake just enough of an ORPort and DirPort to get by */ + MOCK(get_configured_ports, mock_get_configured_ports); + mocked_configured_ports = smartlist_new(); + + memset(&orport, 0, sizeof(orport)); + orport.type = CONN_TYPE_OR_LISTENER; + orport.addr.family = AF_INET; + orport.port = 9000; + smartlist_add(mocked_configured_ports, &orport); + + memset(&dirport, 0, sizeof(dirport)); + dirport.type = CONN_TYPE_DIR_LISTENER; + dirport.addr.family = AF_INET; + dirport.port = 9003; + smartlist_add(mocked_configured_ports, &dirport); buf = router_dump_router_to_string(r1, pk2, NULL, NULL, NULL); + UNMOCK(get_configured_ports); + smartlist_free(mocked_configured_ports); + mocked_configured_ports = NULL; + tor_free(options->ContactInfo); tt_assert(buf); @@ -308,6 +340,16 @@ test_dir_formats(void *arg) strlcat(buf2, "tunnelled-dir-server\n", sizeof(buf2)); strlcat(buf2, "router-sig-ed25519 ", sizeof(buf2)); + /* Fake just enough of an ORPort to get by */ + MOCK(get_configured_ports, mock_get_configured_ports); + mocked_configured_ports = smartlist_new(); + + memset(&orport, 0, sizeof(orport)); + orport.type = CONN_TYPE_OR_LISTENER; + orport.addr.family = AF_INET; + orport.port = 9005; + smartlist_add(mocked_configured_ports, &orport); + buf = router_dump_router_to_string(r2, pk1, pk2, &r2_onion_keypair, &kp2); tt_assert(buf); buf[strlen(buf2)] = '\0'; /* Don't compare the sig; it's never the same @@ -318,6 +360,10 @@ test_dir_formats(void *arg) buf = router_dump_router_to_string(r2, pk1, NULL, NULL, NULL); + UNMOCK(get_configured_ports); + smartlist_free(mocked_configured_ports); + mocked_configured_ports = NULL; + /* Reset for later */ cp = buf; rp2 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL); diff --git a/src/test/test_policy.c b/src/test/test_policy.c index 48e82551e3..a939ebf54f 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -716,10 +716,9 @@ test_policies_reject_exit_address(void *arg) } static smartlist_t *test_configured_ports = NULL; -const smartlist_t *mock_get_configured_ports(void); /** Returns test_configured_ports */ -const smartlist_t * +static const smartlist_t * mock_get_configured_ports(void) { return test_configured_ports; |