diff options
-rw-r--r-- | changes/bug23347 | 9 | ||||
-rw-r--r-- | doc/tor.1.txt | 21 | ||||
-rw-r--r-- | src/or/bridges.c | 22 | ||||
-rw-r--r-- | src/or/bridges.h | 2 | ||||
-rw-r--r-- | src/or/config.c | 16 | ||||
-rw-r--r-- | src/or/directory.c | 13 | ||||
-rw-r--r-- | src/or/or.h | 4 | ||||
-rw-r--r-- | src/test/test_dir.c | 48 | ||||
-rw-r--r-- | src/test/test_entrynodes.c | 2 | ||||
-rw-r--r-- | src/test/test_options.c | 1 |
10 files changed, 112 insertions, 26 deletions
diff --git a/changes/bug23347 b/changes/bug23347 new file mode 100644 index 0000000000..e73aa48f01 --- /dev/null +++ b/changes/bug23347 @@ -0,0 +1,9 @@ + o Minor fixes (bridge client bootstrap): + - Make bridge clients with no running bridges try to download + bridge descriptors immediately. But when bridge clients have + running bridges, make them wait at least 3 hours before + refreshing recently received bridge descriptors. + Download schedules used to start with an implicit 0, but the + fix for 17750 changed this undocumented behaviour, and made + bridge clients hang for 15 minutes before bootstrapping. + Fixes bug 23347, not in any released version of Tor. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index aec1767059..c375e032d3 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1582,8 +1582,8 @@ The following options are useful only for clients (that is, if live consensus). Only used by clients fetching from a list of fallback directory mirrors. This schedule is advanced by (potentially concurrent) connection attempts, unlike other schedules, which are advanced by - connection failures. (Default: 10, 11, 3600, 10800, 25200, 54000, - 111600, 262800) + connection failures. (Default: 6, 11, 3600, 10800, 25200, 54000, 111600, + 262800) [[ClientBootstrapConsensusFallbackDownloadSchedule]] **ClientBootstrapConsensusFallbackDownloadSchedule** __N__,__N__,__...__:: Schedule for when clients should download consensuses from fallback @@ -1613,7 +1613,7 @@ The following options are useful only for clients (that is, if [[ClientBootstrapConsensusMaxInProgressTries]] **ClientBootstrapConsensusMaxInProgressTries** __NUM__:: Try this many simultaneous connections to download a consensus before - waiting for one to complete, timeout, or error out. (Default: 4) + waiting for one to complete, timeout, or error out. (Default: 3) SERVER OPTIONS -------------- @@ -2486,7 +2486,8 @@ The following options are used for running a testing Tor network. TestingClientDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60 TestingServerConsensusDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60 TestingClientConsensusDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60 - TestingBridgeDownloadSchedule 60, 30, 30, 60 + TestingBridgeDownloadSchedule 10, 30, 60 + TestingBridgeBootstrapDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60 TestingClientMaxIntervalWithoutRequest 5 seconds TestingDirConnectionMaxStall 30 seconds TestingConsensusMaxDownloadTries 80 @@ -2551,8 +2552,16 @@ The following options are used for running a testing Tor network. 1800, 3600, 3600, 3600, 10800, 21600, 43200) [[TestingBridgeDownloadSchedule]] **TestingBridgeDownloadSchedule** __N__,__N__,__...__:: - Schedule for when clients should download bridge descriptors. Changing this - requires that **TestingTorNetwork** is set. (Default: 3600, 900, 900, 3600) + Schedule for when clients should download each bridge descriptor when they + know that one or more of their configured bridges are running. Changing + this requires that **TestingTorNetwork** is set. (Default: 10800, 25200, + 54000, 111600, 262800) + +[[TestingBridgeBootstrapDownloadSchedule]] **TestingBridgeBootstrapDownloadSchedule** __N__,__N__,__...__:: + Schedule for when clients should download each bridge descriptor when they + have just started, or when they can not contact any of their bridges. + Changing this requires that **TestingTorNetwork** is set. (Default: 0, 30, + 90, 600, 3600, 10800, 25200, 54000, 111600, 262800) [[TestingClientMaxIntervalWithoutRequest]] **TestingClientMaxIntervalWithoutRequest** __N__ **seconds**|**minutes**:: When directory clients have only a few descriptors to request, they batch diff --git a/src/or/bridges.c b/src/or/bridges.c index 0818fb0812..461f86260f 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -454,6 +454,9 @@ bridge_add_from_config(bridge_line_t *bridge_line) b->transport_name = bridge_line->transport_name; b->fetch_status.schedule = DL_SCHED_BRIDGE; b->fetch_status.backoff = DL_SCHED_RANDOM_EXPONENTIAL; + b->fetch_status.increment_on = DL_SCHED_INCREMENT_ATTEMPT; + /* This will fail if UseBridges is not set */ + download_status_reset(&b->fetch_status); b->socks_args = bridge_line->socks_args; if (!bridge_list) bridge_list = smartlist_new(); @@ -632,8 +635,13 @@ fetch_bridge_descriptors(const or_options_t *options, time_t now) continue; } - /* schedule another fetch as if this one will fail, in case it does */ - download_status_failed(&bridge->fetch_status, 0); + /* schedule the next attempt + * we can't increment after a failure, because sometimes we use the + * bridge authority, and sometimes we use the bridge direct */ + download_status_increment_attempt( + &bridge->fetch_status, + safe_str_client(fmt_and_decorate_addr(&bridge->addr)), + now); can_use_bridge_authority = !tor_digest_is_zero(bridge->identity) && num_bridge_auths; @@ -787,8 +795,12 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) if (bridge) { /* if we actually want to use this one */ node_t *node; /* it's here; schedule its re-fetch for a long time from now. */ - if (!from_cache) + if (!from_cache) { + /* This schedules the re-fetch at a constant interval, which produces + * a pattern of bridge traffic. But it's better than trying all + * configured briges several times in the first few minutes. */ download_status_reset(&bridge->fetch_status); + } node = node_get_mutable_by_id(ri->cache_info.identity_digest); tor_assert(node); @@ -820,8 +832,8 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) * We use this function to decide if we're ready to start building * circuits through our bridges, or if we need to wait until the * directory "server/authority" requests finish. */ -int -any_bridge_descriptors_known(void) +MOCK_IMPL(int, +any_bridge_descriptors_known, (void)) { tor_assert(get_options()->UseBridges); diff --git a/src/or/bridges.h b/src/or/bridges.h index 3bfc782f9a..19341818f4 100644 --- a/src/or/bridges.h +++ b/src/or/bridges.h @@ -45,7 +45,7 @@ void bridge_add_from_config(struct bridge_line_t *bridge_line); void retry_bridge_descriptor_fetch_directly(const char *digest); void fetch_bridge_descriptors(const or_options_t *options, time_t now); void learned_bridge_descriptor(routerinfo_t *ri, int from_cache); -int any_bridge_descriptors_known(void); +MOCK_DECL(int, any_bridge_descriptors_known, (void)); const smartlist_t *get_socks_args_by_bridge_addrport(const tor_addr_t *addr, uint16_t port); diff --git a/src/or/config.c b/src/or/config.c index 9d6cc9a26b..9a7251c41f 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -587,7 +587,16 @@ static config_var_t option_vars_[] = { * blackholed. Clients will try 3 directories simultaneously. * (Relays never use simultaneous connections.) */ V(ClientBootstrapConsensusMaxInProgressTries, UINT, "3"), - V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "1200, 900, 900, 3600"), + /* When a client has any running bridges, check each bridge occasionally, + * whether or not that bridge is actually up. */ + V(TestingBridgeDownloadSchedule, CSV_INTERVAL, + "10800, 25200, 54000, 111600, 262800"), + /* When a client is just starting, or has no running bridges, check each + * bridge a few times quickly, and then try again later. These schedules + * are much longer than the other schedules, because we try each and every + * configured bridge with this schedule. */ + V(TestingBridgeBootstrapDownloadSchedule, CSV_INTERVAL, + "0, 30, 90, 600, 3600, 10800, 25200, 54000, 111600, 262800"), V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "10 minutes"), V(TestingDirConnectionMaxStall, INTERVAL, "5 minutes"), V(TestingConsensusMaxDownloadTries, UINT, "8"), @@ -649,7 +658,9 @@ static const config_var_t testing_tor_network_defaults[] = { "15, 20, 30, 60"), V(TestingClientConsensusDownloadSchedule, CSV_INTERVAL, "0, 0, 5, 10, " "15, 20, 30, 60"), - V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "60, 30, 30, 60"), + V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "10, 30, 60"), + V(TestingBridgeBootstrapDownloadSchedule, CSV_INTERVAL, "0, 0, 5, 10, " + "15, 20, 30, 60"), V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "5 seconds"), V(TestingDirConnectionMaxStall, INTERVAL, "30 seconds"), V(TestingConsensusMaxDownloadTries, UINT, "80"), @@ -4098,6 +4109,7 @@ options_validate(or_options_t *old_options, or_options_t *options, CHECK_DEFAULT(TestingServerConsensusDownloadSchedule); CHECK_DEFAULT(TestingClientConsensusDownloadSchedule); CHECK_DEFAULT(TestingBridgeDownloadSchedule); + CHECK_DEFAULT(TestingBridgeBootstrapDownloadSchedule); CHECK_DEFAULT(TestingClientMaxIntervalWithoutRequest); CHECK_DEFAULT(TestingDirConnectionMaxStall); CHECK_DEFAULT(TestingConsensusMaxDownloadTries); diff --git a/src/or/directory.c b/src/or/directory.c index 007235d108..fed4277dec 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -5362,7 +5362,14 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options) } } case DL_SCHED_BRIDGE: - return options->TestingBridgeDownloadSchedule; + /* A bridge client downloading bridge descriptors */ + if (any_bridge_descriptors_known()) { + /* A bridge client with one or more running bridges */ + return options->TestingBridgeDownloadSchedule; + } else { + /* A bridge client with no running bridges */ + return options->TestingBridgeBootstrapDownloadSchedule; + } default: tor_assert(0); } @@ -5683,8 +5690,8 @@ download_status_get_initial_delay_from_now(const download_status_t *dls) * (We find the zeroth element of the download schedule, and set * next_attempt_at to be the appropriate offset from 'now'. In most * cases this means setting it to 'now', so the item will be immediately - * downloadable; in the case of bridge descriptors, the zeroth element - * is an hour from now.) */ + * downloadable; when using authorities with fallbacks, there is a few seconds' + * delay.) */ void download_status_reset(download_status_t *dls) { diff --git a/src/or/or.h b/src/or/or.h index 161d80ed96..de03a99035 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4340,6 +4340,10 @@ typedef struct { * altered on testing networks. */ smartlist_t *TestingBridgeDownloadSchedule; + /** Schedule for when clients should download bridge descriptors when they + * have no running bridges. Only altered on testing networks. */ + smartlist_t *TestingBridgeBootstrapDownloadSchedule; + /** When directory clients have only a few descriptors to request, they * batch them until they have more, or until this amount of time has * passed. Only altered on testing networks. */ diff --git a/src/test/test_dir.c b/src/test/test_dir.c index b6b0936f83..dd910cd0c5 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -18,6 +18,7 @@ #define RELAY_PRIVATE #include "or.h" +#include "bridges.h" #include "confparse.h" #include "config.h" #include "control.h" @@ -4245,7 +4246,7 @@ test_dir_download_status_increment(void *arg) /* Put it in the options */ mock_options = &test_options; reset_options(mock_options, &mock_get_options_calls); - mock_options->TestingBridgeDownloadSchedule = schedule; + mock_options->TestingBridgeBootstrapDownloadSchedule = schedule; mock_options->TestingClientDownloadSchedule = schedule; MOCK(get_options, mock_get_options); @@ -4425,6 +4426,7 @@ test_dir_download_status_increment(void *arg) /* Check that failure increments do happen on attempt-based schedules, * but that the retry is set at the end of time */ + mock_options->UseBridges = 1; mock_get_options_calls = 0; next_at = download_status_increment_failure(&dls_attempt, 404, "test", 0, current_time); @@ -4539,6 +4541,7 @@ test_dir_download_status_increment(void *arg) tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 0); tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 0); tt_int_op(mock_get_options_calls, OP_GE, 1); + mock_options->UseBridges = 0; /* Check that attempt increments don't happen on failure-based schedules, * and that the attempt is set at the end of time */ @@ -5859,9 +5862,17 @@ mock_networkstatus_consensus_can_use_extra_fallbacks( return mock_networkstatus_consensus_can_use_extra_fallbacks_value; } -/* data is a 2 character nul-terminated string. +static int mock_any_bridge_descriptors_known_value = 0; +static int +mock_any_bridge_descriptors_known(void) +{ + return mock_any_bridge_descriptors_known_value; +} + +/* data is a 3 character nul-terminated string. * If data[0] is 'b', set bootstrapping, anything else means not bootstrapping * If data[1] is 'f', set extra fallbacks, anything else means no extra + * If data[2] is 'f', set running bridges, anything else means no extra * fallbacks. */ static void @@ -5869,7 +5880,7 @@ test_dir_find_dl_schedule(void* data) { const char *str = (const char *)data; - tt_assert(strlen(data) == 2); + tt_assert(strlen(data) == 3); if (str[0] == 'b') { mock_networkstatus_consensus_is_bootstrapping_value = 1; @@ -5883,15 +5894,23 @@ test_dir_find_dl_schedule(void* data) mock_networkstatus_consensus_can_use_extra_fallbacks_value = 0; } + if (str[2] == 'r') { + mock_any_bridge_descriptors_known_value = 1; + } else { + mock_any_bridge_descriptors_known_value = 0; + } + MOCK(networkstatus_consensus_is_bootstrapping, mock_networkstatus_consensus_is_bootstrapping); MOCK(networkstatus_consensus_can_use_extra_fallbacks, mock_networkstatus_consensus_can_use_extra_fallbacks); + MOCK(any_bridge_descriptors_known, + mock_any_bridge_descriptors_known); download_status_t dls; smartlist_t server, client, server_cons, client_cons; smartlist_t client_boot_auth_only_cons, client_boot_auth_cons; - smartlist_t client_boot_fallback_cons, bridge; + smartlist_t client_boot_fallback_cons, bridge, bridge_bootstrap; mock_options = tor_malloc(sizeof(or_options_t)); reset_options(mock_options, &mock_get_options_calls); @@ -5908,6 +5927,7 @@ test_dir_find_dl_schedule(void* data) mock_options->ClientBootstrapConsensusFallbackDownloadSchedule = &client_boot_fallback_cons; mock_options->TestingBridgeDownloadSchedule = &bridge; + mock_options->TestingBridgeBootstrapDownloadSchedule = &bridge_bootstrap; dls.schedule = DL_SCHED_GENERIC; /* client */ @@ -5996,11 +6016,17 @@ test_dir_find_dl_schedule(void* data) dls.schedule = DL_SCHED_BRIDGE; /* client */ mock_options->ClientOnly = 1; - tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge); + mock_options->UseBridges = 1; + if (any_bridge_descriptors_known()) { + tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge); + } else { + tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge_bootstrap); + } done: UNMOCK(networkstatus_consensus_is_bootstrapping); UNMOCK(networkstatus_consensus_can_use_extra_fallbacks); + UNMOCK(any_bridge_descriptors_known); UNMOCK(get_options); tor_free(mock_options); mock_options = NULL; @@ -6165,10 +6191,14 @@ struct testcase_t dir_tests[] = { DIR(dump_unparseable_descriptors, 0), DIR(populate_dump_desc_fifo, 0), DIR(populate_dump_desc_fifo_2, 0), - DIR_ARG(find_dl_schedule, TT_FORK, "bf"), - DIR_ARG(find_dl_schedule, TT_FORK, "ba"), - DIR_ARG(find_dl_schedule, TT_FORK, "cf"), - DIR_ARG(find_dl_schedule, TT_FORK, "ca"), + DIR_ARG(find_dl_schedule, TT_FORK, "bfd"), + DIR_ARG(find_dl_schedule, TT_FORK, "bad"), + DIR_ARG(find_dl_schedule, TT_FORK, "cfd"), + DIR_ARG(find_dl_schedule, TT_FORK, "cad"), + DIR_ARG(find_dl_schedule, TT_FORK, "bfr"), + DIR_ARG(find_dl_schedule, TT_FORK, "bar"), + DIR_ARG(find_dl_schedule, TT_FORK, "cfr"), + DIR_ARG(find_dl_schedule, TT_FORK, "car"), DIR(assumed_flags, 0), DIR(networkstatus_compute_bw_weights_v10, 0), END_OF_TESTCASES diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index f30bb5ffa1..d87670d82b 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -917,6 +917,7 @@ test_entry_guard_node_filter(void *arg) routerset_parse(get_options_mutable()->ExcludeNodes, "144.144.0.0/16", ""); /* 4: Bridge. */ + get_options_mutable()->UseBridges = 1; sweep_bridge_list(); bl = tor_malloc_zero(sizeof(bridge_line_t)); tor_addr_from_ipv4h(&bl->addr, n[4]->rs->addr); @@ -924,6 +925,7 @@ test_entry_guard_node_filter(void *arg) memcpy(bl->digest, n[4]->identity, 20); bridge_add_from_config(bl); bl = NULL; // prevent free. + get_options_mutable()->UseBridges = 0; /* 5: Unreachable. This stays in the filter, but isn't in usable-filtered */ g[5]->last_tried_to_connect = approx_time(); // prevent retry. diff --git a/src/test/test_options.c b/src/test/test_options.c index 92e6982823..65b678cf5b 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -2232,6 +2232,7 @@ test_options_validate__testing(void *ignored) ENSURE_DEFAULT(TestingServerConsensusDownloadSchedule, 3000); ENSURE_DEFAULT(TestingClientConsensusDownloadSchedule, 3000); ENSURE_DEFAULT(TestingBridgeDownloadSchedule, 3000); + ENSURE_DEFAULT(TestingBridgeBootstrapDownloadSchedule, 3000); ENSURE_DEFAULT(TestingClientMaxIntervalWithoutRequest, 3000); ENSURE_DEFAULT(TestingDirConnectionMaxStall, 3000); ENSURE_DEFAULT(TestingConsensusMaxDownloadTries, 3000); |