Merge branch 'maint-0.2.8'

changes/bug18929

@@ -0,0 +1,5 @@
+  o Minor bugfixes (IPv6):
+    - Make directory node selection more reliable, mainly for
+      IPv6-only clients and clients with few reachable addresses.
+      Resolves #18929, bugfix on #17840 in 0.2.8.1-alpha.
+      Patch by "teor".

changes/feature18483

@@ -0,0 +1,4 @@
+  o Minor features (clients):
+    - Make clients, onion services, and bridge relays always
+      use an encrypted begindir connection for directory requests.
+      Resolves #18483. Patch by "teor".

src/or/directory.c

@@ -630,6 +630,7 @@ directory_choose_address_routerstatus(const routerstatus_t *status,
   tor_assert(use_or_ap != NULL);
   tor_assert(use_dir_ap != NULL);
 
+  const or_options_t *options = get_options();
   int have_or = 0, have_dir = 0;
 
   /* We expect status to have at least one reachable address if we're
@@ -671,10 +672,11 @@ directory_choose_address_routerstatus(const routerstatus_t *status,
   }
 
   /* DirPort connections
-   * DIRIND_ONEHOP uses ORPort, but may fall back to the DirPort */
+   * DIRIND_ONEHOP uses ORPort, but may fall back to the DirPort on relays */
   if (indirection == DIRIND_DIRECT_CONN ||
       indirection == DIRIND_ANON_DIRPORT ||
-      indirection == DIRIND_ONEHOP) {
+      (indirection == DIRIND_ONEHOP
+       && !directory_must_use_begindir(options))) {
     have_dir = fascist_firewall_choose_address_rs(status,
                                                   FIREWALL_DIR_CONNECTION, 0,
                                                   use_dir_ap);
@@ -964,6 +966,16 @@ connection_dir_download_cert_failed(dir_connection_t *conn, int status)
   update_certificate_downloads(time(NULL));
 }
 
+/* Should this tor instance only use begindir for all its directory requests?
+ */
+int
+directory_must_use_begindir(const or_options_t *options)
+{
+  /* Clients, onion services, and bridges must use begindir,
+   * relays and authorities do not have to */
+  return !public_server_mode(options);
+}
+
 /** Evaluate the situation and decide if we should use an encrypted
  * "begindir-style" connection for this directory request.
  * 1) If or_port is 0, or it's a direct conn and or_port is firewalled
@@ -971,23 +983,48 @@ connection_dir_download_cert_failed(dir_connection_t *conn, int status)
  * 2) If we prefer to avoid begindir conns, and we're not fetching or
  *    publishing a bridge relay descriptor, no.
  * 3) Else yes.
+ * If returning 0, return in *reason why we can't use begindir.
+ * reason must not be NULL.
  */
 static int
 directory_command_should_use_begindir(const or_options_t *options,
                                       const tor_addr_t *addr,
                                       int or_port, uint8_t router_purpose,
-                                      dir_indirection_t indirection)
+                                      dir_indirection_t indirection,
+                                      const char **reason)
 {
   (void) router_purpose;
-  if (!or_port)
+  tor_assert(reason);
+  *reason = NULL;
+
+  /* Reasons why we can't possibly use begindir */
+  if (!or_port) {
+    *reason = "directory with unknown ORPort";
     return 0; /* We don't know an ORPort -- no chance. */
-  if (indirection == DIRIND_DIRECT_CONN || indirection == DIRIND_ANON_DIRPORT)
+  }
+  if (indirection == DIRIND_DIRECT_CONN ||
+      indirection == DIRIND_ANON_DIRPORT) {
+    *reason = "DirPort connection";
     return 0;
-  if (indirection == DIRIND_ONEHOP)
+  }
+  if (indirection == DIRIND_ONEHOP) {
+    /* We're firewalled and want a direct OR connection */
     if (!fascist_firewall_allows_address_addr(addr, or_port,
-                                              FIREWALL_OR_CONNECTION, 0, 0) ||
-        directory_fetches_from_authorities(options))
-      return 0; /* We're firewalled or are acting like a relay -- also no. */
+                                              FIREWALL_OR_CONNECTION, 0, 0)) {
+      *reason = "ORPort not reachable";
+      return 0;
+    }
+  }
+  /* Reasons why we want to avoid using begindir */
+  if (indirection == DIRIND_ONEHOP) {
+    if (!directory_must_use_begindir(options)) {
+      *reason = "in relay mode";
+      return 0;
+    }
+  }
+  /* DIRIND_ONEHOP on a client, or DIRIND_ANONYMOUS
+   */
+  *reason = "(using begindir)";
   return 1;
 }
 
