December 2016 - tor-commits - lists.torproject.org

[tor/master] Fix pathbias interactions with entry guards
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit d98b9b6d65946e14ee325327d5beac1a60ace6cc Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 22 15:30:12 2016 -0500 Fix pathbias interactions with entry guards entry_guard_get_by_id_digest() was always returning NULL, which was causing "adventure" and "fun" --- src/or/entrynodes.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index f1fc055..81751f5 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2351,6 +2351,11 @@ entry_guard_get_by_id_digest_for_guard_selection(guard_selection_t *gs, { tor_assert(gs != NULL); + SMARTLIST_FOREACH(gs->sampled_entry_guards, entry_guard_t *, entry, + if (tor_memeq(digest, entry->identity, DIGEST_LEN)) + return entry; + ); + SMARTLIST_FOREACH(gs->chosen_entry_guards, entry_guard_t *, entry, if (tor_memeq(digest, entry->identity, DIGEST_LEN)) return entry;

1 0

[tor/master] Maintain a list of all the origin circuits.
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit de617a471442342fc2abafdde4e250fd31eb45ac Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 22 09:05:52 2016 -0500 Maintain a list of all the origin circuits. We'll want this for upgrading waiting circuits. --- src/or/circuitlist.c | 39 +++++++++++++++++++++++++++++++++++++++ src/or/or.h | 4 ++++ 2 files changed, 43 insertions(+) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 0189412..c274534 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -85,6 +85,10 @@ /** A global list of all circuits at this hop. */ static smartlist_t *global_circuitlist = NULL; +/** A global list of all origin circuits. Every element of this is also + * an element of global_circuitlist. */ +static smartlist_t *global_origin_circuit_list = NULL; + /** A list of all the circuits in CIRCUIT_STATE_CHAN_WAIT. */ static smartlist_t *circuits_pending_chans = NULL; @@ -523,6 +527,19 @@ circuit_close_all_marked(void) } circ->global_circuitlist_idx = -1; + /* Remove it from the origin circuit list, if appropriate. */ + if (CIRCUIT_IS_ORIGIN(circ)) { + origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ); + int origin_idx = origin_circ->global_origin_circuit_list_idx; + smartlist_del(global_origin_circuit_list, origin_idx); + if (origin_idx < smartlist_len(global_origin_circuit_list)) { + origin_circuit_t *replacement = + smartlist_get(global_origin_circuit_list, origin_idx); + replacement->global_origin_circuit_list_idx = origin_idx; + } + origin_circ->global_origin_circuit_list_idx = -1; + } + circuit_about_to_free(circ); circuit_free(circ); } SMARTLIST_FOREACH_END(circ); @@ -780,6 +797,13 @@ origin_circuit_new(void) init_circuit_base(TO_CIRCUIT(circ)); + /* Add to origin-list. */ + if (!global_origin_circuit_list) + global_origin_circuit_list = smartlist_new(); + smartlist_add(global_origin_circuit_list, circ); + circ->global_origin_circuit_list_idx = + smartlist_len(global_origin_circuit_list) - 1; + circuit_build_times_update_last_circ(get_circuit_build_times_mutable()); return circ; @@ -837,6 +861,18 @@ circuit_free(circuit_t *circ) mem = ocirc; memlen = sizeof(origin_circuit_t); tor_assert(circ->magic == ORIGIN_CIRCUIT_MAGIC); + + if (ocirc->global_origin_circuit_list_idx != -1) { + int idx = ocirc->global_origin_circuit_list_idx; + origin_circuit_t *c2 = smartlist_get(global_origin_circuit_list, idx); + tor_assert(c2 == ocirc); + smartlist_del(global_origin_circuit_list, idx); + if (idx < smartlist_len(global_origin_circuit_list)) { + c2 = smartlist_get(global_origin_circuit_list, idx); + c2->global_origin_circuit_list_idx = idx; + } + } + if (ocirc->build_state) { extend_info_free(ocirc->build_state->chosen_exit); circuit_free_cpath_node(ocirc->build_state->pending_final_cpath); @@ -977,6 +1013,9 @@ circuit_free_all(void) smartlist_free(lst); global_circuitlist = NULL; + smartlist_free(global_origin_circuit_list); + global_origin_circuit_list = NULL; + smartlist_free(circuits_pending_chans); circuits_pending_chans = NULL; diff --git a/src/or/or.h b/src/or/or.h index 8282731..c8f39f9 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3157,6 +3157,10 @@ typedef struct origin_circuit_t { * whether this circuit can be used. */ struct circuit_guard_state_t *guard_state; + /** Index into global_origin_circuit_list for this circuit. -1 if not + * present. */ + int global_origin_circuit_list_idx; + /** How many more relay_early cells can we send on this circuit, according * to the specification? */ unsigned int remaining_relay_early_cells : 4;

1 0

[tor/master] Make pathbias fields persistent for new guards
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit 783fa2f58637f896d5476d907aa460cae067e51a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 22 15:12:31 2016 -0500 Make pathbias fields persistent for new guards --- src/or/entrynodes.c | 173 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 133 insertions(+), 40 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 4e32154..f1fc055 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -156,6 +156,8 @@ static const node_t *choose_random_entry_impl(guard_selection_t *gs, static void entry_guard_set_filtered_flags(const or_options_t *options, guard_selection_t *gs, entry_guard_t *guard); +static void pathbias_check_use_success_count(entry_guard_t *guard); +static void pathbias_check_close_success_count(entry_guard_t *guard); /** Return 0 if we should apply guardfraction information found in the * consensus. A specific consensus can be specified with the @@ -1694,6 +1696,30 @@ entry_guard_encode_for_state(entry_guard_t *guard) smartlist_add_asprintf(result, "confirmed_idx=%d", guard->confirmed_idx); } + const double EPSILON = 1.0e-6; + + /* Make a copy of the pathbias object, since we will want to update + some of them */ + guard_pathbias_t *pb = tor_memdup(&guard->pb, sizeof(*pb)); + pb->use_successes = pathbias_get_use_success_count(guard); + pb->successful_circuits_closed = pathbias_get_close_success_count(guard); + + #define PB_FIELD(field) do { \ + if (pb->field >= EPSILON) { \ + smartlist_add_asprintf(result, "pb_" #field "=%f", pb->field); \ + } \ + } while (0) + PB_FIELD(use_attempts); + PB_FIELD(use_successes); + PB_FIELD(circ_attempts); + PB_FIELD(circ_successes); + PB_FIELD(successful_circuits_closed); + PB_FIELD(collapsed_circuits); + PB_FIELD(unusable_circuits); + PB_FIELD(timeouts); + tor_free(pb); +#undef PB_FIELD + if (guard->extra_state_fields) smartlist_add_strdup(result, guard->extra_state_fields); @@ -1726,21 +1752,42 @@ entry_guard_parse_from_state(const char *s) char *confirmed_on = NULL; char *confirmed_idx = NULL; + // pathbias + char *pb_use_attempts = NULL; + char *pb_use_successes = NULL; + char *pb_circ_attempts = NULL; + char *pb_circ_successes = NULL; + char *pb_successful_circuits_closed = NULL; + char *pb_collapsed_circuits = NULL; + char *pb_unusable_circuits = NULL; + char *pb_timeouts = NULL; + /* Split up the entries. Put the ones we know about in strings and the * rest in "extra". */ { 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); - strmap_set(vals, "sampled_by", &sampled_by); - strmap_set(vals, "unlisted_since", &unlisted_since); - strmap_set(vals, "listed", &listed); - strmap_set(vals, "confirmed_on", &confirmed_on); - strmap_set(vals, "confirmed_idx", &confirmed_idx); +#define FIELD(f) \ + strmap_set(vals, #f, &f); + FIELD(in); + FIELD(rsa_id); + FIELD(nickname); + FIELD(sampled_on); + FIELD(sampled_by); + FIELD(unlisted_since); + FIELD(listed); + FIELD(confirmed_on); + FIELD(confirmed_idx); + FIELD(pb_use_attempts); + FIELD(pb_use_successes); + FIELD(pb_circ_attempts); + FIELD(pb_circ_successes); + FIELD(pb_successful_circuits_closed); + FIELD(pb_collapsed_circuits); + FIELD(pb_unusable_circuits); + FIELD(pb_timeouts); +#undef FIELD smartlist_split_string(entries, s, " ", SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); @@ -1848,7 +1895,7 @@ entry_guard_parse_from_state(const char *s) int ok=1; long idx = tor_parse_long(confirmed_idx, 10, 0, INT_MAX, &ok, NULL); if (! ok) { - log_warn(LD_CIRC, "Guard has invalid confirmed_idx %s", + log_warn(LD_GUARD, "Guard has invalid confirmed_idx %s", escaped(confirmed_idx)); } else { guard->confirmed_idx = (int)idx; @@ -1863,7 +1910,34 @@ entry_guard_parse_from_state(const char *s) /* initialize non-persistent fields */ guard->is_reachable = GUARD_REACHABLE_MAYBE; - /* XXXX prop271 Update everything on this guard. */ +#define PB_FIELD(field) \ + do { \ + if (pb_ ## field) { \ + int ok = 1; \ + double r = tor_parse_double(pb_ ## field, 0.0, 1e9, &ok, NULL); \ + if (! ok) { \ + log_warn(LD_CIRC, "Guard has invalid pb_%s %s", \ + #field, pb_ ## field); \ + } else { \ + guard->pb.field = r; \ + } \ + } \ + } while (0) + PB_FIELD(use_attempts); + PB_FIELD(use_successes); + PB_FIELD(circ_attempts); + PB_FIELD(circ_successes); + PB_FIELD(successful_circuits_closed); + PB_FIELD(collapsed_circuits); + PB_FIELD(unusable_circuits); + PB_FIELD(timeouts); +#undef PB_FIELD + + pathbias_check_use_success_count(guard); + pathbias_check_close_success_count(guard); + + /* We update everything on this guard later, after we've parsed + * everything. */ goto done; @@ -1882,6 +1956,15 @@ entry_guard_parse_from_state(const char *s) tor_free(listed); tor_free(confirmed_on); tor_free(confirmed_idx); + tor_free(pb_use_attempts); + tor_free(pb_use_successes); + tor_free(pb_circ_attempts); + tor_free(pb_circ_successes); + tor_free(pb_successful_circuits_closed); + tor_free(pb_collapsed_circuits); + tor_free(pb_unusable_circuits); + tor_free(pb_timeouts); + SMARTLIST_FOREACH(extra, char *, cp, tor_free(cp)); smartlist_free(extra); @@ -1912,8 +1995,6 @@ entry_guards_update_guards_in_state(or_state_t *state) gs->dirty = 0; } SMARTLIST_FOREACH_END(gs); - /* XXXXX prop271 circuitpathbias */ - config_free_lines(state->Guard); state->Guard = lines; } @@ -1926,7 +2007,6 @@ entry_guards_update_guards_in_state(or_state_t *state) 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; @@ -3282,6 +3362,42 @@ choose_random_entry_impl(guard_selection_t *gs, return node; } +static void +pathbias_check_use_success_count(entry_guard_t *node) +{ + const or_options_t *options = get_options(); + /* Note: We rely on the < comparison here to allow us to set a 0 + * rate and disable the feature entirely. If refactoring, don't + * change to <= */ + if (pathbias_get_use_success_count(node)/node->pb.use_attempts + < pathbias_get_extreme_use_rate(options) && + pathbias_get_dropguards(options)) { + node->pb.path_bias_disabled = 1; + log_info(LD_GENERAL, + "Path use bias is too high (%f/%f); disabling node %s", + node->pb.circ_successes, node->pb.circ_attempts, + node->nickname); + } +} + +static void +pathbias_check_close_success_count(entry_guard_t *node) +{ + const or_options_t *options = get_options(); + /* Note: We rely on the < comparison here to allow us to set a 0 + * rate and disable the feature entirely. If refactoring, don't + * change to <= */ + if (pathbias_get_close_success_count(node)/node->pb.circ_attempts + < pathbias_get_extreme_rate(options) && + pathbias_get_dropguards(options)) { + node->pb.path_bias_disabled = 1; + log_info(LD_GENERAL, + "Path bias is too high (%f/%f); disabling node %s", + node->pb.circ_successes, node->pb.circ_attempts, + node->nickname); + } +} + /** Parse state and learn about the entry guards it describes. * If set is true, and there are no errors, replace the guard * list in the provided guard selection context with what we find. @@ -3386,7 +3502,6 @@ entry_guards_parse_state_for_guard_selection( } digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1)); } else if (!strcasecmp(line->key, "EntryGuardPathUseBias")) { - const or_options_t *options = get_options(); double use_cnt, success_cnt; if (!node) { @@ -3423,20 +3538,9 @@ entry_guards_parse_state_for_guard_selection( log_info(LD_GENERAL, "Read %f/%f path use bias for node %s", node->pb.use_successes, node->pb.use_attempts, node->nickname); - /* Note: We rely on the < comparison here to allow us to set a 0 - * rate and disable the feature entirely. If refactoring, don't - * change to <= */ - if (pathbias_get_use_success_count(node)/node->pb.use_attempts - < pathbias_get_extreme_use_rate(options) && - pathbias_get_dropguards(options)) { - node->pb.path_bias_disabled = 1; - log_info(LD_GENERAL, - "Path use bias is too high (%f/%f); disabling node %s", - node->pb.circ_successes, node->pb.circ_attempts, - node->nickname); - } + pathbias_check_use_success_count(node); + } else if (!strcasecmp(line->key, "EntryGuardPathBias")) { - const or_options_t *options = get_options(); double hop_cnt, success_cnt, timeouts, collapsed, successful_closed, unusable; @@ -3494,19 +3598,8 @@ entry_guards_parse_state_for_guard_selection( log_info(LD_GENERAL, "Read %f/%f path bias for node %s", node->pb.circ_successes, node->pb.circ_attempts, node->nickname); - /* Note: We rely on the < comparison here to allow us to set a 0 - * rate and disable the feature entirely. If refactoring, don't - * change to <= */ - if (pathbias_get_close_success_count(node)/node->pb.circ_attempts - < pathbias_get_extreme_rate(options) && - pathbias_get_dropguards(options)) { - node->pb.path_bias_disabled = 1; - log_info(LD_GENERAL, - "Path bias is too high (%f/%f); disabling node %s", - node->pb.circ_successes, node->pb.circ_attempts, - node->nickname); - } + pathbias_check_close_success_count(node); } else { log_warn(LD_BUG, "Unexpected key %s", line->key); }

1 0

[tor/master] Make sure primary-guards are up-to-date when we inspect them.
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit ac67819396ac9e96c3dd65a5b5b23715e11eeec5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Nov 23 10:04:23 2016 -0500 Make sure primary-guards are up-to-date when we inspect them. (Plus some magic to prevent and detect recursive invocation of entry_guards_update_primary(), since that can cause some pretty tricky misbehavior.) --- src/or/entrynodes.c | 58 +++++++++++++++++++++++++++++++++------------- src/or/entrynodes.h | 8 +++++++ src/test/test_entrynodes.c | 3 ++- 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 9a753e6..bd30078 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -79,9 +79,6 @@ **/ /* DOCDOC -- expand this. * - * XXXX prop271 -- make sure we check all of these properties everywhere we - * should. - * * Information invariants: * * [x] whenever a guard becomes unreachable, clear its usable_filtered flag. @@ -100,11 +97,11 @@ * [x] Whenever we remove a guard from the sample, remove it from the primary * and confirmed lists. * - * [ ] When we make a guard confirmed, update the primary list. + * [x] When we make a guard confirmed, update the primary list. * - * [ ] When we make a guard filtered or unfiltered, update the primary list. + * [x] When we make a guard filtered or unfiltered, update the primary list. * - * [ ] When we are about to pick a guard, make sure that the primary list is + * [x] When we are about to pick a guard, make sure that the primary list is * full. * * [x] Before calling sample_reachable_filtered_entry_guards(), make sure @@ -682,9 +679,12 @@ sampled_guards_update_from_consensus(guard_selection_t *gs) } SMARTLIST_FOREACH_END(guard); if (n_changes) { - /* Regnerate other things. XXXXXX prop271 */ - // XXXX prop271 rebuild confirmed list. + gs->primary_guards_up_to_date = 0; entry_guards_update_filtered_sets(gs); + /* We don't need to rebuild the confirmed list right here -- we may have + * removed confirmed guards above, but we can't have added any new + * confirmed guards. + */ entry_guards_changed_for_guard_selection(gs); } } @@ -749,6 +749,7 @@ entry_guard_set_filtered_flags(const or_options_t *options, guard_selection_t *gs, entry_guard_t *guard) { + unsigned was_filtered = guard->is_filtered_guard; guard->is_filtered_guard = 0; guard->is_usable_filtered_guard = 0; @@ -763,6 +764,11 @@ entry_guard_set_filtered_flags(const or_options_t *options, log_debug(LD_GUARD, "Updated sampled guard %s: filtered=%d; " "reachable_filtered=%d.", entry_guard_describe(guard), guard->is_filtered_guard, guard->is_usable_filtered_guard); + + if (!bool_eq(was_filtered, guard->is_filtered_guard)) { + /* This guard might now be primary or nonprimary. */ + gs->primary_guards_up_to_date = 0; + } } /** @@ -795,6 +801,7 @@ sample_reachable_filtered_entry_guards(guard_selection_t *gs, const unsigned exclude_confirmed = flags & SAMPLE_EXCLUDE_CONFIRMED; const unsigned exclude_primary = flags & SAMPLE_EXCLUDE_PRIMARY; const unsigned exclude_pending = flags & SAMPLE_EXCLUDE_PENDING; + const unsigned no_update_primary = flags & SAMPLE_NO_UPDATE_PRIMARY; SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) { entry_guard_consider_retry(guard); @@ -810,6 +817,9 @@ sample_reachable_filtered_entry_guards(guard_selection_t *gs, entry_guards_expand_sample(gs); } + if (exclude_primary && !gs->primary_guards_up_to_date && !no_update_primary) + entry_guards_update_primary(gs); + /* Build the set of reachable filtered guards. */ smartlist_t *reachable_filtered_sample = smartlist_new(); SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) { @@ -908,24 +918,34 @@ make_guard_confirmed(guard_selection_t *gs, entry_guard_t *guard) guard->confirmed_idx = gs->next_confirmed_idx++; smartlist_add(gs->confirmed_entry_guards, guard); + // This confirmed guard might kick something else out of the primary + // guards. + gs->primary_guards_up_to_date = 0; + entry_guards_changed_for_guard_selection(gs); } /** * Recalculate the list of primary guards (the ones we'd prefer to use) from * the filtered sample and the confirmed list. - * - * XXXXX prop271 are calling this enough ??? */ STATIC void entry_guards_update_primary(guard_selection_t *gs) { tor_assert(gs); + // prevent recursion. Recursion is potentially very bad here. + static int running = 0; + tor_assert(!running); + running = 1; + smartlist_t *new_primary_guards = smartlist_new(); smartlist_t *old_primary_guards = smartlist_new(); smartlist_add_all(old_primary_guards, gs->primary_entry_guards); + /* Set this flag now, to prevent the calls below from recursing. */ + gs->primary_guards_up_to_date = 1; + /* First, can we fill it up with confirmed guards? */ SMARTLIST_FOREACH_BEGIN(gs->confirmed_entry_guards, entry_guard_t *, guard) { if (smartlist_len(new_primary_guards) >= N_PRIMARY_GUARDS) @@ -964,7 +984,8 @@ entry_guards_update_primary(guard_selection_t *gs) while (smartlist_len(new_primary_guards) < N_PRIMARY_GUARDS) { entry_guard_t *guard = sample_reachable_filtered_entry_guards(gs, SAMPLE_EXCLUDE_CONFIRMED| - SAMPLE_EXCLUDE_PRIMARY); + SAMPLE_EXCLUDE_PRIMARY| + SAMPLE_NO_UPDATE_PRIMARY); if (!guard) break; guard->is_primary = 1; @@ -1007,6 +1028,8 @@ entry_guards_update_primary(guard_selection_t *gs) smartlist_free(old_primary_guards); smartlist_free(gs->primary_entry_guards); gs->primary_entry_guards = new_primary_guards; + gs->primary_guards_up_to_date = 1; + running = 0; } /** @@ -1104,6 +1127,9 @@ select_entry_guard_for_circuit(guard_selection_t *gs, unsigned *state_out) tor_assert(gs); tor_assert(state_out); + if (!gs->primary_guards_up_to_date) + entry_guards_update_primary(gs); + /* "If any entry in PRIMARY_GUARDS has {is_reachable} status of <maybe> or <yes>, return the first such guard." */ SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) { @@ -1191,6 +1217,9 @@ entry_guards_note_guard_failure(guard_selection_t *gs, STATIC void mark_primary_guards_maybe_reachable(guard_selection_t *gs) { + if (!gs->primary_guards_up_to_date) + entry_guards_update_primary(gs); + SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) { if (guard->is_reachable != GUARD_REACHABLE_NO) continue; @@ -1230,7 +1259,6 @@ entry_guards_note_guard_success(guard_selection_t *gs, guard->is_usable_filtered_guard = 1; if (guard->confirmed_idx < 0) { - // XXXX prop271 XXXX update primary guards, since we confirmed something? make_guard_confirmed(gs, guard); } @@ -1248,9 +1276,6 @@ entry_guards_note_guard_success(guard_selection_t *gs, } } - // XXXX prop271 XXXX update primary guards, since we confirmed something? - // XXXX prop261 XXXX if so, here or above? - log_info(LD_GUARD, "Recorded success for %s%sguard %s", guard->is_primary?"primary ":"", guard->confirmed_idx>=0?"confirmed ":"", @@ -1468,8 +1493,9 @@ entry_guard_chan_failed(guard_selection_t *gs, static int entry_guards_all_primary_guards_are_down(guard_selection_t *gs) { - /* XXXXX prop271 do we have to call entry_guards_update_primary() ?? */ tor_assert(gs); + if (!gs->primary_guards_up_to_date) + entry_guards_update_primary(gs); SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) { entry_guard_consider_retry(guard); if (guard->is_reachable != GUARD_REACHABLE_NO) diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 4cbfbf5..a514c13 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -223,6 +223,13 @@ struct guard_selection_s { int dirty; /** + * A value of 1 means that primary_entry_guards is up-to-date; 0 + * means we need to recalculate it before using primary_entry_guards + * or the is_primary flag on any guard. + */ + int primary_guards_up_to_date; + + /** * A list of the sampled entry guards, as entry_guard_t structures. * Not in any particular order. When we 'sample' a guard, we are * noting it as a possible guard to pick in the future. The use of @@ -428,6 +435,7 @@ STATIC void entry_guards_update_filtered_sets(guard_selection_t *gs); #define SAMPLE_EXCLUDE_CONFIRMED (1u<<0) #define SAMPLE_EXCLUDE_PRIMARY (1u<<1) #define SAMPLE_EXCLUDE_PENDING (1u<<2) +#define SAMPLE_NO_UPDATE_PRIMARY (1u<<3) /**@}*/ STATIC entry_guard_t *sample_reachable_filtered_entry_guards( guard_selection_t *gs, diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index d8b9ccb..cdf8672 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -1839,6 +1839,7 @@ test_entry_guard_sample_reachable_filtered(void *arg) g->pb.path_bias_disabled = 1; entry_guards_update_filtered_sets(gs); + gs->primary_guards_up_to_date = 1; tt_int_op(num_reachable_filtered_guards(gs), OP_EQ, n_guards - 1); tt_int_op(smartlist_len(gs->sampled_entry_guards), OP_EQ, n_guards); @@ -1851,7 +1852,7 @@ test_entry_guard_sample_reachable_filtered(void *arg) } tests[] = { { 0, -1 }, { SAMPLE_EXCLUDE_CONFIRMED, 1 }, - { SAMPLE_EXCLUDE_PRIMARY, 2 }, + { SAMPLE_EXCLUDE_PRIMARY|SAMPLE_NO_UPDATE_PRIMARY, 2 }, { SAMPLE_EXCLUDE_PENDING, 0 }, { -1, -1}, };

1 0

[tor/master] Mark some more BUG lines as unreachable.
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit bce0f79252e12a791c50e9b11ceb5867eeb07559 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Nov 23 15:33:02 2016 -0500 Mark some more BUG lines as unreachable. --- src/or/entrynodes.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index cf35b02..1501bf7 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -539,7 +539,9 @@ remove_guard_from_confirmed_and_primary_lists(guard_selection_t *gs, found_guard = smartlist_get(gs->confirmed_entry_guards, guard->confirmed_idx); if (BUG(guard != found_guard)) { + // LCOV_EXCL_START smartlist_remove_keeporder(gs->confirmed_entry_guards, guard); + // LCOV_EXCL_STOP } else { smartlist_del_keeporder(gs->confirmed_entry_guards, guard->confirmed_idx); @@ -548,7 +550,9 @@ remove_guard_from_confirmed_and_primary_lists(guard_selection_t *gs, guard->confirmed_on_date = 0; } else { if (BUG(smartlist_contains(gs->confirmed_entry_guards, guard))) { + // LCOV_EXCL_START smartlist_remove_keeporder(gs->confirmed_entry_guards, guard); + // LCOV_EXCL_STOP } } } @@ -903,10 +907,10 @@ STATIC void make_guard_confirmed(guard_selection_t *gs, entry_guard_t *guard) { if (BUG(guard->confirmed_on_date && guard->confirmed_idx >= 0)) - return; + return; // LCOV_EXCL_LINE if (BUG(smartlist_contains(gs->confirmed_entry_guards, guard))) - return; + return; // LCOV_EXCL_LINE const int GUARD_LIFETIME = GUARD_LIFETIME_DAYS * 86400; guard->confirmed_on_date = randomize_time(approx_time(), GUARD_LIFETIME/10);

1 0

[tor/master] Avoid division-by-zero in pathbias_check_*_success_count
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit 526b0e2ce2c5d31c70eb3e48eda59b34e9eb681d Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Nov 23 13:05:22 2016 -0500 Avoid division-by-zero in pathbias_check_*_success_count --- src/or/entrynodes.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index bd30078..860be9b 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -3402,10 +3402,13 @@ static void pathbias_check_use_success_count(entry_guard_t *node) { const or_options_t *options = get_options(); + const double EPSILON = 1.0e-9; + /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if (pathbias_get_use_success_count(node)/node->pb.use_attempts + if (node->pb.use_attempts > EPSILON && + pathbias_get_use_success_count(node)/node->pb.use_attempts < pathbias_get_extreme_use_rate(options) && pathbias_get_dropguards(options)) { node->pb.path_bias_disabled = 1; @@ -3420,10 +3423,13 @@ static void pathbias_check_close_success_count(entry_guard_t *node) { const or_options_t *options = get_options(); + const double EPSILON = 1.0e-9; + /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if (pathbias_get_close_success_count(node)/node->pb.circ_attempts + if (node->pb.circ_attempts > EPSILON && + pathbias_get_close_success_count(node)/node->pb.circ_attempts < pathbias_get_extreme_rate(options) && pathbias_get_dropguards(options)) { node->pb.path_bias_disabled = 1;

1 0

[tor/master] Turn #defines for prop271 into networkstatus params
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit d2af9826fd0a75efee8612b96709c39f24196f53 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Nov 26 10:06:50 2016 -0500 Turn #defines for prop271 into networkstatus params Some of these will get torrc options to override them too; this is just the mechanical conversion. Also, add documentation for a couple of undocumented (but now used) parameters. --- src/or/entrynodes.c | 144 ++++++++++++++++++++++++++++++++++++++++----- src/or/entrynodes.h | 45 +++++++++----- src/test/test_entrynodes.c | 14 ++--- 3 files changed, 166 insertions(+), 37 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 1c9349e..f1fe9f1 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -324,6 +324,118 @@ randomize_time,(time_t now, time_t max_backdate)) } /** + * @name parameters for networkstatus algorithm + * + * These parameters are taken from the consensus; some are overrideable in + * the torrc. + */ +/**@{*/ +/** + * We never let our sampled guard set grow larger than this fraction + * of the guards on the network. + */ +STATIC double +get_max_sample_threshold(void) +{ + int32_t pct = + networkstatus_get_param(NULL, "guard-max-sample-threshold-percent", + DFLT_MAX_SAMPLE_THRESHOLD_PERCENT, + 1, 100); + return pct / 100.0; +} +/** + * We always try to make our sample contain at least this many guards. + * + * XXXX prop271 There was a MIN_SAMPLE_THRESHOLD in the proposal, but I + * removed it in favor of MIN_FILTERED_SAMPLE_SIZE. -NM + */ +STATIC int +get_min_filtered_sample_size(void) +{ + return networkstatus_get_param(NULL, "guard-min-filtered-sample-size", + DFLT_MIN_FILTERED_SAMPLE_SIZE, + 1, INT32_MAX); +} +/** + * If a guard is unlisted for this many days in a row, we remove it. + */ +STATIC int +get_remove_unlisted_guards_after_days(void) +{ + return networkstatus_get_param(NULL, + "guard-remove-unlisted-guards-after-days", + DFLT_REMOVE_UNLISTED_GUARDS_AFTER_DAYS, + 1, 365*10); +} +/** + * We remove unconfirmed guards from the sample after this many days, + * regardless of whether they are listed or unlisted. + */ +STATIC int +get_guard_lifetime_days(void) +{ + return networkstatus_get_param(NULL, + "guard-lifetime-days", + DFLT_GUARD_LIFETIME_DAYS, 1, 365*10); +} +/** + * We remove confirmed guards from the sample if they were sampled + * GUARD_LIFETIME_DAYS ago and confirmed this many days ago. + */ +STATIC int +get_guard_confirmed_min_lifetime_days(void) +{ + return networkstatus_get_param(NULL, "guard-confirmed-min-lifetime-days", + DFLT_GUARD_CONFIRMED_MIN_LIFETIME_DAYS, + 1, 365*10); +} +/** + * How many guards do we try to keep on our primary guard list? + */ +STATIC int +get_n_primary_guards(void) +{ + return networkstatus_get_param(NULL, "guard-n-primary-guards", + DFLT_N_PRIMARY_GUARDS, 1, INT32_MAX); +} +/** + * If we haven't successfully built or used a circuit in this long, then + * consider that the internet is probably down. + */ +STATIC int +get_internet_likely_down_interval(void) +{ + return networkstatus_get_param(NULL, "guard-internet-likely-down-interval", + DFLT_INTERNET_LIKELY_DOWN_INTERVAL, + 1, INT32_MAX); +} +/** + * If we're trying to connect to a nonprimary guard for at least this + * many seconds, and we haven't gotten the connection to work, we will treat + * lower-priority guards as usable. + */ +STATIC int +get_nonprimary_guard_connect_timeout(void) +{ + return networkstatus_get_param(NULL, + "guard-nonprimary-guard-connect-timeout", + DFLT_NONPRIMARY_GUARD_CONNECT_TIMEOUT, + 1, INT32_MAX); +} +/** + * If a circuit has been sitting around in 'waiting for better guard' state + * for at least this long, we'll expire it. + */ +STATIC int +get_nonprimary_guard_idle_timeout(void) +{ + return networkstatus_get_param(NULL, + "guard-nonprimary-guard-idle-timeout", + (10*60), 1, INT32_MAX); +} +/**@}*/ + +/** * Return true iff node has all the flags needed for us to consider it * a possible guard when sampling guards. */ @@ -377,7 +489,7 @@ STATIC entry_guard_t * entry_guard_add_to_sample(guard_selection_t *gs, const node_t *node) { - const int GUARD_LIFETIME = GUARD_LIFETIME_DAYS * 86400; + const int GUARD_LIFETIME = get_guard_lifetime_days() * 86400; tor_assert(gs); tor_assert(node); @@ -470,8 +582,8 @@ entry_guards_expand_sample(guard_selection_t *gs) if (! smartlist_len(eligible_guards)) goto done; - const int max_sample = (int)(n_guards * MAX_SAMPLE_THRESHOLD); - const int min_filtered_sample = MIN_FILTERED_SAMPLE_SIZE; + const int max_sample = (int)(n_guards * get_max_sample_threshold()); + const int min_filtered_sample = get_min_filtered_sample_size(); log_info(LD_GUARD, "Expanding the sample guard set. We have %d guards " "in the sample, and %d eligible guards to extend it with.", @@ -570,7 +682,7 @@ sampled_guards_update_from_consensus(guard_selection_t *gs) { tor_assert(gs); const int REMOVE_UNLISTED_GUARDS_AFTER = - (REMOVE_UNLISTED_GUARDS_AFTER_DAYS * 86400); + (get_remove_unlisted_guards_after_days() * 86400); const int unlisted_since_slop = REMOVE_UNLISTED_GUARDS_AFTER / 5; // It's important to use only a live consensus here; we don't want to @@ -637,9 +749,9 @@ sampled_guards_update_from_consensus(guard_selection_t *gs) const time_t remove_if_unlisted_since = approx_time() - REMOVE_UNLISTED_GUARDS_AFTER; const time_t maybe_remove_if_sampled_before = - approx_time() - (GUARD_LIFETIME_DAYS * 86400); + approx_time() - (get_guard_lifetime_days() * 86400); const time_t remove_if_confirmed_before = - approx_time() - (GUARD_CONFIRMED_MIN_LIFETIME_DAYS * 86400); + approx_time() - (get_guard_confirmed_min_lifetime_days() * 86400); /* Then: remove the ones that have been junk for too long */ SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) { @@ -656,7 +768,7 @@ sampled_guards_update_from_consensus(guard_selection_t *gs) */ log_info(LD_GUARD, "Removing sampled guard %s: it has been unlisted " "for over %d days", entry_guard_describe(guard), - REMOVE_UNLISTED_GUARDS_AFTER_DAYS); + get_remove_unlisted_guards_after_days()); remove = 1; } else if (guard->sampled_on_date < maybe_remove_if_sampled_before) { /* We have a live consensus, and {ADDED_ON_DATE} is over @@ -668,13 +780,14 @@ sampled_guards_update_from_consensus(guard_selection_t *gs) log_info(LD_GUARD, "Removing sampled guard %s: it was sampled " "over %d days ago, but never confirmed.", entry_guard_describe(guard), - GUARD_LIFETIME_DAYS); + get_guard_lifetime_days()); } else if (guard->confirmed_on_date < remove_if_confirmed_before) { remove = 1; log_info(LD_GUARD, "Removing sampled guard %s: it was sampled " "over %d days ago, and confirmed over %d days ago.", entry_guard_describe(guard), - GUARD_LIFETIME_DAYS, GUARD_CONFIRMED_MIN_LIFETIME_DAYS); + get_guard_lifetime_days(), + get_guard_confirmed_min_lifetime_days()); } } @@ -820,7 +933,8 @@ sample_reachable_filtered_entry_guards(guard_selection_t *gs, log_info(LD_GUARD, "Trying to sample a reachable guard: We know of %d " "in the USABLE_FILTERED set.", n_reachable_filtered); - if (n_reachable_filtered < MIN_FILTERED_SAMPLE_SIZE) { + const int min_filtered_sample = get_min_filtered_sample_size(); + if (n_reachable_filtered < min_filtered_sample) { log_info(LD_GUARD, " (That isn't enough. Trying to expand the sample.)"); entry_guards_expand_sample(gs); } @@ -916,7 +1030,7 @@ make_guard_confirmed(guard_selection_t *gs, entry_guard_t *guard) if (BUG(smartlist_contains(gs->confirmed_entry_guards, guard))) return; // LCOV_EXCL_LINE - const int GUARD_LIFETIME = GUARD_LIFETIME_DAYS * 86400; + const int GUARD_LIFETIME = get_guard_lifetime_days() * 86400; guard->confirmed_on_date = randomize_time(approx_time(), GUARD_LIFETIME/10); log_info(LD_GUARD, "Marking %s as a confirmed guard (index %d)", @@ -947,6 +1061,8 @@ entry_guards_update_primary(guard_selection_t *gs) tor_assert(!running); running = 1; + const int N_PRIMARY_GUARDS = get_n_primary_guards(); + smartlist_t *new_primary_guards = smartlist_new(); smartlist_t *old_primary_guards = smartlist_new(); smartlist_add_all(old_primary_guards, gs->primary_entry_guards); @@ -1278,7 +1394,7 @@ entry_guards_note_guard_success(guard_selection_t *gs, old_state == GUARD_CIRC_STATE_USABLE_IF_NO_BETTER_GUARD); new_state = GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD; - if (last_time_on_internet + INTERNET_LIKELY_DOWN_INTERVAL + if (last_time_on_internet + get_internet_likely_down_interval() < approx_time()) { mark_primary_guards_maybe_reachable(gs); } @@ -1620,7 +1736,7 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, */ int n_blockers_found = 0; const time_t state_set_at_cutoff = - approx_time() - NONPRIMARY_GUARD_CONNECT_TIMEOUT; + approx_time() - get_nonprimary_guard_connect_timeout(); SMARTLIST_FOREACH_BEGIN(all_circuits, origin_circuit_t *, circ) { circuit_guard_state_t *state = origin_circuit_get_guard_state(circ); if (state == NULL) @@ -1678,7 +1794,7 @@ entry_guard_state_should_expire(circuit_guard_state_t *guard_state) if (guard_state == NULL) return 0; const time_t expire_if_waiting_since = - approx_time() - NONPRIMARY_GUARD_IDLE_TIMEOUT; + approx_time() - get_nonprimary_guard_idle_timeout(); return (guard_state->state == GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD && guard_state->state_set_at < expire_if_waiting_since); } diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 648e599..0ed94cb 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -347,63 +347,76 @@ int num_bridges_usable(void); #ifdef ENTRYNODES_PRIVATE /** - * @name Parameters for the new (prop271) entry guard algorithm. + * @name Default values for the parameters for the new (prop271) entry guard + * algorithm. */ -/* XXXX prop271 some of these should be networkstatus parameters */ /**@{*/ /** - * We never let our sampled guard set grow larger than this fraction + * We never let our sampled guard set grow larger than this percentage * of the guards on the network. */ -#define MAX_SAMPLE_THRESHOLD 0.30 +#define DFLT_MAX_SAMPLE_THRESHOLD_PERCENT 30 /** * We always try to make our sample contain at least this many guards. * * XXXX prop271 There was a MIN_SAMPLE_THRESHOLD in the proposal, but I * removed it in favor of MIN_FILTERED_SAMPLE_SIZE. -NM */ -#define MIN_FILTERED_SAMPLE_SIZE 20 +#define DFLT_MIN_FILTERED_SAMPLE_SIZE 20 /** * If a guard is unlisted for this many days in a row, we remove it. */ -#define REMOVE_UNLISTED_GUARDS_AFTER_DAYS 20 +#define DFLT_REMOVE_UNLISTED_GUARDS_AFTER_DAYS 20 /** * We remove unconfirmed guards from the sample after this many days, * regardless of whether they are listed or unlisted. */ -#define GUARD_LIFETIME_DAYS 120 +#define DFLT_GUARD_LIFETIME_DAYS 120 /** * We remove confirmed guards from the sample if they were sampled * GUARD_LIFETIME_DAYS ago and confirmed this many days ago. */ -#define GUARD_CONFIRMED_MIN_LIFETIME_DAYS 60 +#define DFLT_GUARD_CONFIRMED_MIN_LIFETIME_DAYS 60 /** * How many guards do we try to keep on our primary guard list? */ -#define N_PRIMARY_GUARDS 3 +#define DFLT_N_PRIMARY_GUARDS 3 /** * If we haven't successfully built or used a circuit in this long, then * consider that the internet is probably down. */ -#define INTERNET_LIKELY_DOWN_INTERVAL (10*60) +#define DFLT_INTERNET_LIKELY_DOWN_INTERVAL (10*60) /** - * DOCDOC. not yet used; see prop271. + * If we're trying to connect to a nonprimary guard for at least this + * many seconds, and we haven't gotten the connection to work, we will treat + * lower-priority guards as usable. */ -#define NONPRIMARY_GUARD_CONNECT_TIMEOUT 15 +#define DFLT_NONPRIMARY_GUARD_CONNECT_TIMEOUT 15 /** - * DOCDOC. not yet used; see prop271. + * If a circuit has been sitting around in 'waiting for better guard' state + * for at least this long, we'll expire it. */ -#define NONPRIMARY_GUARD_IDLE_TIMEOUT (10*60) +#define DLFT_NONPRIMARY_GUARD_IDLE_TIMEOUT (10*60) /** * DOCDOC. not yet used; see prop271. */ -#define MEANINGFUL_RESTRICTION_FRAC 0.2 +#define DFLT_MEANINGFUL_RESTRICTION_FRAC 0.2 /** * DOCDOC. not yet used. see prop271. */ -#define EXTREME_RESTRICTION_FRAC 0.01 +#define DFLT_EXTREME_RESTRICTION_FRAC 0.01 /**@}*/ +STATIC double get_max_sample_threshold(void); +STATIC int get_min_filtered_sample_size(void); +STATIC int get_remove_unlisted_guards_after_days(void); +STATIC int get_guard_lifetime_days(void); +STATIC int get_guard_confirmed_min_lifetime_days(void); +STATIC int get_n_primary_guards(void); +STATIC int get_internet_likely_down_interval(void); +STATIC int get_nonprimary_guard_connect_timeout(void); +STATIC int get_nonprimary_guard_idle_timeout(void); + // ---------- XXXX these functions and definitions are post-prop271. HANDLE_DECL(entry_guard, entry_guard_t, STATIC) STATIC guard_selection_t *guard_selection_new(const char *name); diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index de36142..ee08375 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -1425,7 +1425,7 @@ test_entry_guard_expand_sample(void *arg) num_reachable_filtered_guards(gs)); /* Make sure we got the right number. */ - tt_int_op(MIN_FILTERED_SAMPLE_SIZE, OP_EQ, + tt_int_op(DFLT_MIN_FILTERED_SAMPLE_SIZE, OP_EQ, num_reachable_filtered_guards(gs)); // Make sure everything we got was from our fake node list, and everything @@ -1444,7 +1444,7 @@ test_entry_guard_expand_sample(void *arg) // make no changes. guard = entry_guards_expand_sample(gs); tt_assert(! guard); // no guard was added. - tt_int_op(MIN_FILTERED_SAMPLE_SIZE, OP_EQ, + tt_int_op(DFLT_MIN_FILTERED_SAMPLE_SIZE, OP_EQ, num_reachable_filtered_guards(gs)); // Make a few guards unreachable. @@ -1454,13 +1454,13 @@ test_entry_guard_expand_sample(void *arg) guard->is_usable_filtered_guard = 0; guard = smartlist_get(gs->sampled_entry_guards, 2); guard->is_usable_filtered_guard = 0; - tt_int_op(MIN_FILTERED_SAMPLE_SIZE - 3, OP_EQ, + tt_int_op(DFLT_MIN_FILTERED_SAMPLE_SIZE - 3, OP_EQ, num_reachable_filtered_guards(gs)); // This time, expanding the sample will add some more guards. guard = entry_guards_expand_sample(gs); tt_assert(guard); // no guard was added. - tt_int_op(MIN_FILTERED_SAMPLE_SIZE, OP_EQ, + tt_int_op(DFLT_MIN_FILTERED_SAMPLE_SIZE, OP_EQ, num_reachable_filtered_guards(gs)); tt_int_op(smartlist_len(gs->sampled_entry_guards), OP_EQ, num_reachable_filtered_guards(gs)+3); @@ -1468,7 +1468,7 @@ test_entry_guard_expand_sample(void *arg) // Still idempotent. guard = entry_guards_expand_sample(gs); tt_assert(! guard); // no guard was added. - tt_int_op(MIN_FILTERED_SAMPLE_SIZE, OP_EQ, + tt_int_op(DFLT_MIN_FILTERED_SAMPLE_SIZE, OP_EQ, num_reachable_filtered_guards(gs)); // Now, do a nasty trick: tell the filter to exclude 31/32 of the guards. @@ -1486,7 +1486,7 @@ test_entry_guard_expand_sample(void *arg) // Surely (p ~ 1-2**-60), one of our guards has been excluded. tt_int_op(num_reachable_filtered_guards(gs), OP_LT, - MIN_FILTERED_SAMPLE_SIZE); + DFLT_MIN_FILTERED_SAMPLE_SIZE); // Try to regenerate the guards. guard = entry_guards_expand_sample(gs); @@ -1494,7 +1494,7 @@ test_entry_guard_expand_sample(void *arg) /* this time, it's possible that we didn't add enough sampled guards. */ tt_int_op(num_reachable_filtered_guards(gs), OP_LE, - MIN_FILTERED_SAMPLE_SIZE); + DFLT_MIN_FILTERED_SAMPLE_SIZE); /* but we definitely didn't exceed the sample maximum. */ tt_int_op(smartlist_len(gs->sampled_entry_guards), OP_LE, (int)((271 / 2) * .3));

1 0

[tor/master] Use the new guard notification/selection APIs throughout Tor
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit dbbaa515183e250e20c40fa7b4c00df9487058fa Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 21 17:23:25 2016 -0500 Use the new guard notification/selection APIs throughout Tor This patch doesn't cover every case; omitted cases are marked with "XXXX prop271", as usual. It leaves both the old interface and the new interface for guard status notification, since they don't actually work in the same way: the new API wants to be told when a circuit has failed or succeeded, whereas the old API wants to know when a channel has failed or succeeded. I ran into some trouble with directory guard stuff, since when we pick the directory guard, we don't actually have a circuit to associate it with. I solved that by allowing guard states to be associated with directory connections, not just circuits. --- src/or/bridges.c | 2 ++ src/or/channel.c | 1 + src/or/circuitbuild.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++---- src/or/circuitbuild.h | 6 +++- src/or/circuitlist.c | 42 +++++++++++++++++++++++++++- src/or/circuitlist.h | 2 ++ src/or/circuituse.c | 2 ++ src/or/connection.c | 1 + src/or/connection_or.c | 6 ++++ src/or/directory.c | 74 +++++++++++++++++++++++++++++++++++++++---------- src/or/directory.h | 6 ++-- src/or/entrynodes.c | 60 ++++++++++++++++++++++++++++++++++++---- src/or/entrynodes.h | 2 +- src/or/main.c | 5 +++- src/or/or.h | 4 +++ src/or/rendclient.c | 2 +- src/or/rendservice.c | 2 +- src/or/routerlist.c | 4 +-- src/test/test_dir.c | 8 ++++-- 19 files changed, 268 insertions(+), 36 deletions(-) diff --git a/src/or/bridges.c b/src/or/bridges.c index 508c77f..2170cc6 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -724,6 +724,7 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) from_cache ? "cached" : "fresh", router_describe(ri)); /* set entry->made_contact so if it goes down we don't drop it from * our entry node list */ + // XXXX prop271 use new interface here when we hit bridges? entry_guard_register_connect_status(ri->cache_info.identity_digest, 1, 0, now); if (first) { @@ -743,6 +744,7 @@ int any_bridge_descriptors_known(void) { tor_assert(get_options()->UseBridges); + // XXXX prop271 this needs to get fixed. -- bridges return choose_random_entry(NULL) != NULL; } diff --git a/src/or/channel.c b/src/or/channel.c index af58107..1e3e99c 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -2538,6 +2538,7 @@ channel_do_open_actions(channel_t *chan) if (started_here) { circuit_build_times_network_is_live(get_circuit_build_times_mutable()); rep_hist_note_connect_succeeded(chan->identity_digest, now); + // XXXX prop271 this call is no longer useful with the new algorithm. if (entry_guard_register_connect_status( chan->identity_digest, 1, 0, now) < 0) { /* Close any circuits pending on this channel. We leave it in state diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index a33c2ca..2f4ce7a 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -964,7 +964,35 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) memset(&ec, 0, sizeof(ec)); if (!hop) { /* done building the circuit. whew. */ - circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_OPEN); + int r; + if (! circ->guard_state) { + if (circuit_get_cpath_len(circ) != 1) { + log_warn(LD_BUG, "%d-hop circuit %p with purpose %d has no " + "guard state", + circuit_get_cpath_len(circ), circ, circ->base_.purpose); + } + r = 1; + } else { + r = entry_guard_succeeded(get_guard_selection_info(), + &circ->guard_state); + } + const int is_usable_for_streams = (r == 1); + if (r == 1) { + circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_OPEN); + } else if (r == 0) { + // XXXX prop271 we might want to probe for whether this + // XXXX one is ready even before the next second rolls over. + circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_GUARD_WAIT); + } else { + return - END_CIRC_REASON_INTERNAL; + } + + /* XXXX prop271 -- the rest of this branch needs careful thought! + * Some of the things here need to happen when a circuit becomes + * mechanically open; some need to happen when it is actually usable. + * I think I got them right, but more checking would be wise. -NM + */ + if (circuit_timeout_want_to_count_circ(circ)) { struct timeval end; long timediff; @@ -1006,7 +1034,8 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) pathbias_count_build_success(circ); circuit_rep_hist_note_result(circ); - circuit_has_opened(circ); /* do other actions as necessary */ + if (is_usable_for_streams) + circuit_has_opened(circ); /* do other actions as necessary */ if (!have_completed_a_circuit() && !circ->build_state->onehop_tunnel) { const or_options_t *options = get_options(); @@ -2206,9 +2235,20 @@ choose_good_middle_server(uint8_t purpose, * * If state is NULL, we're choosing a router to serve as an entry * guard, not for any particular circuit. + * + * Set *guard_state_out to information about the guard that + * we're selecting, which we'll use later to remember whether the + * guard worked or not. + * + * XXXX prop271 this function is used in four ways: picking out guards for + * the old (pre-prop271) guard algorithm; picking out guards for circuits; + * picking out guards for testing circuits on non-bridgees; + * picking out entries when entry guards are disabled. These options + * should be disentangled. */ const node_t * -choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) +choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state, + circuit_guard_state_t **guard_state_out) { const node_t *choice; smartlist_t *excluded; @@ -2223,7 +2263,8 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) (purpose != CIRCUIT_PURPOSE_TESTING || options->BridgeRelay)) { /* This request is for an entry server to use for a regular circuit, * and we use entry guard nodes. Just return one of the guard nodes. */ - return choose_random_entry(state); + tor_assert(guard_state_out); + return guards_choose_guard(state, guard_state_out); } excluded = smartlist_new(); @@ -2306,7 +2347,8 @@ onion_extend_cpath(origin_circuit_t *circ) if (cur_len == state->desired_path_len - 1) { /* Picking last node */ info = extend_info_dup(state->chosen_exit); } else if (cur_len == 0) { /* picking first node */ - const node_t *r = choose_good_entry_server(purpose, state); + const node_t *r = choose_good_entry_server(purpose, state, + &circ->guard_state); if (r) { /* If we're a client, use the preferred address rather than the primary address, for potentially connecting to an IPv6 OR @@ -2574,3 +2616,26 @@ extend_info_has_preferred_onion_key(const extend_info_t* ei) return extend_info_supports_ntor(ei); } +/** Find the circuits that are waiting to find out whether their guards are + * usable, and if any are ready to become usable, mark them open and try + * attaching streams as appropriate. */ +void +circuit_upgrade_circuits_from_guard_wait(void) +{ + smartlist_t *to_upgrade = + circuit_find_circuits_to_upgrade_from_guard_wait(); + + if (to_upgrade == NULL) + return; + + log_info(LD_GUARD, "Upgrading %d circuits from 'waiting for better guard' " + "to 'open'.", smartlist_len(to_upgrade)); + + SMARTLIST_FOREACH_BEGIN(to_upgrade, origin_circuit_t *, circ) { + circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_OPEN); + circuit_has_opened(circ); + } SMARTLIST_FOREACH_END(circ); + + smartlist_free(to_upgrade); +} + diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 56f66a1..2c83a16 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -64,8 +64,12 @@ int extend_info_has_preferred_onion_key(const extend_info_t* ei); const node_t *build_state_get_exit_node(cpath_build_state_t *state); const char *build_state_get_exit_nickname(cpath_build_state_t *state); +struct circuit_guard_state_t; + const node_t *choose_good_entry_server(uint8_t purpose, - cpath_build_state_t *state); + cpath_build_state_t *state, + struct circuit_guard_state_t **guard_state_out); +void circuit_upgrade_circuits_from_guard_wait(void); #ifdef CIRCUITBUILD_PRIVATE STATIC circid_t get_unique_circ_id_by_chan(channel_t *chan); diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index c274534..2a03f8a 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -92,6 +92,10 @@ static smartlist_t *global_origin_circuit_list = NULL; /** A list of all the circuits in CIRCUIT_STATE_CHAN_WAIT. */ static smartlist_t *circuits_pending_chans = NULL; +/** List of all the (origin) circuits whose state is + * CIRCUIT_STATE_GUARD_WAIT. */ +static smartlist_t *circuits_pending_other_guards = NULL; + /** A list of all the circuits that have been marked with * circuit_mark_for_close and which are waiting for circuit_about_to_free. */ static smartlist_t *circuits_pending_close = NULL; @@ -433,8 +437,10 @@ circuit_set_state(circuit_t *circ, uint8_t state) tor_assert(circ); if (state == circ->state) return; - if (!circuits_pending_chans) + if (PREDICT_UNLIKELY(!circuits_pending_chans)) circuits_pending_chans = smartlist_new(); + if (PREDICT_UNLIKELY(!circuits_pending_other_guards)) + circuits_pending_other_guards = smartlist_new(); if (circ->state == CIRCUIT_STATE_CHAN_WAIT) { /* remove from waiting-circuit list. */ smartlist_remove(circuits_pending_chans, circ); @@ -1022,6 +1028,9 @@ circuit_free_all(void) smartlist_free(circuits_pending_close); circuits_pending_close = NULL; + smartlist_free(circuits_pending_other_guards); + circuits_pending_other_guards = NULL; + { chan_circid_circuit_map_t **elt, **next, *c; for (elt = HT_START(chan_circid_map, &chan_circid_map); @@ -1721,6 +1730,37 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, return best; } +/** + * Check whether any of the origin circuits that are waiting to see if + * their guard is good enough to use can be upgraded to "ready". If so, + * return a new smartlist containing them. Otherwise return NULL. + */ +smartlist_t * +circuit_find_circuits_to_upgrade_from_guard_wait(void) +{ + /* Only if some circuit is actually waiting on an upgrade should we + * run the algorithm. */ + if (! circuits_pending_other_guards || + smartlist_len(circuits_pending_other_guards)==0) + return NULL; + /* Only if we have some origin circuiuts should we run the algorithm. + */ + if (!global_origin_circuit_list) + return NULL; + + /* Okay; we can pass our circuit list to entrynodes.c.*/ + smartlist_t *result = smartlist_new(); + int r = entry_guards_upgrade_waiting_circuits(get_guard_selection_info(), + global_origin_circuit_list, + result); + if (r && smartlist_len(result)) { + return result; + } else { + smartlist_free(result); + return NULL; + } +} + /** Return the number of hops in circuit's path. If circ has no entries, * or is NULL, returns 0. */ int diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index 989c02a..73039cc 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -77,6 +77,8 @@ void channel_note_destroy_pending(channel_t *chan, circid_t id); MOCK_DECL(void, channel_note_destroy_not_pending, (channel_t *chan, circid_t id)); +smartlist_t *circuit_find_circuits_to_upgrade_from_guard_wait(void); + #ifdef CIRCUITLIST_PRIVATE STATIC void circuit_free(circuit_t *circ); STATIC size_t n_cells_in_circ_queues(const circuit_t *c); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 2f972d1..d2a7f20 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1633,6 +1633,8 @@ circuit_build_failed(origin_circuit_t *circ) "Our circuit died before the first hop with no connection"); } if (n_chan_id && !already_marked) { + entry_guard_failed(get_guard_selection_info(), &circ->guard_state); + /* XXXX prop271 -- old API */ entry_guard_register_connect_status(n_chan_id, 0, 1, time(NULL)); /* if there are any one-hop streams waiting on this circuit, fail * them now so they can retry elsewhere. */ diff --git a/src/or/connection.c b/src/or/connection.c index 2964c30..c2a7a87 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -634,6 +634,7 @@ connection_free_(connection_t *conn) cached_dir_decref(dir_conn->cached_dir); rend_data_free(dir_conn->rend_data); + circuit_guard_state_free(dir_conn->guard_state); } if (SOCKET_OK(conn->s)) { diff --git a/src/or/connection_or.c b/src/or/connection_or.c index ecc5a45..fefcc86 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -735,6 +735,9 @@ connection_or_about_to_close(or_connection_t *or_conn) const or_options_t *options = get_options(); connection_or_note_state_when_broken(or_conn); rep_hist_note_connect_failed(or_conn->identity_digest, now); + entry_guard_chan_failed(get_guard_selection_info(), + TLS_CHAN_TO_BASE(or_conn->chan)); + /* XXXX prop271 -- old API */ entry_guard_register_connect_status(or_conn->identity_digest,0, !options->HTTPSProxy, now); if (conn->state >= OR_CONN_STATE_TLS_HANDSHAKING) { @@ -1673,6 +1676,9 @@ connection_or_client_learned_peer_id(or_connection_t *conn, "Tried connecting to router at %s:%d, but identity key was not " "as expected: wanted %s but got %s.%s", conn->base_.address, conn->base_.port, expected, seen, extra_log); + entry_guard_chan_failed(get_guard_selection_info(), + TLS_CHAN_TO_BASE(conn->chan)); + /* XXXX prop271 old API */ entry_guard_register_connect_status(conn->identity_digest, 0, 1, time(NULL)); control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, diff --git a/src/or/directory.c b/src/or/directory.c index efa5a31..4164672 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -128,7 +128,8 @@ static void directory_initiate_command_rend( const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query); + const rend_data_t *rend_query, + circuit_guard_state_t *guard_state); static void connection_dir_close_consensus_fetches( dir_connection_t *except_this_one, const char *resource); @@ -422,7 +423,8 @@ directory_post_to_dirservers(uint8_t dir_purpose, uint8_t router_purpose, directory_initiate_command_routerstatus(rs, dir_purpose, router_purpose, indirection, - NULL, payload, upload_len, 0); + NULL, payload, upload_len, 0, + NULL); } SMARTLIST_FOREACH_END(ds); if (!found) { char *s = authdir_type_to_string(type); @@ -458,7 +460,8 @@ should_use_directory_guards(const or_options_t *options) * information of type type, and return its routerstatus. */ static const routerstatus_t * directory_pick_generic_dirserver(dirinfo_type_t type, int pds_flags, - uint8_t dir_purpose) + uint8_t dir_purpose, + circuit_guard_state_t **guard_state_out) { const routerstatus_t *rs = NULL; const or_options_t *options = get_options(); @@ -467,7 +470,7 @@ directory_pick_generic_dirserver(dirinfo_type_t type, int pds_flags, log_warn(LD_BUG, "Called when we have UseBridges set."); if (should_use_directory_guards(options)) { - const node_t *node = choose_random_dirguard(type); + const node_t *node = guards_choose_dirguard(type, guard_state_out); if (node) rs = node->rs; } else { @@ -548,6 +551,7 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( if (!options->FetchServerDescriptors) return; + circuit_guard_state_t *guard_state = NULL; if (!get_via_tor) { if (options->UseBridges && !(type & BRIDGE_DIRINFO)) { /* We want to ask a running bridge for which we have a descriptor. @@ -556,6 +560,7 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( * sort of dir fetch we'll be doing, so it won't return a bridge * that can't answer our question. */ + // XXXX prop271 update this for bridge support. const node_t *node = choose_random_dirguard(type); if (node && node->ri) { /* every bridge has a routerinfo. */ @@ -605,9 +610,9 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( } } if (!rs && !(type & BRIDGE_DIRINFO)) { - /* */ rs = directory_pick_generic_dirserver(type, pds_flags, - dir_purpose); + dir_purpose, + &guard_state); if (!rs) get_via_tor = 1; /* last resort: try routing it via Tor */ } @@ -630,7 +635,8 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( router_purpose, indirection, resource, NULL, 0, - if_modified_since); + if_modified_since, + guard_state); } else { log_notice(LD_DIR, "While fetching directory info, " @@ -664,7 +670,7 @@ directory_get_from_all_authorities(uint8_t dir_purpose, rs = &ds->fake_status; directory_initiate_command_routerstatus(rs, dir_purpose, router_purpose, DIRIND_ONEHOP, resource, NULL, - 0, 0); + 0, 0, NULL); } SMARTLIST_FOREACH_END(ds); } @@ -775,7 +781,8 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query) + const rend_data_t *rend_query, + circuit_guard_state_t *guard_state) { const or_options_t *options = get_options(); const node_t *node; @@ -830,7 +837,8 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, dir_purpose, router_purpose, indirection, resource, payload, payload_len, if_modified_since, - rend_query); + rend_query, + guard_state); } /** Launch a new connection to the directory server status to @@ -855,13 +863,15 @@ MOCK_IMPL(void, directory_initiate_command_routerstatus, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since)) + time_t if_modified_since, + circuit_guard_state_t *guard_state)) { directory_initiate_command_routerstatus_rend(status, dir_purpose, router_purpose, indirection, resource, payload, payload_len, - if_modified_since, NULL); + if_modified_since, NULL, + guard_state); } /** Return true iff conn is the client side of a directory connection @@ -889,6 +899,11 @@ directory_conn_is_self_reachability_test(dir_connection_t *conn) static void connection_dir_request_failed(dir_connection_t *conn) { + if (conn->guard_state) { + /* We haven't seen a success on this guard state, so consider it to have + * failed. */ + entry_guard_failed(get_guard_selection_info(), &conn->guard_state); + } if (directory_conn_is_self_reachability_test(conn)) { return; /* this was a test fetch. don't retry. */ } @@ -1136,7 +1151,7 @@ directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port, digest, dir_purpose, router_purpose, indirection, resource, payload, payload_len, - if_modified_since, NULL); + if_modified_since, NULL, NULL); } /** Same as directory_initiate_command(), but accepts rendezvous data to @@ -1151,7 +1166,8 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, const char *resource, const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query) + const rend_data_t *rend_query, + circuit_guard_state_t *guard_state) { tor_assert(or_addr_port); tor_assert(dir_addr_port); @@ -1246,12 +1262,18 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, if (!anonymized_connection && !use_begindir) { /* then we want to connect to dirport directly */ + // XXXX prop271 I think that we never use guards in this case. if (options->HTTPProxy) { tor_addr_copy(&addr, &options->HTTPProxyAddr); port = options->HTTPProxyPort; } + // In this case we should not have picked a directory guard. + if (BUG(guard_state)) { + entry_guard_cancel(get_guard_selection_info(), &guard_state); + } + switch (connection_connect(TO_CONN(conn), conn->base_.address, &addr, port, &socket_error)) { case -1: @@ -1288,6 +1310,14 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, else if (anonymized_connection && !use_begindir) rep_hist_note_used_port(time(NULL), conn->base_.port); + // In this case we should not have a directory guard; we'll + // get a regular guard later when we build the circuit. + if (BUG(anonymized_connection && guard_state)) { + entry_guard_cancel(get_guard_selection_info(), &guard_state); + } + + conn->guard_state = guard_state; + /* make an AP connection * populate it and add it at the right state * hook up both sides @@ -2540,6 +2570,22 @@ connection_dir_process_inbuf(dir_connection_t *conn) tor_assert(conn); tor_assert(conn->base_.type == CONN_TYPE_DIR); + if (conn->guard_state) { + /* we count the connection as successful once we can read from it. We do + * not, however, delay use of the circuit here, since it's just for a + * one-hop directory request. */ + /* XXXXprop271 note that this will not do the right thing for other + * waiting circuits that would be triggered by this circuit becoming + * complete/usable. But that's ok, I think. + */ + /* XXXXprop271 should we count this as only a partial success somehow? + */ + entry_guard_succeeded(get_guard_selection_info(), + &conn->guard_state); + circuit_guard_state_free(conn->guard_state); + conn->guard_state = NULL; + } + /* Directory clients write, then read data until they receive EOF; * directory servers read data until they get an HTTP command, then * write their response (when it's finished flushing, they mark for diff --git a/src/or/directory.h b/src/or/directory.h index 589df7b..ee0a198 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -49,7 +49,8 @@ MOCK_DECL(void, directory_initiate_command_routerstatus, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since)); + time_t if_modified_since, + struct circuit_guard_state_t *guard_state)); void directory_initiate_command_routerstatus_rend(const routerstatus_t *status, uint8_t dir_purpose, @@ -59,7 +60,8 @@ void directory_initiate_command_routerstatus_rend(const routerstatus_t *status, const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query); + const rend_data_t *rend_query, + struct circuit_guard_state_t *guard_state); int parse_http_response(const char *headers, int *code, time_t *date, compress_method_t *compression, char **response); diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 24a3448..eca88a9 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -89,7 +89,7 @@ * [x] Whenever a guard becomes reachable or maybe-reachable, if its filtered * flag is set, set its usable_filtered flag. * - * [ ] Whenever we get a new consensus, call update_from_consensus(). (LATER.) + * [x] Whenever we get a new consensus, call update_from_consensus(). (LATER.) * * [ ] Whenever the configuration changes in a relevant way, update the * filtered/usable flags. (LATER.) @@ -1203,8 +1203,6 @@ entry_guards_note_guard_success(guard_selection_t *gs, if (last_time_on_internet + INTERNET_LIKELY_DOWN_INTERVAL < approx_time()) { mark_primary_guards_maybe_reachable(gs); - } else { - // update_waiting_circuits(gs); // XXXX prop271 write this function. } } @@ -1475,7 +1473,7 @@ circ_state_has_higher_priority(origin_circuit_t *a, */ int entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, - smartlist_t *all_circuits, + const smartlist_t *all_circuits, smartlist_t *newly_complete_out) { tor_assert(gs); @@ -2274,7 +2272,7 @@ add_an_entry_guard(guard_selection_t *gs, return NULL; } } else if (!for_directory) { - node = choose_good_entry_server(CIRCUIT_PURPOSE_C_GENERAL, NULL); + node = choose_good_entry_server(CIRCUIT_PURPOSE_C_GENERAL, NULL, NULL); if (!node) return NULL; } else { @@ -3779,6 +3777,58 @@ entries_retry_all(const or_options_t *options) entries_retry_helper(options, 1); } +/** Helper: Update the status of all entry guards, in whatever algorithm + is used. */ +void +guards_update_all(void) +{ + if (get_options()->UseDeprecatedGuardAlgorithm) { + entry_guards_compute_status(get_options(), approx_time()); + } else { + entry_guards_update_all(get_guard_selection_info()); + } +} + +/** Helper: pick a guard for a circuit, with whatever algorithm is + used. */ +const node_t * +guards_choose_guard(cpath_build_state_t *state, + circuit_guard_state_t **guard_state_out) +{ + if (get_options()->UseDeprecatedGuardAlgorithm) { + return choose_random_entry(state); + } else { + // XXXX prop271 we need to look at the chosen exit node if any, and + // not duplicate it. + const node_t *r = NULL; + if (entry_guard_pick_for_circuit(get_guard_selection_info(), + &r, + guard_state_out) < 0) { + tor_assert(r == NULL); + } + return r; + } +} + +/** Helper: pick a directory guard, with whatever algorithm is used. */ +const node_t * +guards_choose_dirguard(dirinfo_type_t info, + circuit_guard_state_t **guard_state_out) +{ + if (get_options()->UseDeprecatedGuardAlgorithm) { + return choose_random_dirguard(info); + } else { + // XXXX prop271 look at info? + const node_t *r = NULL; + if (entry_guard_pick_for_circuit(get_guard_selection_info(), + &r, + guard_state_out) < 0) { + tor_assert(r == NULL); + } + return r; + } +} + /** Free one guard selection context */ STATIC void guard_selection_free(guard_selection_t *gs) diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 60191ab..7dcedd6 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -328,7 +328,7 @@ void entry_guard_chan_failed(guard_selection_t *gs, channel_t *chan); void entry_guards_update_all(guard_selection_t *gs); int entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, - smartlist_t *all_circuits, + const smartlist_t *all_circuits, smartlist_t *newly_complete_out); void entry_guards_note_internet_connectivity(guard_selection_t *gs); diff --git a/src/or/main.c b/src/or/main.c index a508003..65d1c1f 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -979,7 +979,7 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) /* if we have enough dir info, then update our guard status with * whatever we just learned. */ - entry_guards_compute_status(options, now); + guards_update_all(); /* Don't even bother trying to get extrainfo until the rest of our * directory info is up-to-date */ if (options->DownloadExtraInfo) @@ -1376,6 +1376,9 @@ run_scheduled_events(time_t now) /* 0c. If we've deferred log messages for the controller, handle them now */ flush_pending_log_callbacks(); + /* Maybe enough time elapsed for us to reconsider a circuit. */ + circuit_upgrade_circuits_from_guard_wait(); + if (options->UseBridges && !options->DisableNetwork) { fetch_bridge_descriptors(options, now); } diff --git a/src/or/or.h b/src/or/or.h index c8f39f9..8b9ede3 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1786,6 +1786,10 @@ typedef struct dir_connection_t { /** What rendezvous service are we querying for? */ rend_data_t *rend_data; + /** If this is a one-hop connection, tracks the state of the directory guard + * for this connection (if any). */ + struct circuit_guard_state_t *guard_state; + char identity_digest[DIGEST_LEN]; /**< Hash of the public RSA key for * the directory server's signing key. */ diff --git a/src/or/rendclient.c b/src/or/rendclient.c index b0dcf52..06744ad 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -762,7 +762,7 @@ directory_get_from_hs_dir(const char *desc_id, how_to_fetch, desc_id_base32, NULL, 0, 0, - rend_query); + rend_query, NULL); log_info(LD_REND, "Sending fetch request for v2 descriptor for " "service '%s' with descriptor ID '%s', auth type %d, " "and descriptor cookie '%s' to hidden service " diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 8ffd0bc..cf57c7d 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -3452,7 +3452,7 @@ directory_post_to_hs_dir(rend_service_descriptor_t *renddesc, DIRIND_ANONYMOUS, NULL, desc->desc_str, strlen(desc->desc_str), - 0, rend_data); + 0, rend_data, NULL); rend_data_free(rend_data); base32_encode(desc_id_base32, sizeof(desc_id_base32), desc->desc_id, DIGEST_LEN); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index f51b50e..d2f360a 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -971,7 +971,7 @@ authority_certs_fetch_resource_impl(const char *resource, directory_initiate_command_routerstatus(rs, DIR_PURPOSE_FETCH_CERTIFICATE, 0, indirection, resource, NULL, - 0, 0); + 0, 0, NULL); return; } @@ -4946,7 +4946,7 @@ MOCK_IMPL(STATIC void, initiate_descriptor_downloads, directory_initiate_command_routerstatus(source, purpose, ROUTER_PURPOSE_GENERAL, DIRIND_ONEHOP, - resource, NULL, 0, 0); + resource, NULL, 0, 0, NULL); } else { directory_get_from_dirserver(purpose, ROUTER_PURPOSE_GENERAL, resource, pds_flags, DL_WANT_ANY_DIRSERVER); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 6f83cef..4501d6b 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -23,6 +23,7 @@ #include "directory.h" #include "dirserv.h" #include "dirvote.h" +#include "entrynodes.h" #include "hibernate.h" #include "memarea.h" #include "networkstatus.h" @@ -4397,7 +4398,8 @@ directory_initiate_command_routerstatus, (const routerstatus_t *status, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since)); + time_t if_modified_since, + circuit_guard_state_t *guardstate)); static void test_dir_should_not_init_request_to_ourselves(void *data) @@ -4504,7 +4506,8 @@ NS(directory_initiate_command_routerstatus)(const routerstatus_t *status, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since) + time_t if_modified_since, + circuit_guard_state_t *guardstate) { (void)status; (void)dir_purpose; @@ -4514,6 +4517,7 @@ NS(directory_initiate_command_routerstatus)(const routerstatus_t *status, (void)payload; (void)payload_len; (void)if_modified_since; + (void)guardstate; CALLED(directory_initiate_command_routerstatus)++; }

1 0

[tor/master] Test get_guard_selection_by_name
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit a7bc73935b030100b0d7b9f39c5dec5ef6eb0a85 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Nov 23 15:08:07 2016 -0500 Test get_guard_selection_by_name --- src/or/entrynodes.c | 2 +- src/or/entrynodes.h | 2 ++ src/test/test_entrynodes.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 860be9b..cf35b02 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -201,7 +201,7 @@ guard_selection_new(const char *name) * create_if_absent is true, then create and return it. If there * is none, and create_if_absent is false, then return NULL. */ -static guard_selection_t * +STATIC guard_selection_t * get_guard_selection_by_name(const char *name, int create_if_absent) { if (!guard_contexts) { diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index a514c13..285664d 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -416,6 +416,8 @@ 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(const char *name); +STATIC guard_selection_t *get_guard_selection_by_name( + const char *name, int create_if_absent); 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); diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index cdf8672..785503b 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -1236,6 +1236,52 @@ test_entry_guard_parse_from_state_partial_failure(void *arg) } static void +test_entry_guard_get_guard_selection_by_name(void *arg) +{ + (void)arg; + guard_selection_t *gs1, *gs2, *gs3; + + gs1 = get_guard_selection_by_name("unlikely", 0); + tt_assert(gs1 == NULL); + gs1 = get_guard_selection_by_name("unlikely", 1); + tt_assert(gs1 != NULL); + gs2 = get_guard_selection_by_name("unlikely", 1); + tt_assert(gs2 == gs1); + gs2 = get_guard_selection_by_name("unlikely", 0); + tt_assert(gs2 == gs1); + + gs2 = get_guard_selection_by_name("implausible", 0); + tt_assert(gs2 == NULL); + gs2 = get_guard_selection_by_name("implausible", 1); + tt_assert(gs2 != NULL); + tt_assert(gs2 != gs1); + gs3 = get_guard_selection_by_name("implausible", 0); + tt_assert(gs3 == gs2); + + gs3 = get_guard_selection_by_name("default", 0); + tt_assert(gs3 == NULL); + gs3 = get_guard_selection_by_name("default", 1); + tt_assert(gs3 != NULL); + tt_assert(gs3 != gs2); + tt_assert(gs3 != gs1); + tt_assert(gs3 == get_guard_selection_info()); + +#if 0 + or_options_t *options = get_options_mutable(); + options->UseDeprecatedGuardAlgorithm = 1; + gs4 = get_guard_selection_info(); + tt_assert(gs4 != gs3); + tt_assert(gs4 == get_guard_selection_by_name("legacy", 1)); + + options->UseDeprecatedGuardAlgorithm = 0; + tt_assert(gs3 == get_guard_selection_info()); +#endif + + done: + entry_guards_free_all(); +} + +static void test_entry_guard_add_single_guard(void *arg) { (void)arg; @@ -2245,6 +2291,8 @@ struct testcase_t entrynodes_tests[] = { test_entry_guard_parse_from_state_failure, 0, NULL, NULL }, { "parse_from_state_partial_failure", test_entry_guard_parse_from_state_partial_failure, 0, NULL, NULL }, + { "get_guard_selection_by_name", + test_entry_guard_get_guard_selection_by_name, TT_FORK, NULL, NULL }, BFN_TEST(add_single_guard), BFN_TEST(node_filter), BFN_TEST(expand_sample),

1 0

[tor/master] Add a wrapper for a common networkstatus param pattern
by nickm＠torproject.org 16 Dec '16

16 Dec '16

commit 039bd01767d42961cb16ff4914481332b52cf8db Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Nov 26 09:22:04 2016 -0500 Add a wrapper for a common networkstatus param pattern We frequently want to check a networkstatus parameter only when it isn't overridden from the torrc file. --- src/or/networkstatus.c | 19 +++++++++++++++++++ src/or/networkstatus.h | 5 +++++ src/test/test_dir.c | 9 +++++++++ 3 files changed, 33 insertions(+) diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index ec8f77f..ce23d67 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -2304,6 +2304,25 @@ networkstatus_get_param(const networkstatus_t *ns, const char *param_name, } /** + * As networkstatus_get_param(), but check torrc_value before checking the + * consensus. If torrc_value is in-range, then return it instead of the + * value from the consensus. + */ +int32_t +networkstatus_get_overridable_param(const networkstatus_t *ns, + int32_t torrc_value, + const char *param_name, + int32_t default_val, + int32_t min_val, int32_t max_val) +{ + if (torrc_value >= min_val && torrc_value <= max_val) + return torrc_value; + else + return networkstatus_get_param( + ns, param_name, default_val, min_val, max_val); +} + +/** * Retrieve the consensus parameter that governs the * fixed-point precision of our network balancing 'bandwidth-weights' * (which are themselves integer consensus values). We divide them diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index 71f36b6..4b3854d 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -111,6 +111,11 @@ int32_t networkstatus_get_param(const networkstatus_t *ns, const char *param_name, int32_t default_val, int32_t min_val, int32_t max_val); +int32_t networkstatus_get_overridable_param(const networkstatus_t *ns, + int32_t torrc_value, + const char *param_name, + int32_t default_val, + int32_t min_val, int32_t max_val); int getinfo_helper_networkstatus(control_connection_t *conn, const char *question, char **answer, const char **errmsg); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 4501d6b..4ef421f 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -1494,6 +1494,15 @@ test_dir_param_voting(void *arg) tt_int_op(-8,OP_EQ, networkstatus_get_param(&vote4, "ab", -12, -100, -8)); tt_int_op(0,OP_EQ, networkstatus_get_param(&vote4, "foobar", 0, -100, 8)); + tt_int_op(100,OP_EQ, networkstatus_get_overridable_param( + &vote4, -1, "x-yz", 50, 0, 300)); + tt_int_op(30,OP_EQ, networkstatus_get_overridable_param( + &vote4, 30, "x-yz", 50, 0, 300)); + tt_int_op(0,OP_EQ, networkstatus_get_overridable_param( + &vote4, -101, "foobar", 0, -100, 8)); + tt_int_op(-99,OP_EQ, networkstatus_get_overridable_param( + &vote4, -99, "foobar", 0, -100, 8)); + smartlist_add(votes, &vote1); /* Do the first tests without adding all the other votes, for

1 0