Merge branch 'maint-0.3.2'

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.

src/or/directory.c

@@ -116,7 +116,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);
 
@@ -469,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 = guards_choose_dirguard(guard_state_out);
+    const node_t *node = guards_choose_dirguard(dir_purpose, guard_state_out);
     if (node)
       rs = node->rs;
   } else {
@@ -603,7 +604,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;
@@ -1011,48 +1012,6 @@ directory_must_use_begindir(const or_options_t *options)
   return !public_server_mode(options);
 }
 
-struct directory_request_t {
-  /**
-   * These fields specify which directory we're contacting.  Routerstatus,
-   * if present, overrides the other fields.
-   *
-   * @{ */
-  tor_addr_port_t or_addr_port;
-  tor_addr_port_t dir_addr_port;
-  char digest[DIGEST_LEN];
-
-  const routerstatus_t *routerstatus;
-  /** @} */
-  /** One of DIR_PURPOSE_* other than DIR_PURPOSE_SERVER. Describes what
-   * kind of operation we'll be doing (upload/download), and of what kind
-   * of document. */
-  uint8_t dir_purpose;
-  /** One of ROUTER_PURPOSE_*; used for uploads and downloads of routerinfo
-   * and extrainfo docs.  */
-  uint8_t router_purpose;
-  /** Enum: determines whether to anonymize, and whether to use dirport or
-   * orport. */
-  dir_indirection_t indirection;
-  /** Alias to the variable part of the URL for this request */
-  const char *resource;
-  /** Alias to the payload to upload (if any) */
-  const char *payload;
-  /** Number of bytes to upload from payload</b> */
-  size_t payload_len;
-  /** Value to send in an if-modified-since header, or 0 for none. */
-  time_t if_modified_since;
-  /** Hidden-service-specific information v2. */
-  const rend_data_t *rend_query;
-  /** Extra headers to append to the request */
-  config_line_t *additional_headers;
-  /** Hidden-service-specific information for v3+. */
-  const hs_ident_dir_conn_t *hs_ident;
-  /** Used internally to directory.c: gets informed when the attempt to
-   * connect to the directory succeeds or fails, if that attempt bears on the
-   * directory's usability as a directory guard. */
-  circuit_guard_state_t *guard_state;
-};
-
 /** Evaluate the situation and decide if we should use an encrypted
  * "begindir-style" connection for this directory request.
  * 0) If there is no DirPort, yes.
@@ -2245,8 +2204,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 *,
@@ -2914,7 +2871,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)
 {
@@ -2931,6 +2888,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,
@@ -2941,7 +2899,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;
@@ -2953,7 +2911,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,
@@ -5794,13 +5752,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. <b>failed</b> is a list of every microdesc digest we didn't
- * get. <b>status_code</b> is the http status code we received. Reschedule the
- * microdesc downloads as appropriate. */
+/** Called when a connection to download microdescriptors from relay with
+ * <b>dir_id</b> has failed in whole or in part. <b>failed</b> is a list
+ * of every microdesc digest we didn't get. <b>status_code</b> 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);
@@ -5811,17 +5770,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);
     }

src/or/directory.h

@@ -183,6 +183,48 @@ typedef struct response_handler_args_t {
   const char *headers;
 } response_handler_args_t;
 
+struct directory_request_t {
+  /**
+   * These fields specify which directory we're contacting.  Routerstatus,
+   * if present, overrides the other fields.
+   *
+   * @{ */
+  tor_addr_port_t or_addr_port;
+  tor_addr_port_t dir_addr_port;
+  char digest[DIGEST_LEN];
+
+  const routerstatus_t *routerstatus;
+  /** @} */
+  /** One of DIR_PURPOSE_* other than DIR_PURPOSE_SERVER. Describes what
+   * kind of operation we'll be doing (upload/download), and of what kind
+   * of document. */
+  uint8_t dir_purpose;
+  /** One of ROUTER_PURPOSE_*; used for uploads and downloads of routerinfo
+   * and extrainfo docs.  */
+  uint8_t router_purpose;
+  /** Enum: determines whether to anonymize, and whether to use dirport or
+   * orport. */
+  dir_indirection_t indirection;
+  /** Alias to the variable part of the URL for this request */
+  const char *resource;
+  /** Alias to the payload to upload (if any) */
+  const char *payload;
+  /** Number of bytes to upload from payload</b> */
+  size_t payload_len;
+  /** Value to send in an if-modified-since header, or 0 for none. */
+  time_t if_modified_since;
+  /** Hidden-service-specific information v2. */
+  const rend_data_t *rend_query;
+  /** Extra headers to append to the request */
+  config_line_t *additional_headers;
+  /** Hidden-service-specific information for v3+. */
+  const hs_ident_dir_conn_t *hs_ident;
+  /** Used internally to directory.c: gets informed when the attempt to
+   * connect to the directory succeeds or fails, if that attempt bears on the
+   * directory's usability as a directory guard. */
+  struct circuit_guard_state_t *guard_state;
+};
+
 struct get_handler_args_t;
 STATIC int handle_get_hs_descriptor_v3(dir_connection_t *conn,
                                        const struct get_handler_args_t *args);
