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

[tor/master] Add a unittest that reveals the offending case of #23862.
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit 8c8d3b90f767076508944f4eb355180e14b6189e Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Oct 25 19:25:53 2017 +0300 Add a unittest that reveals the offending case of #23862. --- src/or/directory.c | 4 +- src/or/directory.h | 3 ++ src/or/entrynodes.c | 6 +-- src/or/entrynodes.h | 4 ++ src/or/networkstatus.c | 2 +- src/test/test_routerlist.c | 110 +++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 122 insertions(+), 7 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 6470723cd..272b324db 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2237,8 +2237,6 @@ load_downloaded_routers(const char *body, smartlist_t *which, return added; } -static int handle_response_fetch_consensus(dir_connection_t *, - const response_handler_args_t *); static int handle_response_fetch_certificate(dir_connection_t *, const response_handler_args_t *); static int handle_response_fetch_status_vote(dir_connection_t *, @@ -2585,7 +2583,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) * consensus document by checking the consensus, storing it, and marking * router requests as reachable. **/ -static int +STATIC int handle_response_fetch_consensus(dir_connection_t *conn, const response_handler_args_t *args) { diff --git a/src/or/directory.h b/src/or/directory.h index 79984be32..764f0092e 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -194,6 +194,9 @@ STATIC void warn_disallowed_anonymous_compression_method(compress_method_t); STATIC int handle_response_fetch_hsdesc_v3(dir_connection_t *conn, const response_handler_args_t *args); +STATIC int handle_response_fetch_consensus(dir_connection_t *conn, + const response_handler_args_t *args); + #endif /* defined(DIRECTORY_PRIVATE) */ #ifdef TOR_UNIT_TESTS diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 9fbf42643..76a8f591b 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2126,9 +2126,9 @@ circuit_guard_state_free(circuit_guard_state_t *state) /** Allocate and return a new circuit_guard_state_t to track the result * of using <b>guard</b> for a given operation. */ -static circuit_guard_state_t * -circuit_guard_state_new(entry_guard_t *guard, unsigned state, - entry_guard_restriction_t *rst) +MOCK_IMPL(STATIC circuit_guard_state_t *, +circuit_guard_state_new,(entry_guard_t *guard, unsigned state, + entry_guard_restriction_t *rst)) { circuit_guard_state_t *result; diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 9e1e72993..86f0517df 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -488,6 +488,10 @@ STATIC entry_guard_t *get_sampled_guard_with_id(guard_selection_t *gs, MOCK_DECL(STATIC time_t, randomize_time, (time_t now, time_t max_backdate)); +MOCK_DECL(STATIC circuit_guard_state_t *, + circuit_guard_state_new,(entry_guard_t *guard, unsigned state, + entry_guard_restriction_t *rst)); + STATIC entry_guard_t *entry_guard_add_to_sample(guard_selection_t *gs, const node_t *node); STATIC entry_guard_t *entry_guards_expand_sample(guard_selection_t *gs); diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 3a4f06fb7..abcd4d865 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1720,7 +1720,7 @@ networkstatus_set_current_consensus(const char *consensus, { networkstatus_t *c=NULL; int r, result = -1; - time_t now = time(NULL); + time_t now = approx_time(); const or_options_t *options = get_options(); char *unverified_fname = NULL, *consensus_fname = NULL; int flav = networkstatus_parse_flavor_name(flavor); diff --git a/src/test/test_routerlist.c b/src/test/test_routerlist.c index 3b0e943ce..b131dd959 100644 --- a/src/test/test_routerlist.c +++ b/src/test/test_routerlist.c @@ -6,13 +6,17 @@ #include <time.h> #define DIRVOTE_PRIVATE +#define ENTRYNODES_PRIVATE +#define DIRECTORY_PRIVATE #define NETWORKSTATUS_PRIVATE +#define CONNECTION_PRIVATE #define ROUTERLIST_PRIVATE #define TOR_UNIT_TESTING #include "or.h" #include "config.h" #include "connection.h" #include "container.h" +#include "control.h" #include "directory.h" #include "dirvote.h" #include "entrynodes.h" @@ -22,10 +26,13 @@ #include "policies.h" #include "router.h" #include "routerlist.h" +#include "routerset.h" #include "routerparse.h" #include "shared_random.h" +#include "statefile.h" #include "test.h" #include "test_dir_common.h" +#include "log_test_helpers.h" void construct_consensus(char **consensus_text_md); @@ -411,6 +418,107 @@ test_router_pick_directory_server_impl(void *arg) networkstatus_vote_free(con_md); } +static or_state_t *dummy_state = NULL; +static or_state_t * +get_or_state_replacement(void) +{ + return dummy_state; +} + +static void +mock_directory_initiate_request(directory_request_t *req) +{ + (void)req; + return; +} + +static circuit_guard_state_t * +mock_circuit_guard_state_new(entry_guard_t *guard, unsigned state, + entry_guard_restriction_t *rst) +{ + (void) guard; + (void) state; + (void) rst; + return NULL; +} + +/** Test that we will use our directory guards to fetch mds even if we don't + * have any dirinfo (tests bug #23862). */ +static void +test_directory_guard_fetch_with_no_dirinfo(void *arg) +{ + int retval; + char *consensus_text_md = NULL; + or_options_t *options = get_options_mutable(); + + (void) arg; + + /* Initialize the SRV subsystem */ + sr_init(0); + + /* Initialize the entry node configuration from the ticket */ + options->UseEntryGuards = 1; + options->StrictNodes = 1; + get_options_mutable()->EntryNodes = routerset_new(); + routerset_parse(get_options_mutable()->EntryNodes, + "2121212121212121212121212121212121212121", "foo"); + + /* Mock some functions */ + dummy_state = tor_malloc_zero(sizeof(or_state_t)); + MOCK(get_or_state, get_or_state_replacement); + MOCK(directory_initiate_request, mock_directory_initiate_request); + /* we need to mock this one to avoid memleaks */ + MOCK(circuit_guard_state_new, mock_circuit_guard_state_new); + + /* Call guards_update_all() to simulate loading our state file (see + * entry_guards_load_guards_from_state() and ticket #23989). */ + guards_update_all(); + + /* Test logic: Simulate the arrival of a new consensus when we have no + * dirinfo at all. Tor will need to fetch the mds from the consensus. Make + * sure that Tor will use the specified entry guard instead of relying on the + * fallback directories. */ + + /* Fixup the dirconn that will deliver the consensus */ + dir_connection_t *conn = dir_connection_new(AF_INET); + tor_addr_from_ipv4h(&conn->base_.addr, 0x7f000001); + conn->base_.port = 8800; + TO_CONN(conn)->address = tor_strdup("127.0.0.1"); + conn->base_.purpose = DIR_PURPOSE_FETCH_CONSENSUS; + conn->requested_resource = tor_strdup("ns"); + + /* Construct a consensus */ + construct_consensus(&consensus_text_md); + tt_assert(consensus_text_md); + + /* Place the consensus in the dirconn */ + response_handler_args_t args; + memset(&args, 0, sizeof(response_handler_args_t)); + args.status_code = 200; + args.body = consensus_text_md; + args.body_len = strlen(consensus_text_md); + + /* Update approx time so that the consensus is considered live */ + update_approx_time(time(NULL)+1010); + + setup_capture_of_logs(LOG_DEBUG); + + /* Now handle the consensus */ + retval = handle_response_fetch_consensus(conn, &args); + tt_int_op(retval, OP_EQ, 0); + + /* Make sure that our primary guard was chosen */ + /* BUG #23862 falls back to dirserver list!! */ + expect_log_msg_containing("falling back to dirserver list"); + + done: + tor_free(consensus_text_md); + tor_free(dummy_state); + connection_free_(TO_CONN(conn)); + entry_guards_free_all(); + teardown_capture_of_logs(); +} + static connection_t *mocked_connection = NULL; /* Mock connection_get_by_type_addr_port_purpose by returning @@ -494,6 +602,8 @@ struct testcase_t routerlist_tests[] = { NODE(launch_descriptor_downloads, 0), NODE(router_is_already_dir_fetching, TT_FORK), ROUTER(pick_directory_server_impl, TT_FORK), + { "directory_guard_fetch_with_no_dirinfo", + test_directory_guard_fetch_with_no_dirinfo, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Update entry guard state whenever we download a consensus.
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit 10c5eae3fa4f26027c6335cab607a5de550458db Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Oct 25 19:18:25 2017 +0300 Update entry guard state whenever we download a consensus. Update guard state even if we don't have enough dirinfo since that actually affects the future download of dirinfos. Fixes #23862 on 0.3.0.1-alpha --- src/or/main.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index be6162834..4ab5f311f 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -971,6 +971,15 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) { const or_options_t *options = get_options(); + /* if we have enough dir info, then update our guard status with + * whatever we just learned. */ + int invalidate_circs = guards_update_all(); + + if (invalidate_circs) { + circuit_mark_all_unused_circs(); + circuit_mark_all_dirty_circs_as_unusable(); + } + if (!router_have_minimum_dir_info()) { int quiet = suppress_logs || from_cache || directory_too_idle_to_fetch_descriptors(options, now); @@ -984,15 +993,6 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) update_all_descriptor_downloads(now); } - /* if we have enough dir info, then update our guard status with - * whatever we just learned. */ - int invalidate_circs = guards_update_all(); - - if (invalidate_circs) { - circuit_mark_all_unused_circs(); - circuit_mark_all_dirty_circs_as_unusable(); - } - /* Don't even bother trying to get extrainfo until the rest of our * directory info is up-to-date */ if (options->DownloadExtraInfo)

