From 91027218e29090b18d42e1868367cc2a9e149900 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 14 Feb 2013 11:48:47 -0500 Subject: Add some code to bluntly prevent duplicate guards from getting added Apparently something in the directory guard code made it possible for the same node to get added as a guard over and over when there were no actual running guard nodes. --- src/or/entrynodes.c | 29 +++++++++++++++++++++++++++++ src/or/or.h | 3 +++ 2 files changed, 32 insertions(+) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 4ca56cbacf..3e471ed01e 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -369,6 +369,15 @@ add_an_entry_guard(const node_t *chosen, int reset_status, int prepend, if (!node) return NULL; } + if (node->using_as_guard) + return NULL; + if (entry_guard_get_by_id_digest(node->identity) != NULL) { + log_info(LD_CIRC, "I was about to add a duplicate entry guard."); + /* This can happen if we choose a guard, then the node goes away, then + * comes back. */ + ((node_t*) node)->using_as_guard = 1; + return NULL; + } entry = tor_malloc_zero(sizeof(entry_guard_t)); log_info(LD_CIRC, "Chose %s as new entry guard.", node_describe(node)); @@ -384,6 +393,7 @@ add_an_entry_guard(const node_t *chosen, int reset_status, int prepend, * this guard. For details, see the Jan 2010 or-dev thread. */ entry->chosen_on_date = time(NULL) - crypto_rand_int(3600*24*30); entry->chosen_by_version = tor_strdup(VERSION); + ((node_t*)node)->using_as_guard = 1; if (prepend) smartlist_insert(entry_guards, 0, entry); else @@ -723,6 +733,21 @@ entry_nodes_should_be_added(void) should_add_entry_nodes = 1; } +/** Update the using_as_guard fields of all the nodes. We do this after we + * remove entry guards from the list: This is the only function that clears + * the using_as_guard field. */ +static void +update_node_guard_status(void) +{ + smartlist_t *nodes = nodelist_get_list(); + SMARTLIST_FOREACH(nodes, node_t *, node, node->using_as_guard = 0); + SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, entry) { + node_t *node = node_get_mutable_by_id(entry->identity); + if (node) + node->using_as_guard = 1; + } SMARTLIST_FOREACH_END(entry); +} + /** Adjust the entry guards list so that it only contains entries from * EntryNodes, adding new entries from EntryNodes to the list as needed. */ static void @@ -807,6 +832,8 @@ entry_guards_set_from_config(const or_options_t *options) SMARTLIST_FOREACH(old_entry_guards_not_on_list, entry_guard_t *, e, entry_guard_free(e)); + update_node_guard_status(); + smartlist_free(entry_nodes); smartlist_free(worse_entry_nodes); smartlist_free(entry_fps); @@ -1231,6 +1258,8 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) * few lines, so we don't have to re-dirty it */ if (remove_obsolete_entry_guards(now)) entry_guards_dirty = 1; + + update_node_guard_status(); } digestmap_free(added_by, tor_free_); return *msg ? -1 : 0; diff --git a/src/or/or.h b/src/or/or.h index 04640d050a..df69674d34 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2233,6 +2233,9 @@ typedef struct node_t { /** Local info: we treat this node as if it rejects everything */ unsigned int rejects_all:1; + /** Local info: this node is in our list of guards */ + unsigned int using_as_guard:1; + /* Local info: derived. */ /** True if the IPv6 OR port is preferred over the IPv4 OR port. */ -- cgit v1.2.3-54-g00ecf From 1070a720ad7f45fa82b77be0512056a06e535b72 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 14 Feb 2013 12:06:59 -0500 Subject: Be more robust when excluding existing nodes as new dirguards In addition to rejecting them post-hoc, avoid picking them in the first place. This makes us less likely to decide that we can't add guards at all. --- src/or/circuitbuild.c | 1 + src/or/entrynodes.c | 2 +- src/or/or.h | 4 ++++ src/or/routerlist.c | 3 +++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 163afd3d29..c2f395338d 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -3397,6 +3397,7 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) }); } /* and exclude current entry guards and their families, if applicable */ + /*XXXX025 use the using_as_guard flag to accomplish this.*/ if (options->UseEntryGuards) { SMARTLIST_FOREACH(get_entry_guards(), const entry_guard_t *, entry, { diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 3e471ed01e..5dd27905d6 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -362,7 +362,7 @@ add_an_entry_guard(const node_t *chosen, int reset_status, int prepend, } else { const routerstatus_t *rs; rs = router_pick_directory_server(MICRODESC_DIRINFO|V3_DIRINFO, - PDS_PREFER_TUNNELED_DIR_CONNS_); + PDS_PREFER_TUNNELED_DIR_CONNS_|PDS_FOR_GUARD); if (!rs) return NULL; node = node_get_by_id(rs->identity_digest); diff --git a/src/or/or.h b/src/or/or.h index df69674d34..1cb9ef2f0e 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4760,6 +4760,10 @@ typedef struct dir_server_t { #define PDS_NO_EXISTING_SERVERDESC_FETCH (1<<3) #define PDS_NO_EXISTING_MICRODESC_FETCH (1<<4) +/** This node is to be chosen as a directory guard, so don't choose any + * node that's currently a guard. */ +#define PDS_FOR_GUARD (1<<5) + #define PDS_PREFER_TUNNELED_DIR_CONNS_ (1<<16) /** Possible ways to weight routers when choosing one randomly. See diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 90b707bcdb..837245db3e 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1153,6 +1153,7 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags) int requireother = ! (flags & PDS_ALLOW_SELF); int fascistfirewall = ! (flags & PDS_IGNORE_FASCISTFIREWALL); int prefer_tunnel = (flags & PDS_PREFER_TUNNELED_DIR_CONNS_); + int for_guard = (flags & PDS_FOR_GUARD); int try_excluding = 1, n_excluded = 0; if (!consensus) @@ -1192,6 +1193,8 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags) if ((type & MICRODESC_DIRINFO) && !is_trusted && !node->rs->version_supports_microdesc_cache) continue; + if (for_guard && node->using_as_guard) + continue; /* Don't make the same node a guard twice. */ if (try_excluding && routerset_contains_routerstatus(options->ExcludeNodes, status, country)) { -- cgit v1.2.3-54-g00ecf From 44095312fa63c2623418346eb0cb487b45cf9c50 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 14 Feb 2013 12:12:48 -0500 Subject: Changes file for bug8231 (duplicate directory guards) --- changes/bug8231 | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changes/bug8231 diff --git a/changes/bug8231 b/changes/bug8231 new file mode 100644 index 0000000000..fd87a1daec --- /dev/null +++ b/changes/bug8231 @@ -0,0 +1,5 @@ + o Major bugfixes: + - When unable to find any working directory nodes to use as a + directory guard, give up rather than adding the same non-working + nodes to the list over and over. Fixes bug 8231; bugfix on + 0.2.4.8-alpha. -- cgit v1.2.3-54-g00ecf