@@ -1070,11 +1107,13 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
   dir_connection_t *conn;
   const or_options_t *options = get_options();
   int socket_error = 0;
+  const char *begindir_reason = NULL;
   /* Should the connection be to a relay's OR port (and inside that we will
    * send our directory request)? */
   const int use_begindir = directory_command_should_use_begindir(options,
                                      &or_addr_port->addr, or_addr_port->port,
-                                     router_purpose, indirection);
+                                     router_purpose, indirection,
+                                     &begindir_reason);
   /* Will the connection go via a three-hop Tor circuit? Note that this
    * is separate from whether it will use_begindir. */
   const int anonymized_connection = dirind_is_anon(indirection);
@@ -1100,6 +1139,14 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
   (void)is_sensitive_dir_purpose;
 #endif
 
+  /* use encrypted begindir connections for everything except relays
+   * this provides better protection for directory fetches */
+  if (!use_begindir && directory_must_use_begindir(options)) {
+    log_warn(LD_BUG, "Client could not use begindir connection: %s",
+             begindir_reason ? begindir_reason : "(NULL)");
+    return;
+  }
+
   /* ensure that we don't make direct connections when a SOCKS server is
    * configured. */
   if (!anonymized_connection && !use_begindir && !options->HTTPProxy &&

src/or/directory.h

@@ -28,8 +28,8 @@ void directory_get_from_all_authorities(uint8_t dir_purpose,
 
 /** Enumeration of ways to connect to a directory server */
 typedef enum {
-  /** Default: connect over a one-hop Tor circuit but fall back to direct
-   * connection */
+  /** Default: connect over a one-hop Tor circuit. Relays fall back to direct
+   * DirPort connections, clients, onion services, and bridges do not */
   DIRIND_ONEHOP=0,
   /** Connect over a multi-hop anonymizing Tor circuit */
   DIRIND_ANONYMOUS=1,
@@ -39,6 +39,8 @@ typedef enum {
   DIRIND_ANON_DIRPORT,
 } dir_indirection_t;
 
+int directory_must_use_begindir(const or_options_t *options);
+
 MOCK_DECL(void, directory_initiate_command_routerstatus,
                 (const routerstatus_t *status,
                  uint8_t dir_purpose,

src/or/or.h

@@ -2220,7 +2220,7 @@ typedef struct routerstatus_t {
   /** Digest of the router's most recent descriptor or microdescriptor.
    * If it's a descriptor, we only use the first DIGEST_LEN bytes. */
   char descriptor_digest[DIGEST256_LEN];
-  uint32_t addr; /**< IPv4 address for this router. */
+  uint32_t addr; /**< IPv4 address for this router, in host order. */
   uint16_t or_port; /**< OR port for this router. */
   uint16_t dir_port; /**< Directory port for this router. */
   tor_addr_t ipv6_addr; /**< IPv6 address for this router. */

src/or/routerlist.c

@@ -1597,11 +1597,10 @@ router_picked_poor_directory_log(const routerstatus_t *rs)
   STMT_BEGIN                                                                  \
     if (result == NULL && try_ip_pref && options->ClientUseIPv4               \
         && fascist_firewall_use_ipv6(options) && !server_mode(options)        \
-        && n_not_preferred && !n_busy) {                                      \
+        && !n_busy) {                                                         \
       n_excluded = 0;                                                         \
       n_busy = 0;                                                             \
       try_ip_pref = 0;                                                        \
-      n_not_preferred = 0;                                                    \
       goto retry_label;                                                       \
     }                                                                         \
   STMT_END                                                                    \
@@ -1620,7 +1619,6 @@ router_picked_poor_directory_log(const routerstatus_t *rs)
       n_excluded = 0;                                                         \
       n_busy = 0;                                                             \
       try_ip_pref = 1;                                                        \
-      n_not_preferred = 0;                                                    \
       goto retry_label;                                                       \
     }                                                                         \
   STMT_END
@@ -1673,7 +1671,7 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
   const int no_microdesc_fetching = (flags & PDS_NO_EXISTING_MICRODESC_FETCH);
   const int for_guard = (flags & PDS_FOR_GUARD);
   int try_excluding = 1, n_excluded = 0, n_busy = 0;
-  int try_ip_pref = 1, n_not_preferred = 0;
+  int try_ip_pref = 1;
 
   if (!consensus)
     return NULL;
@@ -1687,8 +1685,9 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
   overloaded_direct = smartlist_new();
   overloaded_tunnel = smartlist_new();
 
-  const int skip_or = router_skip_or_reachability(options, try_ip_pref);
-  const int skip_dir = router_skip_dir_reachability(options, try_ip_pref);
+  const int skip_or_fw = router_skip_or_reachability(options, try_ip_pref);
+  const int skip_dir_fw = router_skip_dir_reachability(options, try_ip_pref);
+  const int must_have_or = directory_must_use_begindir(options);
 
   /* Find all the running dirservers we know about. */
   SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) {
@@ -1740,18 +1739,16 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
      * address for each router (if any). (To ensure correct load-balancing
      * we try routers that only have one address both times.)
      */
-    if (!fascistfirewall || skip_or ||
-        fascist_firewall_allows_rs(status, FIREWALL_OR_CONNECTION,
-                                   try_ip_pref))
+    if (!fascistfirewall || skip_or_fw ||
+        fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION,
+                                     try_ip_pref))
       smartlist_add(is_trusted ? trusted_tunnel :
                     is_overloaded ? overloaded_tunnel : tunnel, (void*)node);
-    else if (skip_dir ||
-             fascist_firewall_allows_rs(status, FIREWALL_DIR_CONNECTION,
-                                        try_ip_pref))
+    else if (!must_have_or && (skip_dir_fw ||
+             fascist_firewall_allows_node(node, FIREWALL_DIR_CONNECTION,
+                                          try_ip_pref)))
       smartlist_add(is_trusted ? trusted_direct :
                     is_overloaded ? overloaded_direct : direct, (void*)node);
-    else if (!tor_addr_is_null(&status->ipv6_addr))
-      ++n_not_preferred;
   } SMARTLIST_FOREACH_END(node);
 
   if (smartlist_len(tunnel)) {
@@ -1839,7 +1836,7 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
   smartlist_t *pick_from;
   int n_busy = 0;
   int try_excluding = 1, n_excluded = 0;
-  int try_ip_pref = 1, n_not_preferred = 0;
+  int try_ip_pref = 1;
 
   if (!sourcelist)
     return NULL;
@@ -1851,8 +1848,9 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
   overloaded_direct = smartlist_new();
   overloaded_tunnel = smartlist_new();
 
-  const int skip_or = router_skip_or_reachability(options, try_ip_pref);
-  const int skip_dir = router_skip_dir_reachability(options, try_ip_pref);
+  const int skip_or_fw = router_skip_or_reachability(options, try_ip_pref);
+  const int skip_dir_fw = router_skip_dir_reachability(options, try_ip_pref);
+  const int must_have_or = directory_must_use_begindir(options);
 
   SMARTLIST_FOREACH_BEGIN(sourcelist, const dir_server_t *, d)
     {
@@ -1888,16 +1886,14 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
        * address for each router (if any). (To ensure correct load-balancing
        * we try routers that only have one address both times.)
        */
-      if (!fascistfirewall || skip_or ||
+      if (!fascistfirewall || skip_or_fw ||
           fascist_firewall_allows_dir_server(d, FIREWALL_OR_CONNECTION,
                                              try_ip_pref))
         smartlist_add(is_overloaded ? overloaded_tunnel : tunnel, (void*)d);
-      else if (skip_dir ||
+      else if (!must_have_or && (skip_dir_fw ||
                fascist_firewall_allows_dir_server(d, FIREWALL_DIR_CONNECTION,
-                                                  try_ip_pref))
+                                                  try_ip_pref)))
         smartlist_add(is_overloaded ? overloaded_direct : direct, (void*)d);
-      else if (!tor_addr_is_null(&d->ipv6_addr))
-        ++n_not_preferred;
     }
   SMARTLIST_FOREACH_END(d);