Merge remote-tracking branch 'teor/bug19530-v2'

4 files changed, 99 insertions, 53 deletions

diff --git a/changes/bug19530 b/changes/bug19530
new file mode 100644
index 0000000000..355d778886
--- /dev/null
+++ b/changes/bug19530
@@ -0,0 +1,6 @@
+  o Minor bugfixes (directory downloads):
+    - Hex-encode a relay identity fingerprint before printing it.
+    - When downloading authority certificates, re-download from
+      the last successful bridge before retrying a random bridge.
+      Fixes bug 19530; bugfix on #18963, not in any released version
+      of tor.
diff --git a/src/or/directory.c b/src/or/directory.c
index 1fe4d6804d..68ec3b7503 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -122,7 +122,7 @@ static void connection_dir_close_consensus_fetches(
 /** Return true iff the directory purpose <b>dir_purpose</b> (and if it's
  * fetching descriptors, it's fetching them for <b>router_purpose</b>)
  * must use an anonymous connection to a directory. */
-STATIC int
+int
 purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose)
 {
   if (get_options()->AllDirActionsPrivate)
diff --git a/src/or/directory.h b/src/or/directory.h
index afa3bcc611..f04e7ab315 100644
--- a/src/or/directory.h
+++ b/src/or/directory.h
@@ -132,12 +132,12 @@ int download_status_get_n_failures(const download_status_t *dls);
 int download_status_get_n_attempts(const download_status_t *dls);
 time_t download_status_get_next_attempt_at(const download_status_t *dls);
 
+int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose);
+
 #ifdef TOR_UNIT_TESTS
 /* Used only by directory.c and test_dir.c */
 
 STATIC int parse_http_url(const char *headers, char **url);
-STATIC int purpose_needs_anonymity(uint8_t dir_purpose,
-                                   uint8_t router_purpose);
 STATIC dirinfo_type_t dir_fetch_type(int dir_purpose, int router_purpose,
                                      const char *resource);
 STATIC int directory_handle_command_get(dir_connection_t *conn,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index bd6fc86463..220441fc83 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -834,6 +834,68 @@ authority_cert_dl_looks_uncertain(const char *id_digest)
   return n_failures >= N_AUTH_CERT_DL_FAILURES_TO_BUG_USER;
 }
 
+/* Fetch the authority certificates specified in resource.
+ * If we are a bridge client, and node is a configured bridge, fetch from node
+ * using dir_hint as the fingerprint. Otherwise, if rs is not NULL, fetch from
+ * rs. Otherwise, fetch from a random directory mirror. */
+static void
+authority_certs_fetch_resource_impl(const char *resource,
+                                    const char *dir_hint,
+                                    const node_t *node,
+                                    const routerstatus_t *rs)
+{
+  const or_options_t *options = get_options();
+  int get_via_tor = purpose_needs_anonymity(DIR_PURPOSE_FETCH_CERTIFICATE, 0);
+
+  /* Make sure bridge clients never connect to anything but a bridge */
+  if (options->UseBridges) {
+    if (node && !node_is_a_configured_bridge(node)) {
+      /* If we're using bridges, and node is not a bridge, use a 3-hop path. */
+      get_via_tor = 1;
+    } else if (!node) {
+      /* If we're using bridges, and there's no node, use a 3-hop path. */
+      get_via_tor = 1;
+    }
+  }
+
+  const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS
+                                                    : DIRIND_ONEHOP;
+
+  /* If we've just downloaded a consensus from a bridge, re-use that
+   * bridge */
+  if (options->UseBridges && node && !get_via_tor) {
+    /* clients always make OR connections to bridges */
+    tor_addr_port_t or_ap;
+    /* we are willing to use a non-preferred address if we need to */
+    fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0,
+                                         &or_ap);
+    directory_initiate_command(&or_ap.addr, or_ap.port,
+                               NULL, 0, /*no dirport*/
+                               dir_hint,
+                               DIR_PURPOSE_FETCH_CERTIFICATE,
+                               0,
+                               indirection,
+                               resource, NULL, 0, 0);
+    return;
+  }
+
+  if (rs) {
+    /* If we've just downloaded a consensus from a directory, re-use that
+     * directory */
+    directory_initiate_command_routerstatus(rs,
+                                            DIR_PURPOSE_FETCH_CERTIFICATE,
+                                            0, indirection, resource, NULL,
+                                            0, 0);
+    return;
+  }
+
+  /* Otherwise, we want certs from a random fallback or directory
+   * mirror, because they will almost always succeed. */
+  directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0,
+                               resource, PDS_RETRY_IF_NO_SERVERS,
+                               DL_WANT_ANY_DIRSERVER);
+}
+
 /** Try to download any v3 authority certificates that we may be missing.  If
  * <b>status</b> is provided, try to get all the ones that were used to sign
  * <b>status</b>.  Additionally, try to have a non-expired certificate for
@@ -865,12 +927,13 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
   smartlist_t *missing_cert_digests, *missing_id_digests;
   char *resource = NULL;
   cert_list_t *cl;
-  const int cache = directory_caches_unknown_auth_certs(get_options());
+  const or_options_t *options = get_options();
+  const int cache = directory_caches_unknown_auth_certs(options);
   fp_pair_t *fp_tmp = NULL;
   char id_digest_str[2*DIGEST_LEN+1];
   char sk_digest_str[2*DIGEST_LEN+1];
 
-  if (should_delay_dir_fetches(get_options(), NULL))
+  if (should_delay_dir_fetches(options, NULL))
     return;
 
   pending_cert = fp_pair_map_new();
@@ -911,7 +974,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
     } SMARTLIST_FOREACH_END(cert);
     if (!found &&
         download_status_is_ready(&(cl->dl_status_by_id), now,
-                                 get_options()->TestingCertMaxDownloadTries) &&
+                                 options->TestingCertMaxDownloadTries) &&
         !digestmap_get(pending_id, ds->v3_identity_digest)) {
       log_info(LD_DIR,
                "No current certificate known for authority %s "
@@ -973,7 +1036,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
         }
         if (download_status_is_ready_by_sk_in_cl(
               cl, sig->signing_key_digest,
-              now, get_options()->TestingCertMaxDownloadTries) &&
+              now, options->TestingCertMaxDownloadTries) &&
             !fp_pair_map_get_by_digests(pending_cert,
                                         voter->identity_digest,
                                         sig->signing_key_digest)) {
@@ -1010,7 +1073,10 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
     } SMARTLIST_FOREACH_END(voter);
   }
 
-  /* Look up the routerstatus for the dir_hint  */
+  /* Bridge clients look up the node for the dir_hint  */
+  const node_t *node = NULL;
+  /* All clients, including bridge clients, look up the routerstatus for the
+   * dir_hint */
   const routerstatus_t *rs = NULL;
 
   /* If we still need certificates, try the directory that just successfully
@@ -1018,12 +1084,16 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
    * As soon as the directory fails to provide additional certificates, we try
    * another, randomly selected directory. This avoids continual retries.
    * (We only ever have one outstanding request per certificate.)
-   *
-   * Bridge clients won't find their bridges using this hint, so they will
-   * fall back to using directory_get_from_dirserver, which selects a bridge.
    */
   if (dir_hint) {
-    /* First try the consensus routerstatus, then the fallback
+    if (options->UseBridges) {
+      /* Bridge clients try the nodelist. If the dir_hint is from an authority,
+       * or something else fetched over tor, we won't find the node here, but
+       * we will find the rs. */
+      node = node_get_by_id(dir_hint);
+    }
+
+    /* All clients try the consensus routerstatus, then the fallback
      * routerstatus */
     rs = router_get_consensus_status_by_id(dir_hint);
     if (!rs) {
@@ -1035,9 +1105,11 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
       }
     }
 
