June 2019 - tor-commits - lists.torproject.org

[tor/master] Merge remote-tracking branch 'tor-github/pr/1119'
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 0fe9657c8c9181a6623fdb69000f28a3cd2c9a59 Merge: 167dd097e 9644f3462 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 26 09:50:00 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1119' changes/ticket30871 | 6 ++++ scripts/maint/practracker/exceptions.txt | 4 +-- src/feature/client/entrynodes.c | 4 +++ src/test/test_circuitbuild.c | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-)

1 0

[tor/master] Merge branch 'maint-0.4.1'
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit c2a326d34f18b23f4e772f9290bde6bfa180c220 Merge: 0fe9657c8 648d5df62 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 26 09:50:12 2019 -0400 Merge branch 'maint-0.4.1' "ours" to avoid the 0.4.1 version of #30871.

1 0

[tor/master] Make the great grand practracker happy...
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 9644f3462a4c069a914beb04fda73820bcf29779 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jun 18 13:33:10 2019 -0400 Make the great grand practracker happy... Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- scripts/maint/practracker/exceptions.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 815d0ed09..613f046a2 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -135,8 +135,8 @@ problem function-size /src/feature/client/addressmap.c:addressmap_rewrite() 112 problem function-size /src/feature/client/bridges.c:rewrite_node_address_for_bridge() 126 problem function-size /src/feature/client/circpathbias.c:pathbias_measure_close_rate() 108 problem function-size /src/feature/client/dnsserv.c:evdns_server_callback() 153 -problem file-size /src/feature/client/entrynodes.c 3820 -problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 153 +problem file-size /src/feature/client/entrynodes.c 3824 +problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 157 problem function-size /src/feature/client/entrynodes.c:entry_guard_parse_from_state() 246 problem function-size /src/feature/client/transports.c:handle_proxy_line() 108 problem function-size /src/feature/client/transports.c:parse_method_line_helper() 112

1 0

[tor/release-0.4.1] guard: Ignore marked for close circuit when changing state to open
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 6a0763cd665e50174df5a7ea6ae0a27db7c9945f Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jun 18 13:32:45 2019 -0400 guard: Ignore marked for close circuit when changing state to open When we consider all circuits in "waiting for guard" state to be promoted to an "open" state, we were considering all circuits, even the one marked for close. This ultiamtely triggers a "circuit_has_opened()" called on the circuit that is marked for close which then leads to possible undesirable behaviors within a subsystem. For instance, the HS subsystem would be unable to find the authentication key of the introduction point circuit leading to a BUG() warning and a duplicate mark for close on the circuit. This commit also adds a unit test to make sure we never select marked for close circuits when upgrading its guard state from waiting for guard to open. Fixes #30871 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket30871 | 6 ++++++ src/feature/client/entrynodes.c | 4 ++++ src/test/test_circuitbuild.c | 47 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/changes/ticket30871 b/changes/ticket30871 new file mode 100644 index 000000000..81c076bb0 --- /dev/null +++ b/changes/ticket30871 @@ -0,0 +1,6 @@ + o Major bugfixes (circuit build, guard): + - When considering upgrading circuits from "waiting for guard" to "open", + always ignore the ones that are mark for close. Else, we can end up in + the situation where a subsystem is notified of that circuit opening but + still marked for close leading to undesirable behavior. Fixes bug 30871; + bugfix on 0.3.0.1-alpha. diff --git a/src/feature/client/entrynodes.c b/src/feature/client/entrynodes.c index 4afcee202..54a9238d8 100644 --- a/src/feature/client/entrynodes.c +++ b/src/feature/client/entrynodes.c @@ -2611,6 +2611,10 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, entry_guard_t *guard = entry_guard_handle_get(state->guard); if (!guard || guard->in_selection != gs) continue; + if (TO_CIRCUIT(circ)->marked_for_close) { + /* Don't consider any marked for close circuits. */ + continue; + } smartlist_add(all_circuits, circ); } SMARTLIST_FOREACH_END(circ); diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 47218a559..0c2309159 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -4,6 +4,8 @@ /* See LICENSE for licensing information */ #define CIRCUITBUILD_PRIVATE +#define CIRCUITLIST_PRIVATE +#define ENTRYNODES_PRIVATE #include "core/or/or.h" #include "test/test.h" @@ -13,7 +15,11 @@ #include "core/or/circuitbuild.h" #include "core/or/circuitlist.h" +#include "core/or/cpath_build_state_st.h" #include "core/or/extend_info_st.h" +#include "core/or/origin_circuit_st.h" + +#include "feature/client/entrynodes.h" /* Dummy nodes smartlist for testing */ static smartlist_t dummy_nodes; @@ -126,10 +132,51 @@ test_new_route_len_unhandled_exit(void *arg) UNMOCK(count_acceptable_nodes); } +static void +test_upgrade_from_guard_wait(void *arg) +{ + circuit_t *circ = NULL; + origin_circuit_t *orig_circ = NULL; + entry_guard_t *guard = NULL; + smartlist_t *list = NULL; + + (void) arg; + + circ = dummy_origin_circuit_new(0); + orig_circ = TO_ORIGIN_CIRCUIT(circ); + tt_assert(orig_circ); + + orig_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t)); + + circuit_set_state(circ, CIRCUIT_STATE_GUARD_WAIT); + + /* Put it in guard wait state. */ + guard = tor_malloc_zero(sizeof(*guard)); + guard->in_selection = get_guard_selection_info(); + + orig_circ->guard_state = + circuit_guard_state_new(guard, GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD, + NULL); + + /* Mark the circuit for close. */ + circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL); + tt_int_op(circ->marked_for_close, OP_NE, 0); + + /* We shouldn't pick the mark for close circuit. */ + list = circuit_find_circuits_to_upgrade_from_guard_wait(); + tt_assert(!list); + + done: + circuit_free(circ); + entry_guard_free_(guard); +} + struct testcase_t circuitbuild_tests[] = { { "noexit", test_new_route_len_noexit, 0, NULL, NULL }, { "safe_exit", test_new_route_len_safe_exit, 0, NULL, NULL }, { "unsafe_exit", test_new_route_len_unsafe_exit, 0, NULL, NULL }, { "unhandled_exit", test_new_route_len_unhandled_exit, 0, NULL, NULL }, + { "upgrade_from_guard_wait", test_upgrade_from_guard_wait, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/release-0.4.1] Merge remote-tracking branch 'tor-github/pr/1118' into maint-0.4.1
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 648d5df628295f6bde4c94b5fb5302b982ef031f Merge: 00acccac6 05b6f73f1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 26 09:49:40 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1118' into maint-0.4.1 changes/ticket30871 | 6 ++++ scripts/maint/practracker/exceptions.txt | 4 +-- src/feature/client/entrynodes.c | 4 +++ src/test/test_circuitbuild.c | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-)