1 0

[tor/master] Remove a duplicate call to update_microdesc_downloads()
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit dfd3ed5bdc252ad2c3afed922e5dcd955eff1d68 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Oct 25 19:18:38 2017 +0300 Remove a duplicate call to update_microdesc_downloads() This call happens before we update our entry guards, so it needs to be removed for the fix to #23862 to work. --- src/or/directory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 272b324db..777972d57 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2667,11 +2667,11 @@ handle_response_fetch_consensus(dir_connection_t *conn, /* If we launched other fetches for this consensus, cancel them. */ connection_dir_close_consensus_fetches(conn, flavname); - /* launches router downloads as needed */ + /* update the list of routers and directory guards */ routers_update_all_from_networkstatus(now, 3); update_microdescs_from_networkstatus(now); - update_microdesc_downloads(now); directory_info_has_arrived(now, 0, 0); + if (authdir_mode_v3(get_options())) { sr_act_post_consensus( networkstatus_get_latest_consensus_by_flavor(FLAV_NS));

1 0

[tor/master] remove now-redundant changes file
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit 636d0ad68bf134a7dbeabdf83b70763f88fbc970 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Oct 26 09:09:37 2017 -0400 remove now-redundant changes file --- changes/bug23682 | 5 ----- 1 file changed, 5 deletions(-) diff --git a/changes/bug23682 b/changes/bug23682 deleted file mode 100644 index 301ce7367..000000000 --- a/changes/bug23682 +++ /dev/null @@ -1,5 +0,0 @@ - o Minor bugfixes (entry guards): - - Tor now updates its guard state when it reads a consensus regardless of - whether it's missing descriptors. That makes tor use its primary guards - to fetch descriptors in some edge cases where it would have used fallback - directories in the past. Fixes bug 23862; bugfix on 0.3.0.1-alpha. \ No newline at end of file