@@ -193,6 +235,8 @@ STATIC void warn_disallowed_anonymous_compression_method(compress_method_t);
 
 STATIC int handle_response_fetch_hsdesc_v3(dir_connection_t *conn,
                                           const response_handler_args_t *args);
+STATIC int handle_response_fetch_microdesc(dir_connection_t *conn,
+                                 const response_handler_args_t *args);
 
 STATIC int handle_response_fetch_consensus(dir_connection_t *conn,
                                          const response_handler_args_t *args);

src/or/entrynodes.c

@@ -1460,6 +1460,96 @@ guard_in_node_family(const entry_guard_t *guard, const node_t *node)
   }
 }
 
+/* Allocate and return a new exit guard restriction (where <b>exit_id</b> 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 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 network is too restricted, since we don't want to
+ *  blacklist all our nodes. */
+static int
+should_set_md_dirserver_restriction(void)
+{
+  const guard_selection_t *gs = get_guard_selection_info();
+
+  /* Compute the number of filtered guards */
+  int n_filtered_guards = 0;
+  SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) {
+    if (guard->is_filtered_guard) {
+      ++n_filtered_guards;
+    }
+  } SMARTLIST_FOREACH_END(guard);
+
+  /* Do we have enough filtered guards that we feel okay about blacklisting
+   * some for MD restriction? */
+  return (n_filtered_guards >= MIN_GUARDS_FOR_MD_RESTRICTION);
+}
+
+/** 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 <b>guard</b> obeys the exit restriction <b>rst</b>. */
+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 <b>guard</b> 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 <b>guard</b> obeys the restrictions defined in <b>rst</b>.
  * (If <b>rst</b> is NULL, there are no restrictions.)
@@ -1472,13 +1562,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;
 }
 
 /**
@@ -2105,7 +2196,7 @@ entry_guard_has_higher_priority(entry_guard_t *a, entry_guard_t *b)
 }
 
 /** Release all storage held in <b>restriction</b> */
-static void
+STATIC void
 entry_guard_restriction_free(entry_guard_restriction_t *rst)
 {
   tor_free(rst);
@@ -3358,8 +3449,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,
@@ -3411,12 +3502,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);

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 <em>one particular circuit</em>.
  */
 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
@@ -554,6 +561,14 @@ 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);
+
+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);

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
+ *  <b>relay_digest</b> (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 <b>relay_digest</b> (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 <b>md</b> into <b>f</b>, with appropriate annotations.
  * On success, return the total number of bytes written, and set
  * *<b>annotation_len_out</b> 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 <b>cache</b> whose sha256 digest is

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);
 
+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) */
 

src/or/networkstatus.c

@@ -2023,6 +2023,9 @@ networkstatus_set_current_consensus(const char *consensus,
     tor_free(flavormsg);
   }
 
+  /* We got a new consesus. Reset our md fetch fail cache */
+  microdesc_reset_outdated_dirservers_list();
+
   router_dir_info_changed();
 
   result = 0;

src/or/scheduler.c