1 0

[tor/release-0.4.1] Make the great grand practracker happy...
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 05b6f73f12374e40e38bb1b35f3df593aa287610 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jun 18 13:33:10 2019 -0400 Make the great grand practracker happy... Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- scripts/maint/practracker/exceptions.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index db9bf34e2..9073f2f1d 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -135,8 +135,8 @@ problem function-size /src/feature/client/addressmap.c:addressmap_rewrite() 112 problem function-size /src/feature/client/bridges.c:rewrite_node_address_for_bridge() 126 problem function-size /src/feature/client/circpathbias.c:pathbias_measure_close_rate() 108 problem function-size /src/feature/client/dnsserv.c:evdns_server_callback() 153 -problem file-size /src/feature/client/entrynodes.c 3820 -problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 153 +problem file-size /src/feature/client/entrynodes.c 3824 +problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 157 problem function-size /src/feature/client/entrynodes.c:entry_guard_parse_from_state() 246 problem function-size /src/feature/client/transports.c:handle_proxy_line() 108 problem function-size /src/feature/client/transports.c:parse_method_line_helper() 112

1 0

[tor/release-0.4.1] Merge branch 'maint-0.4.1' into release-0.4.1
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 89ca35ff54e21a172034652a9b152d4fc8103a54 Merge: 8c77a9444 648d5df62 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 26 09:51:32 2019 -0400 Merge branch 'maint-0.4.1' into release-0.4.1 changes/ticket30871 | 6 ++++ scripts/maint/practracker/exceptions.txt | 4 +-- src/feature/client/entrynodes.c | 4 +++ src/test/test_circuitbuild.c | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-)

1 0