1 0

[tor/maint-0.3.2] Update entry guard state whenever we download a consensus.
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit cbc70437a7318f8e5abef0829ba8e02298ae53e2 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Oct 25 19:18:25 2017 +0300 Update entry guard state whenever we download a consensus. Update guard state even if we don't have enough dirinfo since that actually affects the future download of dirinfos. Fixes #23862 on 0.3.0.1-alpha --- src/or/main.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 65b0b8f4d..c340e4128 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -971,6 +971,15 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) { const or_options_t *options = get_options(); + /* if we have enough dir info, then update our guard status with + * whatever we just learned. */ + int invalidate_circs = guards_update_all(); + + if (invalidate_circs) { + circuit_mark_all_unused_circs(); + circuit_mark_all_dirty_circs_as_unusable(); + } + if (!router_have_minimum_dir_info()) { int quiet = suppress_logs || from_cache || directory_too_idle_to_fetch_descriptors(options, now); @@ -984,15 +993,6 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) update_all_descriptor_downloads(now); } - /* if we have enough dir info, then update our guard status with - * whatever we just learned. */ - int invalidate_circs = guards_update_all(); - - if (invalidate_circs) { - circuit_mark_all_unused_circs(); - circuit_mark_all_dirty_circs_as_unusable(); - } - /* Don't even bother trying to get extrainfo until the rest of our * directory info is up-to-date */ if (options->DownloadExtraInfo)

