From 93b39c51629ed0ded2bf807cb6a06d59f3a55347 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jan 2017 11:35:57 -0500 Subject: [PATCH 1/5] Downgrade assertion to nonfatal for #21242 This assertion triggered in the (error) case where we got a result from guards_choose_guard() without a descriptor. That's not supposed to be possible, but it's not worth crashing over. --- src/or/circuitbuild.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index f11c865ad0..88445f9248 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2369,7 +2369,7 @@ onion_extend_cpath(origin_circuit_t *circ) int client = (server_mode(get_options()) == 0); info = extend_info_from_node(r, client); /* Clients can fail to find an allowed address */ - tor_assert(info || client); + tor_assert_nonfatal(info || client); } } else { const node_t *r = From 26957a127a80404ca243e50c3f0c2bac01e480b8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jan 2017 11:46:06 -0500 Subject: [PATCH 2/5] entry_guard_pick_for_circuit(): TRAFFIC guards must have descriptors This relates to the 21242 fix -- entry_guard_pick_for_circuit() should never yield nodes without descriptors when the node is going to be used for traffic, since we won't be able to extend through them. --- src/or/entrynodes.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index c5fb92e35a..344b69d65f 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2068,6 +2068,8 @@ entry_guard_pick_for_circuit(guard_selection_t *gs, // XXXX prop271 check Ed ID. if (! node) goto fail; + if (BUG(usage != GUARD_USAGE_DIRGUARD && !node_has_descriptor(node))) + goto fail; *chosen_node_out = node; *guard_state_out = tor_malloc_zero(sizeof(circuit_guard_state_t)); From 02da24f8e5b2ce34da941cc25ce9808b447065ac Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jan 2017 12:30:33 -0500 Subject: [PATCH 3/5] Don't (usually) return any guards that are missing descriptors. Actually, it's _fine_ to use a descriptorless guard for fetching directory info -- we just shouldn't use it when building circuits. Fortunately, we already have a "usage" flag that we can use here. Partial fix for bug 21242. --- src/or/entrynodes.c | 25 ++++++++++++++++++++++++- src/or/entrynodes.h | 1 + 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 344b69d65f..1cf1ff2c8e 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -185,6 +185,16 @@ should_apply_guardfraction(const networkstatus_t *ns) return options->UseGuardFraction; } +/** Return true iff we know a descriptor for guard */ +static int +guard_has_descriptor(const entry_guard_t *guard) +{ + const node_t *node = node_get_by_id(guard->identity); + if (!node) + return 0; + return node_has_descriptor(node); +} + /** * Try to determine the correct type for a selection named "name", * if type is GS_TYPE_INFER. @@ -1447,6 +1457,7 @@ sample_reachable_filtered_entry_guards(guard_selection_t *gs, 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; + const unsigned need_descriptor = flags & SAMPLE_EXCLUDE_NO_DESCRIPTOR; SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) { entry_guard_consider_retry(guard); @@ -1480,6 +1491,8 @@ sample_reachable_filtered_entry_guards(guard_selection_t *gs, continue; if (exclude_pending && guard->is_pending) continue; + if (need_descriptor && !guard_has_descriptor(guard)) + continue; smartlist_add(reachable_filtered_sample, guard); } SMARTLIST_FOREACH_END(guard); @@ -1777,6 +1790,7 @@ select_entry_guard_for_circuit(guard_selection_t *gs, const entry_guard_restriction_t *rst, unsigned *state_out) { + const int need_descriptor = (usage == GUARD_USAGE_TRAFFIC); tor_assert(gs); tor_assert(state_out); @@ -1793,6 +1807,9 @@ select_entry_guard_for_circuit(guard_selection_t *gs, if (! entry_guard_obeys_restriction(guard, rst)) continue; if (guard->is_reachable != GUARD_REACHABLE_NO) { + if (need_descriptor && BUG(!guard_has_descriptor(guard))) { + continue; + } *state_out = GUARD_CIRC_STATE_USABLE_ON_COMPLETION; guard->last_tried_to_connect = approx_time(); smartlist_add(usable_primary_guards, guard); @@ -1821,6 +1838,8 @@ select_entry_guard_for_circuit(guard_selection_t *gs, continue; entry_guard_consider_retry(guard); if (guard->is_usable_filtered_guard && ! guard->is_pending) { + if (need_descriptor && !guard_has_descriptor(guard)) + continue; /* not a bug */ guard->is_pending = 1; guard->last_tried_to_connect = approx_time(); *state_out = GUARD_CIRC_STATE_USABLE_IF_NO_BETTER_GUARD; @@ -1836,11 +1855,15 @@ select_entry_guard_for_circuit(guard_selection_t *gs, random from {USABLE_FILTERED_GUARDS}." */ { entry_guard_t *guard; + unsigned flags = 0; + if (need_descriptor) + flags |= SAMPLE_EXCLUDE_NO_DESCRIPTOR; guard = sample_reachable_filtered_entry_guards(gs, rst, SAMPLE_EXCLUDE_CONFIRMED | SAMPLE_EXCLUDE_PRIMARY | - SAMPLE_EXCLUDE_PENDING); + SAMPLE_EXCLUDE_PENDING | + flags); if (guard == NULL) { log_info(LD_GUARD, "Absolutely no sampled guards were available."); return NULL; diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index d469df7e3d..0c29292c17 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -498,6 +498,7 @@ STATIC int entry_guards_all_primary_guards_are_down(guard_selection_t *gs); #define SAMPLE_EXCLUDE_PRIMARY (1u<<1) #define SAMPLE_EXCLUDE_PENDING (1u<<2) #define SAMPLE_NO_UPDATE_PRIMARY (1u<<3) +#define SAMPLE_EXCLUDE_NO_DESCRIPTOR (1u<<4) /**@}*/ STATIC entry_guard_t *sample_reachable_filtered_entry_guards( guard_selection_t *gs, From 746d959100f0275e3ad4442e89901796f669413e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jan 2017 12:31:43 -0500 Subject: [PATCH 4/5] Don't build circuits till primary guards have descriptors In addition to not wanting to build circuits until we can see most of the paths in the network, and in addition to not wanting to build circuits until we have a consensus ... we shouldn't build circuits till all of our (in-use) primary guards have descriptors that we can use for them. This is another bug 21242 fix. --- src/or/entrynodes.c | 37 +++++++++++++++++++++++++++++++++++++ src/or/entrynodes.h | 5 +++++ src/or/nodelist.c | 8 ++++++++ 3 files changed, 50 insertions(+) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 1cf1ff2c8e..6249e847a9 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -3335,6 +3335,43 @@ guards_retry_optimistic(const or_options_t *options) return 1; } +/** + * Return true iff we know enough directory information to construct + * circuits through all of the primary guards we'd currently use. + */ +int +guard_selection_have_enough_dir_info_to_build_circuits(guard_selection_t *gs) +{ + if (!gs->primary_guards_up_to_date) + entry_guards_update_primary(gs); + + const int num_primary = get_n_primary_guards_to_use(GUARD_USAGE_TRAFFIC); + int n_missing_descriptors = 0; + int n_considered = 0; + + SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) { + entry_guard_consider_retry(guard); + if (guard->is_reachable == GUARD_REACHABLE_NO) + continue; + n_considered++; + if (!guard_has_descriptor(guard)) + n_missing_descriptors++; + if (n_considered >= num_primary) + break; + } SMARTLIST_FOREACH_END(guard); + + return n_missing_descriptors == 0; +} + +/** As guard_selection_have_enough_dir_info_to_build_circuits, but uses + * the default guard selection. */ +int +entry_guards_have_enough_dir_info_to_build_circuits(void) +{ + return guard_selection_have_enough_dir_info_to_build_circuits( + get_guard_selection_info()); +} + /** 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 0c29292c17..f02901f5d7 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -482,6 +482,7 @@ STATIC entry_guard_t *get_sampled_guard_with_id(guard_selection_t *gs, const uint8_t *rsa_id); MOCK_DECL(STATIC time_t, randomize_time, (time_t now, time_t max_backdate)); + STATIC entry_guard_t *entry_guard_add_to_sample(guard_selection_t *gs, const node_t *node); STATIC entry_guard_t *entry_guards_expand_sample(guard_selection_t *gs); @@ -566,6 +567,10 @@ int getinfo_helper_entry_guards(control_connection_t *conn, int entries_known_but_down(const or_options_t *options); void entries_retry_all(const or_options_t *options); +int guard_selection_have_enough_dir_info_to_build_circuits( + guard_selection_t *gs); +int entry_guards_have_enough_dir_info_to_build_circuits(void); + void entry_guards_free_all(void); double pathbias_get_close_success_count(entry_guard_t *guard); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 804af297ba..96e95baf5a 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -43,6 +43,7 @@ #include "config.h" #include "control.h" #include "dirserv.h" +#include "entrynodes.h" #include "geoip.h" #include "main.h" #include "microdesc.h" @@ -1991,6 +1992,13 @@ update_router_have_minimum_dir_info(void) using_md = consensus->flavor == FLAV_MICRODESC; + if (! entry_guards_have_enough_dir_info_to_build_circuits()) { + strlcpy(dir_info_status, "We're missing descriptors for some of our " + "primary entry guards", sizeof(dir_info_status)); + res = 0; + goto done; + } + /* Check fraction of available paths */ { char *status = NULL; From 957b9f8b83429a3dcd0258a2eab0f266adcbf87b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jan 2017 12:35:44 -0500 Subject: [PATCH 5/5] changes file for bug21242 --- changes/bug21242 | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changes/bug21242 diff --git a/changes/bug21242 b/changes/bug21242 new file mode 100644 index 0000000000..0bdc203afc --- /dev/null +++ b/changes/bug21242 @@ -0,0 +1,10 @@ + o Major bugfixes (entry guards): + - Stop trying to build circuits through entry guards for which we + have no descriptor yet. Also, stop crashing if we *do* accidentally + try to build a circuit in such a state. Fixes bug 21242; bugfix + on 0.3.0.1-alpha. + + o Minor features (entry guards): + - Do not try to build circuits until we have descriptors for our + primary entry guards. Related to fix for bug 21242. +