[tor-commits] [tor/master] Make new prop271 entry guards persistent

commit 858c8f5593e573cdf36c360141cf6e96d91d6474 Author: Nick Mathewson nickm@torproject.org Date: Tue Nov 22 14:22:54 2016 -0500

Make new prop271 entry guards persistent

To do this, it makes sense to treat legacy guards as a separate guard_selection_t *, and handle them separately. This also means we add support here for having multiple guard selections.

Note that we don't persist pathbias information yet; that will take some refactoring. --- src/or/entrynodes.c | 186 ++++++++++++++++++++++++++++++++++++++++----- src/or/entrynodes.h | 14 +++- src/or/or.h | 5 +- src/or/statefile.c | 2 + src/test/test_entrynodes.c | 50 ++++++++---- 5 files changed, 218 insertions(+), 39 deletions(-)

diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index eca88a9..4e32154 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -179,14 +179,16 @@ should_apply_guardfraction(const networkstatus_t *ns) return options->UseGuardFraction; }

-/** Allocate and return a new guard_selection_t */ - +/** + * Allocate and return a new guard_selection_t, with the name <b>name</b>. + */ STATIC guard_selection_t * -guard_selection_new(void) +guard_selection_new(const char *name) { guard_selection_t *gs;

gs = tor_malloc_zero(sizeof(*gs)); + gs->name = tor_strdup(name); gs->chosen_entry_guards = smartlist_new(); gs->sampled_entry_guards = smartlist_new(); gs->confirmed_entry_guards = smartlist_new(); @@ -195,6 +197,37 @@ guard_selection_new(void) return gs; }

+/** + * Return the guard selection called <b>name</b>. If there is none, and + * <b>create_if_absent</b> is true, then create and return it. If there + * is none, and <b>create_if_absent</b> is false, then return NULL. + */ +static guard_selection_t * +get_guard_selection_by_name(const char *name, int create_if_absent) +{ + if (!guard_contexts) { + guard_contexts = smartlist_new(); + } + SMARTLIST_FOREACH_BEGIN(guard_contexts, guard_selection_t *, gs) { + if (!strcmp(gs->name, name)) + return gs; + } SMARTLIST_FOREACH_END(gs); + + if (! create_if_absent) + return NULL; + + guard_selection_t *new_selection = guard_selection_new(name); + smartlist_add(guard_contexts, new_selection); + + const char *default_name = get_options()->UseDeprecatedGuardAlgorithm ? + "legacy" : "default"; + + if (!strcmp(name, default_name)) + curr_guard_context = new_selection; + + return new_selection; +} + /** Get current default guard_selection_t, creating it if necessary */ guard_selection_t * get_guard_selection_info(void) @@ -204,7 +237,9 @@ get_guard_selection_info(void) }

if (!curr_guard_context) { - curr_guard_context = guard_selection_new(); + const char *name = get_options()->UseDeprecatedGuardAlgorithm ? + "legacy" : "default"; + curr_guard_context = guard_selection_new(name); smartlist_add(guard_contexts, curr_guard_context); }

