nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-11-10 13:13:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

guard: Ignore marked for close circuit when changing state to open

Browse Source 
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@torproject.org>
This commit is contained in:
David Goulet 2019-06-18 13:32:45 -04:00
parent 125a6300da
commit 16a0b7ed67
3 changed files with 57 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
changes/ticket30871 Normal file
View File

@ -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.

4
src/feature/client/entrynodes.c
View File

@ -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);

47
src/test/test_circuitbuild.c
View File

@ -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,
&helper_pubsub_setup, NULL },
END_OF_TESTCASES
};