commit 19a4abf2a99c6a3de2c9a2fdf3e6d7b7c404f8f8 Author: teor teor2345@gmail.com Date: Mon Dec 11 02:29:05 2017 +1100
Make sure bridges are definitely running before delaying directory fetches
Retry directory downloads when we get our first bridge descriptor during bootstrap or while reconnecting to the network. Keep retrying every time we get a bridge descriptor, until we have a reachable bridge.
Stop delaying bridge descriptor fetches when we have cached bridge descriptors. Instead, only delay bridge descriptor fetches when we have at least one reachable bridge.
Fixes bug 24367; bugfix on 0.2.0.3-alpha. --- changes/bug24367 | 16 +++++++++++----- src/or/bridges.c | 10 +++++++--- src/or/directory.c | 9 +++++---- src/or/entrynodes.c | 11 +++++++++-- src/or/entrynodes.h | 2 +- src/or/networkstatus.c | 4 +++- src/test/test_dir.c | 5 +++-- 7 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/changes/bug24367 b/changes/bug24367 index 6081f7b5d..09ef3bb87 100644 --- a/changes/bug24367 +++ b/changes/bug24367 @@ -1,7 +1,13 @@ o Minor bugfixes (bridge clients, bootstrap): - - Stop checking for bridge descriptors when we actually want to know if - any bridges are usable. This avoids potential bootstrapping issues. + - Retry directory downloads when we get our first bridge descriptor + during bootstrap or while reconnecting to the network. Keep retrying + every time we get a bridge descriptor, until we have a reachable bridge. + Fixes bug 24367; bugfix on 0.2.0.3-alpha. + - Stop delaying bridge descriptor fetches when we have cached bridge + descriptors. Instead, only delay bridge descriptor fetches when we + have at least one reachable bridge. + Fixes bug 24367; bugfix on 0.2.0.3-alpha. + - Stop delaying directory fetches when we have cached bridge descriptors. + Instead, only delay bridge descriptor fetches when all our bridges are + definitely unreachable. Fixes bug 24367; bugfix on 0.2.0.3-alpha. - - Stop stalling when bridges are changed at runtime. Stop stalling when - old bridge descriptors are cached, but they are not in use. - Fixes bug 24367; bugfix on 23347 in 0.3.2.1-alpha. diff --git a/src/or/bridges.c b/src/or/bridges.c index ac759493c..0b1bbbd15 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -796,7 +796,11 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) tor_assert(ri); tor_assert(ri->purpose == ROUTER_PURPOSE_BRIDGE); if (get_options()->UseBridges) { - int first = num_bridges_usable() <= 1; + /* Retry directory downloads whenever we get a bridge descriptor: + * - when bootstrapping, and + * - when we aren't sure if any of our bridges are reachable. + * Keep on retrying until we have at least one reachable bridge. */ + int first = num_bridges_usable(0) < 1; bridge_info_t *bridge = get_configured_bridge_by_routerinfo(ri); time_t now = time(NULL); router_set_status(ri->cache_info.identity_digest, 1); @@ -826,8 +830,8 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache)
log_notice(LD_DIR, "new bridge descriptor '%s' (%s): %s", ri->nickname, from_cache ? "cached" : "fresh", router_describe(ri)); - /* set entry->made_contact so if it goes down we don't drop it from - * our entry node list */ + /* If we didn't have a reachable bridge before this one, try directory + * documents again. */ if (first) { routerlist_retry_directory_downloads(now); } diff --git a/src/or/directory.c b/src/or/directory.c index e49fd595c..f14bdde90 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -5337,12 +5337,13 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options) } } case DL_SCHED_BRIDGE: - /* A bridge client downloading bridge descriptors */ - if (options->UseBridges && num_bridges_usable() > 0) { - /* A bridge client with one or more running bridges */ + if (options->UseBridges && num_bridges_usable(0) > 0) { + /* A bridge client that is sure that one or more of its bridges are + * running can afford to wait longer to update bridge descriptors. */ return options->TestingBridgeDownloadSchedule; } else { - /* A bridge client with no running bridges */ + /* A bridge client which might have no running bridges, must try to + * get bridge descriptors straight away. */ return options->TestingBridgeBootstrapDownloadSchedule; } default: diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 100286f10..90b728726 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -3135,13 +3135,16 @@ entry_list_is_constrained(const or_options_t *options) }
/** Return the number of bridges that have descriptors that are marked with - * purpose 'bridge' and are running. + * purpose 'bridge' and are running. If use_maybe_reachable is + * true, include bridges that might be reachable in the count. + * Otherwise, if it is false, only include bridges that have recently been + * found running in the count. * * 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. */ MOCK_IMPL(int, -num_bridges_usable,(void)) +num_bridges_usable,(int use_maybe_reachable)) { int n_options = 0;
@@ -3154,8 +3157,12 @@ num_bridges_usable,(void)) }
SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) { + /* Definitely not usable */ if (guard->is_reachable == GUARD_REACHABLE_NO) continue; + /* If we want to be really sure the bridges will work, skip maybes */ + if (!use_maybe_reachable && guard->is_reachable == GUARD_REACHABLE_MAYBE) + continue; if (tor_digest_is_zero(guard->identity)) continue; const node_t *node = node_get_by_id(guard->identity); diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index ad6b47936..d909115b1 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -383,7 +383,7 @@ void entry_guards_note_internet_connectivity(guard_selection_t *gs);
int update_guard_selection_choice(const or_options_t *options);
-MOCK_DECL(int,num_bridges_usable,(void)); +MOCK_DECL(int,num_bridges_usable,(int use_maybe_reachable));
#ifdef ENTRYNODES_PRIVATE /** diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 2075fccc9..e0a3e4cdc 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1209,7 +1209,9 @@ should_delay_dir_fetches(const or_options_t *options, const char **msg_out) }
if (options->UseBridges) { - if (num_bridges_usable() == 0) { + /* If we know that none of our bridges can possibly work, avoid fetching + * directory documents. But if some of them might work, try again. */ + if (num_bridges_usable(1) == 0) { if (msg_out) { *msg_out = "No running bridges"; } diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 0b34e8b33..997b110d6 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -5877,8 +5877,9 @@ mock_networkstatus_consensus_can_use_extra_fallbacks(
static int mock_num_bridges_usable_value = 0; static int -mock_num_bridges_usable(void) +mock_num_bridges_usable(int use_maybe_reachable) { + (void)use_maybe_reachable; return mock_num_bridges_usable_value; }
@@ -6031,7 +6032,7 @@ test_dir_find_dl_schedule(void* data) /* client */ mock_options->ClientOnly = 1; mock_options->UseBridges = 1; - if (num_bridges_usable() > 0) { + if (num_bridges_usable(0) > 0) { 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);