-    if (!rs) {
-      log_warn(LD_BUG, "Directory %s delivered a consensus, but a "
-               "routerstatus could not be found for it.", dir_hint);
+    if (!node && !rs) {
+      log_warn(LD_BUG, "Directory %s delivered a consensus, but %s"
+               "no routerstatus could be found for it.",
+               options->UseBridges ? "no node and " : "",
+               hex_str(dir_hint, DIGEST_LEN));
     }
   }
 
@@ -1070,25 +1142,9 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
 
     if (smartlist_len(fps) > 1) {
       resource = smartlist_join_strings(fps, "", 0, NULL);
-
-      /* If we've just downloaded a consensus from a directory, re-use that
-       * directory */
-      if (rs) {
-        /* Certificate fetches are one-hop, unless AllDirActionsPrivate is 1 */
-        int get_via_tor = get_options()->AllDirActionsPrivate;
-        const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS
-                                                          : DIRIND_ONEHOP;
-        directory_initiate_command_routerstatus(rs,
-                                                DIR_PURPOSE_FETCH_CERTIFICATE,
-                                                0, indirection, resource, NULL,
-                                                0, 0);
-      } else {
-        /* Otherwise, we want certs from a random fallback or directory
-         * mirror, because they will almost always succeed. */
-        directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0,
-                                     resource, PDS_RETRY_IF_NO_SERVERS,
-                                     DL_WANT_ANY_DIRSERVER);
-      }
+      /* node and rs are directories that just gave us a consensus or
+       * certificates */
+      authority_certs_fetch_resource_impl(resource, dir_hint, node, rs);
       tor_free(resource);
     }
     /* else we didn't add any: they were all pending */
@@ -1131,25 +1187,9 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
 
     if (smartlist_len(fp_pairs) > 1) {
       resource = smartlist_join_strings(fp_pairs, "", 0, NULL);
-
-      /* If we've just downloaded a consensus from a directory, re-use that
-       * directory */
-      if (rs) {
-        /* Certificate fetches are one-hop, unless AllDirActionsPrivate is 1 */
-        int get_via_tor = get_options()->AllDirActionsPrivate;
-        const dir_indirection_t indirection = get_via_tor ? DIRIND_ANONYMOUS
-                                                          : DIRIND_ONEHOP;
-        directory_initiate_command_routerstatus(rs,
-                                                DIR_PURPOSE_FETCH_CERTIFICATE,
-                                                0, indirection, resource, NULL,
-                                                0, 0);
-      } else {
-        /* Otherwise, we want certs from a random fallback or directory
-         * mirror, because they will almost always succeed. */
-        directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0,
-                                     resource, PDS_RETRY_IF_NO_SERVERS,
-                                     DL_WANT_ANY_DIRSERVER);
-      }
+      /* node and rs are directories that just gave us a consensus or
+       * certificates */
+      authority_certs_fetch_resource_impl(resource, dir_hint, node, rs);
       tor_free(resource);
     }
     /* else they were all pending */