[tor/master] guard: Ignore marked for close circuit when changing state to open
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 6a0763cd665e50174df5a7ea6ae0a27db7c9945f Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jun 18 13:32:45 2019 -0400 guard: Ignore marked for close circuit when changing state to open When we consider all circuits in "waiting for guard" state to be promoted to an "open" state, we were considering all circuits, even the one marked for close. This ultiamtely triggers a "circuit_has_opened()" called on the circuit that is marked for close which then leads to possible undesirable behaviors within a subsystem. For instance, the HS subsystem would be unable to find the authentication key of the introduction point circuit leading to a BUG() warning and a duplicate mark for close on the circuit. This commit also adds a unit test to make sure we never select marked for close circuits when upgrading its guard state from waiting for guard to open. Fixes #30871 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket30871 | 6 ++++++ src/feature/client/entrynodes.c | 4 ++++ src/test/test_circuitbuild.c | 47 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/changes/ticket30871 b/changes/ticket30871 new file mode 100644 index 000000000..81c076bb0 --- /dev/null +++ b/changes/ticket30871 @@ -0,0 +1,6 @@ + o Major bugfixes (circuit build, guard): + - When considering upgrading circuits from "waiting for guard" to "open", + always ignore the ones that are mark for close. Else, we can end up in + the situation where a subsystem is notified of that circuit opening but + still marked for close leading to undesirable behavior. Fixes bug 30871; + bugfix on 0.3.0.1-alpha. diff --git a/src/feature/client/entrynodes.c b/src/feature/client/entrynodes.c index 4afcee202..54a9238d8 100644 --- a/src/feature/client/entrynodes.c +++ b/src/feature/client/entrynodes.c @@ -2611,6 +2611,10 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, entry_guard_t *guard = entry_guard_handle_get(state->guard); if (!guard || guard->in_selection != gs) continue; + if (TO_CIRCUIT(circ)->marked_for_close) { + /* Don't consider any marked for close circuits. */ + continue; + } smartlist_add(all_circuits, circ); } SMARTLIST_FOREACH_END(circ); diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 47218a559..0c2309159 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -4,6 +4,8 @@ /* See LICENSE for licensing information */ #define CIRCUITBUILD_PRIVATE +#define CIRCUITLIST_PRIVATE +#define ENTRYNODES_PRIVATE #include "core/or/or.h" #include "test/test.h" @@ -13,7 +15,11 @@ #include "core/or/circuitbuild.h" #include "core/or/circuitlist.h" +#include "core/or/cpath_build_state_st.h" #include "core/or/extend_info_st.h" +#include "core/or/origin_circuit_st.h" + +#include "feature/client/entrynodes.h" /* Dummy nodes smartlist for testing */ static smartlist_t dummy_nodes; @@ -126,10 +132,51 @@ test_new_route_len_unhandled_exit(void *arg) UNMOCK(count_acceptable_nodes); } +static void +test_upgrade_from_guard_wait(void *arg) +{ + circuit_t *circ = NULL; + origin_circuit_t *orig_circ = NULL; + entry_guard_t *guard = NULL; + smartlist_t *list = NULL; + + (void) arg; + + circ = dummy_origin_circuit_new(0); + orig_circ = TO_ORIGIN_CIRCUIT(circ); + tt_assert(orig_circ); + + orig_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t)); + + circuit_set_state(circ, CIRCUIT_STATE_GUARD_WAIT); + + /* Put it in guard wait state. */ + guard = tor_malloc_zero(sizeof(*guard)); + guard->in_selection = get_guard_selection_info(); + + orig_circ->guard_state = + circuit_guard_state_new(guard, GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD, + NULL); + + /* Mark the circuit for close. */ + circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL); + tt_int_op(circ->marked_for_close, OP_NE, 0); + + /* We shouldn't pick the mark for close circuit. */ + list = circuit_find_circuits_to_upgrade_from_guard_wait(); + tt_assert(!list); + + done: + circuit_free(circ); + entry_guard_free_(guard); +} + struct testcase_t circuitbuild_tests[] = { { "noexit", test_new_route_len_noexit, 0, NULL, NULL }, { "safe_exit", test_new_route_len_safe_exit, 0, NULL, NULL }, { "unsafe_exit", test_new_route_len_unsafe_exit, 0, NULL, NULL }, { "unhandled_exit", test_new_route_len_unhandled_exit, 0, NULL, NULL }, + { "upgrade_from_guard_wait", test_upgrade_from_guard_wait, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/maint-0.4.1] Merge remote-tracking branch 'tor-github/pr/1118' into maint-0.4.1
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 648d5df628295f6bde4c94b5fb5302b982ef031f Merge: 00acccac6 05b6f73f1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jun 26 09:49:40 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1118' into maint-0.4.1 changes/ticket30871 | 6 ++++ scripts/maint/practracker/exceptions.txt | 4 +-- src/feature/client/entrynodes.c | 4 +++ src/test/test_circuitbuild.c | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-)

1 0

[tor/maint-0.4.1] Make the great grand practracker happy...
by nickm＠torproject.org 26 Jun '19

26 Jun '19

commit 05b6f73f12374e40e38bb1b35f3df593aa287610 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jun 18 13:33:10 2019 -0400 Make the great grand practracker happy... Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- scripts/maint/practracker/exceptions.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index db9bf34e2..9073f2f1d 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -135,8 +135,8 @@ problem function-size /src/feature/client/addressmap.c:addressmap_rewrite() 112 problem function-size /src/feature/client/bridges.c:rewrite_node_address_for_bridge() 126 problem function-size /src/feature/client/circpathbias.c:pathbias_measure_close_rate() 108 problem function-size /src/feature/client/dnsserv.c:evdns_server_callback() 153 -problem file-size /src/feature/client/entrynodes.c 3820 -problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 153 +problem file-size /src/feature/client/entrynodes.c 3824 +problem function-size /src/feature/client/entrynodes.c:entry_guards_upgrade_waiting_circuits() 157 problem function-size /src/feature/client/entrynodes.c:entry_guard_parse_from_state() 246 problem function-size /src/feature/client/transports.c:handle_proxy_line() 108 problem function-size /src/feature/client/transports.c:parse_method_line_helper() 112

1 0