@@ -258,9 +258,10 @@ select_scheduler(void)
           /* We should only log this once in most cases. If it was the kernel
            * losing support for kist that caused scheduler_can_use_kist() to
            * return false, then this flag makes sure we only log this message
-           * once. If it was the consensus that switched from "yes use kist" to
-           * "no don't use kist", then we still set the flag so we log once, but
-           * we unset the flag elsewhere if we ever can_use_kist() again.
+           * once. If it was the consensus that switched from "yes use kist"
+           * to "no don't use kist", then we still set the flag so we log
+           * once, but we unset the flag elsewhere if we ever can_use_kist()
+           * again.
            */
           have_logged_kist_suddenly_disabled = 1;
           log_notice(LD_SCHED, "Scheduler type KIST has been disabled by "

src/test/test_entrynodes.c

@@ -7,6 +7,7 @@
 #define STATEFILE_PRIVATE
 #define ENTRYNODES_PRIVATE
 #define ROUTERLIST_PRIVATE
+#define DIRECTORY_PRIVATE
 
 #include "or.h"
 #include "test.h"
@@ -15,6 +16,7 @@
 #include "circuitlist.h"
 #include "config.h"
 #include "confparse.h"
+#include "directory.h"
 #include "entrynodes.h"
 #include "nodelist.h"
 #include "networkstatus.h"
@@ -129,6 +131,14 @@ big_fake_network_setup(const struct testcase_t *testcase)
     n->rs->has_bandwidth = 1;
     n->rs->bandwidth_kb = 30;
 
+    /* Make a random nickname for each node */
+    {
+      char nickname_binary[8];
+      crypto_rand(nickname_binary, sizeof(nickname_binary));
+      base64_encode(n->rs->nickname, sizeof(n->rs->nickname),
+                    nickname_binary, sizeof(nickname_binary), 0);
+    }
+
     /* Call half of the nodes a possible guard. */
     if (i % 2 == 0) {
       n->is_possible_guard = 1;
@@ -1719,6 +1729,7 @@ test_entry_guard_select_for_circuit_no_confirmed(void *arg)
   /* Simpler cases: no gaurds are confirmed yet. */
   (void)arg;
   guard_selection_t *gs = guard_selection_new("default", GS_TYPE_NORMAL);
+  entry_guard_restriction_t *rst = NULL;
 
   /* simple starting configuration */
   entry_guards_update_primary(gs);
@@ -1800,14 +1811,13 @@ test_entry_guard_select_for_circuit_no_confirmed(void *arg)
   tt_ptr_op(g2, OP_EQ, g);
 
   /* But if we impose a restriction, we don't get the same guard */
-  entry_guard_restriction_t rst;
-  memset(&rst, 0, sizeof(rst));
-  memcpy(rst.exclude_id, g->identity, DIGEST_LEN);
-  g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, &rst, &state);
+  rst = guard_create_exit_restriction((uint8_t*)g->identity);
+  g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, rst, &state);
   tt_ptr_op(g2, OP_NE, g);
 
  done:
   guard_selection_free(gs);
+  entry_guard_restriction_free(rst);
 }
 
 static void
@@ -1817,6 +1827,7 @@ test_entry_guard_select_for_circuit_confirmed(void *arg)
      guards, we use a confirmed guard. */
   (void)arg;
   int i;
+  entry_guard_restriction_t *rst = NULL;
   guard_selection_t *gs = guard_selection_new("default", GS_TYPE_NORMAL);
   const int N_CONFIRMED = 10;
 
@@ -1877,10 +1888,8 @@ test_entry_guard_select_for_circuit_confirmed(void *arg)
   get_options_mutable()->EnforceDistinctSubnets = 0;
   g = smartlist_get(gs->confirmed_entry_guards,
                      smartlist_len(gs->primary_entry_guards)+2);
-  entry_guard_restriction_t rst;
-  memset(&rst, 0, sizeof(rst));
-  memcpy(rst.exclude_id, g->identity, DIGEST_LEN);
-  g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, &rst, &state);
+  rst = guard_create_exit_restriction((uint8_t*)g->identity);
+  g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, rst, &state);
   tt_ptr_op(g2, OP_NE, NULL);
   tt_ptr_op(g2, OP_NE, g);
   tt_int_op(g2->confirmed_idx, OP_EQ,
@@ -1906,13 +1915,13 @@ test_entry_guard_select_for_circuit_confirmed(void *arg)
   // Regression test for bug 22753/TROVE-2017-006.
   get_options_mutable()->EnforceDistinctSubnets = 1;
   g = smartlist_get(gs->confirmed_entry_guards, 0);
-  memset(&rst, 0, sizeof(rst));
-  memcpy(rst.exclude_id, g->identity, DIGEST_LEN);
-  g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, &rst, &state);
+  memcpy(rst->exclude_id, g->identity, DIGEST_LEN);
+  g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, rst, &state);
   tt_ptr_op(g2, OP_EQ, NULL);
 
  done:
   guard_selection_free(gs);
+  entry_guard_restriction_free(rst);
 }
 
 static void
@@ -2493,9 +2502,7 @@ test_entry_guard_upgrade_not_blocked_by_restricted_circ_complete(void *arg)
   /* Once more, let circ1 become complete. But this time, we'll claim
    * that circ2 was restricted to not use the same guard as circ1. */
   data->guard2_state->restrictions =
-    tor_malloc_zero(sizeof(entry_guard_restriction_t));
-  memcpy(data->guard2_state->restrictions->exclude_id,
-         data->guard1->identity, DIGEST_LEN);
+    guard_create_exit_restriction((uint8_t*)data->guard1->identity);
 
   smartlist_t *result = smartlist_new();
   int r;
@@ -2604,9 +2611,7 @@ test_entry_guard_upgrade_not_blocked_by_restricted_circ_pending(void *arg)
   }
 
   data->guard2_state->restrictions =
-    tor_malloc_zero(sizeof(entry_guard_restriction_t));
-  memcpy(data->guard2_state->restrictions->exclude_id,
-         data->guard1->identity, DIGEST_LEN);
+    guard_create_exit_restriction((uint8_t*)data->guard1->identity);
 
   smartlist_t *result = smartlist_new();
   int r;
