From 818b44cc7c22696a3206229e4e15eeb9f130e2c0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 8 Dec 2016 12:10:06 -0500 Subject: [PATCH 1/3] Repair the (deprecated, ugly) DROPGUARDS controller function. This actually is much easier to write now that guard_selection_t is first-class. --- src/or/control.c | 13 ++++++++----- src/or/entrynodes.c | 36 ++++++++++++++++++++++++++++++++++++ src/or/entrynodes.h | 6 +++++- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/or/control.c b/src/or/control.c index 364d75c976..857b7325ae 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -4064,17 +4064,20 @@ handle_control_dropguards(control_connection_t *conn, smartlist_split_string(args, body, " ", SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); -#ifdef ENABLE_LEGACY_GUARD_ALGORITHM + static int have_warned = 0; + if (! have_warned) { + log_warn(LD_CONTROL, "DROPGUARDS is dangerous; make sure you understand " + "the risks before using it. It may be removed in a future " + "version of Tor."); + have_warned = 1; + } + if (smartlist_len(args)) { connection_printf_to_buf(conn, "512 Too many arguments to DROPGUARDS\r\n"); } else { remove_all_entry_guards(); send_control_done(conn); } -#else - // XXXX - connection_printf_to_buf(conn, "512 not supported\r\n"); -#endif SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); smartlist_free(args); diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index a4021ccc1a..63b54f41f0 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -3252,6 +3252,42 @@ guards_choose_guard(cpath_build_state_t *state, return r; } +/** Remove all currently listed entry guards for a given guard selection + * context. This frees and replaces gs, so don't use gs + * after calling this function. */ +void +remove_all_entry_guards_for_guard_selection(guard_selection_t *gs) +{ + // This function shouldn't exist. XXXX + tor_assert(gs != NULL); + char *old_name = tor_strdup(gs->name); + guard_selection_type_t old_type = gs->type; + + SMARTLIST_FOREACH(gs->sampled_entry_guards, entry_guard_t *, entry, { + control_event_guard(entry->nickname, entry->identity, "DROPPED"); + }); + + if (gs == curr_guard_context) { + curr_guard_context = NULL; + } + + smartlist_remove(guard_contexts, gs); + guard_selection_free(gs); + + gs = get_guard_selection_by_name(old_name, old_type, 1); + entry_guards_changed_for_guard_selection(gs); + tor_free(old_name); +} + +/** Remove all currently listed entry guards. So new ones will be chosen. */ +void +remove_all_entry_guards(void) +{ + // XXXX prop271 this function shouldn't exist, in the new order. + // This function shouldn't exist. + remove_all_entry_guards_for_guard_selection(get_guard_selection_info()); +} + /** Helper: pick a directory guard, with whatever algorithm is used. */ const node_t * guards_choose_dirguard(circuit_guard_state_t **guard_state_out) diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 6bbdff1e66..bd501d001f 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -170,7 +170,8 @@ struct entry_guard_t { * we saw them in the state, even if we don't understand them. */ char *extra_state_fields; - /** Backpointer to the guard selection that this guard belongs to. */ + /** Backpointer to the guard selection that this guard belongs to. + * The entry_guard_t must never outlive its guard_selection. */ guard_selection_t *in_selection; /**@}*/ @@ -548,6 +549,9 @@ STATIC int entry_guard_has_higher_priority(entry_guard_t *a, entry_guard_t *b); STATIC char *getinfo_helper_format_single_entry_guard(const entry_guard_t *e); #endif +void remove_all_entry_guards_for_guard_selection(guard_selection_t *gs); +void remove_all_entry_guards(void); + struct bridge_info_t; void entry_guard_learned_bridge_identity(const tor_addr_port_t *addrport, const uint8_t *rsa_id_digest); From 12efa1f1cc57ddbd64ab446d00ac6dae668110cc Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 2 Jan 2017 10:45:32 -0500 Subject: [PATCH 2/3] Add a unit test for dropguards --- src/test/test_entrynodes.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 031177e469..bcb56e3a6b 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -2194,6 +2194,35 @@ test_entry_guard_select_and_cancel(void *arg) circuit_guard_state_free(guard); } +static void +test_entry_guard_drop_guards(void *arg) +{ + (void) arg; + int r; + const node_t *node = NULL; + circuit_guard_state_t *guard; + guard_selection_t *gs = get_guard_selection_info(); + + // Pick a guard, to get things set up. + r = entry_guard_pick_for_circuit(gs, GUARD_USAGE_TRAFFIC, NULL, + &node, &guard); + tt_int_op(r, OP_EQ, 0); + tt_int_op(smartlist_len(gs->sampled_entry_guards), OP_GE, + DFLT_MIN_FILTERED_SAMPLE_SIZE); + tt_ptr_op(gs, OP_EQ, get_guard_selection_info()); + + // Drop all the guards! (This is a bad idea....) + remove_all_entry_guards_for_guard_selection(gs); + gs = get_guard_selection_info(); + tt_int_op(smartlist_len(gs->sampled_entry_guards), OP_EQ, 0); + tt_int_op(smartlist_len(gs->primary_entry_guards), OP_EQ, 0); + tt_int_op(smartlist_len(gs->confirmed_entry_guards), OP_EQ, 0); + + done: + circuit_guard_state_free(guard); + guard_selection_free(gs); +} + /* Unit test setup function: Create a fake network, and set everything up * for testing the upgrade-a-waiting-circuit code. */ typedef struct { @@ -2667,6 +2696,7 @@ struct testcase_t entrynodes_tests[] = { BFN_TEST(select_for_circuit_highlevel_confirm_other), BFN_TEST(select_for_circuit_highlevel_primary_retry), BFN_TEST(select_and_cancel), + BFN_TEST(drop_guards), UPGRADE_TEST(upgrade_a_circuit, "c1-done c2-done"), UPGRADE_TEST(upgrade_blocked_by_live_primary_guards, "c1-done c2-done"), From 33dcd0c44b2721d22239ec6664de7b54f16861d3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 24 Jan 2017 09:19:44 -0500 Subject: [PATCH 3/3] changes file for DROPGUARDS --- changes/bug20824 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/bug20824 diff --git a/changes/bug20824 b/changes/bug20824 new file mode 100644 index 0000000000..0cd52054ed --- /dev/null +++ b/changes/bug20824 @@ -0,0 +1,3 @@ + o Minor bugfixes (controller): + - Restore the (deprecated) DROPGUARDS controller command. + Fixes bug 20824; bugfix on 0.3.0.1-alpha.