diff --git a/changes/bug23817 b/changes/bug23817
new file mode 100644
index 000000000..474094279
--- /dev/null
+++ b/changes/bug23817
@@ -0,0 +1,3 @@
+ o Minor bugfixes (descriptors):
+ - Don't try fetching microdescriptors from relays that have failed to
+ deliver them in the past. Fixes bug 23817; bugfix on 0.3.0.1-alpha.
diff --git a/src/or/directory.c b/src/or/directory.c
index f54bf176c..5ceea2fb3 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -117,7 +117,8 @@ static void dir_routerdesc_download_failed(smartlist_t *failed,
int was_extrainfo,
int was_descriptor_digests);
static void dir_microdesc_download_failed(smartlist_t *failed,
- int status_code);
+ int status_code,
+ const char *dir_id);
static int client_likes_consensus(const struct consensus_cache_entry_t *ent,
const char *want_url);
@@ -463,7 +464,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 = guards_choose_dirguard(guard_state_out);
+ const node_t *node = guards_choose_dirguard(dir_purpose, guard_state_out);
if (node)
rs = node->rs;
} else {
@@ -597,7 +598,7 @@ 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.
*/
- const node_t *node = guards_choose_dirguard(&guard_state);
+ const node_t *node = guards_choose_dirguard(dir_purpose, &guard_state);
if (node && node->ri) {
/* every bridge has a routerinfo. */
routerinfo_t *ri = node->ri;
@@ -2178,8 +2179,6 @@ static int handle_response_fetch_detached_signatures(dir_connection_t *,
const response_handler_args_t *);
static int handle_response_fetch_desc(dir_connection_t *,
const response_handler_args_t *);
-static int handle_response_fetch_microdesc(dir_connection_t *,
- const response_handler_args_t *);
static int handle_response_upload_dir(dir_connection_t *,
const response_handler_args_t *);
static int handle_response_upload_vote(dir_connection_t *,
@@ -2839,7 +2838,7 @@ handle_response_fetch_desc(dir_connection_t *conn,
* Handler function: processes a response to a request for a group of
* microdescriptors
**/
-static int
+STATIC int
handle_response_fetch_microdesc(dir_connection_t *conn,
const response_handler_args_t *args)
{
@@ -2856,6 +2855,7 @@ handle_response_fetch_microdesc(dir_connection_t *conn,
conn->base_.port);
tor_assert(conn->requested_resource &&
!strcmpstart(conn->requested_resource, "d/"));
+ tor_assert_nonfatal(!tor_mem_is_zero(conn->identity_digest, DIGEST_LEN));
which = smartlist_new();
dir_split_resource_into_fingerprints(conn->requested_resource+2,
which, NULL,
@@ -2866,7 +2866,7 @@ handle_response_fetch_microdesc(dir_connection_t *conn,
"soon.",
status_code, escaped(reason), conn->base_.address,
(int)conn->base_.port, conn->requested_resource);
- dir_microdesc_download_failed(which, status_code);
+ dir_microdesc_download_failed(which, status_code, conn->identity_digest);
SMARTLIST_FOREACH(which, char *, cp, tor_free(cp));
smartlist_free(which);
return 0;
@@ -2878,7 +2878,7 @@ handle_response_fetch_microdesc(dir_connection_t *conn,
now, which);
if (smartlist_len(which)) {
/* Mark remaining ones as failed. */
- dir_microdesc_download_failed(which, status_code);
+ dir_microdesc_download_failed(which, status_code, conn->identity_digest);
}
if (mds && smartlist_len(mds)) {
control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
@@ -5546,13 +5546,14 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code,
* every 10 or 60 seconds (FOO_DESCRIPTOR_RETRY_INTERVAL) in main.c. */
}
-/** Called when a connection to download microdescriptors has failed in whole
- * or in part. failed is a list of every microdesc digest we didn't
- * get. status_code is the http status code we received. Reschedule the
- * microdesc downloads as appropriate. */
+/** Called when a connection to download microdescriptors from relay with
+ * dir_id has failed in whole or in part. failed is a list
+ * of every microdesc digest we didn't get. status_code is the http
+ * status code we received. Reschedule the microdesc downloads as
+ * appropriate. */
static void
dir_microdesc_download_failed(smartlist_t *failed,
- int status_code)
+ int status_code, const char *dir_id)
{
networkstatus_t *consensus
= networkstatus_get_latest_consensus_by_flavor(FLAV_MICRODESC);
@@ -5563,17 +5564,26 @@ dir_microdesc_download_failed(smartlist_t *failed,
if (! consensus)
return;
+
+ /* We failed to fetch a microdescriptor from 'dir_id', note it down
+ * so that we don't try the same relay next time... */
+ microdesc_note_outdated_dirserver(dir_id);
+
SMARTLIST_FOREACH_BEGIN(failed, const char *, d) {
rs = router_get_mutable_consensus_status_by_descriptor_digest(consensus,d);
if (!rs)
continue;
dls = &rs->dl_status;
if (dls->n_download_failures >=
- get_options()->TestingMicrodescMaxDownloadTries)
+ get_options()->TestingMicrodescMaxDownloadTries) {
continue;
- {
+ }
+
+ { /* Increment the failure count for this md fetch */
char buf[BASE64_DIGEST256_LEN+1];
digest256_to_base64(buf, d);
+ log_info(LD_DIR, "Failed to download md %s from %s",
+ buf, hex_str(dir_id, DIGEST_LEN));
download_status_increment_failure(dls, status_code, buf,
server, now);
}
diff --git a/src/or/directory.h b/src/or/directory.h
index 14d5ae9ef..571c30a0f 100644
--- a/src/or/directory.h
+++ b/src/or/directory.h
@@ -166,7 +166,12 @@ STATIC char *accept_encoding_header(void);
STATIC int allowed_anonymous_connection_compression_method(compress_method_t);
STATIC void warn_disallowed_anonymous_compression_method(compress_method_t);
-#endif
+struct response_handler_args_t;
+
+STATIC int handle_response_fetch_microdesc(dir_connection_t *conn,
+ const struct response_handler_args_t *args);
+
+#endif /* defined(DIRECTORY_PRIVATE) */
#ifdef TOR_UNIT_TESTS
/* Used only by test_dir.c */
diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index d6cd58ced..2cbc8019d 100644
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -967,7 +967,7 @@ entry_guard_learned_bridge_identity(const tor_addr_port_t *addrport,
* violate it.
*/
STATIC int
-num_reachable_filtered_guards(guard_selection_t *gs,
+num_reachable_filtered_guards(const guard_selection_t *gs,
const entry_guard_restriction_t *rst)
{
int n_reachable_filtered_guards = 0;
@@ -1461,6 +1461,94 @@ guard_in_node_family(const entry_guard_t *guard, const node_t *node)
}
}
+/* Allocate and return a new exit guard restriction (where exit_id is of
+ * size DIGEST_LEN) */
+STATIC entry_guard_restriction_t *
+guard_create_exit_restriction(const uint8_t *exit_id)
+{
+ entry_guard_restriction_t *rst = NULL;
+ rst = tor_malloc_zero(sizeof(entry_guard_restriction_t));
+ rst->type = RST_EXIT_NODE;
+ memcpy(rst->exclude_id, exit_id, DIGEST_LEN);
+ return rst;
+}
+
+/** If we have fewer than this many possible usable guards, don't set
+ * MD-availability-based restrictions: we might blacklist all of them. */
+#define MIN_GUARDS_FOR_MD_RESTRICTION 10
+
+/** Return true if we should set md dirserver restrictions. We might not want
+ * to set those if our guard options are too restricted, since we don't want
+ * to blacklist all of them. */
+static int
+should_set_md_dirserver_restriction(void)
+{
+ const guard_selection_t *gs = get_guard_selection_info();
+ int num_usable_guards = num_reachable_filtered_guards(gs, NULL);
+
+ /* Don't set restriction if too few reachable filtered guards. */
+ if (num_usable_guards < MIN_GUARDS_FOR_MD_RESTRICTION) {
+ log_info(LD_GUARD, "Not setting md restriction: only %d"
+ " usable guards.", num_usable_guards);
+ return 0;
+ }
+
+ /* We have enough usable guards: set MD restriction */
+ return 1;
+}
+
+/** Allocate and return an outdated md guard restriction. Return NULL if no
+ * such restriction is needed. */
+STATIC entry_guard_restriction_t *
+guard_create_dirserver_md_restriction(void)
+{
+ entry_guard_restriction_t *rst = NULL;
+
+ if (!should_set_md_dirserver_restriction()) {
+ log_debug(LD_GUARD, "Not setting md restriction: too few "
+ "filtered guards.");
+ return NULL;
+ }
+
+ rst = tor_malloc_zero(sizeof(entry_guard_restriction_t));
+ rst->type = RST_OUTDATED_MD_DIRSERVER;
+
+ return rst;
+}
+
+/* Return True if guard obeys the exit restriction rst. */
+static int
+guard_obeys_exit_restriction(const entry_guard_t *guard,
+ const entry_guard_restriction_t *rst)
+{
+ tor_assert(rst->type == RST_EXIT_NODE);
+
+ // Exclude the exit ID and all of its family.
+ const node_t *node = node_get_by_id((const char*)rst->exclude_id);
+ if (node && guard_in_node_family(guard, node))
+ return 0;
+
+ return tor_memneq(guard->identity, rst->exclude_id, DIGEST_LEN);
+}
+
+/** Return True if guard should be used as a dirserver for fetching
+ * microdescriptors. */
+static int
+guard_obeys_md_dirserver_restriction(const entry_guard_t *guard)
+{
+ /* If this guard is an outdated dirserver, don't use it. */
+ if (microdesc_relay_is_outdated_dirserver(guard->identity)) {
+ log_info(LD_GENERAL, "Skipping %s dirserver: outdated",
+ hex_str(guard->identity, DIGEST_LEN));
+ return 0;
+ }
+
+ log_debug(LD_GENERAL, "%s dirserver obeys md restrictions",
+ hex_str(guard->identity, DIGEST_LEN));
+
+ return 1;
+}
+
/**
* Return true iff guard obeys the restrictions defined in rst.
* (If rst is NULL, there are no restrictions.)
@@ -1473,13 +1561,14 @@ entry_guard_obeys_restriction(const entry_guard_t *guard,
if (! rst)
return 1; // No restriction? No problem.
- // Only one kind of restriction exists right now: excluding an exit
- // ID and all of its family.
- const node_t *node = node_get_by_id((const char*)rst->exclude_id);
- if (node && guard_in_node_family(guard, node))
- return 0;
+ if (rst->type == RST_EXIT_NODE) {
+ return guard_obeys_exit_restriction(guard, rst);
+ } else if (rst->type == RST_OUTDATED_MD_DIRSERVER) {
+ return guard_obeys_md_dirserver_restriction(guard);
+ }
- return tor_memneq(guard->identity, rst->exclude_id, DIGEST_LEN);
+ tor_assert_nonfatal_unreached();
+ return 0;
}
/**
@@ -2106,7 +2195,7 @@ entry_guard_has_higher_priority(entry_guard_t *a, entry_guard_t *b)
}
/** Release all storage held in restriction */
-static void
+STATIC void
entry_guard_restriction_free(entry_guard_restriction_t *rst)
{
tor_free(rst);
@@ -3359,8 +3448,8 @@ guards_choose_guard(cpath_build_state_t *state,
/* We're building to a targeted exit node, so that node can't be
* chosen as our guard for this circuit. Remember that fact in a
* restriction. */
- rst = tor_malloc_zero(sizeof(entry_guard_restriction_t));
- memcpy(rst->exclude_id, exit_id, DIGEST_LEN);
+ rst = guard_create_exit_restriction(exit_id);
+ tor_assert(rst);
}
if (entry_guard_pick_for_circuit(get_guard_selection_info(),
GUARD_USAGE_TRAFFIC,
@@ -3412,12 +3501,20 @@ remove_all_entry_guards(void)
/** Helper: pick a directory guard, with whatever algorithm is used. */
const node_t *
-guards_choose_dirguard(circuit_guard_state_t **guard_state_out)
+guards_choose_dirguard(uint8_t dir_purpose,
+ circuit_guard_state_t **guard_state_out)
{
const node_t *r = NULL;
+ entry_guard_restriction_t *rst = NULL;
+
+ /* If we are fetching microdescs, don't query outdated dirservers. */
+ if (dir_purpose == DIR_PURPOSE_FETCH_MICRODESC) {
+ rst = guard_create_dirserver_md_restriction();
+ }
+
if (entry_guard_pick_for_circuit(get_guard_selection_info(),
GUARD_USAGE_DIRGUARD,
- NULL,
+ rst,
&r,
guard_state_out) < 0) {
tor_assert(r == NULL);
diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h
index 735c7738b..39bff1438 100644
--- a/src/or/entrynodes.h
+++ b/src/or/entrynodes.h
@@ -272,22 +272,28 @@ struct guard_selection_s {
struct entry_guard_handle_t;
+/** Types of restrictions we impose when picking guard nodes */
+typedef enum guard_restriction_type_t {
+ /* Don't pick the same guard node as our exit node (or its family) */
+ RST_EXIT_NODE = 0,
+ /* Don't pick dirguards that have previously shown to be outdated */
+ RST_OUTDATED_MD_DIRSERVER = 1
+} guard_restriction_type_t;
+
/**
* A restriction to remember which entry guards are off-limits for a given
* circuit.
*
- * Right now, we only use restrictions to block a single guard and its family
- * from being selected; this mechanism is designed to be more extensible in
- * the future, however.
- *
* Note: This mechanism is NOT for recording which guards are never to be
* used: only which guards cannot be used on one particular circuit.
*/
struct entry_guard_restriction_t {
- /**
- * The guard's RSA identity digest must not equal this; and it must not
- * be in the same family as any node with this digest.
- */
+ /* What type of restriction are we imposing? */
+ guard_restriction_type_t type;
+
+ /* In case of restriction type RST_EXIT_NODE, the guard's RSA identity
+ * digest must not equal this; and it must not be in the same family as any
+ * node with this digest. */
uint8_t exclude_id[DIGEST_LEN];
};
@@ -316,7 +322,8 @@ struct circuit_guard_state_t {
int guards_update_all(void);
const node_t *guards_choose_guard(cpath_build_state_t *state,
circuit_guard_state_t **guard_state_out);
-const node_t *guards_choose_dirguard(circuit_guard_state_t **guard_state_out);
+const node_t *guards_choose_dirguard(uint8_t dir_purpose,
+ circuit_guard_state_t **guard_state_out);
#if 1
/* XXXX NM I would prefer that all of this stuff be private to
@@ -514,7 +521,7 @@ STATIC void entry_guard_consider_retry(entry_guard_t *guard);
STATIC void make_guard_confirmed(guard_selection_t *gs, entry_guard_t *guard);
STATIC void entry_guards_update_confirmed(guard_selection_t *gs);
STATIC void entry_guards_update_primary(guard_selection_t *gs);
-STATIC int num_reachable_filtered_guards(guard_selection_t *gs,
+STATIC int num_reachable_filtered_guards(const guard_selection_t *gs,
const entry_guard_restriction_t *rst);
STATIC void sampled_guards_update_from_consensus(guard_selection_t *gs);
/**
@@ -550,7 +557,17 @@ STATIC unsigned entry_guards_note_guard_success(guard_selection_t *gs,
unsigned old_state);
STATIC int entry_guard_has_higher_priority(entry_guard_t *a, entry_guard_t *b);
STATIC char *getinfo_helper_format_single_entry_guard(const entry_guard_t *e);
-#endif
+
+STATIC entry_guard_restriction_t *
+guard_create_exit_restriction(const uint8_t *exit_id);
+
+STATIC entry_guard_restriction_t *
+guard_create_dirserver_md_restriction(void);
+
+STATIC void
+entry_guard_restriction_free(entry_guard_restriction_t *rst);
+
+#endif /* defined(ENTRYNODES_PRIVATE) */
void remove_all_entry_guards_for_guard_selection(guard_selection_t *gs);
void remove_all_entry_guards(void);
diff --git a/src/or/microdesc.c b/src/or/microdesc.c
index a4e6b409c..32242d005 100644
--- a/src/or/microdesc.c
+++ b/src/or/microdesc.c
@@ -74,6 +74,102 @@ HT_GENERATE2(microdesc_map, microdesc_t, node,
microdesc_hash_, microdesc_eq_, 0.6,
tor_reallocarray_, tor_free_)
+/************************* md fetch fail cache *****************************/
+
+/* If we end up with too many outdated dirservers, something probably went
+ * wrong so clean up the list. */
+#define TOO_MANY_OUTDATED_DIRSERVERS 30
+
+/** List of dirservers with outdated microdesc information. The smartlist is
+ * filled with the hex digests of outdated dirservers. */
+static smartlist_t *outdated_dirserver_list = NULL;
+
+/** Note that we failed to fetch a microdescriptor from the relay with
+ * relay_digest (of size DIGEST_LEN). */
+void
+microdesc_note_outdated_dirserver(const char *relay_digest)
+{
+ char relay_hexdigest[HEX_DIGEST_LEN+1];
+
+ /* Don't register outdated dirservers if we don't have a live consensus,
+ * since we might be trying to fetch microdescriptors that are not even
+ * currently active. */
+ if (!networkstatus_get_live_consensus(approx_time())) {
+ return;
+ }
+
+ if (!outdated_dirserver_list) {
+ outdated_dirserver_list = smartlist_new();
+ }
+
+ tor_assert(outdated_dirserver_list);
+
+ /* If the list grows too big, clean it up */
+ if (BUG(smartlist_len(outdated_dirserver_list) >
+ TOO_MANY_OUTDATED_DIRSERVERS)) {
+ microdesc_reset_outdated_dirservers_list();
+ }
+
+ /* Turn the binary relay digest to a hex since smartlists have better support
+ * for strings than digests. */
+ base16_encode(relay_hexdigest,sizeof(relay_hexdigest),
+ relay_digest, DIGEST_LEN);
+
+ /* Make sure we don't add a dirauth as an outdated dirserver */
+ if (router_get_trusteddirserver_by_digest(relay_digest)) {
+ log_info(LD_GENERAL, "Auth %s gave us outdated dirinfo.", relay_hexdigest);
+ return;
+ }
+
+ /* Don't double-add outdated dirservers */
+ if (smartlist_contains_string(outdated_dirserver_list, relay_hexdigest)) {
+ return;
+ }
+
+ /* Add it to the list of outdated dirservers */
+ smartlist_add_strdup(outdated_dirserver_list, relay_hexdigest);
+
+ log_info(LD_GENERAL, "Noted %s as outdated md dirserver", relay_hexdigest);
+}
+
+/** Return True if the relay with relay_digest (size DIGEST_LEN) is an
+ * outdated dirserver */
+int
+microdesc_relay_is_outdated_dirserver(const char *relay_digest)
+{
+ char relay_hexdigest[HEX_DIGEST_LEN+1];
+
+ if (!outdated_dirserver_list) {
+ return 0;
+ }
+
+ /* Convert identity digest to hex digest */
+ base16_encode(relay_hexdigest, sizeof(relay_hexdigest),
+ relay_digest, DIGEST_LEN);
+
+ /* Last time we tried to fetch microdescs, was this directory mirror missing
+ * any mds we asked for? */
+ if (smartlist_contains_string(outdated_dirserver_list, relay_hexdigest)) {
+ return 1;
+ }
+
+ return 0;
+}
+
+/** Reset the list of outdated dirservers. */
+void
+microdesc_reset_outdated_dirservers_list(void)
+{
+ if (!outdated_dirserver_list) {
+ return;
+ }
+
+ SMARTLIST_FOREACH(outdated_dirserver_list, char *, cp, tor_free(cp));
+ smartlist_clear(outdated_dirserver_list);
+}
+
+/****************************************************************************/
+
/** Write the body of md into f, with appropriate annotations.
* On success, return the total number of bytes written, and set
* *annotation_len_out to the number of bytes written as
@@ -789,6 +885,11 @@ microdesc_free_all(void)
tor_free(the_microdesc_cache->journal_fname);
tor_free(the_microdesc_cache);
}
+
+ if (outdated_dirserver_list) {
+ SMARTLIST_FOREACH(outdated_dirserver_list, char *, cp, tor_free(cp));
+ smartlist_free(outdated_dirserver_list);
+ }
}
/** If there is a microdescriptor in cache whose sha256 digest is
diff --git a/src/or/microdesc.h b/src/or/microdesc.h
index 943873066..1be12156a 100644
--- a/src/or/microdesc.h
+++ b/src/or/microdesc.h
@@ -50,5 +50,9 @@ int we_fetch_microdescriptors(const or_options_t *options);
int we_fetch_router_descriptors(const or_options_t *options);
int we_use_microdescriptors_for_circuits(const or_options_t *options);
-#endif
+void microdesc_note_outdated_dirserver(const char *relay_digest);
+int microdesc_relay_is_outdated_dirserver(const char *relay_digest);
+void microdesc_reset_outdated_dirservers_list(void);
+
+#endif /* !defined(TOR_MICRODESC_H) */
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 997280de5..36e62020e 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -2041,6 +2041,9 @@ networkstatus_set_current_consensus(const char *consensus,
"CLOCK_SKEW MIN_SKEW=%ld SOURCE=CONSENSUS", delta);
}
+ /* We got a new consesus. Reset our md fetch fail cache */
+ microdesc_reset_outdated_dirservers_list();
+
router_dir_info_changed();
result = 0;