From a8297cdbd3324ac707165ae9922ecf478c4608a1 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Wed, 25 Jan 2012 20:18:51 -0500 Subject: [PATCH 1/3] use microdescriptors if *any* of our bridges can handle them Now as we move into a future where most bridges can handle microdescs we will generally find ourselves using them, rather than holding back just because one of our bridges doesn't use them. --- changes/feature4994 | 7 +++++ src/or/circuitbuild.c | 3 ++- src/or/directory.c | 12 +++++---- src/or/entrynodes.c | 59 ++++++++++++++++++++++++++++--------------- src/or/entrynodes.h | 5 ++-- src/or/microdesc.c | 4 +-- 6 files changed, 60 insertions(+), 30 deletions(-) create mode 100644 changes/feature4994 diff --git a/changes/feature4994 b/changes/feature4994 new file mode 100644 index 0000000000..4fa0e037b7 --- /dev/null +++ b/changes/feature4994 @@ -0,0 +1,7 @@ + o Minor features: + - Teach bridge-using clients to avoid 0.2.2 bridges when making + microdescriptor-related dir requests, and only fall back to normal + descriptors if none of their bridges can handle microdescriptors + (as opposed to the fix in ticket 4013, which caused them to fall + back to normal descriptors if *any* of their bridges preferred + them). Resolves ticket 4994. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 5a5a3afea7..8c86aac90c 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -25,6 +25,7 @@ #include "directory.h" #include "entrynodes.h" #include "main.h" +#include "microdesc.h" #include "networkstatus.h" #include "nodelist.h" #include "onion.h" @@ -3372,7 +3373,7 @@ 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); + return choose_random_entry(state, 0); } excluded = smartlist_new(); diff --git a/src/or/directory.c b/src/or/directory.c index c101418446..c1bbe6bd81 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -472,12 +472,14 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose, if (options->UseBridges && type != BRIDGE_DIRINFO) { /* We want to ask a running bridge for which we have a descriptor. * - * Be careful here: we should only ask questions that we know our - * bridges can answer. So far we're solving that by backing off to - * the behavior supported by our oldest bridge; see for example - * any_bridges_dont_support_microdescriptors(). + * When we ask choose_random_entry() for a bridge, we specify that + * we're going to be using it for a dir fetch: if any of our bridges + * can handle microdescriptor questions, we'll get one of the ones + * that can. */ - const node_t *node = choose_random_entry(NULL); + /* XXX024 Not all bridges handle conditional consensus downloading, + * so, for now, never assume the server supports that. -PP */ + const node_t *node = choose_random_entry(NULL, 1); if (node && node->ri) { /* every bridge has a routerinfo. */ tor_addr_t addr; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 4ca56cbacf..d029d85935 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -23,6 +23,7 @@ #include "directory.h" #include "entrynodes.h" #include "main.h" +#include "microdesc.h" #include "nodelist.h" #include "policies.h" #include "router.h" @@ -63,7 +64,8 @@ static int entry_guards_dirty = 0; static void bridge_free(bridge_info_t *bridge); static const node_t *choose_random_entry_impl(cpath_build_state_t *state, int for_directory, - dirinfo_type_t dirtype); + dirinfo_type_t dirtype, + int prefer_microdescs); /** Return the list of entry guards, creating it if necessary. */ const smartlist_t * @@ -829,15 +831,33 @@ entry_list_is_constrained(const or_options_t *options) return 0; } +/** Return true iff this node can answer directory questions about + * microdescriptors. */ +static int +node_understands_microdescriptors(const node_t *node) +{ + tor_assert(node); + if (node->rs && node->rs->version_supports_microdesc_cache) + return 1; + if (node->ri && tor_version_supports_microdescriptors(node->ri->platform)) + return 1; + return 0; +} + /** Pick a live (up and listed) entry guard from entry_guards. If * state is non-NULL, this is for a specific circuit -- * make sure not to pick this circuit's exit or any node in the * exit's family. If state is NULL, we're looking for a random - * guard (likely a bridge). */ + * guard (likely a bridge). + * + * If the prefer_microdescs flag is set, we are looking for a bridge to + * use for directory fetching and pick a bridge that supports microdescriptors + * if we know any that do so. + */ const node_t * -choose_random_entry(cpath_build_state_t *state) +choose_random_entry(cpath_build_state_t *state, int prefer_microdescs) { - return choose_random_entry_impl(state, 0, 0); + return choose_random_entry_impl(state, 0, 0, prefer_microdescs); } /** Pick a live (up and listed) directory guard from entry_guards for @@ -845,13 +865,13 @@ choose_random_entry(cpath_build_state_t *state) const node_t * choose_random_dirguard(dirinfo_type_t type) { - return choose_random_entry_impl(NULL, 1, type); + return choose_random_entry_impl(NULL, 1, type, 0); } /** Helper for choose_random{entry,dirguard}. */ static const node_t * choose_random_entry_impl(cpath_build_state_t *state, int for_directory, - dirinfo_type_t dirinfo_type) + dirinfo_type_t dirinfo_type, int prefer_microdescs) { const or_options_t *options = get_options(); smartlist_t *live_entry_guards = smartlist_new(); @@ -903,6 +923,10 @@ choose_random_entry_impl(cpath_build_state_t *state, int for_directory, continue; /* don't pick the same node for entry and exit */ if (consider_exit_family && smartlist_contains(exit_family, node)) continue; /* avoid relays that are family members of our exit */ + if (prefer_microdescs && + we_use_microdescriptors_for_circuits(options) && + !node_understands_microdescriptors(node)) + continue; /* this node won't be able to answer our dir questions */ #if 0 /* since EntryNodes is always strict now, this clause is moot */ if (options->EntryNodes && !routerset_contains_node(options->EntryNodes, node)) { @@ -1982,7 +2006,7 @@ int any_bridge_descriptors_known(void) { tor_assert(get_options()->UseBridges); - return choose_random_entry(NULL)!=NULL ? 1 : 0; + return choose_random_entry(NULL, 0)!=NULL ? 1 : 0; } /** Return 1 if there are any directory conns fetching bridge descriptors @@ -2064,29 +2088,24 @@ entries_retry_all(const or_options_t *options) entries_retry_helper(options, 1); } -/** Return true if we've ever had a bridge running a Tor version that can't - * provide microdescriptors to us. In that case fall back to asking for - * full descriptors. Eventually all bridges will support microdescriptors - * and we can take this check out; see bug 4013. */ +/** Return true if at least one of our bridges runs a Tor version that can + * provide microdescriptors to us. If not, we'll fall back to asking for + * full descriptors. */ int -any_bridges_dont_support_microdescriptors(void) +any_bridge_supports_microdescriptors(void) { const node_t *node; - static int ever_answered_yes = 0; if (!get_options()->UseBridges || !entry_guards) return 0; - if (ever_answered_yes) - return 1; /* if we ever answer 'yes', always answer 'yes' */ SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, e) { node = node_get_by_id(e->identity); - if (node && node->ri && + if (node && node->is_running && node_is_bridge(node) && node_is_a_configured_bridge(node) && - !tor_version_supports_microdescriptors(node->ri->platform)) { + node_understands_microdescriptors(node)) { /* This is one of our current bridges, and we know enough about - * it to know that it won't be able to answer our microdescriptor + * it to know that it will be able to answer our microdescriptor * questions. */ - ever_answered_yes = 1; - return 1; + return 1; } } SMARTLIST_FOREACH_END(e); return 0; diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index e6c973c95a..8ed429c8fb 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -78,7 +78,8 @@ int entry_guard_register_connect_status(const char *digest, int succeeded, int mark_relay_status, time_t now); void entry_nodes_should_be_added(void); int entry_list_is_constrained(const or_options_t *options); -const node_t *choose_random_entry(cpath_build_state_t *state); +const node_t *choose_random_entry(cpath_build_state_t *state, + int prefer_microdescs); const node_t *choose_random_dirguard(dirinfo_type_t t); int entry_guards_parse_state(or_state_t *state, int set, char **msg); void entry_guards_update_state(or_state_t *state); @@ -104,7 +105,7 @@ int any_pending_bridge_descriptor_fetches(void); int entries_known_but_down(const or_options_t *options); void entries_retry_all(const or_options_t *options); -int any_bridges_dont_support_microdescriptors(void); +int any_bridge_supports_microdescriptors(void); void entry_guards_free_all(void); diff --git a/src/or/microdesc.c b/src/or/microdesc.c index e99b3ebe78..ac48930faf 100644 --- a/src/or/microdesc.c +++ b/src/or/microdesc.c @@ -730,9 +730,9 @@ we_use_microdescriptors_for_circuits(const or_options_t *options) int ret = options->UseMicrodescriptors; if (ret == -1) { /* UseMicrodescriptors is "auto"; we need to decide: */ - /* If we are configured to use bridges and one of our bridges doesn't + /* If we are configured to use bridges and none of our bridges * know what a microdescriptor is, the answer is no. */ - if (options->UseBridges && any_bridges_dont_support_microdescriptors()) + if (options->UseBridges && !any_bridge_supports_microdescriptors()) return 0; /* Otherwise, we decide that we'll use microdescriptors iff we are * not a server, and we're not autofetching everything. */ From bce5019eff37fc741747ef76c5d0a387569f9265 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Wed, 25 Jan 2012 20:49:32 -0500 Subject: [PATCH 2/3] generalize choose_random_entry()'s dirinfo parameter Now we can specify to skip bridges that wouldn't be able to answer the type of dir fetch we're launching. It's still the responsibility of the rest of the code to prevent us from launching a given dir fetch if we have no bridges that could handle it. --- src/or/circuitbuild.c | 2 +- src/or/directory.c | 9 ++++----- src/or/entrynodes.c | 38 ++++++++++++++++++++++---------------- src/or/entrynodes.h | 2 +- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 8c86aac90c..cea1cbd52b 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -3373,7 +3373,7 @@ 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, 0); + return choose_random_entry(state, NO_DIRINFO); } excluded = smartlist_new(); diff --git a/src/or/directory.c b/src/or/directory.c index c1bbe6bd81..a1ac2ad2e6 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -472,14 +472,13 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose, if (options->UseBridges && type != BRIDGE_DIRINFO) { /* We want to ask a running bridge for which we have a descriptor. * - * When we ask choose_random_entry() for a bridge, we specify that - * we're going to be using it for a dir fetch: if any of our bridges - * can handle microdescriptor questions, we'll get one of the ones - * that can. + * When we ask choose_random_entry() for a bridge, we specify what + * sort of dir fetch we'll be doing, so it won't return a bridge + * that can't answer our question. */ /* XXX024 Not all bridges handle conditional consensus downloading, * so, for now, never assume the server supports that. -PP */ - const node_t *node = choose_random_entry(NULL, 1); + const node_t *node = choose_random_entry(NULL, type); if (node && node->ri) { /* every bridge has a routerinfo. */ tor_addr_t addr; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index d029d85935..dab081d1fc 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -64,8 +64,7 @@ static int entry_guards_dirty = 0; static void bridge_free(bridge_info_t *bridge); static const node_t *choose_random_entry_impl(cpath_build_state_t *state, int for_directory, - dirinfo_type_t dirtype, - int prefer_microdescs); + dirinfo_type_t dirtype); /** Return the list of entry guards, creating it if necessary. */ const smartlist_t * @@ -844,20 +843,28 @@ node_understands_microdescriptors(const node_t *node) return 0; } +/** Return true iff node is able to answer directory questions + * of type dirinfo. */ +static int +node_can_handle_dirinfo(const node_t *node, dirinfo_type_t dirinfo) +{ + if ((dirinfo & MICRODESC_DIRINFO) && + !node_understands_microdescriptors(node)) + return 0; + return 1; +} + /** Pick a live (up and listed) entry guard from entry_guards. If * state is non-NULL, this is for a specific circuit -- * make sure not to pick this circuit's exit or any node in the * exit's family. If state is NULL, we're looking for a random - * guard (likely a bridge). - * - * If the prefer_microdescs flag is set, we are looking for a bridge to - * use for directory fetching and pick a bridge that supports microdescriptors - * if we know any that do so. - */ + * guard (likely a bridge). If dirinfo is not NO_DIRINFO, then + * only select from nodes that know how to answer directory questions + * of that type. */ const node_t * -choose_random_entry(cpath_build_state_t *state, int prefer_microdescs) +choose_random_entry(cpath_build_state_t *state, dirinfo_type_t dirinfo) { - return choose_random_entry_impl(state, 0, 0, prefer_microdescs); + return choose_random_entry_impl(state, 0, dirinfo); } /** Pick a live (up and listed) directory guard from entry_guards for @@ -865,13 +872,13 @@ choose_random_entry(cpath_build_state_t *state, int prefer_microdescs) const node_t * choose_random_dirguard(dirinfo_type_t type) { - return choose_random_entry_impl(NULL, 1, type, 0); + return choose_random_entry_impl(NULL, 1, type); } /** Helper for choose_random{entry,dirguard}. */ static const node_t * choose_random_entry_impl(cpath_build_state_t *state, int for_directory, - dirinfo_type_t dirinfo_type, int prefer_microdescs) + dirinfo_type_t dirinfo_type) { const or_options_t *options = get_options(); smartlist_t *live_entry_guards = smartlist_new(); @@ -923,9 +930,8 @@ choose_random_entry_impl(cpath_build_state_t *state, int for_directory, continue; /* don't pick the same node for entry and exit */ if (consider_exit_family && smartlist_contains(exit_family, node)) continue; /* avoid relays that are family members of our exit */ - if (prefer_microdescs && - we_use_microdescriptors_for_circuits(options) && - !node_understands_microdescriptors(node)) + if (dirinfo_type != NO_DIRINFO && + !node_can_handle_dirinfo(node, dirinfo_type)) continue; /* this node won't be able to answer our dir questions */ #if 0 /* since EntryNodes is always strict now, this clause is moot */ if (options->EntryNodes && @@ -2006,7 +2012,7 @@ int any_bridge_descriptors_known(void) { tor_assert(get_options()->UseBridges); - return choose_random_entry(NULL, 0)!=NULL ? 1 : 0; + return choose_random_entry(NULL, NO_DIRINFO)!=NULL ? 1 : 0; } /** Return 1 if there are any directory conns fetching bridge descriptors diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 8ed429c8fb..235ef4bfd1 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -79,7 +79,7 @@ int entry_guard_register_connect_status(const char *digest, int succeeded, void entry_nodes_should_be_added(void); int entry_list_is_constrained(const or_options_t *options); const node_t *choose_random_entry(cpath_build_state_t *state, - int prefer_microdescs); + dirinfo_type_t dirinfo); const node_t *choose_random_dirguard(dirinfo_type_t t); int entry_guards_parse_state(or_state_t *state, int set, char **msg); void entry_guards_update_state(or_state_t *state); From d7089ff228227259137b5a8bc32d0764a0ad4155 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 12 Feb 2013 16:23:12 -0500 Subject: [PATCH 3/3] Restore the entry/dirguard distinction. We shouldn't be calling choose_random_entry() for directory conncetions; that's what choose_random_dirguard() is for. --- src/or/circuitbuild.c | 2 +- src/or/directory.c | 2 +- src/or/entrynodes.c | 20 +++++++++++--------- src/or/entrynodes.h | 3 +-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index cea1cbd52b..753eaf8c16 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -3373,7 +3373,7 @@ 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, NO_DIRINFO); + return choose_random_entry(state); } excluded = smartlist_new(); diff --git a/src/or/directory.c b/src/or/directory.c index a1ac2ad2e6..6b61fc6a99 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -478,7 +478,7 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose, */ /* XXX024 Not all bridges handle conditional consensus downloading, * so, for now, never assume the server supports that. -PP */ - const node_t *node = choose_random_entry(NULL, type); + const node_t *node = choose_random_dirguard(type); if (node && node->ri) { /* every bridge has a routerinfo. */ tor_addr_t addr; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index dab081d1fc..51c3a56742 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -848,6 +848,14 @@ node_understands_microdescriptors(const node_t *node) static int node_can_handle_dirinfo(const node_t *node, dirinfo_type_t dirinfo) { + /* Checking dirinfo for any type other than microdescriptors isn't required + yet, since we only choose directory guards that can support microdescs, + routerinfos, and networkstatuses, AND we don't use directory guards if + we're configured to do direct downloads of anything else. The only case + where we might have a guard that doesn't know about a type of directory + information is when we're retrieving directory information from a + bridge. */ + if ((dirinfo & MICRODESC_DIRINFO) && !node_understands_microdescriptors(node)) return 0; @@ -862,9 +870,9 @@ node_can_handle_dirinfo(const node_t *node, dirinfo_type_t dirinfo) * only select from nodes that know how to answer directory questions * of that type. */ const node_t * -choose_random_entry(cpath_build_state_t *state, dirinfo_type_t dirinfo) +choose_random_entry(cpath_build_state_t *state) { - return choose_random_entry_impl(state, 0, dirinfo); + return choose_random_entry_impl(state, 0, 0); } /** Pick a live (up and listed) directory guard from entry_guards for @@ -893,12 +901,6 @@ choose_random_entry_impl(cpath_build_state_t *state, int for_directory, const int num_needed = for_directory ? options->NumDirectoryGuards : options->NumEntryGuards; - /* Checking dirinfo_type isn't required yet, since we only choose directory - guards that can support microdescs, routerinfos, and networkstatuses, AND - we don't use directory guards if we're configured to do direct downloads - of anything else. */ - (void) dirinfo_type; - if (chosen_exit) { nodelist_add_node_and_family(exit_family, chosen_exit); consider_exit_family = 1; @@ -2012,7 +2014,7 @@ int any_bridge_descriptors_known(void) { tor_assert(get_options()->UseBridges); - return choose_random_entry(NULL, NO_DIRINFO)!=NULL ? 1 : 0; + return choose_random_entry(NULL) != NULL; } /** Return 1 if there are any directory conns fetching bridge descriptors diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 235ef4bfd1..e774a61a4f 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -78,8 +78,7 @@ int entry_guard_register_connect_status(const char *digest, int succeeded, int mark_relay_status, time_t now); void entry_nodes_should_be_added(void); int entry_list_is_constrained(const or_options_t *options); -const node_t *choose_random_entry(cpath_build_state_t *state, - dirinfo_type_t dirinfo); +const node_t *choose_random_entry(cpath_build_state_t *state); const node_t *choose_random_dirguard(dirinfo_type_t t); int entry_guards_parse_state(or_state_t *state, int set, char **msg); void entry_guards_update_state(or_state_t *state);