@@ -2674,6 +2679,112 @@ test_enty_guard_should_expire_waiting(void *arg)
   tor_free(fake_state);
 }
 
+static void
+mock_directory_initiate_request(directory_request_t *req)
+{
+  if (req->guard_state) {
+    circuit_guard_state_free(req->guard_state);
+  }
+}
+
+static networkstatus_t *mock_ns_val = NULL;
+static networkstatus_t *
+mock_ns_get_by_flavor(consensus_flavor_t f)
+{
+  (void)f;
+  return mock_ns_val;
+}
+
+/** Test that when we fetch microdescriptors we skip guards that have
+ *  previously failed to serve us needed microdescriptors. */
+static void
+test_entry_guard_outdated_dirserver_exclusion(void *arg)
+{
+  int retval;
+  response_handler_args_t *args = NULL;
+  dir_connection_t *conn = NULL;
+  (void) arg;
+
+  /* Test prep: Make a new guard selection */
+  guard_selection_t *gs = get_guard_selection_by_name("default",
+                                                      GS_TYPE_NORMAL, 1);
+
+  /* ... we want to use entry guards */
+  or_options_t *options = get_options_mutable();
+  options->UseEntryGuards = 1;
+  options->UseBridges = 0;
+
+  /* ... prepare some md digests we want to download in the future */
+  smartlist_t *digests = smartlist_new();
+  const char *prose = "unhurried and wise, we perceive.";
+  for (int i = 0; i < 20; i++) {
+    smartlist_add(digests, (char*)prose);
+  }
+
+  /* ... now mock some functions */
+  mock_ns_val = tor_malloc_zero(sizeof(networkstatus_t));
+  MOCK(networkstatus_get_latest_consensus_by_flavor, mock_ns_get_by_flavor);
+  MOCK(directory_initiate_request, mock_directory_initiate_request);
+
+  /* Test logic:
+   *  0. Create a proper guard set and primary guard list.
+   *  1. Pretend to fail microdescriptor fetches from all the primary guards.
+   *  2. Order another microdescriptor fetch and make sure that primary guards
+   *     get skipped since they failed previous fetches.
+   */
+
+  { /* Setup primary guard list */
+    int i;
+    entry_guards_update_primary(gs);
+    for (i = 0; i < DFLT_N_PRIMARY_GUARDS; ++i) {
+      entry_guard_t *guard = smartlist_get(gs->sampled_entry_guards, i);
+      make_guard_confirmed(gs, guard);
+    }
+    entry_guards_update_primary(gs);
+  }
+
+  {
+    /* Fail microdesc fetches with all the primary guards */
+    args = tor_malloc_zero(sizeof(response_handler_args_t));
+    args->status_code = 404;
+    args->reason = NULL;
+    args->body = NULL;
+    args->body_len = 0;
+
+    conn = tor_malloc_zero(sizeof(dir_connection_t));
+    conn->requested_resource = tor_strdup("d/jlinblackorigami");
+    conn->base_.purpose = DIR_PURPOSE_FETCH_MICRODESC;
+
+    /* Pretend to fail fetches with all primary guards */
+    SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards,const entry_guard_t *,g) {
+      memcpy(conn->identity_digest, g->identity, DIGEST_LEN);
+
+      retval = handle_response_fetch_microdesc(conn, args);
+      tt_int_op(retval, OP_EQ, 0);
+    } SMARTLIST_FOREACH_END(g);
+  }
+
+  {
+    /* Now order the final md download */
+    setup_full_capture_of_logs(LOG_INFO);
+    initiate_descriptor_downloads(NULL, DIR_PURPOSE_FETCH_MICRODESC,
+                                  digests, 3, 7, 0);
+
+    /* ... and check that because we failed to fetch microdescs from all our
+     * primaries, we didnt end up selecting a primary for fetching dir info */
+    expect_log_msg_containing("No primary or confirmed guards available.");
+    teardown_capture_of_logs();
+  }
+
+ done:
+  smartlist_free(digests);
+  tor_free(args);
+  if (conn) {
+    tor_free(conn->requested_resource);
+    tor_free(conn);
+  }
+}
+
 static const struct testcase_setup_t big_fake_network = {
   big_fake_network_setup, big_fake_network_cleanup
 };
@@ -2734,6 +2845,7 @@ struct testcase_t entrynodes_tests[] = {
   BFN_TEST(select_for_circuit_highlevel_primary_retry),
   BFN_TEST(select_and_cancel),
   BFN_TEST(drop_guards),
+  BFN_TEST(outdated_dirserver_exclusion),
 
   UPGRADE_TEST(upgrade_a_circuit, "c1-done c2-done"),
   UPGRADE_TEST(upgrade_blocked_by_live_primary_guards, "c1-done c2-done"),