December 2017 - tor-commits - lists.torproject.org

[tor/maint-0.3.2] Make sandbox.c compile when libseccomp-dev is installed on arm64
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 15b41fa6ae6a1356d5453242ccb7d7d301dd5e67 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:46:03 2017 -0500 Make sandbox.c compile when libseccomp-dev is installed on arm64 Fixes ticket 24424. Patch from weasel. --- changes/bug24424 | 3 +++ src/common/sandbox.c | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/changes/bug24424 b/changes/bug24424 new file mode 100644 index 000000000..63c2d39ba --- /dev/null +++ b/changes/bug24424 @@ -0,0 +1,3 @@ + o Minor features (portability): + - Tor now compiles correctly on arm64 with libseccomp-dev installed. + (It doesn't yet work with the sandbox enabled.) Closes ticket 24424. diff --git a/src/common/sandbox.c b/src/common/sandbox.c index ba6c3efb9..8827370c2 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -106,6 +106,11 @@ #define M_SYSCALL arm_r7 +#elif defined(__aarch64__) && defined(__LP64__) + +#define REG_SYSCALL 8 +#define M_SYSCALL regs[REG_SYSCALL] + #endif /* defined(__i386__) || ... */ /**Determines if at least one sandbox is active.*/

1 0

[tor/release-0.3.2] Merge branch 'maint-0.3.2' into release-0.3.2
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 37fba82c155dda2ba5d4b3bdd63ea4cd54f264ee Merge: dc9f2cdf7 15b41fa6a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:46:53 2017 -0500 Merge branch 'maint-0.3.2' into release-0.3.2 changes/bug24424 | 3 +++ src/common/sandbox.c | 5 +++++ 2 files changed, 8 insertions(+)

1 0

[tor/release-0.3.2] Make sandbox.c compile when libseccomp-dev is installed on arm64
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 15b41fa6ae6a1356d5453242ccb7d7d301dd5e67 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:46:03 2017 -0500 Make sandbox.c compile when libseccomp-dev is installed on arm64 Fixes ticket 24424. Patch from weasel. --- changes/bug24424 | 3 +++ src/common/sandbox.c | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/changes/bug24424 b/changes/bug24424 new file mode 100644 index 000000000..63c2d39ba --- /dev/null +++ b/changes/bug24424 @@ -0,0 +1,3 @@ + o Minor features (portability): + - Tor now compiles correctly on arm64 with libseccomp-dev installed. + (It doesn't yet work with the sandbox enabled.) Closes ticket 24424. diff --git a/src/common/sandbox.c b/src/common/sandbox.c index ba6c3efb9..8827370c2 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -106,6 +106,11 @@ #define M_SYSCALL arm_r7 +#elif defined(__aarch64__) && defined(__LP64__) + +#define REG_SYSCALL 8 +#define M_SYSCALL regs[REG_SYSCALL] + #endif /* defined(__i386__) || ... */ /**Determines if at least one sandbox is active.*/

1 0

[tor/master] Merge branch 'maint-0.3.2'
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 426110dfa2e27f134e7bf44341e5df6f454e49a3 Merge: 9abef516f 15b41fa6a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:46:53 2017 -0500 Merge branch 'maint-0.3.2' changes/bug24424 | 3 +++ src/common/sandbox.c | 5 +++++ 2 files changed, 8 insertions(+)

1 0

[tor/maint-0.3.2] Avoid crashing if we call num_usable_bridges() when bridges are not enabled
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit d7833c9d27feed9e4d4d4d4b5920b5d17150b82d Author: teor <teor2345(a)gmail.com> Date: Wed Nov 22 02:01:51 2017 +1100 Avoid crashing if we call num_usable_bridges() when bridges are not enabled This applies the changes in 23524 to num_usable_bridges(), because it has replaced any_bridge_descriptors_known(). The original changes file still applies. --- src/or/entrynodes.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 826c0e48a..100286f10 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -3145,9 +3145,13 @@ num_bridges_usable,(void)) { int n_options = 0; - tor_assert(get_options()->UseBridges); + if (BUG(!get_options()->UseBridges)) { + return 0; + } guard_selection_t *gs = get_guard_selection_info(); - tor_assert(gs->type == GS_TYPE_BRIDGE); + if (BUG(gs->type != GS_TYPE_BRIDGE)) { + return 0; + } SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) { if (guard->is_reachable == GUARD_REACHABLE_NO)

1 0

[tor/maint-0.3.2] Make sure bridges are definitely running before delaying directory fetches
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 19a4abf2a99c6a3de2c9a2fdf3e6d7b7c404f8f8 Author: teor <teor2345(a)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);

1 0

[tor/maint-0.3.2] Run the download_status_increment test in a forked process.
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 6f3a862966cd09b9feae357ada0ad4d51b51c0ea Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Dec 1 16:06:25 2017 -0500 Run the download_status_increment test in a forked process. It messes with global state somehow in a way that makes several of the entryconn tests fail now. --- src/test/test_dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 78bf23608..0b34e8b33 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -6218,7 +6218,7 @@ struct testcase_t dir_tests[] = { DIR(download_status_schedule, 0), DIR(download_status_random_backoff, 0), DIR(download_status_random_backoff_ranges, 0), - DIR(download_status_increment, 0), + DIR(download_status_increment, TT_FORK), DIR(authdir_type_to_string, 0), DIR(conn_purpose_to_string, 0), DIR(should_use_directory_guards, 0),

1 0

[tor/master] Stop checking cached bridge descriptors for usable bridges
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 690f646bf8a5de9b099fb5295ba9ff252e5606f4 Author: teor <teor2345(a)gmail.com> Date: Wed Nov 22 01:50:46 2017 +1100 Stop checking cached bridge descriptors for usable bridges Stop checking for bridge descriptors when we actually want to know if any bridges are usable. This avoids potential bootstrapping issues. 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. --- changes/bug24367 | 7 +++++++ src/or/bridges.c | 28 ---------------------------- src/or/bridges.h | 1 - src/or/circuituse.c | 2 +- src/or/directory.c | 2 +- src/or/entrynodes.c | 9 ++++++--- src/or/entrynodes.h | 3 +-- src/or/networkstatus.c | 2 +- src/test/test_dir.c | 19 ++++++++++--------- 9 files changed, 27 insertions(+), 46 deletions(-) diff --git a/changes/bug24367 b/changes/bug24367 new file mode 100644 index 000000000..6081f7b5d --- /dev/null +++ b/changes/bug24367 @@ -0,0 +1,7 @@ + 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. + 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 320f5ee63..ac759493c 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -835,34 +835,6 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) } } -/** Return the number of bridges that have descriptors that - * are marked with purpose 'bridge' and are running. - * - * 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, -any_bridge_descriptors_known, (void)) -{ - if (BUG(!get_options()->UseBridges)) { - return 0; - } - - if (!bridge_list) - return 0; - - SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge) { - const node_t *node; - if (!tor_digest_is_zero(bridge->identity) && - (node = node_get_by_id(bridge->identity)) != NULL && - node->ri) { - return 1; - } - } SMARTLIST_FOREACH_END(bridge); - - return 0; -} - /** Return a smartlist containing all bridge identity digests */ MOCK_IMPL(smartlist_t *, list_bridge_identities, (void)) diff --git a/src/or/bridges.h b/src/or/bridges.h index 263c7d51b..54a625025 100644 --- a/src/or/bridges.h +++ b/src/or/bridges.h @@ -45,7 +45,6 @@ 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); -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/circuituse.c b/src/or/circuituse.c index aa0df9565..8c9859bab 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -2084,7 +2084,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, "used client functionality lately" : "received a consensus with exits", options->UseBridges ? "bridges" : "entrynodes"); - } else if (!options->UseBridges || any_bridge_descriptors_known()) { + } else if (!options->UseBridges || num_bridges_usable() > 0) { log_fn(severity, LD_APP|LD_DIR, "Application request when we haven't %s. " "Optimistically trying directory fetches again.", diff --git a/src/or/directory.c b/src/or/directory.c index 66bdef236..e49fd595c 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -5338,7 +5338,7 @@ 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 && any_bridge_descriptors_known()) { + if (options->UseBridges && num_bridges_usable() > 0) { /* A bridge client with one or more running bridges */ return options->TestingBridgeDownloadSchedule; } else { diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 866e59335..826c0e48a 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -3136,9 +3136,12 @@ 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. - */ -int -num_bridges_usable(void) + * + * 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)) { int n_options = 0; diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index d177f6cff..ad6b47936 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -383,8 +383,7 @@ void entry_guards_note_internet_connectivity(guard_selection_t *gs); int update_guard_selection_choice(const or_options_t *options); -/* Used by bridges.c only. */ -int num_bridges_usable(void); +MOCK_DECL(int,num_bridges_usable,(void)); #ifdef ENTRYNODES_PRIVATE /** diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index f31529733..2075fccc9 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1209,7 +1209,7 @@ should_delay_dir_fetches(const or_options_t *options, const char **msg_out) } if (options->UseBridges) { - if (!any_bridge_descriptors_known()) { + if (num_bridges_usable() == 0) { if (msg_out) { *msg_out = "No running bridges"; } diff --git a/src/test/test_dir.c b/src/test/test_dir.c index ee4a9780b..78bf23608 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -5875,11 +5875,11 @@ mock_networkstatus_consensus_can_use_extra_fallbacks( return mock_networkstatus_consensus_can_use_extra_fallbacks_value; } -static int mock_any_bridge_descriptors_known_value = 0; +static int mock_num_bridges_usable_value = 0; static int -mock_any_bridge_descriptors_known(void) +mock_num_bridges_usable(void) { - return mock_any_bridge_descriptors_known_value; + return mock_num_bridges_usable_value; } /* data is a 3 character nul-terminated string. @@ -5908,17 +5908,18 @@ test_dir_find_dl_schedule(void* data) } if (str[2] == 'r') { - mock_any_bridge_descriptors_known_value = 1; + /* Any positive, non-zero value should work */ + mock_num_bridges_usable_value = 2; } else { - mock_any_bridge_descriptors_known_value = 0; + mock_num_bridges_usable_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); + MOCK(num_bridges_usable, + mock_num_bridges_usable); download_status_t dls; smartlist_t server, client, server_cons, client_cons; @@ -6030,7 +6031,7 @@ test_dir_find_dl_schedule(void* data) /* client */ mock_options->ClientOnly = 1; mock_options->UseBridges = 1; - if (any_bridge_descriptors_known()) { + if (num_bridges_usable() > 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); @@ -6039,7 +6040,7 @@ test_dir_find_dl_schedule(void* data) done: UNMOCK(networkstatus_consensus_is_bootstrapping); UNMOCK(networkstatus_consensus_can_use_extra_fallbacks); - UNMOCK(any_bridge_descriptors_known); + UNMOCK(num_bridges_usable); UNMOCK(get_options); tor_free(mock_options); mock_options = NULL;

1 0

[tor/maint-0.3.2] Simplify some conditionals in circuit_get_open_circ_or_launch()
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 6b5c70670b26b9560febf5dc70f814d5e515c0f8 Author: teor <teor2345(a)gmail.com> Date: Mon Dec 11 01:14:28 2017 +1100 Simplify some conditionals in circuit_get_open_circ_or_launch() When entry_list_is_constrained() is true, guards_retry_optimistic() always returns true. When entry_list_is_constrained() is false, options->UseBridges is always false, therefore !options->UseBridges is always true, therefore (!options->UseBridges || ...) is always true. Cleanup after #24367. --- src/or/circuituse.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 8c9859bab..34b2e2eab 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -2075,8 +2075,12 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, if (!connection_get_by_type(CONN_TYPE_DIR)) { int severity = LOG_NOTICE; /* Retry some stuff that might help the connection work. */ - if (entry_list_is_constrained(options) && - guards_retry_optimistic(options)) { + /* If we are configured with EntryNodes or UseBridges */ + if (entry_list_is_constrained(options)) { + /* Retry all our guards / bridges. + * guards_retry_optimistic() always returns true here. */ + int rv = guards_retry_optimistic(options); + tor_assert_nonfatal_once(rv); log_fn(severity, LD_APP|LD_DIR, "Application request when we haven't %s. " "Optimistically trying known %s again.", @@ -2084,7 +2088,12 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, "used client functionality lately" : "received a consensus with exits", options->UseBridges ? "bridges" : "entrynodes"); - } else if (!options->UseBridges || num_bridges_usable() > 0) { + } else { + /* Getting directory documents doesn't help much if we have a limited + * number of guards */ + tor_assert_nonfatal(!options->UseBridges); + tor_assert_nonfatal(!options->EntryNodes); + /* Retry our directory fetches, so we have a fresh set of guard info */ log_fn(severity, LD_APP|LD_DIR, "Application request when we haven't %s. " "Optimistically trying directory fetches again.",

1 0

[tor/maint-0.3.2] Stop checking cached bridge descriptors for usable bridges
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit 690f646bf8a5de9b099fb5295ba9ff252e5606f4 Author: teor <teor2345(a)gmail.com> Date: Wed Nov 22 01:50:46 2017 +1100 Stop checking cached bridge descriptors for usable bridges Stop checking for bridge descriptors when we actually want to know if any bridges are usable. This avoids potential bootstrapping issues. 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. --- changes/bug24367 | 7 +++++++ src/or/bridges.c | 28 ---------------------------- src/or/bridges.h | 1 - src/or/circuituse.c | 2 +- src/or/directory.c | 2 +- src/or/entrynodes.c | 9 ++++++--- src/or/entrynodes.h | 3 +-- src/or/networkstatus.c | 2 +- src/test/test_dir.c | 19 ++++++++++--------- 9 files changed, 27 insertions(+), 46 deletions(-) diff --git a/changes/bug24367 b/changes/bug24367 new file mode 100644 index 000000000..6081f7b5d --- /dev/null +++ b/changes/bug24367 @@ -0,0 +1,7 @@ + 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. + 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 320f5ee63..ac759493c 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -835,34 +835,6 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) } } -/** Return the number of bridges that have descriptors that - * are marked with purpose 'bridge' and are running. - * - * 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, -any_bridge_descriptors_known, (void)) -{ - if (BUG(!get_options()->UseBridges)) { - return 0; - } - - if (!bridge_list) - return 0; - - SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge) { - const node_t *node; - if (!tor_digest_is_zero(bridge->identity) && - (node = node_get_by_id(bridge->identity)) != NULL && - node->ri) { - return 1; - } - } SMARTLIST_FOREACH_END(bridge); - - return 0; -} - /** Return a smartlist containing all bridge identity digests */ MOCK_IMPL(smartlist_t *, list_bridge_identities, (void)) diff --git a/src/or/bridges.h b/src/or/bridges.h index 263c7d51b..54a625025 100644 --- a/src/or/bridges.h +++ b/src/or/bridges.h @@ -45,7 +45,6 @@ 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); -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/circuituse.c b/src/or/circuituse.c index aa0df9565..8c9859bab 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -2084,7 +2084,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn, "used client functionality lately" : "received a consensus with exits", options->UseBridges ? "bridges" : "entrynodes"); - } else if (!options->UseBridges || any_bridge_descriptors_known()) { + } else if (!options->UseBridges || num_bridges_usable() > 0) { log_fn(severity, LD_APP|LD_DIR, "Application request when we haven't %s. " "Optimistically trying directory fetches again.", diff --git a/src/or/directory.c b/src/or/directory.c index 66bdef236..e49fd595c 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -5338,7 +5338,7 @@ 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 && any_bridge_descriptors_known()) { + if (options->UseBridges && num_bridges_usable() > 0) { /* A bridge client with one or more running bridges */ return options->TestingBridgeDownloadSchedule; } else { diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 866e59335..826c0e48a 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -3136,9 +3136,12 @@ 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. - */ -int -num_bridges_usable(void) + * + * 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)) { int n_options = 0; diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index d177f6cff..ad6b47936 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -383,8 +383,7 @@ void entry_guards_note_internet_connectivity(guard_selection_t *gs); int update_guard_selection_choice(const or_options_t *options); -/* Used by bridges.c only. */ -int num_bridges_usable(void); +MOCK_DECL(int,num_bridges_usable,(void)); #ifdef ENTRYNODES_PRIVATE /** diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index f31529733..2075fccc9 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1209,7 +1209,7 @@ should_delay_dir_fetches(const or_options_t *options, const char **msg_out) } if (options->UseBridges) { - if (!any_bridge_descriptors_known()) { + if (num_bridges_usable() == 0) { if (msg_out) { *msg_out = "No running bridges"; } diff --git a/src/test/test_dir.c b/src/test/test_dir.c index ee4a9780b..78bf23608 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -5875,11 +5875,11 @@ mock_networkstatus_consensus_can_use_extra_fallbacks( return mock_networkstatus_consensus_can_use_extra_fallbacks_value; } -static int mock_any_bridge_descriptors_known_value = 0; +static int mock_num_bridges_usable_value = 0; static int -mock_any_bridge_descriptors_known(void) +mock_num_bridges_usable(void) { - return mock_any_bridge_descriptors_known_value; + return mock_num_bridges_usable_value; } /* data is a 3 character nul-terminated string. @@ -5908,17 +5908,18 @@ test_dir_find_dl_schedule(void* data) } if (str[2] == 'r') { - mock_any_bridge_descriptors_known_value = 1; + /* Any positive, non-zero value should work */ + mock_num_bridges_usable_value = 2; } else { - mock_any_bridge_descriptors_known_value = 0; + mock_num_bridges_usable_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); + MOCK(num_bridges_usable, + mock_num_bridges_usable); download_status_t dls; smartlist_t server, client, server_cons, client_cons; @@ -6030,7 +6031,7 @@ test_dir_find_dl_schedule(void* data) /* client */ mock_options->ClientOnly = 1; mock_options->UseBridges = 1; - if (any_bridge_descriptors_known()) { + if (num_bridges_usable() > 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); @@ -6039,7 +6040,7 @@ test_dir_find_dl_schedule(void* data) done: UNMOCK(networkstatus_consensus_is_bootstrapping); UNMOCK(networkstatus_consensus_can_use_extra_fallbacks); - UNMOCK(any_bridge_descriptors_known); + UNMOCK(num_bridges_usable); UNMOCK(get_options); tor_free(mock_options); mock_options = NULL;

1 0