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

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

13 Dec '17

commit bf89b089306067cffa4d2ca6b7fa7cd54790a961 Merge: cf5ab5934 19a4abf2a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:17:38 2017 -0500 Merge branch 'bug24367_032_squashed' into maint-0.3.2 changes/bug24367 | 13 +++++++++++++ src/or/bridges.c | 38 +++++++------------------------------- src/or/bridges.h | 1 - src/or/circuituse.c | 15 ++++++++++++--- src/or/directory.c | 9 +++++---- src/or/entrynodes.c | 26 ++++++++++++++++++++------ src/or/entrynodes.h | 3 +-- src/or/networkstatus.c | 4 +++- src/test/test_dir.c | 22 ++++++++++++---------- 9 files changed, 73 insertions(+), 58 deletions(-)

1 0

[tor/master] 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/master] 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] 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/release-0.3.2] Merge branch 'bug24367_032_squashed' into maint-0.3.2
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit bf89b089306067cffa4d2ca6b7fa7cd54790a961 Merge: cf5ab5934 19a4abf2a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:17:38 2017 -0500 Merge branch 'bug24367_032_squashed' into maint-0.3.2 changes/bug24367 | 13 +++++++++++++ src/or/bridges.c | 38 +++++++------------------------------- src/or/bridges.h | 1 - src/or/circuituse.c | 15 ++++++++++++--- src/or/directory.c | 9 +++++---- src/or/entrynodes.c | 26 ++++++++++++++++++++------ src/or/entrynodes.h | 3 +-- src/or/networkstatus.c | 4 +++- src/test/test_dir.c | 22 ++++++++++++---------- 9 files changed, 73 insertions(+), 58 deletions(-)

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 dc9f2cdf73a4a55288f2321aa78b72ea188fedec Merge: dd5ca2505 bf89b0893 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:27:14 2017 -0500 Merge branch 'maint-0.3.2' into release-0.3.2 changes/bug24367 | 13 +++++++++++++ src/or/bridges.c | 38 +++++++------------------------------- src/or/bridges.h | 1 - src/or/circuituse.c | 15 ++++++++++++--- src/or/directory.c | 9 +++++---- src/or/entrynodes.c | 26 ++++++++++++++++++++------ src/or/entrynodes.h | 3 +-- src/or/networkstatus.c | 4 +++- src/test/test_dir.c | 22 ++++++++++++---------- 9 files changed, 73 insertions(+), 58 deletions(-)

1 0

[tor/release-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/release-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

[tor/release-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/master] Merge branch 'bug24367_032_squashed' into maint-0.3.2
by nickm＠torproject.org 13 Dec '17

13 Dec '17

commit bf89b089306067cffa4d2ca6b7fa7cd54790a961 Merge: cf5ab5934 19a4abf2a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 12 19:17:38 2017 -0500 Merge branch 'bug24367_032_squashed' into maint-0.3.2 changes/bug24367 | 13 +++++++++++++ src/or/bridges.c | 38 +++++++------------------------------- src/or/bridges.h | 1 - src/or/circuituse.c | 15 ++++++++++++--- src/or/directory.c | 9 +++++---- src/or/entrynodes.c | 26 ++++++++++++++++++++------ src/or/entrynodes.h | 3 +-- src/or/networkstatus.c | 4 +++- src/test/test_dir.c | 22 ++++++++++++---------- 9 files changed, 73 insertions(+), 58 deletions(-)

1 0