@@ -355,6 +390,7 @@ entry_guard_add_to_sample(guard_selection_t *gs, entry_guard_t *guard = tor_malloc_zero(sizeof(entry_guard_t));

/* persistent fields */ + guard->selection_name = tor_strdup(gs->name); memcpy(guard->identity, node->identity, DIGEST_LEN); strlcpy(guard->nickname, node_get_nickname(node), sizeof(guard->nickname)); guard->sampled_on_date = randomize_time(approx_time(), GUARD_LIFETIME/10); @@ -691,8 +727,9 @@ entry_guard_passes_filter(const or_options_t *options, guard_selection_t *gs, return 0;

const node_t *node = node_get_by_id(guard->identity); - if (BUG(node == NULL)) { - // should be impossible, since currently_listed was true. + if (node == NULL) { + // This can happen when currently_listed is true, and we're not updating + // it because we don't have a live consensus. return 0; }

@@ -1627,6 +1664,7 @@ entry_guard_encode_for_state(entry_guard_t *guard)

tor_assert(guard);

+ smartlist_add_asprintf(result, "in=%s", guard->selection_name); smartlist_add_asprintf(result, "rsa_id=%s", hex_str(guard->identity, DIGEST_LEN)); if (strlen(guard->nickname)) { @@ -1678,6 +1716,7 @@ entry_guard_parse_from_state(const char *s) smartlist_t *extra = smartlist_new();

/* These fields get parsed from the string. */ + char *in = NULL; char *rsa_id = NULL; char *nickname = NULL; char *sampled_on = NULL; @@ -1693,6 +1732,7 @@ entry_guard_parse_from_state(const char *s) smartlist_t *entries = smartlist_new();

strmap_t *vals = strmap_new(); // Maps keyword to location + strmap_set(vals, "in", &in); strmap_set(vals, "rsa_id", &rsa_id); strmap_set(vals, "nickname", &nickname); strmap_set(vals, "sampled_on", &sampled_on); @@ -1731,6 +1771,14 @@ entry_guard_parse_from_state(const char *s)

entry_guard_t *guard = tor_malloc_zero(sizeof(entry_guard_t));

+ if (in == NULL) { + log_warn(LD_CIRC, "Guard missing 'in' field"); + goto err; + } + + guard->selection_name = in; + in = NULL; + if (rsa_id == NULL) { log_warn(LD_CIRC, "Guard missing RSA ID field"); goto err; @@ -1825,6 +1873,7 @@ entry_guard_parse_from_state(const char *s) guard = NULL;

done: + tor_free(in); tor_free(rsa_id); tor_free(nickname); tor_free(sampled_on); @@ -1839,16 +1888,95 @@ entry_guard_parse_from_state(const char *s) return guard; }

-/* XXXXprop271 This is a dummy function added for now so that all of the - * new guard code will be counted as reachable. It should get removed. +/** + * Replace the Guards entries in <b>state</b> with a list of all our + * non-legacy sampled guards. */ -__attribute__((noreturn)) void -entry_guards_DUMMY_ENTRY_POINT(void) +static void +entry_guards_update_guards_in_state(or_state_t *state) { - // prop271 XXXXX kludge; remove this - entry_guard_encode_for_state(NULL); - entry_guard_parse_from_state(NULL); - tor_assert(0); + if (!guard_contexts) + return; + config_line_t *lines = NULL; + config_line_t **nextline = &lines; + + SMARTLIST_FOREACH_BEGIN(guard_contexts, guard_selection_t *, gs) { + if (!strcmp(gs->name, "legacy")) + continue; /* This is encoded differently. */ + SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) { + *nextline = tor_malloc_zero(sizeof(config_line_t)); + (*nextline)->key = tor_strdup("Guard"); + (*nextline)->value = entry_guard_encode_for_state(guard); + nextline = &(*nextline)->next; + } SMARTLIST_FOREACH_END(guard); + gs->dirty = 0; + } SMARTLIST_FOREACH_END(gs); + + /* XXXXX prop271 circuitpathbias */ + + config_free_lines(state->Guard); + state->Guard = lines; +} + +/** + * Replace our non-legacy sampled guards from the Guards entries in + * <b>state</b>. Return 0 on success, -1 on failure. (If <b>set</b> is + * true, replace nothing -- only check whether replacing would work.) + */ +static int +entry_guards_load_guards_from_state(or_state_t *state, int set) +{ + /* XXXXX prop271 circuitpathbias */ + const config_line_t *line = state->Guard; + int n_errors = 0; + + if (!guard_contexts) + guard_contexts = smartlist_new(); + + /* Wipe all our existing guard info. (we shouldn't have any, but + * let's be safe.) */ + if (set) { + SMARTLIST_FOREACH_BEGIN(guard_contexts, guard_selection_t *, gs) { + if (!strcmp(gs->name, "legacy")) + continue; + guard_selection_free(gs); + if (curr_guard_context == gs) + curr_guard_context = NULL; + SMARTLIST_DEL_CURRENT(guard_contexts, gs); + } SMARTLIST_FOREACH_END(gs); + } + + for ( ; line != NULL; line = line->next) { + entry_guard_t *guard = entry_guard_parse_from_state(line->value); + if (guard == NULL) { + ++n_errors; + continue; + } + tor_assert(guard->selection_name); + if (!strcmp(guard->selection_name, "legacy")) { + ++n_errors; + entry_guard_free(guard); + continue; + } + + if (set) { + guard_selection_t *gs; + gs = get_guard_selection_by_name(guard->selection_name, 1); + tor_assert(gs); + smartlist_add(gs->sampled_entry_guards, guard); + } else { + entry_guard_free(guard); + } + } + + if (set) { + SMARTLIST_FOREACH_BEGIN(guard_contexts, guard_selection_t *, gs) { + if (!strcmp(gs->name, "legacy")) + continue; + entry_guards_update_all(gs); + } SMARTLIST_FOREACH_END(gs); + } + return n_errors ? -1 : 0; }

/* XXXXX ----------------------------------------------- */ @@ -2403,6 +2531,7 @@ entry_guard_free(entry_guard_t *e) tor_free(e->chosen_by_version); tor_free(e->sampled_by_version); tor_free(e->extra_state_fields); + tor_free(e->selection_name); tor_free(e); }

@@ -3436,9 +3565,19 @@ entry_guards_parse_state_for_guard_selection( int entry_guards_parse_state(or_state_t *state, int set, char **msg) { - return entry_guards_parse_state_for_guard_selection( - get_guard_selection_info(), + int r1 = entry_guards_load_guards_from_state(state, set); + + int r2 = entry_guards_parse_state_for_guard_selection( + get_guard_selection_by_name("legacy", 1), state, set, msg); + + if (r1 < 0 || r2 < 0) { + if (msg && *msg == NULL) { + *msg = tor_strdup("parsing error"); //xxxx prop271 should we try harder? + } + return -1; + } + return 0; }

/** How long will we let a change in our guard nodes stay un-saved @@ -3466,7 +3605,9 @@ entry_guards_changed_for_guard_selection(guard_selection_t *gs) else when = time(NULL) + FAST_GUARD_STATE_FLUSH_TIME;

- /* or_state_save() will call entry_guards_update_state(). */ + /* or_state_save() will call entry_guards_update_state() and + entry_guards_update_guards_in_state() + */ or_state_mark_dirty(get_or_state(), when); }

@@ -3494,11 +3635,14 @@ void entry_guards_update_state(or_state_t *state) { config_line_t **next, *line; - guard_selection_t *gs = get_guard_selection_info();

- tor_assert(gs != NULL); - tor_assert(gs->chosen_entry_guards != NULL); + // Handles all non-legacy guard info. + entry_guards_update_guards_in_state(state);

+ guard_selection_t *gs = get_guard_selection_by_name("legacy", 0); + if (!gs) + return; // nothign to save. + tor_assert(gs->chosen_entry_guards != NULL); if (!gs->dirty) return;

@@ -3835,6 +3979,8 @@ guard_selection_free(guard_selection_t *gs) { if (!gs) return;

+ tor_free(gs->name); + if (gs->chosen_entry_guards) { SMARTLIST_FOREACH(gs->chosen_entry_guards, entry_guard_t *, e, entry_guard_free(e)); diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 7dcedd6..d8468eb 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -121,6 +121,11 @@ struct entry_guard_t { * item should occur in the CONFIRMED_GUARDS ordered * list */

+ /** + * Which selection does this guard belong to? + */ + char *selection_name; + /* ==== Non-persistent fields. */ /* == These are used by sampled guards */ /** When did we last decide to try using this guard for a circuit? 0 for @@ -203,6 +208,11 @@ struct entry_guard_t { */ struct guard_selection_s { /** + * The name for this guard-selection object. (Must not contain spaces). + */ + char *name; + + /** * A value of 1 means that guard_selection_t structures have changed * and those changes need to be flushed to disk. * @@ -398,7 +408,7 @@ int num_bridges_usable(void);

// ---------- XXXX these functions and definitions are post-prop271. HANDLE_DECL(entry_guard, entry_guard_t, STATIC) -STATIC guard_selection_t *guard_selection_new(void); +STATIC guard_selection_t *guard_selection_new(const char *name); STATIC void guard_selection_free(guard_selection_t *gs); STATIC entry_guard_t *get_sampled_guard_with_id(guard_selection_t *gs, const uint8_t *rsa_id); @@ -459,8 +469,6 @@ STATIC unsigned entry_guards_note_guard_success(guard_selection_t *gs, unsigned old_state); STATIC int entry_guard_has_higher_priority(entry_guard_t *a, entry_guard_t *b);

-void entry_guards_DUMMY_ENTRY_POINT(void); - // ---------- XXXX this stuff is pre-prop271.

STATIC const node_t *add_an_entry_guard(guard_selection_t *gs, diff --git a/src/or/or.h b/src/or/or.h index 8b9ede3..04ff548 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4618,9 +4618,12 @@ typedef struct { uint64_t AccountingBytesAtSoftLimit; uint64_t AccountingExpectedUsage;

- /** A list of Entry Guard-related configuration lines. */ + /** A list of Entry Guard-related configuration lines. (pre-prop271) */ config_line_t *EntryGuards;

+ /** A list of guard-related configuration lines. (post-prop271) */ + config_line_t *Guard; + config_line_t *TransportProxies;

/** These fields hold information on the history of bandwidth usage for diff --git a/src/or/statefile.c b/src/or/statefile.c index 8fa4324..a95ba85 100644 --- a/src/or/statefile.c +++ b/src/or/statefile.c @@ -102,6 +102,8 @@ static config_var_t state_vars_[] = { V(BWHistoryDirWriteValues, CSV, ""), V(BWHistoryDirWriteMaxima, CSV, ""),

+ V(Guard, LINELIST, NULL), + V(TorVersion, STRING, NULL),

V(LastRotatedOnionKey, ISOTIME, NULL), diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 6511859..d8b9ccb 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -528,6 +528,8 @@ state_lines_free(smartlist_t *entry_guard_lines) static void test_entry_guards_parse_state_simple(void *arg) { + or_options_t *options = get_options_mutable(); + options->UseDeprecatedGuardAlgorithm = 1; or_state_t *state = or_state_new(); const smartlist_t *all_entry_guards = get_entry_guards(); smartlist_t *entry_state_lines = smartlist_new(); @@ -622,6 +624,8 @@ test_entry_guards_parse_state_simple(void *arg) static void test_entry_guards_parse_state_pathbias(void *arg) { + or_options_t *options = get_options_mutable(); + options->UseDeprecatedGuardAlgorithm = 1; or_state_t *state = or_state_new(); const smartlist_t *all_entry_guards = get_entry_guards(); char *msg = NULL; @@ -1021,6 +1025,7 @@ test_entry_guard_encode_for_state_minimal(void *arg) (void) arg; entry_guard_t *eg = tor_malloc_zero(sizeof(entry_guard_t));

+ eg->selection_name = tor_strdup("wubwub"); memcpy(eg->identity, "plurpyflurpyslurpydo", DIGEST_LEN); eg->sampled_on_date = 1479081600; eg->confirmed_idx = -1; @@ -1029,6 +1034,7 @@ test_entry_guard_encode_for_state_minimal(void *arg) s = entry_guard_encode_for_state(eg);

tt_str_op(s, OP_EQ, + "in=wubwub " "rsa_id=706C75727079666C75727079736C75727079646F " "sampled_on=2016-11-14T00:00:00 " "listed=0"); @@ -1045,6 +1051,7 @@ test_entry_guard_encode_for_state_maximal(void *arg) entry_guard_t *eg = tor_malloc_zero(sizeof(entry_guard_t));

strlcpy(eg->nickname, "Fred", sizeof(eg->nickname)); + eg->selection_name = tor_strdup("default"); memcpy(eg->identity, "plurpyflurpyslurpydo", DIGEST_LEN); eg->sampled_on_date = 1479081600; eg->sampled_by_version = tor_strdup("1.2.3"); @@ -1058,6 +1065,7 @@ test_entry_guard_encode_for_state_maximal(void *arg) s = entry_guard_encode_for_state(eg);

tt_str_op(s, OP_EQ, + "in=default " "rsa_id=706C75727079666C75727079736C75727079646F " "nickname=Fred " "sampled_on=2016-11-14T00:00:00 " @@ -1082,9 +1090,11 @@ test_entry_guard_parse_from_state_minimal(void *arg) time_t t = approx_time();

eg = entry_guard_parse_from_state( + "in=default_plus " "rsa_id=596f75206d6179206e656564206120686f626279"); tt_assert(eg);

+ tt_str_op(eg->selection_name, OP_EQ, "default_plus"); test_mem_op_hex(eg->identity, OP_EQ, "596f75206d6179206e656564206120686f626279"); tt_str_op(eg->nickname, OP_EQ, "$596F75206D6179206E656564206120686F626279"); @@ -1112,6 +1122,7 @@ test_entry_guard_parse_from_state_maximal(void *arg) entry_guard_t *eg = NULL;

eg = entry_guard_parse_from_state( + "in=fred " "rsa_id=706C75727079666C75727079736C75727079646F " "nickname=Fred " "sampled_on=2016-11-14T00:00:00 " @@ -1150,22 +1161,30 @@ test_entry_guard_parse_from_state_failure(void *arg) (void)arg; entry_guard_t *eg = NULL;

+ /* no selection */ + eg = entry_guard_parse_from_state( + "rsa_id=596f75206d6179206e656564206120686f626270"); + tt_assert(! eg); + /* no RSA ID. */ - eg = entry_guard_parse_from_state("nickname=Fred"); + eg = entry_guard_parse_from_state("in=default nickname=Fred"); tt_assert(! eg);

/* Bad RSA ID: bad character. */ eg = entry_guard_parse_from_state( + "in=default " "rsa_id=596f75206d6179206e656564206120686f62627q"); tt_assert(! eg);

/* Bad RSA ID: too long.*/ eg = entry_guard_parse_from_state( + "in=default " "rsa_id=596f75206d6179206e656564206120686f6262703"); tt_assert(! eg);

/* Bad RSA ID: too short.*/ eg = entry_guard_parse_from_state( + "in=default " "rsa_id=596f75206d6179206e65656420612"); tt_assert(! eg);

@@ -1182,6 +1201,7 @@ test_entry_guard_parse_from_state_partial_failure(void *arg) time_t t = approx_time();

eg = entry_guard_parse_from_state( + "in=default " "rsa_id=706C75727079666C75727079736C75727079646F " "nickname=FredIsANodeWithAStrangeNicknameThatIsTooLong " "sampled_on=2016-11-14T00:00:99 " @@ -1219,7 +1239,7 @@ static void test_entry_guard_add_single_guard(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default");

/* 1: Add a single guard to the sample. */ node_t *n1 = smartlist_get(big_fake_net_nodes, 0); @@ -1259,7 +1279,7 @@ static void test_entry_guard_node_filter(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); bridge_line_t *bl = NULL;

/* Initialize a bunch of node objects that are all guards. */ @@ -1334,7 +1354,7 @@ static void test_entry_guard_expand_sample(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); digestmap_t *node_by_id = digestmap_new();

entry_guard_t *guard = entry_guards_expand_sample(gs); @@ -1428,7 +1448,7 @@ static void test_entry_guard_expand_sample_small_net(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default");

/* Fun corner case: not enough guards to make up our whole sample size. */ SMARTLIST_FOREACH(big_fake_net_nodes, node_t *, n, { @@ -1461,7 +1481,7 @@ test_entry_guard_update_from_consensus_status(void *arg) (void)arg; int i; time_t start = approx_time(); - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); networkstatus_t *ns_tmp = NULL;

/* Don't randomly backdate stuff; it will make correctness harder to check.*/ @@ -1566,7 +1586,7 @@ test_entry_guard_update_from_consensus_repair(void *arg) (void)arg; int i; time_t start = approx_time(); - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default");

/* Don't randomly backdate stuff; it will make correctness harder to check.*/ MOCK(randomize_time, mock_randomize_time_no_randomization); @@ -1629,7 +1649,7 @@ test_entry_guard_update_from_consensus_remove(void *arg)

(void)arg; //int i; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); smartlist_t *keep_ids = smartlist_new(); smartlist_t *remove_ids = smartlist_new();

@@ -1727,7 +1747,7 @@ test_entry_guard_confirming_guards(void *arg) (void)arg; /* Now let's check the logic responsible for manipulating the list * of confirmed guards */ - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); MOCK(randomize_time, mock_randomize_time_no_randomization);

/* Create the sample. */ @@ -1797,7 +1817,7 @@ static void test_entry_guard_sample_reachable_filtered(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); entry_guards_expand_sample(gs); const int N = 10000; bitarray_t *selected = NULL; @@ -1867,7 +1887,7 @@ static void test_entry_guard_sample_reachable_filtered_empty(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); /* What if we try to sample from a set of 0? */ SMARTLIST_FOREACH(big_fake_net_nodes, node_t *, n, n->is_possible_guard = 0); @@ -1883,7 +1903,7 @@ static void test_entry_guard_retry_unreachable(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default");

entry_guards_expand_sample(gs); /* Let's say that we have two guards, and they're down. @@ -1942,7 +1962,7 @@ static void test_entry_guard_manage_primary(void *arg) { (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); smartlist_t *prev_guards = smartlist_new();

/* If no guards are confirmed, we should pick a few reachable guards and @@ -2015,7 +2035,7 @@ test_entry_guard_select_for_circuit_no_confirmed(void *arg) { /* Simpler cases: no gaurds are confirmed yet. */ (void)arg; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default");

/* simple starting configuration */ entry_guards_update_primary(gs); @@ -2099,7 +2119,7 @@ test_entry_guard_select_for_circuit_confirmed(void *arg) guards, we use a confirmed guard. */ (void)arg; int i; - guard_selection_t *gs = guard_selection_new(); + guard_selection_t *gs = guard_selection_new("default"); const int N_CONFIRMED = 10;

/* slightly more complicated simple starting configuration */