1 0

[tor/maint-0.3.2] fix check-changes warning
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit 62cd64cc166081b17a0cf1fc9abd422c71e1f17b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Oct 26 09:08:49 2017 -0400 fix check-changes warning --- changes/{bug23682 => bug23862} | 0 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/changes/bug23682 b/changes/bug23862 similarity index 100% rename from changes/bug23682 rename to changes/bug23862

1 0

[tor/maint-0.3.2] Remove a duplicate call to update_microdesc_downloads()
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit 42a47c0e7da425a55b77aafe2dda2fea7563c761 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Oct 25 19:18:38 2017 +0300 Remove a duplicate call to update_microdesc_downloads() This call happens before we update our entry guards, so it needs to be removed for the fix to #23862 to work. --- src/or/directory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 6470723cd..7a1364bd7 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2669,11 +2669,11 @@ handle_response_fetch_consensus(dir_connection_t *conn, /* If we launched other fetches for this consensus, cancel them. */ connection_dir_close_consensus_fetches(conn, flavname); - /* launches router downloads as needed */ + /* update the list of routers and directory guards */ routers_update_all_from_networkstatus(now, 3); update_microdescs_from_networkstatus(now); - update_microdesc_downloads(now); directory_info_has_arrived(now, 0, 0); + if (authdir_mode_v3(get_options())) { sr_act_post_consensus( networkstatus_get_latest_consensus_by_flavor(FLAV_NS));

1 0

[tor/master] Update entry guard state whenever we download a consensus.
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit cbc70437a7318f8e5abef0829ba8e02298ae53e2 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Oct 25 19:18:25 2017 +0300 Update entry guard state whenever we download a consensus. Update guard state even if we don't have enough dirinfo since that actually affects the future download of dirinfos. Fixes #23862 on 0.3.0.1-alpha --- src/or/main.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 65b0b8f4d..c340e4128 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -971,6 +971,15 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) { const or_options_t *options = get_options(); + /* if we have enough dir info, then update our guard status with + * whatever we just learned. */ + int invalidate_circs = guards_update_all(); + + if (invalidate_circs) { + circuit_mark_all_unused_circs(); + circuit_mark_all_dirty_circs_as_unusable(); + } + if (!router_have_minimum_dir_info()) { int quiet = suppress_logs || from_cache || directory_too_idle_to_fetch_descriptors(options, now); @@ -984,15 +993,6 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) update_all_descriptor_downloads(now); } - /* if we have enough dir info, then update our guard status with - * whatever we just learned. */ - int invalidate_circs = guards_update_all(); - - if (invalidate_circs) { - circuit_mark_all_unused_circs(); - circuit_mark_all_dirty_circs_as_unusable(); - } - /* Don't even bother trying to get extrainfo until the rest of our * directory info is up-to-date */ if (options->DownloadExtraInfo)

1 0

[tor/maint-0.3.2] Add changes file for #23862.
by nickm＠torproject.org 26 Oct '17

26 Oct '17

commit e0276dc1c64d4c2c9439e080d006dfb249fc7c53 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Oct 25 19:54:48 2017 +0300 Add changes file for #23862. --- changes/bug23682 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/bug23682 b/changes/bug23682 new file mode 100644 index 000000000..301ce7367 --- /dev/null +++ b/changes/bug23682 @@ -0,0 +1,5 @@ + o Minor bugfixes (entry guards): + - Tor now updates its guard state when it reads a consensus regardless of + whether it's missing descriptors. That makes tor use its primary guards + to fetch descriptors in some edge cases where it would have used fallback + directories in the past. Fixes bug 23862; bugfix on 0.3.0.1-alpha. \ No newline at end of file

1 0

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

26 Oct '17

commit be49fadad02e74a5d1a175ea8bd4b8855f8ec728 Merge: 853bbb911 e0276dc1c Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Oct 26 09:03:21 2017 -0400 Merge branch 'maint-0.3.2' changes/bug23682 | 5 +++++ src/or/directory.c | 4 ++-- src/or/main.c | 18 +++++++++--------- 3 files changed, 16 insertions(+), 11 deletions(-)

1 0