Просмотр исходного кода

Merge remote-tracking branch 'teor/feature17840-v11-merged-v2'

Nick Mathewson 8 лет назад
Родитель
Сommit
ba2be81fc3

+ 9 - 0
changes/feature17840

@@ -0,0 +1,9 @@
+  o Minor feature (IPv6):
+    - Add ClientUseIPv4, which is set to 1 by default. If set to 0, tor
+      avoids using IPv4 for client OR and directory connections.
+    - Add ClientPreferIPv6DirPort, which is set to 0 by default. If set
+      to 1, tor prefers IPv6 directory addresses.
+    - Try harder to fulfil IP version restrictions ClientUseIPv4 0 and
+      ClientUseIPv6 0; and the preferences ClientPreferIPv6ORPort and
+      ClientPreferIPv6DirPort.
+      Closes ticket 17840; patch by "teor".

+ 29 - 8
doc/tor.1.txt

@@ -390,6 +390,7 @@ GENERAL OPTIONS
     authorities.
     By default, the directory authorities are also FallbackDirs. Specifying a
     FallbackDir replaces Tor's default hard-coded FallbackDirs (if any).
+    (See the **DirAuthority** entry for an explanation of each flag.)
 
 [[UseDefaultFallbackDirs]] **UseDefaultFallbackDirs** **0**|**1**::
     Use Tor's default hard-coded FallbackDirs (if any). (When a
@@ -413,6 +414,10 @@ GENERAL OPTIONS
     if an "ipv6=__address__:__orport__" flag is present, then the directory
     authority is listening for IPv6 connections on the indicated IPv6 address
     and OR Port. +
+ +
+    Tor will contact the authority at __address__:__port__ (the DirPort) to
+    download directory documents. If an IPv6 address is supplied, Tor will
+    also download directory documents at the IPv6 address on the DirPort. +
  +
     If no **DirAuthority** line is given, Tor will use the default directory
     authorities. NOTE: this option is intended for setting up a private Tor
@@ -1506,17 +1511,33 @@ The following options are useful only for clients (that is, if
     If no defaults are available there, these options default to 20, .80,
     .60, and 100, respectively.
 
+[[ClientUseIPv4]] **ClientUseIPv4** **0**|**1**::
+    If this option is set to 0, Tor will avoid connecting to directory servers
+    and entry nodes over IPv4. Note that clients with an IPv4
+    address in a **Bridge**, proxy, or pluggable transport line will try
+    connecting over IPv4 even if **ClientUseIPv4** is set to 0. (Default: 1)
+
 [[ClientUseIPv6]] **ClientUseIPv6** **0**|**1**::
-    If this option is set to 1, Tor might connect to entry nodes over
-    IPv6. Note that clients configured with an IPv6 address in a
-    **Bridge** line will try connecting over IPv6 even if
-    **ClientUseIPv6** is set to 0. (Default: 0)
+    If this option is set to 1, Tor might connect to directory servers or
+    entry nodes over IPv6. Note that clients configured with an IPv6 address
+    in a **Bridge**, proxy, or pluggable transport line will try connecting
+    over IPv6 even if **ClientUseIPv6** is set to 0. (Default: 0)
+
+[[ClientPreferIPv6DirPort]] **ClientPreferIPv6DirPort** **0**|**1**|**auto**::
+    If this option is set to 1, Tor prefers a directory port with an IPv6
+    address over one with IPv4, for direct connections, if a given directory
+    server has both. (Tor also prefers an IPv6 DirPort if IPv4Client is set to
+    0.) If this option is set to auto, clients prefer IPv4. Other things may
+    influence the choice. This option breaks a tie to the favor of IPv6.
+    (Default: auto)
 
-[[ClientPreferIPv6ORPort]] **ClientPreferIPv6ORPort** **0**|**1**::
+[[ClientPreferIPv6ORPort]] **ClientPreferIPv6ORPort** **0**|**1**|**auto**::
     If this option is set to 1, Tor prefers an OR port with an IPv6
-    address over one with IPv4 if a given entry node has both. Other
-    things may influence the choice. This option breaks a tie to the
-    favor of IPv6. (Default: 0)
+    address over one with IPv4 if a given entry node has both. (Tor also
+    prefers an IPv6 ORPort if IPv4Client is set to 0.) If this option is set
+    to auto, Tor bridge clients prefer the configured bridge address, and
+    other clients prefer IPv4. Other things may influence the choice. This
+    option breaks a tie to the favor of IPv6. (Default: auto)
 
 [[PathsNeededToBuildCircuits]] **PathsNeededToBuildCircuits** __NUM__::
     Tor clients don't build circuits for user traffic until they know

+ 53 - 0
src/common/address.c

@@ -910,6 +910,59 @@ tor_addr_is_loopback(const tor_addr_t *addr)
   }
 }
 
+/* Is addr valid?
+ * Checks that addr is non-NULL and not tor_addr_is_null().
+ * If for_listening is true, IPv4 addr 0.0.0.0 is allowed.
+ * It means "bind to all addresses on the local machine". */
+int
+tor_addr_is_valid(const tor_addr_t *addr, int for_listening)
+{
+  /* NULL addresses are invalid regardless of for_listening */
+  if (addr == NULL) {
+    return 0;
+  }
+
+  /* Only allow IPv4 0.0.0.0 for_listening. */
+  if (for_listening && addr->family == AF_INET
+      && tor_addr_to_ipv4h(addr) == 0) {
+    return 1;
+  }
+
+  /* Otherwise, the address is valid if it's not tor_addr_is_null() */
+  return !tor_addr_is_null(addr);
+}
+
+/* Is the network-order IPv4 address v4n_addr valid?
+ * Checks that addr is not zero.
+ * Except if for_listening is true, where IPv4 addr 0.0.0.0 is allowed. */
+int
+tor_addr_is_valid_ipv4n(uint32_t v4n_addr, int for_listening)
+{
+  /* Any IPv4 address is valid with for_listening. */
+  if (for_listening) {
+    return 1;
+  }
+
+  /* Otherwise, zero addresses are invalid. */
+  return v4n_addr != 0;
+}
+
+/* Is port valid?
+ * Checks that port is not 0.
+ * Except if for_listening is true, where port 0 is allowed.
+ * It means "OS chooses a port". */
+int
+tor_port_is_valid(uint16_t port, int for_listening)
+{
+  /* Any port value is valid with for_listening. */
+  if (for_listening) {
+    return 1;
+  }
+
+  /* Otherwise, zero ports are invalid. */
+  return port != 0;
+}
+
 /** Set <b>dest</b> to equal the IPv4 address in <b>v4addr</b> (given in
  * network order). */
 void

+ 21 - 0
src/common/address.h

@@ -267,6 +267,27 @@ void tor_addr_from_in6(tor_addr_t *dest, const struct in6_addr *in6);
 int tor_addr_is_null(const tor_addr_t *addr);
 int tor_addr_is_loopback(const tor_addr_t *addr);
 
+int tor_addr_is_valid(const tor_addr_t *addr, int for_listening);
+int tor_addr_is_valid_ipv4n(uint32_t v4n_addr, int for_listening);
+#define tor_addr_is_valid_ipv4h(v4h_addr, for_listening) \
+        tor_addr_is_valid_ipv4n(htonl(v4h_addr), (for_listening))
+int tor_port_is_valid(uint16_t port, int for_listening);
+/* Are addr and port both valid? */
+#define tor_addr_port_is_valid(addr, port, for_listening) \
+        (tor_addr_is_valid((addr), (for_listening)) &&    \
+         tor_port_is_valid((port), (for_listening)))
+/* Are ap->addr and ap->port both valid? */
+#define tor_addr_port_is_valid_ap(ap, for_listening) \
+        tor_addr_port_is_valid(&(ap)->addr, (ap)->port, (for_listening))
+/* Are the network-order v4addr and port both valid? */
+#define tor_addr_port_is_valid_ipv4n(v4n_addr, port, for_listening) \
+        (tor_addr_is_valid_ipv4n((v4n_addr), (for_listening)) &&    \
+         tor_port_is_valid((port), (for_listening)))
+/* Are the host-order v4addr and port both valid? */
+#define tor_addr_port_is_valid_ipv4h(v4h_addr, port, for_listening) \
+        (tor_addr_is_valid_ipv4h((v4h_addr), (for_listening)) &&    \
+         tor_port_is_valid((port), (for_listening)))
+
 int tor_addr_port_split(int severity, const char *addrport,
                         char **address_out, uint16_t *port_out);
 

+ 27 - 21
src/or/circuitbuild.c

@@ -1778,7 +1778,7 @@ pick_tor2web_rendezvous_node(router_crn_flags_t flags,
   router_add_running_nodes_to_smartlist(all_live_nodes,
                                         allow_invalid,
                                         0, 0, 0,
-                                        need_desc);
+                                        need_desc, 0);
 
   /* Filter all_live_nodes to only add live *and* whitelisted RPs to
    * the list whitelisted_live_rps. */
@@ -2144,7 +2144,9 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
   const node_t *choice;
   smartlist_t *excluded;
   const or_options_t *options = get_options();
-  router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC;
+  /* If possible, choose an entry server with a preferred address,
+   * otherwise, choose one with an allowed address */
+  router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC|CRN_PREF_ADDR;
   const node_t *node;
 
   if (state && options->UseEntryGuards &&
@@ -2161,14 +2163,6 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
      * family. */
     nodelist_add_node_and_family(excluded, node);
   }
-  if (firewall_is_fascist_or()) {
-    /* Exclude all ORs that we can't reach through our firewall */
-    smartlist_t *nodes = nodelist_get_list();
-    SMARTLIST_FOREACH(nodes, const node_t *, node, {
-      if (!fascist_firewall_allows_node(node))
-        smartlist_add(excluded, (void*)node);
-    });
-  }
   /* and exclude current entry guards and their families,
    * unless we're in a test network, and excluding guards
    * would exclude all nodes (i.e. we're in an incredibly small tor network,
@@ -2247,9 +2241,11 @@ onion_extend_cpath(origin_circuit_t *circ)
     if (r) {
       /* If we're a client, use the preferred address rather than the
          primary address, for potentially connecting to an IPv6 OR
-         port. */
-      info = extend_info_from_node(r, server_mode(get_options()) == 0);
-      tor_assert(info);
+         port. Servers always want the primary (IPv4) address. */
+      int client = (server_mode(get_options()) == 0);
+      info = extend_info_from_node(r, client);
+      /* Clients can fail to find an allowed address */
+      tor_assert(info || client);
     }
   } else {
     const node_t *r =
@@ -2324,33 +2320,43 @@ extend_info_new(const char *nickname, const char *digest,
  * <b>for_direct_connect</b> is true, in which case the preferred
  * address is used instead. May return NULL if there is not enough
  * info about <b>node</b> to extend to it--for example, if there is no
- * routerinfo_t or microdesc_t.
+ * routerinfo_t or microdesc_t, or if for_direct_connect is true and none of
+ * the node's addresses are allowed by tor's firewall and IP version config.
  **/
 extend_info_t *
 extend_info_from_node(const node_t *node, int for_direct_connect)
 {
   tor_addr_port_t ap;
+  int valid_addr = 0;
 
   if (node->ri == NULL && (node->rs == NULL || node->md == NULL))
     return NULL;
 
+  /* Choose a preferred address first, but fall back to an allowed address.
+   * choose_address returns 1 on success, but get_prim_orport returns 0. */
   if (for_direct_connect)
-    node_get_pref_orport(node, &ap);
+    valid_addr = fascist_firewall_choose_address_node(node,
+                                                      FIREWALL_OR_CONNECTION,
+                                                      0, &ap);
   else
-    node_get_prim_orport(node, &ap);
+    valid_addr = !node_get_prim_orport(node, &ap);
 
-  log_debug(LD_CIRC, "using %s for %s",
-            fmt_addrport(&ap.addr, ap.port),
-            node->ri ? node->ri->nickname : node->rs->nickname);
+  if (valid_addr)
+    log_debug(LD_CIRC, "using %s for %s",
+              fmt_addrport(&ap.addr, ap.port),
+              node->ri ? node->ri->nickname : node->rs->nickname);
+  else
+    log_warn(LD_CIRC, "Could not choose valid address for %s",
+              node->ri ? node->ri->nickname : node->rs->nickname);
 
-  if (node->ri)
+  if (valid_addr && node->ri)
     return extend_info_new(node->ri->nickname,
                              node->identity,
                              node->ri->onion_pkey,
                              node->ri->onion_curve25519_pkey,
                              &ap.addr,
                              ap.port);
-  else if (node->rs && node->md)
+  else if (valid_addr && node->rs && node->md)
     return extend_info_new(node->rs->nickname,
                              node->identity,
                              node->md->onion_pkey,

+ 6 - 1
src/or/circuituse.c

@@ -2006,8 +2006,13 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn,
         if (r && node_has_descriptor(r)) {
           /* We might want to connect to an IPv6 bridge for loading
              descriptors so we use the preferred address rather than
-             the primary.  */
+             the primary. */
           extend_info = extend_info_from_node(r, conn->want_onehop ? 1 : 0);
+          if (!extend_info) {
+            log_warn(LD_CIRC,"Could not make a one-hop connection to %s. "
+                     "Discarding this circuit.", conn->chosen_exit_name);
+            return -1;
+          }
         } else {
           log_debug(LD_DIR, "considering %d, %s",
                     want_onehop, conn->chosen_exit_name);

+ 25 - 5
src/or/config.c

@@ -190,10 +190,12 @@ static config_var_t option_vars_[] = {
   V(CircuitPriorityHalflife,     DOUBLE,  "-100.0"), /*negative:'Use default'*/
   V(ClientDNSRejectInternalAddresses, BOOL,"1"),
   V(ClientOnly,                  BOOL,     "0"),
-  V(ClientPreferIPv6ORPort,      BOOL,     "0"),
+  V(ClientPreferIPv6ORPort,      AUTOBOOL, "auto"),
+  V(ClientPreferIPv6DirPort,     AUTOBOOL, "auto"),
   V(ClientRejectInternalAddresses, BOOL,   "1"),
   V(ClientTransportPlugin,       LINELIST, NULL),
   V(ClientUseIPv6,               BOOL,     "0"),
+  V(ClientUseIPv4,               BOOL,     "1"),
   V(ConsensusParams,             STRING,   NULL),
   V(ConnLimit,                   UINT,     "1000"),
   V(ConnDirectionStatistics,     BOOL,     "0"),
@@ -3078,6 +3080,8 @@ options_validate(or_options_t *old_options, or_options_t *options,
     }
   }
 
+  /* Terminate Reachable*Addresses with reject *
+   */
   for (i=0; i<3; i++) {
     config_line_t **linep =
       (i==0) ? &options->ReachableAddresses :
@@ -3087,8 +3091,6 @@ options_validate(or_options_t *old_options, or_options_t *options,
       continue;
     /* We need to end with a reject *:*, not an implicit accept *:* */
     for (;;) {
-      if (!strcmp((*linep)->value, "reject *:*")) /* already there */
-        break;
       linep = &((*linep)->next);
       if (!*linep) {
         *linep = tor_malloc_zero(sizeof(config_line_t));
@@ -3104,11 +3106,29 @@ options_validate(or_options_t *old_options, or_options_t *options,
 
   if ((options->ReachableAddresses ||
        options->ReachableORAddresses ||
-       options->ReachableDirAddresses) &&
+       options->ReachableDirAddresses ||
+       options->ClientUseIPv4 == 0) &&
       server_mode(options))
     REJECT("Servers must be able to freely connect to the rest "
            "of the Internet, so they must not set Reachable*Addresses "
-           "or FascistFirewall.");
+           "or FascistFirewall or FirewallPorts or ClientUseIPv4 0.");
+
+  /* We check if Reachable*Addresses blocks all addresses in
+   * parse_reachable_addresses(). */
+
+#define WARN_PLEASE_USE_IPV6_LOG_MSG \
+        "ClientPreferIPv6%sPort 1 is ignored unless tor is using IPv6. " \
+        "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges."
+
+  if (!fascist_firewall_use_ipv6(options)
+      && options->ClientPreferIPv6ORPort == 1)
+    log_warn(LD_CONFIG, WARN_PLEASE_USE_IPV6_LOG_MSG, "OR");
+
+  if (!fascist_firewall_use_ipv6(options)
+      && options->ClientPreferIPv6DirPort == 1)
+    log_warn(LD_CONFIG, WARN_PLEASE_USE_IPV6_LOG_MSG, "Dir");
+
+#undef WARN_PLEASE_USE_IPV6_LOG_MSG
 
   if (options->UseBridges &&
       server_mode(options))

+ 71 - 3
src/or/connection.c

@@ -19,6 +19,7 @@
  */
 #define TOR_CHANNEL_INTERNAL_
 #define CONNECTION_PRIVATE
+#include "backtrace.h"
 #include "channel.h"
 #include "channeltls.h"
 #include "circuitbuild.h"
@@ -37,6 +38,7 @@
 #include "ext_orport.h"
 #include "geoip.h"
 #include "main.h"
+#include "nodelist.h"
 #include "policies.h"
 #include "reasons.h"
 #include "relay.h"
@@ -44,6 +46,7 @@
 #include "rendcommon.h"
 #include "rephist.h"
 #include "router.h"
+#include "routerlist.h"
 #include "transports.h"
 #include "routerparse.h"
 #include "transports.h"
@@ -1714,6 +1717,67 @@ connection_connect_sockaddr,(connection_t *conn,
   return inprogress ? 0 : 1;
 }
 
+/* Log a message if connection attempt is made when IPv4 or IPv6 is disabled.
+ * Log a less severe message if we couldn't conform to ClientPreferIPv6ORPort
+ * or ClientPreferIPv6ORPort. */
+static void
+connection_connect_log_client_use_ip_version(const connection_t *conn)
+{
+  const or_options_t *options = get_options();
+
+  /* Only clients care about ClientUseIPv4/6, bail out early on servers, and
+   * on connections we don't care about */
+  if (server_mode(options) || !conn || conn->type == CONN_TYPE_EXIT) {
+    return;
+  }
+
+  /* We're only prepared to log OR and DIR connections here */
+  if (conn->type != CONN_TYPE_OR && conn->type != CONN_TYPE_DIR) {
+    return;
+  }
+
+  const int must_ipv4 = !fascist_firewall_use_ipv6(options);
+  const int must_ipv6 = (options->ClientUseIPv4 == 0);
+  const int pref_ipv6 = (conn->type == CONN_TYPE_OR
+                         ? fascist_firewall_prefer_ipv6_orport(options)
+                         : fascist_firewall_prefer_ipv6_dirport(options));
+  tor_addr_t real_addr;
+  tor_addr_make_null(&real_addr, AF_UNSPEC);
+
+  /* OR conns keep the original address in real_addr, as addr gets overwritten
+   * with the descriptor address */
+  if (conn->type == CONN_TYPE_OR) {
+    const or_connection_t *or_conn = TO_OR_CONN((connection_t *)conn);
+    tor_addr_copy(&real_addr, &or_conn->real_addr);
+  } else if (conn->type == CONN_TYPE_DIR) {
+    tor_addr_copy(&real_addr, &conn->addr);
+  }
+
+  /* Check if we broke a mandatory address family restriction */
+  if ((must_ipv4 && tor_addr_family(&real_addr) == AF_INET6)
+      || (must_ipv6 && tor_addr_family(&real_addr) == AF_INET)) {
+    log_warn(LD_BUG, "%s connection to %s violated ClientUseIPv%s 0.",
+             conn->type == CONN_TYPE_OR ? "OR" : "Dir",
+             fmt_addr(&real_addr),
+             options->ClientUseIPv4 == 0 ? "4" : "6");
+    log_backtrace(LOG_WARN, LD_BUG, "Address came from");
+  }
+
+  /* Check if we couldn't satisfy an address family preference */
+  if ((!pref_ipv6 && tor_addr_family(&real_addr) == AF_INET6)
+      || (pref_ipv6 && tor_addr_family(&real_addr) == AF_INET)) {
+    log_info(LD_NET, "Connection to %s doesn't satisfy ClientPreferIPv6%sPort "
+             "%d, with ClientUseIPv4 %d, and fascist_firewall_use_ipv6 %d "
+             "(ClientUseIPv6 %d and UseBridges %d).",
+             fmt_addr(&real_addr),
+             conn->type == CONN_TYPE_OR ? "OR" : "Dir",
+             conn->type == CONN_TYPE_OR ? options->ClientPreferIPv6ORPort
+                                        : options->ClientPreferIPv6DirPort,
+             options->ClientUseIPv4, fascist_firewall_use_ipv6(options),
+             options->ClientUseIPv6, options->UseBridges);
+  }
+}
+
 /** Take conn, make a nonblocking socket; try to connect to
  * addr:port (port arrives in *host order*). If fail, return -1 and if
  * applicable put your best guess about errno into *<b>socket_error</b>.
@@ -1738,6 +1802,10 @@ connection_connect(connection_t *conn, const char *address,
   const or_options_t *options = get_options();
   int protocol_family;
 
+  /* Log if we didn't stick to ClientUseIPv4/6 or ClientPreferIPv6OR/DirPort
+   */
+  connection_connect_log_client_use_ip_version(conn);
+
   if (tor_addr_family(addr) == AF_INET6)
     protocol_family = PF_INET6;
   else
@@ -4246,10 +4314,10 @@ connection_write_to_buf_impl_,(const char *string, size_t len,
 /** Return a connection with given type, address, port, and purpose;
  * or NULL if no such connection exists (or if all such connections are marked
  * for close). */
-connection_t *
-connection_get_by_type_addr_port_purpose(int type,
+MOCK_IMPL(connection_t *,
+connection_get_by_type_addr_port_purpose,(int type,
                                          const tor_addr_t *addr, uint16_t port,
-                                         int purpose)
+                                         int purpose))
 {
   CONN_GET_TEMPLATE(conn,
        (conn->type == type &&

+ 3 - 3
src/or/connection.h

@@ -186,9 +186,9 @@ connection_get_outbuf_len(connection_t *conn)
 connection_t *connection_get_by_global_id(uint64_t id);
 
 connection_t *connection_get_by_type(int type);
-connection_t *connection_get_by_type_addr_port_purpose(int type,
-                                                   const tor_addr_t *addr,
-                                                   uint16_t port, int purpose);
+MOCK_DECL(connection_t *,connection_get_by_type_addr_port_purpose,(int type,
+                                                  const tor_addr_t *addr,
+                                                  uint16_t port, int purpose));
 connection_t *connection_get_by_type_state(int type, int state);
 connection_t *connection_get_by_type_state_rendquery(int type, int state,
                                                      const char *rendquery);

+ 16 - 2
src/or/control.c

@@ -2864,12 +2864,26 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
   }
 
   /* now circ refers to something that is ready to be extended */
+  int first_node = zero_circ;
   SMARTLIST_FOREACH(nodes, const node_t *, node,
   {
-    extend_info_t *info = extend_info_from_node(node, 0);
-    tor_assert(info); /* True, since node_has_descriptor(node) == true */
+    extend_info_t *info = extend_info_from_node(node, first_node);
+    if (first_node && !info) {
+      log_warn(LD_CONTROL,
+               "controller tried to connect to a node that doesn't have any "
+               "addresses that are allowed by the firewall configuration; "
+               "circuit marked for closing.");
+      circuit_mark_for_close(TO_CIRCUIT(circ), -END_CIRC_REASON_CONNECTFAILED);
+      connection_write_str_to_buf("551 Couldn't start circuit\r\n", conn);
+      goto done;
+    } else {
+      /* True, since node_has_descriptor(node) == true and we are extending
+       * to the node's primary address */
+      tor_assert(info);
+    }
     circuit_append_new_exit(circ, info);
     extend_info_free(info);
+    first_node = 0;
   });
 
   /* now that we've populated the cpath, start extending */

+ 196 - 55
src/or/directory.c

@@ -4,6 +4,7 @@
 /* See LICENSE for licensing information */
 
 #include "or.h"
+#include "backtrace.h"
 #include "buffers.h"
 #include "circuitbuild.h"
 #include "config.h"
@@ -82,18 +83,18 @@ static void dir_microdesc_download_failed(smartlist_t *failed,
 static void note_client_request(int purpose, int compressed, size_t bytes);
 static int client_likes_consensus(networkstatus_t *v, const char *want_url);
 
-static void directory_initiate_command_rend(const tor_addr_t *addr,
-                                            uint16_t or_port,
-                                            uint16_t dir_port,
-                                            const char *digest,
-                                            uint8_t dir_purpose,
-                                            uint8_t router_purpose,
-                                            dir_indirection_t indirection,
-                                            const char *resource,
-                                            const char *payload,
-                                            size_t payload_len,
-                                            time_t if_modified_since,
-                                            const rend_data_t *rend_query);
+static void directory_initiate_command_rend(
+                                          const tor_addr_port_t *or_addr_port,
+                                          const tor_addr_port_t *dir_addr_port,
+                                          const char *digest,
+                                          uint8_t dir_purpose,
+                                          uint8_t router_purpose,
+                                          dir_indirection_t indirection,
+                                          const char *resource,
+                                          const char *payload,
+                                          size_t payload_len,
+                                          time_t if_modified_since,
+                                          const rend_data_t *rend_query);
 
 /********* START VARIABLES **********/
 
@@ -313,7 +314,6 @@ directory_post_to_dirservers(uint8_t dir_purpose, uint8_t router_purpose,
   SMARTLIST_FOREACH_BEGIN(dirservers, dir_server_t *, ds) {
       routerstatus_t *rs = &(ds->fake_status);
       size_t upload_len = payload_len;
-      tor_addr_t ds_addr;
 
       if ((type & ds->type) == 0)
         continue;
@@ -344,11 +344,12 @@ directory_post_to_dirservers(uint8_t dir_purpose, uint8_t router_purpose,
         log_info(LD_DIR, "Uploading an extrainfo too (length %d)",
                  (int) extrainfo_len);
       }
-      tor_addr_from_ipv4h(&ds_addr, ds->addr);
       if (purpose_needs_anonymity(dir_purpose, router_purpose)) {
         indirection = DIRIND_ANONYMOUS;
-      } else if (!fascist_firewall_allows_address_dir(&ds_addr,ds->dir_port)) {
-        if (fascist_firewall_allows_address_or(&ds_addr,ds->or_port))
+      } else if (!fascist_firewall_allows_dir_server(ds,
+                                                     FIREWALL_DIR_CONNECTION,
+                                                     0)) {
+        if (fascist_firewall_allows_dir_server(ds, FIREWALL_OR_CONNECTION, 0))
           indirection = DIRIND_ONEHOP;
         else
           indirection = DIRIND_ANONYMOUS;
@@ -496,11 +497,14 @@ MOCK_IMPL(void, directory_get_from_dirserver, (
       const node_t *node = choose_random_dirguard(type);
       if (node && node->ri) {
         /* every bridge has a routerinfo. */
-        tor_addr_t addr;
         routerinfo_t *ri = node->ri;
-        node_get_addr(node, &addr);
-        directory_initiate_command(&addr,
-                                   ri->or_port, 0/*no dirport*/,
+        /* 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*/
                                    ri->cache_info.identity_digest,
                                    dir_purpose,
                                    router_purpose,
@@ -609,6 +613,80 @@ dirind_is_anon(dir_indirection_t ind)
   return ind == DIRIND_ANON_DIRPORT || ind == DIRIND_ANONYMOUS;
 }
 
+/* Choose reachable OR and Dir addresses and ports from status, copying them
+ * into use_or_ap and use_dir_ap. If indirection is anonymous, then we're
+ * connecting via another relay, so choose the primary IPv4 address and ports.
+ *
+ * status should have at least one reachable address, if we can't choose a
+ * reachable address, warn and return -1. Otherwise, return 0.
+ */
+static int
+directory_choose_address_routerstatus(const routerstatus_t *status,
+                                      dir_indirection_t indirection,
+                                      tor_addr_port_t *use_or_ap,
+                                      tor_addr_port_t *use_dir_ap)
+{
+  tor_assert(status != NULL);
+  tor_assert(use_or_ap != NULL);
+  tor_assert(use_dir_ap != NULL);
+
+  const int anonymized_connection = dirind_is_anon(indirection);
+  int have_or = 0, have_dir = 0;
+
+  /* We expect status to have at least one reachable address if we're
+   * connecting to it directly.
+   *
+   * Therefore, we can simply use the other address if the one we want isn't
+   * allowed by the firewall.
+   *
+   * (When Tor uploads and downloads a hidden service descriptor, it uses
+   * DIRIND_ANONYMOUS, except for Tor2Web, which uses DIRIND_ONEHOP.
+   * So this code will only modify the address for Tor2Web's HS descriptor
+   * fetches. Even Single Onion Servers (NYI) use DIRIND_ANONYMOUS, to avoid
+   * HSDirs denying service by rejecting descriptors.)
+   */
+
+  /* Initialise the OR / Dir addresses */
+  tor_addr_make_null(&use_or_ap->addr, AF_UNSPEC);
+  use_or_ap->port = 0;
+  tor_addr_make_null(&use_dir_ap->addr, AF_UNSPEC);
+  use_dir_ap->port = 0;
+
+  if (anonymized_connection) {
+    /* Use the primary (IPv4) OR address if we're making an indirect
+     * connection. */
+    tor_addr_from_ipv4h(&use_or_ap->addr, status->addr);
+    use_or_ap->port = status->or_port;
+    have_or = 1;
+  } else {
+    /* We use an IPv6 address if we have one and we prefer it.
+     * Use the preferred address and port if they are reachable, otherwise,
+     * use the alternate address and port (if any).
+     */
+    have_or = fascist_firewall_choose_address_rs(status,
+                                                 FIREWALL_OR_CONNECTION, 0,
+                                                 use_or_ap);
+  }
+
+  have_dir = fascist_firewall_choose_address_rs(status,
+                                                FIREWALL_DIR_CONNECTION, 0,
+                                                use_dir_ap);
+
+  /* We rejected both addresses. This isn't great. */
+  if (!have_or && !have_dir) {
+    log_warn(LD_BUG, "Rejected all OR and Dir addresses from %s when "
+             "launching a directory connection to: IPv4 %s OR %d Dir %d "
+             "IPv6 %s OR %d Dir %d", routerstatus_describe(status),
+             fmt_addr32(status->addr), status->or_port,
+             status->dir_port, fmt_addr(&status->ipv6_addr),
+             status->ipv6_orport, status->dir_port);
+    log_backtrace(LOG_WARN, LD_BUG, "Addresses came from");
+    return -1;
+  }
+
+  return 0;
+}
+
 /** Same as directory_initiate_command_routerstatus(), but accepts
  * rendezvous data to fetch a hidden service descriptor. */
 void
@@ -624,8 +702,11 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status,
 {
   const or_options_t *options = get_options();
   const node_t *node;
-  tor_addr_t addr;
+  tor_addr_port_t use_or_ap, use_dir_ap;
   const int anonymized_connection = dirind_is_anon(indirection);
+
+  tor_assert(status != NULL);
+
   node = node_get_by_id(status->identity_digest);
 
   if (!node && anonymized_connection) {
@@ -634,7 +715,6 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status,
              routerstatus_describe(status));
     return;
   }
-  tor_addr_from_ipv4h(&addr, status->addr);
 
   if (options->ExcludeNodes && options->StrictNodes &&
       routerset_contains_routerstatus(options->ExcludeNodes, status, -1)) {
@@ -646,13 +726,30 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status,
     return;
   }
 
-  directory_initiate_command_rend(&addr,
-                             status->or_port, status->dir_port,
-                             status->identity_digest,
-                             dir_purpose, router_purpose,
-                             indirection, resource,
-                             payload, payload_len, if_modified_since,
-                             rend_query);
+  /* At this point, if we are a clients making a direct connection to a
+   * directory server, we have selected a server that has at least one address
+   * allowed by ClientUseIPv4/6 and Reachable{"",OR,Dir}Addresses. This
+   * selection uses the preference in ClientPreferIPv6{OR,Dir}Port, if
+   * possible. (If UseBridges is set, clients always use IPv6, and prefer it
+   * by default.)
+   *
+   * Now choose an address that we can use to connect to the directory server.
+   */
+  if (directory_choose_address_routerstatus(status, indirection, &use_or_ap,
+                                            &use_dir_ap) < 0) {
+    return;
+  }
+
+  /* We don't retry the alternate OR/Dir address for the same directory if
+   * the address we choose fails (#6772).
+   * Instead, we'll retry another directory on failure. */
+
+  directory_initiate_command_rend(&use_or_ap, &use_dir_ap,
+                                  status->identity_digest,
+                                  dir_purpose, router_purpose,
+                                  indirection, resource,
+                                  payload, payload_len, if_modified_since,
+                                  rend_query);
 }
 
 /** Launch a new connection to the directory server <b>status</b> to
@@ -874,27 +971,52 @@ directory_command_should_use_begindir(const or_options_t *options,
   if (indirection == DIRIND_DIRECT_CONN || indirection == DIRIND_ANON_DIRPORT)
     return 0;
   if (indirection == DIRIND_ONEHOP)
-    if (!fascist_firewall_allows_address_or(addr, or_port) ||
+    if (!fascist_firewall_allows_address_addr(addr, or_port,
+                                              FIREWALL_OR_CONNECTION, 0) ||
         directory_fetches_from_authorities(options))
       return 0; /* We're firewalled or are acting like a relay -- also no. */
   return 1;
 }
 
-/** Helper for directory_initiate_command_routerstatus: send the
- * command to a server whose address is <b>address</b>, whose IP is
- * <b>addr</b>, whose directory port is <b>dir_port</b>, whose tor version
- * <b>supports_begindir</b>, and whose identity key digest is
- * <b>digest</b>. */
+/** Helper for directory_initiate_command_rend: send the
+ * command to a server whose OR address/port is <b>or_addr</b>/<b>or_port</b>,
+ * whose directory address/port is <b>dir_addr</b>/<b>dir_port</b>, whose
+ * identity key digest is <b>digest</b>, with purposes <b>dir_purpose</b> and
+ * <b>router_purpose</b>, making an (in)direct connection as specified in
+ * <b>indirection</b>, with command <b>resource</b>, <b>payload</b> of
+ * <b>payload_len</b>, and asking for a result only <b>if_modified_since</b>.
+ */
 void
-directory_initiate_command(const tor_addr_t *_addr,
-                           uint16_t or_port, uint16_t dir_port,
+directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port,
+                           const tor_addr_t *dir_addr, uint16_t dir_port,
                            const char *digest,
                            uint8_t dir_purpose, uint8_t router_purpose,
                            dir_indirection_t indirection, const char *resource,
                            const char *payload, size_t payload_len,
                            time_t if_modified_since)
 {
-  directory_initiate_command_rend(_addr, or_port, dir_port,
+  tor_addr_port_t or_ap, dir_ap;
+
+  /* Use the null tor_addr and 0 port if the address or port isn't valid. */
+  if (tor_addr_port_is_valid(or_addr, or_port, 0)) {
+    tor_addr_copy(&or_ap.addr, or_addr);
+    or_ap.port = or_port;
+  } else {
+    /* the family doesn't matter here, so make it IPv4 */
+    tor_addr_make_null(&or_ap.addr, AF_INET);
+    or_port = 0;
+  }
+
+  if (tor_addr_port_is_valid(dir_addr, dir_port, 0)) {
+    tor_addr_copy(&dir_ap.addr, dir_addr);
+    dir_ap.port = dir_port;
+  } else {
+    /* the family doesn't matter here, so make it IPv4 */
+    tor_addr_make_null(&dir_ap.addr, AF_INET);
+    dir_port = 0;
+  }
+
+  directory_initiate_command_rend(&or_ap, &dir_ap,
                              digest, dir_purpose,
                              router_purpose, indirection,
                              resource, payload, payload_len,
@@ -914,10 +1036,11 @@ is_sensitive_dir_purpose(uint8_t dir_purpose)
 }
 
 /** Same as directory_initiate_command(), but accepts rendezvous data to
- * fetch a hidden service descriptor. */
+ * fetch a hidden service descriptor, and takes its address & port arguments
+ * as tor_addr_port_t. */
 static void
-directory_initiate_command_rend(const tor_addr_t *_addr,
-                                uint16_t or_port, uint16_t dir_port,
+directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
+                                const tor_addr_port_t *dir_addr_port,
                                 const char *digest,
                                 uint8_t dir_purpose, uint8_t router_purpose,
                                 dir_indirection_t indirection,
@@ -926,29 +1049,34 @@ directory_initiate_command_rend(const tor_addr_t *_addr,
                                 time_t if_modified_since,
                                 const rend_data_t *rend_query)
 {
+  tor_assert(or_addr_port);
+  tor_assert(dir_addr_port);
+  tor_assert(or_addr_port->port || dir_addr_port->port);
+  tor_assert(digest);
+
   dir_connection_t *conn;
   const or_options_t *options = get_options();
   int socket_error = 0;
-  int use_begindir = directory_command_should_use_begindir(options, _addr,
-                                     or_port, router_purpose, indirection);
+  const int use_begindir = directory_command_should_use_begindir(options,
+                                     &or_addr_port->addr, or_addr_port->port,
+                                     router_purpose, indirection);
   const int anonymized_connection = dirind_is_anon(indirection);
-  tor_addr_t addr;
+  const int or_connection = use_begindir || anonymized_connection;
 
-  tor_assert(_addr);
-  tor_assert(or_port || dir_port);
-  tor_assert(digest);
-
-  tor_addr_copy(&addr, _addr);
+  tor_addr_t addr;
+  tor_addr_copy(&addr, &(or_connection ? or_addr_port : dir_addr_port)->addr);
+  uint16_t port = (or_connection ? or_addr_port : dir_addr_port)->port;
+  uint16_t dir_port = dir_addr_port->port;
 
   log_debug(LD_DIR, "anonymized %d, use_begindir %d.",
             anonymized_connection, use_begindir);
 
   if (!dir_port && !use_begindir) {
     char ipaddr[TOR_ADDR_BUF_LEN];
-    tor_addr_to_str(ipaddr, _addr, TOR_ADDR_BUF_LEN, 0);
+    tor_addr_to_str(ipaddr, &addr, TOR_ADDR_BUF_LEN, 0);
     log_warn(LD_BUG, "Cannot use directory server without dirport or "
-                     "begindir! Address: %s, ORPort: %d, DirPort: %d",
-                     escaped_safe_str_client(ipaddr), or_port, dir_port);
+                     "begindir! Address: %s, DirPort: %d, Connection Port: %d",
+                     escaped_safe_str_client(ipaddr), dir_port, port);
     return;
   }
 
@@ -963,13 +1091,26 @@ directory_initiate_command_rend(const tor_addr_t *_addr,
 
   /* ensure that we don't make direct connections when a SOCKS server is
    * configured. */
-  if (!anonymized_connection && !use_begindir && !options->HTTPProxy &&
+  if (!or_connection && !options->HTTPProxy &&
       (options->Socks4Proxy || options->Socks5Proxy)) {
     log_warn(LD_DIR, "Cannot connect to a directory server through a "
                      "SOCKS proxy!");
     return;
   }
 
+  if (or_connection && (!or_addr_port->port
+                        || tor_addr_is_null(&or_addr_port->addr))) {
+    log_warn(LD_DIR, "Cannot make an OR connection without an OR port.");
+    log_backtrace(LOG_WARN, LD_BUG, "Address came from");
+    return;
+  } else if (!or_connection && (!dir_addr_port->port
+                                || tor_addr_is_null(&dir_addr_port->addr))) {
+    log_warn(LD_DIR, "Cannot make a Dir connection without a Dir port.");
+    log_backtrace(LOG_WARN, LD_BUG, "Address came from");
+
+    return;
+  }
+
   /* ensure we don't make excess connections when we're already downloading
    * a consensus during bootstrap */
   if (connection_dir_avoid_extra_connection_for_purpose(dir_purpose)) {
@@ -980,7 +1121,7 @@ directory_initiate_command_rend(const tor_addr_t *_addr,
 
   /* set up conn so it's got all the data we need to remember */
   tor_addr_copy(&conn->base_.addr, &addr);
-  conn->base_.port = use_begindir ? or_port : dir_port;
+  conn->base_.port = port;
   conn->base_.address = tor_dup_addr(&addr);
   memcpy(conn->identity_digest, digest, DIGEST_LEN);
 
@@ -998,7 +1139,7 @@ directory_initiate_command_rend(const tor_addr_t *_addr,
   if (rend_query)
     conn->rend_data = rend_data_dup(rend_query);
 
-  if (!anonymized_connection && !use_begindir) {
+  if (!or_connection) {
     /* then we want to connect to dirport directly */
 
     if (options->HTTPProxy) {

+ 2 - 2
src/or/directory.h

@@ -68,8 +68,8 @@ int connection_dir_process_inbuf(dir_connection_t *conn);
 int connection_dir_finished_flushing(dir_connection_t *conn);
 int connection_dir_finished_connecting(dir_connection_t *conn);
 void connection_dir_about_to_close(dir_connection_t *dir_conn);
-void directory_initiate_command(const tor_addr_t *addr,
-                                uint16_t or_port, uint16_t dir_port,
+void directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port,
+                                const tor_addr_t *dir_addr, uint16_t dir_port,
                                 const char *digest,
                                 uint8_t dir_purpose, uint8_t router_purpose,
                                 dir_indirection_t indirection,

+ 36 - 12
src/or/entrynodes.c

@@ -87,7 +87,7 @@ get_entry_guards(void)
 
 /** Check whether the entry guard <b>e</b> is usable, given the directory
  * authorities' opinion about the router (stored in <b>ri</b>) and the user's
- * configuration (in <b>options</b>). Set <b>e</b>-&gt;bad_since
+ * configuration (in <b>options</b>). Set <b>e</b>->bad_since
  * accordingly. Return true iff the entry guard's status changes.
  *
  * If it's not usable, set *<b>reason</b> to a static string explaining why.
@@ -117,6 +117,9 @@ entry_guard_set_status(entry_guard_t *e, const node_t *node,
     *reason = "not recommended as a guard";
   else if (routerset_contains_node(options->ExcludeNodes, node))
     *reason = "excluded";
+  /* We only care about OR connection connectivity for entry guards. */
+  else if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, 0))
+    *reason = "unreachable by config";
   else if (e->path_bias_disabled)
     *reason = "path-biased";
 
@@ -268,7 +271,7 @@ entry_is_live(const entry_guard_t *e, entry_is_live_flags_t flags,
     *msg = "not fast/stable";
     return NULL;
   }
-  if (!fascist_firewall_allows_node(node)) {
+  if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, 0)) {
     *msg = "unreachable by config";
     return NULL;
   }
@@ -918,7 +921,8 @@ entry_guards_set_from_config(const or_options_t *options)
     } else if (routerset_contains_node(options->ExcludeNodes, node)) {
       SMARTLIST_DEL_CURRENT(entry_nodes, node);
       continue;
-    } else if (!fascist_firewall_allows_node(node)) {
+    } else if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION,
+                                             0)) {
       SMARTLIST_DEL_CURRENT(entry_nodes, node);
       continue;
     } else if (! node->is_possible_guard) {
@@ -2116,8 +2120,17 @@ launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge)
     return;
   }
 
-  directory_initiate_command(&bridge->addr,
-                             bridge->port, 0/*no dirport*/,
+  /* Until we get a descriptor for the bridge, we only know one address for
+   * it. If we  */
+  if (!fascist_firewall_allows_address_addr(&bridge->addr, bridge->port,
+                                            FIREWALL_OR_CONNECTION, 0)) {
+    log_notice(LD_CONFIG, "Tried to fetch a descriptor directly from a bridge, "
+               "but that bridge is not reachable through our firewall.");
+    return;
+  }
+
+  directory_initiate_command(&bridge->addr, bridge->port,
+                             NULL, 0, /*no dirport*/
                              bridge->identity,
                              DIR_PURPOSE_FETCH_SERVERDESC,
                              ROUTER_PURPOSE_BRIDGE,
@@ -2178,7 +2191,8 @@ fetch_bridge_descriptors(const or_options_t *options, time_t now)
                 !options->UpdateBridgesFromAuthority, !num_bridge_auths);
 
       if (ask_bridge_directly &&
-          !fascist_firewall_allows_address_or(&bridge->addr, bridge->port)) {
+          !fascist_firewall_allows_address_addr(&bridge->addr, bridge->port,
+                                                FIREWALL_OR_CONNECTION, 0)) {
         log_notice(LD_DIR, "Bridge at '%s' isn't reachable by our "
                    "firewall policy. %s.",
                    fmt_addrport(&bridge->addr, bridge->port),
@@ -2226,6 +2240,7 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node)
    *   does so through an address from any source other than node_get_addr().
    */
   tor_addr_t addr;
+  const or_options_t *options = get_options();
 
   if (node->ri) {
     routerinfo_t *ri = node->ri;
@@ -2258,9 +2273,15 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node)
       }
     }
 
-    /* Mark which address to use based on which bridge_t we got. */
-    node->ipv6_preferred = (tor_addr_family(&bridge->addr) == AF_INET6 &&
-                            !tor_addr_is_null(&node->ri->ipv6_addr));
+    if (options->ClientPreferIPv6ORPort == -1) {
+      /* Mark which address to use based on which bridge_t we got. */
+      node->ipv6_preferred = (tor_addr_family(&bridge->addr) == AF_INET6 &&
+                              !tor_addr_is_null(&node->ri->ipv6_addr));
+    } else {
+      /* Mark which address to use based on user preference */
+      node->ipv6_preferred = (fascist_firewall_prefer_ipv6_orport(options) &&
+                              !tor_addr_is_null(&node->ri->ipv6_addr));
+    }
 
     /* XXXipv6 we lack support for falling back to another address for
        the same relay, warn the user */
@@ -2269,10 +2290,13 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node)
       node_get_pref_orport(node, &ap);
       log_notice(LD_CONFIG,
                  "Bridge '%s' has both an IPv4 and an IPv6 address.  "
-                 "Will prefer using its %s address (%s).",
+                 "Will prefer using its %s address (%s) based on %s.",
                  ri->nickname,
-                 tor_addr_family(&ap.addr) == AF_INET6 ? "IPv6" : "IPv4",
-                 fmt_addrport(&ap.addr, ap.port));
+                 node->ipv6_preferred ? "IPv6" : "IPv4",
+                 fmt_addrport(&ap.addr, ap.port),
+                 options->ClientPreferIPv6ORPort == -1 ?
+                 "the configured Bridge address" :
+                 "ClientPreferIPv6ORPort");
     }
   }
   if (node->rs) {

+ 238 - 69
src/or/nodelist.c

@@ -224,7 +224,6 @@ nodelist_set_consensus(networkstatus_t *ns)
 {
   const or_options_t *options = get_options();
   int authdir = authdir_mode_v3(options);
-  int client = !server_mode(options);
 
   init_nodelist();
   if (ns->flavor == FLAV_MICRODESC)
@@ -261,7 +260,7 @@ nodelist_set_consensus(networkstatus_t *ns)
       node->is_bad_exit = rs->is_bad_exit;
       node->is_hs_dir = rs->is_hs_dir;
       node->ipv6_preferred = 0;
-      if (client && options->ClientPreferIPv6ORPort == 1 &&
+      if (fascist_firewall_prefer_ipv6_orport(options) &&
           (tor_addr_is_null(&rs->ipv6_addr) == 0 ||
            (node->md && tor_addr_is_null(&node->md->ipv6_addr) == 0)))
         node->ipv6_preferred = 1;
@@ -761,6 +760,40 @@ node_exit_policy_is_exact(const node_t *node, sa_family_t family)
   return 1;
 }
 
+/* Check if the "addr" and port_field fields from r are a valid non-listening
+ * address/port. If so, set valid to true and add a newly allocated
+ * tor_addr_port_t containing "addr" and port_field to sl.
+ * "addr" is an IPv4 host-order address and port_field is a uint16_t.
+ * r is typically a routerinfo_t or routerstatus_t.
+ */
+#define SL_ADD_NEW_IPV4_AP(r, port_field, sl, valid) \
+  STMT_BEGIN \
+    if (tor_addr_port_is_valid_ipv4h((r)->addr, (r)->port_field, 0)) { \
+      valid = 1; \
+      tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); \
+      tor_addr_from_ipv4h(&ap->addr, (r)->addr); \
+      ap->port = (r)->port_field; \
+      smartlist_add((sl), ap); \
+    } \
+  STMT_END
+
+/* Check if the "addr" and port_field fields from r are a valid non-listening
+ * address/port. If so, set valid to true and add a newly allocated
+ * tor_addr_port_t containing "addr" and port_field to sl.
+ * "addr" is a tor_addr_t and port_field is a uint16_t.
+ * r is typically a routerinfo_t or routerstatus_t.
+ */
+#define SL_ADD_NEW_IPV6_AP(r, port_field, sl, valid) \
+  STMT_BEGIN \
+    if (tor_addr_port_is_valid(&(r)->ipv6_addr, (r)->port_field, 0)) { \
+      valid = 1; \
+      tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); \
+      tor_addr_copy(&ap->addr, &(r)->ipv6_addr); \
+      ap->port = (r)->port_field; \
+      smartlist_add((sl), ap); \
+    } \
+  STMT_END
+
 /** Return list of tor_addr_port_t with all OR ports (in the sense IP
  * addr + TCP port) for <b>node</b>.  Caller must free all elements
  * using tor_free() and free the list using smartlist_free().
@@ -773,30 +806,38 @@ smartlist_t *
 node_get_all_orports(const node_t *node)
 {
   smartlist_t *sl = smartlist_new();
+  int valid = 0;
 
+  /* Find a valid IPv4 address and port */
   if (node->ri != NULL) {
-    if (node->ri->addr != 0) {
-      tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t));
-      tor_addr_from_ipv4h(&ap->addr, node->ri->addr);
-      ap->port = node->ri->or_port;
-      smartlist_add(sl, ap);
-    }
-    if (!tor_addr_is_null(&node->ri->ipv6_addr)) {
-      tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t));
-      tor_addr_copy(&ap->addr, &node->ri->ipv6_addr);
-      ap->port = node->ri->or_port;
-      smartlist_add(sl, ap);
-    }
-  } else if (node->rs != NULL) {
-      tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t));
-      tor_addr_from_ipv4h(&ap->addr, node->rs->addr);
-      ap->port = node->rs->or_port;
-      smartlist_add(sl, ap);
+    SL_ADD_NEW_IPV4_AP(node->ri, or_port, sl, valid);
+  }
+
+  /* If we didn't find a valid address/port in the ri, try the rs */
+  if (!valid && node->rs != NULL) {
+    SL_ADD_NEW_IPV4_AP(node->rs, or_port, sl, valid);
+  }
+
+  /* Find a valid IPv6 address and port */
+  valid = 0;
+  if (node->ri != NULL) {
+    SL_ADD_NEW_IPV6_AP(node->ri, ipv6_orport, sl, valid);
+  }
+
+  if (!valid && node->rs != NULL) {
+    SL_ADD_NEW_IPV6_AP(node->rs, ipv6_orport, sl, valid);
+  }
+
+  if (!valid && node->md != NULL) {
+    SL_ADD_NEW_IPV6_AP(node->md, ipv6_orport, sl, valid);
   }
 
   return sl;
 }
 
+#undef SL_ADD_NEW_IPV4_AP
+#undef SL_ADD_NEW_IPV6_AP
+
 /** Wrapper around node_get_prim_orport for backward
     compatibility.  */
 void
@@ -812,9 +853,13 @@ node_get_addr(const node_t *node, tor_addr_t *addr_out)
 uint32_t
 node_get_prim_addr_ipv4h(const node_t *node)
 {
-  if (node->ri) {
+  /* Don't check the ORPort or DirPort, as this function isn't port-specific,
+   * and the node might have a valid IPv4 address, yet have a zero
+   * ORPort or DirPort.
+   */
+  if (node->ri && tor_addr_is_valid_ipv4h(node->ri->addr, 0)) {
     return node->ri->addr;
-  } else if (node->rs) {
+  } else if (node->rs && tor_addr_is_valid_ipv4h(node->rs->addr, 0)) {
     return node->rs->addr;
   }
   return 0;
@@ -825,13 +870,13 @@ node_get_prim_addr_ipv4h(const node_t *node)
 void
 node_get_address_string(const node_t *node, char *buf, size_t len)
 {
-  if (node->ri) {
-    strlcpy(buf, fmt_addr32(node->ri->addr), len);
-  } else if (node->rs) {
+  uint32_t ipv4_addr = node_get_prim_addr_ipv4h(node);
+
+  if (tor_addr_is_valid_ipv4h(ipv4_addr, 0)) {
     tor_addr_t addr;
-    tor_addr_from_ipv4h(&addr, node->rs->addr);
+    tor_addr_from_ipv4h(&addr, ipv4_addr);
     tor_addr_to_str(buf, &addr, len, 0);
-  } else {
+  } else if (len > 0) {
     buf[0] = '\0';
   }
 }
@@ -890,30 +935,80 @@ node_get_declared_family(const node_t *node)
     return NULL;
 }
 
+/* Does this node have a valid IPv6 address?
+ * Prefer node_has_ipv6_orport() or node_has_ipv6_dirport() for
+ * checking specific ports. */
+int
+node_has_ipv6_addr(const node_t *node)
+{
+  /* Don't check the ORPort or DirPort, as this function isn't port-specific,
+   * and the node might have a valid IPv6 address, yet have a zero
+   * ORPort or DirPort.
+   */
+  if (node->ri && tor_addr_is_valid(&node->ri->ipv6_addr, 0))
+    return 1;
+  if (node->rs && tor_addr_is_valid(&node->rs->ipv6_addr, 0))
+    return 1;
+  if (node->md && tor_addr_is_valid(&node->md->ipv6_addr, 0))
+    return 1;
+
+  return 0;
+}
+
+/* Does this node have a valid IPv6 ORPort? */
+int
+node_has_ipv6_orport(const node_t *node)
+{
+  tor_addr_port_t ipv6_orport;
+  node_get_pref_ipv6_orport(node, &ipv6_orport);
+  return tor_addr_port_is_valid_ap(&ipv6_orport, 0);
+}
+
+/* Does this node have a valid IPv6 DirPort? */
+int
+node_has_ipv6_dirport(const node_t *node)
+{
+  tor_addr_port_t ipv6_dirport;
+  node_get_pref_ipv6_dirport(node, &ipv6_dirport);
+  return tor_addr_port_is_valid_ap(&ipv6_dirport, 0);
+}
+
 /** Return 1 if we prefer the IPv6 address and OR TCP port of
  * <b>node</b>, else 0.
  *
- *  We prefer the IPv6 address if the router has an IPv6 address and
+ *  We prefer the IPv6 address if the router has an IPv6 address,
+ *  and we can use IPv6 addresses, and:
  *  i) the node_t says that it prefers IPv6
  *  or
- *  ii) the router has no IPv4 address. */
+ *  ii) the router has no IPv4 OR address.
+ */
 int
-node_ipv6_preferred(const node_t *node)
+node_ipv6_or_preferred(const node_t *node)
 {
+  const or_options_t *options = get_options();
   tor_addr_port_t ipv4_addr;
   node_assert_ok(node);
 
-  if (node->ipv6_preferred || node_get_prim_orport(node, &ipv4_addr)) {
-    if (node->ri)
-      return !tor_addr_is_null(&node->ri->ipv6_addr);
-    if (node->md)
-      return !tor_addr_is_null(&node->md->ipv6_addr);
-    if (node->rs)
-      return !tor_addr_is_null(&node->rs->ipv6_addr);
+  /* XX/teor - node->ipv6_preferred is set from
+   * fascist_firewall_prefer_ipv6_orport() each time the consensus is loaded.
+   */
+  if (!fascist_firewall_use_ipv6(options)) {
+    return 0;
+  } else if (node->ipv6_preferred || node_get_prim_orport(node, &ipv4_addr)) {
+    return node_has_ipv6_orport(node);
   }
   return 0;
 }
 
+#define RETURN_IPV4_AP(r, port_field, ap_out) \
+  STMT_BEGIN \
+    if (r && tor_addr_port_is_valid_ipv4h((r)->addr, (r)->port_field, 0)) { \
+      tor_addr_from_ipv4h(&(ap_out)->addr, (r)->addr); \
+      (ap_out)->port = (r)->port_field; \
+      return 0; \
+    } \
+  STMT_END
+
 /** Copy the primary (IPv4) OR port (IP address and TCP port) for
  * <b>node</b> into *<b>ap_out</b>. Return 0 if a valid address and
  * port was copied, else return non-zero.*/
@@ -923,20 +1018,10 @@ node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out)
   node_assert_ok(node);
   tor_assert(ap_out);
 
-  if (node->ri) {
-    if (node->ri->addr == 0 || node->ri->or_port == 0)
-      return -1;
-    tor_addr_from_ipv4h(&ap_out->addr, node->ri->addr);
-    ap_out->port = node->ri->or_port;
-    return 0;
-  }
-  if (node->rs) {
-    if (node->rs->addr == 0 || node->rs->or_port == 0)
-      return -1;
-    tor_addr_from_ipv4h(&ap_out->addr, node->rs->addr);
-    ap_out->port = node->rs->or_port;
-    return 0;
-  }
+  RETURN_IPV4_AP(node->ri, or_port, ap_out);
+  RETURN_IPV4_AP(node->rs, or_port, ap_out);
+  /* Microdescriptors only have an IPv6 address */
+
   return -1;
 }
 
@@ -945,21 +1030,12 @@ node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out)
 void
 node_get_pref_orport(const node_t *node, tor_addr_port_t *ap_out)
 {
-  const or_options_t *options = get_options();
   tor_assert(ap_out);
 
-  /* Cheap implementation of config option ClientUseIPv6 -- simply
-     don't prefer IPv6 when ClientUseIPv6 is not set and we're not a
-     client running with bridges. See #4455 for more on this subject.
-
-     Note that this filter is too strict since we're hindering not
-     only clients! Erring on the safe side shouldn't be a problem
-     though. XXX move this check to where outgoing connections are
-     made? -LN */
-  if ((options->ClientUseIPv6 || options->UseBridges) &&
-      node_ipv6_preferred(node)) {
+  if (node_ipv6_or_preferred(node)) {
     node_get_pref_ipv6_orport(node, ap_out);
   } else {
+    /* the primary ORPort is always on IPv4 */
     node_get_prim_orport(node, ap_out);
   }
 }
@@ -972,20 +1048,113 @@ node_get_pref_ipv6_orport(const node_t *node, tor_addr_port_t *ap_out)
   node_assert_ok(node);
   tor_assert(ap_out);
 
-  /* We prefer the microdesc over a potential routerstatus here. They
-     are not being synchronised atm so there might be a chance that
-     they differ at some point, f.ex. when flipping
-     UseMicrodescriptors? -LN */
+  /* Prefer routerstatus over microdesc for consistency with the
+   * fascist_firewall_* functions. Also check if the address or port are valid,
+   * and try another alternative if they are not. */
 
-  if (node->ri) {
+  if (node->ri && tor_addr_port_is_valid(&node->ri->ipv6_addr,
+                                         node->ri->ipv6_orport, 0)) {
     tor_addr_copy(&ap_out->addr, &node->ri->ipv6_addr);
     ap_out->port = node->ri->ipv6_orport;
-  } else if (node->md) {
+  } else if (node->rs && tor_addr_port_is_valid(&node->rs->ipv6_addr,
+                                                 node->rs->ipv6_orport, 0)) {
+    tor_addr_copy(&ap_out->addr, &node->rs->ipv6_addr);
+    ap_out->port = node->rs->ipv6_orport;
+  } else if (node->md && tor_addr_port_is_valid(&node->md->ipv6_addr,
+                                                 node->md->ipv6_orport, 0)) {
     tor_addr_copy(&ap_out->addr, &node->md->ipv6_addr);
     ap_out->port = node->md->ipv6_orport;
-  } else if (node->rs) {
+  } else {
+    tor_addr_make_null(&ap_out->addr, AF_INET6);
+    ap_out->port = 0;
+  }
+}
+
+/** Return 1 if we prefer the IPv6 address and Dir TCP port of
+ * <b>node</b>, else 0.
+ *
+ *  We prefer the IPv6 address if the router has an IPv6 address,
+ *  and we can use IPv6 addresses, and:
+ *  i) the router has no IPv4 Dir address.
+ *  or
+ *  ii) our preference is for IPv6 Dir addresses.
+ */
+int
+node_ipv6_dir_preferred(const node_t *node)
+{
+  const or_options_t *options = get_options();
+  tor_addr_port_t ipv4_addr;
+  node_assert_ok(node);
+
+  /* node->ipv6_preferred is set from fascist_firewall_prefer_ipv6_orport(),
+   * so we can't use it to determine DirPort IPv6 preference.
+   * This means that bridge clients will use IPv4 DirPorts by default.
+   */
+  if (!fascist_firewall_use_ipv6(options)) {
+    return 0;
+  } else if (node_get_prim_dirport(node, &ipv4_addr)
+      || fascist_firewall_prefer_ipv6_dirport(get_options())) {
+    return node_has_ipv6_dirport(node);
+  }
+  return 0;
+}
+
+/** Copy the primary (IPv4) Dir port (IP address and TCP port) for
+ * <b>node</b> into *<b>ap_out</b>. Return 0 if a valid address and
+ * port was copied, else return non-zero.*/
+int
+node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out)
+{
+  node_assert_ok(node);
+  tor_assert(ap_out);
+
+  RETURN_IPV4_AP(node->ri, dir_port, ap_out);
+  RETURN_IPV4_AP(node->rs, dir_port, ap_out);
+  /* Microdescriptors only have an IPv6 address */
+
+  return -1;
+}
+
+#undef RETURN_IPV4_AP
+
+/** Copy the preferred Dir port (IP address and TCP port) for
+ * <b>node</b> into *<b>ap_out</b>.  */
+void
+node_get_pref_dirport(const node_t *node, tor_addr_port_t *ap_out)
+{
+  tor_assert(ap_out);
+
+  if (node_ipv6_dir_preferred(node)) {
+    node_get_pref_ipv6_dirport(node, ap_out);
+  } else {
+    /* the primary DirPort is always on IPv4 */
+    node_get_prim_dirport(node, ap_out);
+  }
+}
+
+/** Copy the preferred IPv6 Dir port (IP address and TCP port) for
+ * <b>node</b> into *<b>ap_out</b>. */
+void
+node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out)
+{
+  node_assert_ok(node);
+  tor_assert(ap_out);
+
+  /* Check if the address or port are valid, and try another alternative if
+   * they are not. Note that microdescriptors have no dir_port. */
+
+  /* Assume IPv4 and IPv6 dirports are the same */
+  if (node->ri && tor_addr_port_is_valid(&node->ri->ipv6_addr,
+                                         node->ri->dir_port, 0)) {
+    tor_addr_copy(&ap_out->addr, &node->ri->ipv6_addr);
+    ap_out->port = node->ri->dir_port;
+  } else if (node->rs && tor_addr_port_is_valid(&node->rs->ipv6_addr,
+                                                node->rs->dir_port, 0)) {
     tor_addr_copy(&ap_out->addr, &node->rs->ipv6_addr);
-    ap_out->port = node->rs->ipv6_orport;
+    ap_out->port = node->rs->dir_port;
+  } else {
+    tor_addr_make_null(&ap_out->addr, AF_INET6);
+    ap_out->port = 0;
   }
 }
 

+ 11 - 1
src/or/nodelist.h

@@ -55,10 +55,20 @@ void node_get_address_string(const node_t *node, char *cp, size_t len);
 long node_get_declared_uptime(const node_t *node);
 time_t node_get_published_on(const node_t *node);
 const smartlist_t *node_get_declared_family(const node_t *node);
-int node_ipv6_preferred(const node_t *node);
+
+int node_has_ipv6_addr(const node_t *node);
+int node_has_ipv6_orport(const node_t *node);
+int node_has_ipv6_dirport(const node_t *node);
+/* Deprecated - use node_ipv6_or_preferred or node_ipv6_dir_preferred */
+#define node_ipv6_preferred(node) node_ipv6_or_preferred(node)
+int node_ipv6_or_preferred(const node_t *node);
 int node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out);
 void node_get_pref_orport(const node_t *node, tor_addr_port_t *ap_out);
 void node_get_pref_ipv6_orport(const node_t *node, tor_addr_port_t *ap_out);
+int node_ipv6_dir_preferred(const node_t *node);
+int node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out);
+void node_get_pref_dirport(const node_t *node, tor_addr_port_t *ap_out);
+void node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out);
 int node_has_curve25519_onion_key(const node_t *node);
 
 MOCK_DECL(smartlist_t *, nodelist_get_list, (void));

+ 23 - 6
src/or/or.h

@@ -2414,7 +2414,8 @@ typedef struct node_t {
 
   /* Local info: derived. */
 
-  /** True if the IPv6 OR port is preferred over the IPv4 OR port.  */
+  /** True if the IPv6 OR port is preferred over the IPv4 OR port.
+   * XX/teor - can this become out of date if the torrc changes? */
   unsigned int ipv6_preferred:1;
 
   /** According to the geoip db what country is this router in? */
@@ -4081,12 +4082,24 @@ typedef struct {
    * over randomly chosen exits. */
   int ClientRejectInternalAddresses;
 
-  /** If true, clients may connect over IPv6. XXX we don't really
-      enforce this -- clients _may_ set up outgoing IPv6 connections
-      even when this option is not set. */
+  /** If true, clients may connect over IPv4. If false, they will avoid
+   * connecting over IPv4. We enforce this for OR and Dir connections. */
+  int ClientUseIPv4;
+  /** If true, clients may connect over IPv6. If false, they will avoid
+   * connecting over IPv4. We enforce this for OR and Dir connections.
+   * Use fascist_firewall_use_ipv6() instead of accessing this value
+   * directly. */
   int ClientUseIPv6;
-  /** If true, prefer an IPv6 OR port over an IPv4 one. */
+  /** If true, prefer an IPv6 OR port over an IPv4 one for entry node
+   * connections. If auto, bridge clients prefer IPv6, and other clients
+   * prefer IPv4. Use fascist_firewall_prefer_ipv6_orport() instead of accessing
+   * this value directly. */
   int ClientPreferIPv6ORPort;
+  /** If true, prefer an IPv6 directory port over an IPv4 one for direct
+   * directory connections. If auto, bridge clients prefer IPv6, and other
+   * clients prefer IPv4. Use fascist_firewall_prefer_ipv6_dirport() instead of
+   * accessing this value directly.  */
+  int ClientPreferIPv6DirPort;
 
   /** The length of time that we think a consensus should be fresh. */
   int V3AuthVotingInterval;
@@ -5145,6 +5158,8 @@ typedef struct dir_server_t {
   char *description;
   char *nickname;
   char *address; /**< Hostname. */
+  /* XX/teor - why do we duplicate the address and port fields here and in
+   *           fake_status? Surely we could just use fake_status (#17867). */
   tor_addr_t ipv6_addr; /**< IPv6 address if present; AF_UNSPEC if not */
   uint32_t addr; /**< IPv4 address. */
   uint16_t dir_port; /**< Directory port. */
@@ -5230,7 +5245,9 @@ typedef enum {
   CRN_ALLOW_INVALID = 1<<3,
   /* XXXX not used, apparently. */
   CRN_WEIGHT_AS_EXIT = 1<<5,
-  CRN_NEED_DESC = 1<<6
+  CRN_NEED_DESC = 1<<6,
+  /* On clients, only provide nodes that satisfy ClientPreferIPv6OR */
+  CRN_PREF_ADDR = 1<<7
 } router_crn_flags_t;
 
 /** Return value for router_add_to_routerlist() and dirserv_add_descriptor() */

+ 666 - 29
src/or/policies.c

@@ -13,6 +13,7 @@
 #include "or.h"
 #include "config.h"
 #include "dirserv.h"
+#include "networkstatus.h"
 #include "nodelist.h"
 #include "policies.h"
 #include "router.h"
@@ -270,16 +271,76 @@ parse_reachable_addresses(void)
                "Error parsing ReachableDirAddresses entry; ignoring.");
     ret = -1;
   }
+
+  /* We ignore ReachableAddresses for relays */
+  if (!server_mode(options)) {
+    if ((reachable_or_addr_policy
+         && policy_is_reject_star(reachable_or_addr_policy, AF_UNSPEC))
+        || (reachable_dir_addr_policy
+            && policy_is_reject_star(reachable_dir_addr_policy, AF_UNSPEC))) {
+      log_warn(LD_CONFIG, "Tor cannot connect to the Internet if "
+               "ReachableAddresses, ReachableORAddresses, or "
+               "ReachableDirAddresses reject all addresses. Please accept "
+               "some addresses in these options.");
+    } else if (options->ClientUseIPv4 == 1
+       && ((reachable_or_addr_policy
+            && policy_is_reject_star(reachable_or_addr_policy, AF_INET))
+        || (reachable_dir_addr_policy
+            && policy_is_reject_star(reachable_dir_addr_policy, AF_INET)))) {
+          log_warn(LD_CONFIG, "You have set ClientUseIPv4 1, but "
+                   "ReachableAddresses, ReachableORAddresses, or "
+                   "ReachableDirAddresses reject all IPv4 addresses. "
+                   "Tor will not connect using IPv4.");
+    } else if (fascist_firewall_use_ipv6(options)
+       && ((reachable_or_addr_policy
+            && policy_is_reject_star(reachable_or_addr_policy, AF_INET6))
+        || (reachable_dir_addr_policy
+            && policy_is_reject_star(reachable_dir_addr_policy, AF_INET6)))) {
+          log_warn(LD_CONFIG, "You have configured tor to use IPv6 "
+                   "(ClientUseIPv6 1 or UseBridges 1), but "
+                   "ReachableAddresses, ReachableORAddresses, or "
+                   "ReachableDirAddresses reject all IPv6 addresses. "
+                   "Tor will not connect using IPv6.");
+    }
+  }
+
   return ret;
 }
 
-/** Return true iff the firewall options might block any address:port
- * combination.
+/* Return true iff ClientUseIPv4 0 or ClientUseIPv6 0 might block any OR or Dir
+ * address:port combination. */
+static int
+firewall_is_fascist_impl(void)
+{
+  const or_options_t *options = get_options();
+  /* Assume every non-bridge relay has an IPv4 address.
+   * Clients which use bridges may only know the IPv6 address of their
+   * bridge. */
+  return (options->ClientUseIPv4 == 0
+          || (!fascist_firewall_use_ipv6(options)
+              && options->UseBridges == 1));
+}
+
+/** Return true iff the firewall options, including ClientUseIPv4 0 and
+ * ClientUseIPv6 0, might block any OR address:port combination.
+ * Address preferences may still change which address is selected even if
+ * this function returns false.
  */
 int
 firewall_is_fascist_or(void)
 {
-  return reachable_or_addr_policy != NULL;
+  return (reachable_or_addr_policy != NULL || firewall_is_fascist_impl());
+}
+
+/** Return true iff the firewall options, including ClientUseIPv4 0 and
+ * ClientUseIPv6 0, might block any Dir address:port combination.
+ * Address preferences may still change which address is selected even if
+ * this function returns false.
+ */
+int
+firewall_is_fascist_dir(void)
+{
+  return (reachable_dir_addr_policy != NULL || firewall_is_fascist_impl());
 }
 
 /** Return true iff <b>policy</b> (possibly NULL) will allow a
@@ -317,49 +378,625 @@ addr_policy_permits_address(uint32_t addr, uint16_t port,
   return addr_policy_permits_tor_addr(&a, port, policy);
 }
 
-/** Return true iff we think our firewall will let us make an OR connection to
- * addr:port. */
-int
-fascist_firewall_allows_address_or(const tor_addr_t *addr, uint16_t port)
+/** Return true iff we think our firewall will let us make a connection to
+ * addr:port.
+ *
+ * If we are configured as a server, ignore any address family preference and
+ * just use IPv4.
+ * Otherwise:
+ *  - return false for all IPv4 addresses:
+ *    - if ClientUseIPv4 is 0, or
+ *      if pref_only and pref_ipv6 are both true;
+ *  - return false for all IPv6 addresses:
+ *    - if fascist_firewall_use_ipv6() is 0, or
+ *    - if pref_only is true and pref_ipv6 is false.
+ *
+ * Return false if addr is NULL or tor_addr_is_null(), or if port is 0. */
+STATIC int
+fascist_firewall_allows_address(const tor_addr_t *addr,
+                                uint16_t port,
+                                smartlist_t *firewall_policy,
+                                int pref_only, int pref_ipv6)
 {
+  const or_options_t *options = get_options();
+
+  if (!addr || tor_addr_is_null(addr) || !port) {
+    return 0;
+  }
+
+  if (!server_mode(options)) {
+    if (tor_addr_family(addr) == AF_INET &&
+        (!options->ClientUseIPv4 || (pref_only && pref_ipv6)))
+      return 0;
+
+    /* Bridges can always use IPv6 */
+    if (tor_addr_family(addr) == AF_INET6 &&
+        (!fascist_firewall_use_ipv6(options) || (pref_only && !pref_ipv6)))
+      return 0;
+  }
+
   return addr_policy_permits_tor_addr(addr, port,
-                                     reachable_or_addr_policy);
+                                      firewall_policy);
 }
 
-/** Return true iff we think our firewall will let us make an OR connection to
- * <b>ri</b>. */
+/** Is this client configured to use IPv6?
+ */
+int fascist_firewall_use_ipv6(const or_options_t *options)
+{
+  /* Clients use IPv6 if it's set, or they use bridges, or they don't use
+   * IPv4 */
+  return (options->ClientUseIPv6 == 1 || options->UseBridges == 1
+          || options->ClientUseIPv4 == 0);
+}
+
+/** Do we prefer to connect to IPv6, ignoring ClientPreferIPv6ORPort and
+ * ClientPreferIPv6DirPort?
+ * If we're unsure, return -1, otherwise, return 1 for IPv6 and 0 for IPv4.
+ */
+static int
+fascist_firewall_prefer_ipv6_impl(const or_options_t *options)
+{
+  /*
+   Cheap implementation of config options ClientUseIPv4 & ClientUseIPv6 --
+   If we're a server or IPv6 is disabled, use IPv4.
+   If IPv4 is disabled, use IPv6.
+   */
+
+  if (server_mode(options) || !fascist_firewall_use_ipv6(options)) {
+    return 0;
+  }
+
+  if (!options->ClientUseIPv4) {
+    return 1;
+  }
+
+  return -1;
+}
+
+/** Do we prefer to connect to IPv6 ORPorts?
+ */
 int
-fascist_firewall_allows_or(const routerinfo_t *ri)
+fascist_firewall_prefer_ipv6_orport(const or_options_t *options)
+{
+  /* node->ipv6_preferred is set from fascist_firewall_prefer_ipv6_orport()
+   * each time the consensus is loaded.
+   * If our preferences change, we will only reset ipv6_preferred on the node
+   * when the next consensus is loaded. But the consensus is realoded when the
+   * configuration changes after a HUP. So as long as the result of this
+   * function only depends on Tor's options, everything should work ok.
+   */
+  int pref_ipv6 = fascist_firewall_prefer_ipv6_impl(options);
+
+  if (pref_ipv6 >= 0) {
+    return pref_ipv6;
+  }
+
+  /* We can use both IPv4 and IPv6 - which do we prefer? */
+  if (options->ClientPreferIPv6ORPort == 1) {
+    return 1;
+  }
+
+  return 0;
+}
+
+/** Do we prefer to connect to IPv6 DirPorts?
+ */
+int
+fascist_firewall_prefer_ipv6_dirport(const or_options_t *options)
 {
-  /* XXXX proposal 118 */
-  tor_addr_t addr;
-  tor_addr_from_ipv4h(&addr, ri->addr);
-  return fascist_firewall_allows_address_or(&addr, ri->or_port);
+  int pref_ipv6 = fascist_firewall_prefer_ipv6_impl(options);
+
+  if (pref_ipv6 >= 0) {
+    return pref_ipv6;
+  }
+
+  /* We can use both IPv4 and IPv6 - which do we prefer? */
+  if (options->ClientPreferIPv6DirPort == 1) {
+    return 1;
+  }
+
+  return 0;
 }
 
-/** Return true iff we think our firewall will let us make an OR connection to
- * <b>node</b>. */
+/** Return true iff we think our firewall will let us make a connection to
+ * addr:port. Uses ReachableORAddresses or ReachableDirAddresses based on
+ * fw_connection.
+ * If pref_only, return false if addr is not in the client's preferred address
+ * family.
+ */
 int
-fascist_firewall_allows_node(const node_t *node)
+fascist_firewall_allows_address_addr(const tor_addr_t *addr, uint16_t port,
+                                     firewall_connection_t fw_connection,
+                                     int pref_only)
 {
-  if (node->ri) {
-    return fascist_firewall_allows_or(node->ri);
-  } else if (node->rs) {
-    tor_addr_t addr;
-    tor_addr_from_ipv4h(&addr, node->rs->addr);
-    return fascist_firewall_allows_address_or(&addr, node->rs->or_port);
+  const or_options_t *options = get_options();
+
+  if (fw_connection == FIREWALL_OR_CONNECTION) {
+    return fascist_firewall_allows_address(addr, port,
+                                        reachable_or_addr_policy,
+                                        pref_only,
+                                        fascist_firewall_prefer_ipv6_orport(options));
+  } else if (fw_connection == FIREWALL_DIR_CONNECTION) {
+    return fascist_firewall_allows_address(addr, port,
+                                        reachable_dir_addr_policy,
+                                        pref_only,
+                                        fascist_firewall_prefer_ipv6_dirport(options));
   } else {
+    log_warn(LD_BUG, "Bad firewall_connection_t value %d.",
+             fw_connection);
+    return 0;
+  }
+}
+
+/** Return true iff we think our firewall will let us make a connection to
+ * addr:port (ap). Uses ReachableORAddresses or ReachableDirAddresses based on
+ * fw_connection.
+ * If pref_only, return false if addr is not in the client's preferred address
+ * family.
+ */
+int
+fascist_firewall_allows_address_ap(const tor_addr_port_t *ap,
+                                   firewall_connection_t fw_connection,
+                                   int pref_only)
+{
+  tor_assert(ap);
+  return fascist_firewall_allows_address_addr(&ap->addr, ap->port,
+                                              fw_connection, pref_only);
+}
+
+/* Return true iff we think our firewall will let us make a connection to
+ * ipv4h_or_addr:ipv4_or_port. ipv4h_or_addr is interpreted in host order.
+ * Uses ReachableORAddresses or ReachableDirAddresses based on
+ * fw_connection.
+ * If pref_only, return false if addr is not in the client's preferred address
+ * family. */
+int
+fascist_firewall_allows_address_ipv4h(uint32_t ipv4h_or_addr,
+                                          uint16_t ipv4_or_port,
+                                          firewall_connection_t fw_connection,
+                                          int pref_only)
+{
+  tor_addr_t ipv4_or_addr;
+  tor_addr_from_ipv4h(&ipv4_or_addr, ipv4h_or_addr);
+  return fascist_firewall_allows_address_addr(&ipv4_or_addr, ipv4_or_port,
+                                              fw_connection, pref_only);
+}
+
+/** Return true iff we think our firewall will let us make a connection to
+ * ipv4h_addr/ipv6_addr. Uses ipv4_orport/ipv6_orport/ReachableORAddresses or
+ * ipv4_dirport/ipv6_dirport/ReachableDirAddresses based on IPv4/IPv6 and
+ * <b>fw_connection</b>.
+ * If pref_only, return false if addr is not in the client's preferred address
+ * family. */
+static int
+fascist_firewall_allows_base(uint32_t ipv4h_addr, uint16_t ipv4_orport,
+                             uint16_t ipv4_dirport,
+                             const tor_addr_t *ipv6_addr, uint16_t ipv6_orport,
+                             uint16_t ipv6_dirport,
+                             firewall_connection_t fw_connection,
+                             int pref_only)
+{
+  if (fascist_firewall_allows_address_ipv4h(ipv4h_addr,
+                                      (fw_connection == FIREWALL_OR_CONNECTION
+                                       ? ipv4_orport
+                                       : ipv4_dirport),
+                                      fw_connection,
+                                      pref_only)) {
+    return 1;
+  }
+
+  if (fascist_firewall_allows_address_addr(ipv6_addr,
+                                      (fw_connection == FIREWALL_OR_CONNECTION
+                                       ? ipv6_orport
+                                       : ipv6_dirport),
+                                      fw_connection,
+                                      pref_only)) {
     return 1;
   }
+
+  return 0;
 }
 
-/** Return true iff we think our firewall will let us make a directory
- * connection to addr:port. */
+/** Like fascist_firewall_allows_ri, but doesn't consult the node. */
+static int
+fascist_firewall_allows_ri_impl(const routerinfo_t *ri,
+                                firewall_connection_t fw_connection,
+                                int pref_only)
+{
+  if (!ri) {
+    return 0;
+  }
+
+  /* Assume IPv4 and IPv6 DirPorts are the same */
+  return fascist_firewall_allows_base(ri->addr, ri->or_port, ri->dir_port,
+                                      &ri->ipv6_addr, ri->ipv6_orport,
+                                      ri->dir_port, fw_connection, pref_only);
+}
+
+/** Like fascist_firewall_allows_rs, but doesn't consult the node. */
+static int
+fascist_firewall_allows_rs_impl(const routerstatus_t *rs,
+                                firewall_connection_t fw_connection,
+                                int pref_only)
+{
+  if (!rs) {
+    return 0;
+  }
+
+  /* Assume IPv4 and IPv6 DirPorts are the same */
+  return fascist_firewall_allows_base(rs->addr, rs->or_port, rs->dir_port,
+                                      &rs->ipv6_addr, rs->ipv6_orport,
+                                      rs->dir_port, fw_connection, pref_only);
+}
+
+/** Return true iff we think our firewall will let us make a connection to
+ * <b>rs</b> on either its IPv4 or IPv6 address. Uses
+ * or_port/ipv6_orport/ReachableORAddresses or dir_port/ReachableDirAddresses
+ * based on IPv4/IPv6 and <b>fw_connection</b>.
+ * If pref_only, return false if addr is not in the client's preferred address
+ * family.
+ * Consults the corresponding node if the addresses in rs are not permitted. */
 int
-fascist_firewall_allows_address_dir(const tor_addr_t *addr, uint16_t port)
+fascist_firewall_allows_rs(const routerstatus_t *rs,
+                           firewall_connection_t fw_connection, int pref_only)
 {
-  return addr_policy_permits_tor_addr(addr, port,
-                                      reachable_dir_addr_policy);
+  if (!rs) {
+    return 0;
+  }
+
+  /* Assume IPv4 and IPv6 DirPorts are the same */
+  if (fascist_firewall_allows_rs_impl(rs, fw_connection, pref_only)) {
+    return 1;
+  } else {
+    const node_t *node = node_get_by_id(rs->identity_digest);
+    return fascist_firewall_allows_node(node, fw_connection, pref_only);
+  }
+}
+
+/** Like fascist_firewall_allows_md, but doesn't consult the node. */
+static int
+fascist_firewall_allows_md_impl(const microdesc_t *md,
+                                firewall_connection_t fw_connection,
+                                int pref_only)
+{
+  if (!md) {
+    return 0;
+  }
+
+  /* Can't check dirport, it doesn't have one */
+  if (fw_connection == FIREWALL_DIR_CONNECTION) {
+    return 0;
+  }
+
+  /* Also can't check IPv4, doesn't have that either */
+  return fascist_firewall_allows_address_addr(&md->ipv6_addr, md->ipv6_orport,
+                                              fw_connection, pref_only);
+}
+
+/** Return true iff we think our firewall will let us make a connection to
+ * <b>node</b>:
+ *  - if <b>preferred</b> is true, on its preferred address,
+ *  - if not, on either its IPv4 or IPv6 address.
+ * Uses or_port/ipv6_orport/ReachableORAddresses or
+ * dir_port/ReachableDirAddresses based on IPv4/IPv6 and <b>fw_connection</b>.
+ * If pref_only, return false if addr is not in the client's preferred address
+ * family. */
+int
+fascist_firewall_allows_node(const node_t *node,
+                             firewall_connection_t fw_connection,
+                             int pref_only)
+{
+  if (!node) {
+    return 0;
+  }
+
+  node_assert_ok(node);
+
+  /* Sometimes, the rs is missing the IPv6 address info, and we need to go
+   * all the way to the md */
+  if (node->ri && fascist_firewall_allows_ri_impl(node->ri, fw_connection,
+                                                  pref_only)) {
+    return 1;
+  } else if (node->rs && fascist_firewall_allows_rs_impl(node->rs,
+                                                         fw_connection,
+                                                         pref_only)) {
+    return 1;
+  } else if (node->md && fascist_firewall_allows_md_impl(node->md,
+                                                         fw_connection,
+                                                         pref_only)) {
+    return 1;
+  } else {
+    /* If we know nothing, assume it's unreachable, we'll never get an address
+     * to connect to. */
+    return 0;
+  }
+}
+
+/** Return true iff we think our firewall will let us make a connection to
+ * <b>ds</b> on either its IPv4 or IPv6 address. Uses ReachableORAddresses or
+ * ReachableDirAddresses based on <b>fw_connection</b> (some directory
+ * connections are tunneled over ORPorts).
+ * If pref_only, return false if addr is not in the client's preferred address
+ * family. */
+int
+fascist_firewall_allows_dir_server(const dir_server_t *ds,
+                                   firewall_connection_t fw_connection,
+                                   int pref_only)
+{
+  if (!ds) {
+    return 0;
+  }
+
+  /* A dir_server_t always has a fake_status. As long as it has the same
+   * addresses/ports in both fake_status and dir_server_t, this works fine.
+   * (See #17867.)
+   * This function relies on fascist_firewall_allows_rs looking up the node on
+   * failure, because it will get the latest info for the relay. */
+  return fascist_firewall_allows_rs(&ds->fake_status, fw_connection,
+                                    pref_only);
+}
+
+/** If a and b are both valid and allowed by fw_connection,
+ * choose one based on want_a and return it.
+ * Otherwise, return whichever is allowed.
+ * Otherwise, return NULL.
+ * If pref_only, only return an address if it's in the client's preferred
+ * address family. */
+static const tor_addr_port_t *
+fascist_firewall_choose_address_impl(const tor_addr_port_t *a,
+                                     const tor_addr_port_t *b,
+                                     int want_a,
+                                     firewall_connection_t fw_connection,
+                                     int pref_only)
+{
+  const tor_addr_port_t *use_a = NULL;
+  const tor_addr_port_t *use_b = NULL;
+
+  if (fascist_firewall_allows_address_ap(a, fw_connection, pref_only)) {
+    use_a = a;
+  }
+
+  if (fascist_firewall_allows_address_ap(b, fw_connection, pref_only)) {
+    use_b = b;
+  }
+
+  /* If both are allowed */
+  if (use_a && use_b) {
+    /* Choose a if we want it */
+    return (want_a ? use_a : use_b);
+  } else {
+    /* Choose a if we have it */
+    return (use_a ? use_a : use_b);
+  }
+}
+
+/** If a and b are both valid and preferred by fw_connection,
+ * choose one based on want_a and return it.
+ * Otherwise, return whichever is preferred.
+ * If neither are preferred, and pref_only is false:
+ *  - If a and b are both allowed by fw_connection,
+ *    choose one based on want_a and return it.
+ *  - Otherwise, return whichever is preferred.
+ * Otherwise, return NULL. */
+const tor_addr_port_t *
+fascist_firewall_choose_address(const tor_addr_port_t *a,
+                                const tor_addr_port_t *b,
+                                int want_a,
+                                firewall_connection_t fw_connection,
+                                int pref_only)
+{
+  const tor_addr_port_t *pref = fascist_firewall_choose_address_impl(
+                                                                a, b, want_a,
+                                                                fw_connection,
+                                                                1);
+  if (pref_only || pref) {
+    /* If there is a preferred address, use it. If we can only use preferred
+     * addresses, and neither address is preferred, pref will be NULL, and we
+     * want to return NULL, so return it. */
+    return pref;
+  } else {
+    /* If there's no preferred address, and we can return addresses that are
+     * not preferred, use an address that's allowed */
+    return fascist_firewall_choose_address_impl(a, b, want_a, fw_connection,
+                                                0);
+  }
+}
+
+/** Copy an address and port into <b>ap</b> that we think our firewall will
+ * let us connect to. Uses ipv4_addr/ipv6_addr and
+ * ipv4_orport/ipv6_orport/ReachableORAddresses or
+ * ipv4_dirport/ipv6_dirport/ReachableDirAddresses based on IPv4/IPv6 and
+ * <b>fw_connection</b>.
+ * If pref_only, only choose preferred addresses. In either case, choose
+ * a preferred address before an address that's not preferred.
+ * If neither address is chosen, return 0, else return 1. */
+static int
+fascist_firewall_choose_address_base(const tor_addr_t *ipv4_addr,
+                                     uint16_t ipv4_orport,
+                                     uint16_t ipv4_dirport,
+                                     const tor_addr_t *ipv6_addr,
+                                     uint16_t ipv6_orport,
+                                     uint16_t ipv6_dirport,
+                                     firewall_connection_t fw_connection,
+                                     int pref_only,
+                                     tor_addr_port_t* ap)
+{
+  const tor_addr_port_t *result = NULL;
+  /* This argument is ignored as long as the address pair is IPv4/IPv6,
+   * because we always have a preference in a client.
+   * For bridge clients, this selects the preferred address, which was
+   * previously IPv6 (if a bridge has both), so we keep that behaviour. */
+  const int bridge_client_prefer_ipv4 = 0;
+
+  tor_assert(ipv6_addr);
+  tor_assert(ap);
+
+  tor_addr_port_t ipv4_ap;
+  tor_addr_copy(&ipv4_ap.addr, ipv4_addr);
+  ipv4_ap.port = (fw_connection == FIREWALL_OR_CONNECTION
+                  ? ipv4_orport
+                  : ipv4_dirport);
+
+  tor_addr_port_t ipv6_ap;
+  tor_addr_copy(&ipv6_ap.addr, ipv6_addr);
+  ipv6_ap.port = (fw_connection == FIREWALL_OR_CONNECTION
+                  ? ipv6_orport
+                  : ipv6_dirport);
+
+  result = fascist_firewall_choose_address(&ipv4_ap, &ipv6_ap,
+                                           bridge_client_prefer_ipv4,
+                                           fw_connection, pref_only);
+
+  if (result) {
+    tor_addr_copy(&ap->addr, &result->addr);
+    ap->port = result->port;
+    return 1;
+  } else {
+    return 0;
+  }
+}
+
+/** Like fascist_firewall_choose_address_base, but takes a host-order IPv4
+ * address as the first parameter. */
+static int
+fascist_firewall_choose_address_ipv4h(uint32_t ipv4h_addr,
+                                      uint16_t ipv4_orport,
+                                      uint16_t ipv4_dirport,
+                                      const tor_addr_t *ipv6_addr,
+                                      uint16_t ipv6_orport,
+                                      uint16_t ipv6_dirport,
+                                      firewall_connection_t fw_connection,
+                                      int pref_only,
+                                      tor_addr_port_t* ap)
+{
+  tor_addr_t ipv4_addr;
+  tor_addr_from_ipv4h(&ipv4_addr, ipv4h_addr);
+  return fascist_firewall_choose_address_base(&ipv4_addr, ipv4_orport,
+                                              ipv4_dirport, ipv6_addr,
+                                              ipv6_orport, ipv6_dirport,
+                                              fw_connection, pref_only, ap);
+}
+
+#define IPV6_OR_LOOKUP(r, identity_digest, ipv6_or_ap) \
+  STMT_BEGIN \
+    if (!(r)->ipv6_orport || tor_addr_is_null(&(r)->ipv6_addr)) { \
+      const node_t *node = node_get_by_id((identity_digest)); \
+      if (node) { \
+        node_get_pref_ipv6_orport(node, &(ipv6_or_ap)); \
+      } else { \
+        tor_addr_make_null(&(ipv6_or_ap).addr, AF_INET6); \
+        (ipv6_or_ap).port = 0; \
+      } \
+    } else { \
+      tor_addr_copy(&(ipv6_or_ap).addr, &(r)->ipv6_addr); \
+      (ipv6_or_ap).port = (r)->ipv6_orport; \
+    } \
+  STMT_END
+
+/** Copy an address and port from <b>rs</b> into <b>ap</b> that we think our
+ * firewall will let us connect to. Uses ipv4h_addr/ipv6_addr and
+ * ipv4_orport/ipv6_orport/ReachableORAddresses or
+ * ipv4_dirport/ipv6_dirport/ReachableDirAddresses based on IPv4/IPv6 and
+ * <b>fw_connection</b>.
+ * If pref_only, only choose preferred addresses. In either case, choose
+ * a preferred address before an address that's not preferred.
+ * If neither address is chosen, return 0, else return 1.
+ * Consults the corresponding node if the addresses in rs are not valid. */
+int
+fascist_firewall_choose_address_rs(const routerstatus_t *rs,
+                                   firewall_connection_t fw_connection,
+                                   int pref_only, tor_addr_port_t* ap)
+{
+  if (!rs) {
+    return 0;
+  }
+
+  tor_assert(ap);
+
+  /* Don't do the lookup if the IPv6 address/port in rs is OK.
+   * If it's OK, assume the dir_port is also OK. */
+  tor_addr_port_t ipv6_or_ap;
+  IPV6_OR_LOOKUP(rs, rs->identity_digest, ipv6_or_ap);
+
+  /* Assume IPv4 and IPv6 DirPorts are the same.
+   * Assume the IPv6 OR and Dir addresses are the same. */
+  return fascist_firewall_choose_address_ipv4h(rs->addr,
+                                               rs->or_port,
+                                               rs->dir_port,
+                                               &ipv6_or_ap.addr,
+                                               ipv6_or_ap.port,
+                                               rs->dir_port,
+                                               fw_connection,
+                                               pref_only,
+                                               ap);
+}
+
+/** Copy an address and port from <b>node</b> into <b>ap</b> that we think our
+ * firewall will let us connect to. Uses ipv4h_addr/ipv6_addr and
+ * ipv4_orport/ipv6_orport/ReachableORAddresses or
+ * ipv4_dirport/ipv6_dirport/ReachableDirAddresses based on IPv4/IPv6 and
+ * <b>fw_connection</b>.
+ * If pref_only, only choose preferred addresses. In either case, choose
+ * a preferred address before an address that's not preferred.
+ * If neither address is chosen, return 0, else return 1. */
+int
+fascist_firewall_choose_address_node(const node_t *node,
+                                     firewall_connection_t fw_connection,
+                                     int pref_only, tor_addr_port_t *ap)
+{
+  if (!node) {
+    return 0;
+  }
+
+  node_assert_ok(node);
+
+  tor_addr_port_t ipv4_or_ap;
+  node_get_prim_orport(node, &ipv4_or_ap);
+  tor_addr_port_t ipv4_dir_ap;
+  node_get_prim_dirport(node, &ipv4_dir_ap);
+
+  tor_addr_port_t ipv6_or_ap;
+  node_get_pref_ipv6_orport(node, &ipv6_or_ap);
+  tor_addr_port_t ipv6_dir_ap;
+  node_get_pref_ipv6_dirport(node, &ipv6_dir_ap);
+
+  /* Assume the IPv6 OR and Dir addresses are the same. */
+  return fascist_firewall_choose_address_base(&ipv4_or_ap.addr,
+                                              ipv4_or_ap.port,
+                                              ipv4_dir_ap.port,
+                                              &ipv6_or_ap.addr,
+                                              ipv6_or_ap.port,
+                                              ipv6_dir_ap.port,
+                                              fw_connection,
+                                              pref_only,
+                                              ap);
+}
+
+/** Copy an address and port from <b>ds</b> into <b>ap</b> that we think our
+ * firewall will let us connect to. Uses ipv4h_addr/ipv6_addr and
+ * ipv4_orport/ipv6_orport/ReachableORAddresses or
+ * ipv4_dirport/ipv6_dirport/ReachableDirAddresses based on IPv4/IPv6 and
+ * <b>fw_connection</b>.
+ * If pref_only, only choose preferred addresses. In either case, choose
+ * a preferred address before an address that's not preferred.
+ * If neither address is chosen, return 0, else return 1. */
+int
+fascist_firewall_choose_address_dir_server(const dir_server_t *ds,
+                                           firewall_connection_t fw_connection,
+                                           int pref_only, tor_addr_port_t *ap)
+{
+  if (!ds) {
+    return 0;
+  }
+
+  /* A dir_server_t always has a fake_status. As long as it has the same
+   * addresses/ports in both fake_status and dir_server_t, this works fine.
+   * (See #17867.)
+   * This function relies on fascist_firewall_choose_address_rs looking up the
+   * addresses from the node if it can, because that will get the latest info
+   * for the relay. */
+  return fascist_firewall_choose_address_rs(&ds->fake_status, fw_connection,
+                                            pref_only, ap);
 }
 
 /** Return 1 if <b>addr</b> is permitted to connect to our dir port,

+ 50 - 4
src/or/policies.h

@@ -22,13 +22,55 @@
 #define EXIT_POLICY_REJECT_PRIVATE (1 << 1)
 #define EXIT_POLICY_ADD_DEFAULT    (1 << 2)
 
+typedef enum firewall_connection_t {
+  FIREWALL_OR_CONNECTION      = 0,
+  FIREWALL_DIR_CONNECTION     = 1
+} firewall_connection_t;
+
 typedef int exit_policy_parser_cfg_t;
 
 int firewall_is_fascist_or(void);
-int fascist_firewall_allows_address_or(const tor_addr_t *addr, uint16_t port);
-int fascist_firewall_allows_or(const routerinfo_t *ri);
-int fascist_firewall_allows_node(const node_t *node);
-int fascist_firewall_allows_address_dir(const tor_addr_t *addr, uint16_t port);
+int firewall_is_fascist_dir(void);
+int fascist_firewall_use_ipv6(const or_options_t *options);
+int fascist_firewall_prefer_ipv6_orport(const or_options_t *options);
+int fascist_firewall_prefer_ipv6_dirport(const or_options_t *options);
+
+int fascist_firewall_allows_address_addr(const tor_addr_t *addr, uint16_t port,
+                                         firewall_connection_t fw_connection,
+                                         int pref_only);
+int fascist_firewall_allows_address_ap(const tor_addr_port_t *ap,
+                                       firewall_connection_t fw_connection,
+                                       int pref_only);
+int fascist_firewall_allows_address_ipv4h(uint32_t ipv4h_or_addr,
+                                          uint16_t ipv4_or_port,
+                                          firewall_connection_t fw_connection,
+                                          int pref_only);
+int fascist_firewall_allows_rs(const routerstatus_t *rs,
+                               firewall_connection_t fw_connection,
+                               int pref_only);
+int fascist_firewall_allows_node(const node_t *node,
+                                 firewall_connection_t fw_connection,
+                                 int pref_only);
+int fascist_firewall_allows_dir_server(const dir_server_t *ds,
+                                       firewall_connection_t fw_connection,
+                                       int pref_only);
+
+const tor_addr_port_t * fascist_firewall_choose_address(
+                                          const tor_addr_port_t *a,
+                                          const tor_addr_port_t *b,
+                                          int want_a,
+                                          firewall_connection_t fw_connection,
+                                          int pref_only);
+int fascist_firewall_choose_address_rs(const routerstatus_t *rs,
+                                       firewall_connection_t fw_connection,
+                                       int pref_only, tor_addr_port_t* ap);
+int fascist_firewall_choose_address_node(const node_t *node,
+                                         firewall_connection_t fw_connection,
+                                         int pref_only, tor_addr_port_t* ap);
+int fascist_firewall_choose_address_dir_server(const dir_server_t *ds,
+                                          firewall_connection_t fw_connection,
+                                           int pref_only, tor_addr_port_t* ap);
+
 int dir_policy_permits_address(const tor_addr_t *addr);
 int socks_policy_permits_address(const tor_addr_t *addr);
 int authdir_policy_permits_address(uint32_t addr, uint16_t port);
@@ -94,6 +136,10 @@ addr_policy_result_t compare_tor_addr_to_short_policy(
 
 #ifdef POLICIES_PRIVATE
 STATIC void append_exit_policy_string(smartlist_t **policy, const char *more);
+STATIC int fascist_firewall_allows_address(const tor_addr_t *addr,
+                                           uint16_t port,
+                                           smartlist_t *firewall_policy,
+                                           int pref_only, int pref_ipv6);
 #endif
 
 #endif

+ 10 - 2
src/or/rendclient.c

@@ -1367,11 +1367,19 @@ rend_client_get_random_intro_impl(const rend_cache_entry_t *entry,
       smartlist_del(usable_nodes, i);
       goto again;
     }
+#ifdef ENABLE_TOR2WEB_MODE
+    new_extend_info = extend_info_from_node(node, options->Tor2webMode);
+#else
     new_extend_info = extend_info_from_node(node, 0);
+#endif
     if (!new_extend_info) {
+      const char *alternate_reason = "";
+#ifdef ENABLE_TOR2WEB_MODE
+      alternate_reason = ", or we cannot connect directly to it";
+#endif
       log_info(LD_REND, "We don't have a descriptor for the intro-point relay "
-               "'%s'; trying another.",
-               extend_info_describe(intro->extend_info));
+               "'%s'%s; trying another.",
+               extend_info_describe(intro->extend_info), alternate_reason);
       smartlist_del(usable_nodes, i);
       goto again;
     } else {

+ 10 - 21
src/or/router.c

@@ -1293,14 +1293,15 @@ consider_testing_reachability(int test_or, int test_dir)
     extend_info_free(ei);
   }
 
+  /* XXX IPv6 self testing */
   tor_addr_from_ipv4h(&addr, me->addr);
   if (test_dir && !check_whether_dirport_reachable() &&
       !connection_get_by_type_addr_port_purpose(
                 CONN_TYPE_DIR, &addr, me->dir_port,
                 DIR_PURPOSE_FETCH_SERVERDESC)) {
     /* ask myself, via tor, for my server descriptor. */
-    directory_initiate_command(&addr,
-                               me->or_port, me->dir_port,
+    directory_initiate_command(&addr, me->or_port,
+                               &addr, me->dir_port,
                                me->cache_info.identity_digest,
                                DIR_PURPOSE_FETCH_SERVERDESC,
                                ROUTER_PURPOSE_GENERAL,
@@ -3413,28 +3414,16 @@ router_free_all(void)
 
 /** Return a smartlist of tor_addr_port_t's with all the OR ports of
     <b>ri</b>. Note that freeing of the items in the list as well as
-    the smartlist itself is the callers responsibility.
-
-    XXX duplicating code from node_get_all_orports(). */
+    the smartlist itself is the callers responsibility. */
 smartlist_t *
 router_get_all_orports(const routerinfo_t *ri)
 {
-  smartlist_t *sl = smartlist_new();
   tor_assert(ri);
-
-  if (ri->addr != 0) {
-    tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t));
-    tor_addr_from_ipv4h(&ap->addr, ri->addr);
-    ap->port = ri->or_port;
-    smartlist_add(sl, ap);
-  }
-  if (!tor_addr_is_null(&ri->ipv6_addr)) {
-    tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t));
-    tor_addr_copy(&ap->addr, &ri->ipv6_addr);
-    ap->port = ri->or_port;
-    smartlist_add(sl, ap);
-  }
-
-  return sl;
+  node_t fake_node;
+  memset(&fake_node, 0, sizeof(fake_node));
+  /* we don't modify ri, fake_node is passed as a const node_t *
+   */
+  fake_node.ri = (routerinfo_t *)ri;
+  return node_get_all_orports(&fake_node);
 }
 

+ 259 - 73
src/or/routerlist.c

@@ -13,6 +13,7 @@
 
 #define ROUTERLIST_PRIVATE
 #include "or.h"
+#include "backtrace.h"
 #include "crypto_ed25519.h"
 #include "circuitstats.h"
 #include "config.h"
@@ -1460,9 +1461,190 @@ router_pick_dirserver_generic(smartlist_t *sourcelist,
   return router_pick_trusteddirserver_impl(sourcelist, type, flags, NULL);
 }
 
+/* Check if we already have a directory fetch from ap, for serverdesc
+ * (including extrainfo) or microdesc documents.
+ * If so, return 1, if not, return 0.
+ * Also returns 0 if addr is NULL, tor_addr_is_null(addr), or dir_port is 0.
+ */
+STATIC int
+router_is_already_dir_fetching(const tor_addr_port_t *ap, int serverdesc,
+                               int microdesc)
+{
+  if (!ap || tor_addr_is_null(&ap->addr) || !ap->port) {
+    return 0;
+  }
+
+  /* XX/teor - we're not checking tunnel connections here, see #17848
+   */
+  if (serverdesc && (
+     connection_get_by_type_addr_port_purpose(
+       CONN_TYPE_DIR, &ap->addr, ap->port, DIR_PURPOSE_FETCH_SERVERDESC)
+  || connection_get_by_type_addr_port_purpose(
+       CONN_TYPE_DIR, &ap->addr, ap->port, DIR_PURPOSE_FETCH_EXTRAINFO))) {
+    return 1;
+  }
+
+  if (microdesc && (
+     connection_get_by_type_addr_port_purpose(
+       CONN_TYPE_DIR, &ap->addr, ap->port, DIR_PURPOSE_FETCH_MICRODESC))) {
+    return 1;
+  }
+
+  return 0;
+}
+
+/* Check if we already have a directory fetch from ds, for serverdesc
+ * (including extrainfo) or microdesc documents.
+ * If so, return 1, if not, return 0.
+ */
+static int
+router_is_already_dir_fetching_ds(const dir_server_t *ds,
+                                  int serverdesc,
+                                  int microdesc)
+{
+  tor_addr_port_t ipv4_dir_ap, ipv6_dir_ap;
+
+  /* Assume IPv6 DirPort is the same as IPv4 DirPort */
+  tor_addr_from_ipv4h(&ipv4_dir_ap.addr, ds->addr);
+  ipv4_dir_ap.port = ds->dir_port;
+  tor_addr_copy(&ipv6_dir_ap.addr, &ds->ipv6_addr);
+  ipv6_dir_ap.port = ds->dir_port;
+
+  return (router_is_already_dir_fetching(&ipv4_dir_ap, serverdesc, microdesc)
+       || router_is_already_dir_fetching(&ipv6_dir_ap, serverdesc, microdesc));
+}
+
+/* Check if we already have a directory fetch from rs, for serverdesc
+ * (including extrainfo) or microdesc documents.
+ * If so, return 1, if not, return 0.
+ */
+static int
+router_is_already_dir_fetching_rs(const routerstatus_t *rs,
+                                  int serverdesc,
+                                  int microdesc)
+{
+  tor_addr_port_t ipv4_dir_ap, ipv6_dir_ap;
+
+  /* Assume IPv6 DirPort is the same as IPv4 DirPort */
+  tor_addr_from_ipv4h(&ipv4_dir_ap.addr, rs->addr);
+  ipv4_dir_ap.port = rs->dir_port;
+  tor_addr_copy(&ipv6_dir_ap.addr, &rs->ipv6_addr);
+  ipv6_dir_ap.port = rs->dir_port;
+
+  return (router_is_already_dir_fetching(&ipv4_dir_ap, serverdesc, microdesc)
+       || router_is_already_dir_fetching(&ipv6_dir_ap, serverdesc, microdesc));
+}
+
+#ifndef LOG_FALSE_POSITIVES_DURING_BOOTSTRAP
+#define LOG_FALSE_POSITIVES_DURING_BOOTSTRAP 0
+#endif
+
+/* Log a message if rs is not found or not a preferred address */
+static void
+router_picked_poor_directory_log(const routerstatus_t *rs)
+{
+  const networkstatus_t *usable_consensus;
+  usable_consensus = networkstatus_get_reasonably_live_consensus(time(NULL),
+                                                 usable_consensus_flavor());
+
+#if !LOG_FALSE_POSITIVES_DURING_BOOTSTRAP
+  /* Don't log early in the bootstrap process, it's normal to pick from a
+   * small pool of nodes. Of course, this won't help if we're trying to
+   * diagnose bootstrap issues. */
+  if (!smartlist_len(nodelist_get_list()) || !usable_consensus
+      || !router_have_minimum_dir_info()) {
+    return;
+  }
+#endif
+
+  /* We couldn't find a node, or the one we have doesn't fit our preferences.
+   * This might be a bug. */
+  if (!rs) {
+    log_warn(LD_BUG, "Firewall denied all OR and Dir addresses for all relays "
+             "when searching for a directory.");
+    log_backtrace(LOG_WARN, LD_BUG, "Node search initiated by");
+  } else if (!fascist_firewall_allows_rs(rs, FIREWALL_OR_CONNECTION, 1)
+             && !fascist_firewall_allows_rs(rs, FIREWALL_DIR_CONNECTION, 1)
+             ) {
+    log_warn(LD_BUG, "Selected a directory %s with non-preferred OR and Dir "
+             "addresses for launching a connection: "
+             "IPv4 %s OR %d Dir %d IPv6 %s OR %d Dir %d",
+             routerstatus_describe(rs),
+             fmt_addr32(rs->addr), rs->or_port,
+             rs->dir_port, fmt_addr(&rs->ipv6_addr),
+             rs->ipv6_orport, rs->dir_port);
+    log_backtrace(LOG_WARN, LD_BUG, "Node search initiated by");
+  }
+}
+
+#undef LOG_FALSE_POSITIVES_DURING_BOOTSTRAP
+
 /** How long do we avoid using a directory server after it's given us a 503? */
 #define DIR_503_TIMEOUT (60*60)
 
+/* Common retry code for router_pick_directory_server_impl and
+ * router_pick_trusteddirserver_impl. Retry with the non-preferred IP version.
+ * Must be called before RETRY_WITHOUT_EXCLUDE().
+ *
+ * If we got no result, and we are applying IP preferences, and we are a
+ * client that could use an alternate IP version, try again with the
+ * opposite preferences. */
+#define RETRY_ALTERNATE_IP_VERSION(retry_label)                               \
+  STMT_BEGIN                                                                  \
+    if (result == NULL && try_ip_pref && options->ClientUseIPv4               \
+        && fascist_firewall_use_ipv6(options) && !server_mode(options)        \
+        && n_not_preferred && !n_busy) {                                      \
+      n_excluded = 0;                                                         \
+      n_busy = 0;                                                             \
+      try_ip_pref = 0;                                                        \
+      n_not_preferred = 0;                                                    \
+      goto retry_label;                                                       \
+    }                                                                         \
+  STMT_END                                                                    \
+
+/* Common retry code for router_pick_directory_server_impl and
+ * router_pick_trusteddirserver_impl. Retry without excluding nodes, but with
+ * the preferred IP version. Must be called after RETRY_ALTERNATE_IP_VERSION().
+ *
+ * If we got no result, and we are excluding nodes, and StrictNodes is
+ * not set, try again without excluding nodes. */
+#define RETRY_WITHOUT_EXCLUDE(retry_label)                                    \
+  STMT_BEGIN                                                                  \
+    if (result == NULL && try_excluding && !options->StrictNodes              \
+        && n_excluded && !n_busy) {                                           \
+      try_excluding = 0;                                                      \
+      n_excluded = 0;                                                         \
+      n_busy = 0;                                                             \
+      try_ip_pref = 1;                                                        \
+      n_not_preferred = 0;                                                    \
+      goto retry_label;                                                       \
+    }                                                                         \
+  STMT_END
+
+/* When iterating through the routerlist, can OR address/port preference
+ * and reachability checks be skipped?
+ */
+static int
+router_skip_or_reachability(const or_options_t *options, int try_ip_pref)
+{
+  /* Servers always have and prefer IPv4.
+   * And if clients are checking against the firewall for reachability only,
+   * but there's no firewall, don't bother checking */
+  return server_mode(options) || (!try_ip_pref && !firewall_is_fascist_or());
+}
+
+/* When iterating through the routerlist, can Dir address/port preference
+ * and reachability checks be skipped?
+ */
+static int
+router_skip_dir_reachability(const or_options_t *options, int try_ip_pref)
+{
+  /* Servers always have and prefer IPv4.
+   * And if clients are checking against the firewall for reachability only,
+   * but there's no firewall, don't bother checking */
+  return server_mode(options) || (!try_ip_pref && !firewall_is_fascist_dir());
+}
+
 /** Pick a random running valid directory server/mirror from our
  * routerlist.  Arguments are as for router_pick_directory_server(), except:
  *
@@ -1487,11 +1669,12 @@ 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;
 
   if (!consensus)
     return NULL;
 
- retry_without_exclude:
+ retry_search:
 
   direct = smartlist_new();
   tunnel = smartlist_new();
@@ -1500,11 +1683,13 @@ 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);
+
   /* Find all the running dirservers we know about. */
   SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) {
     int is_trusted, is_trusted_extrainfo;
     int is_overloaded;
-    tor_addr_t addr;
     const routerstatus_t *status = node->rs;
     const country_t country = node->country;
     if (!status)
@@ -1535,36 +1720,34 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
       continue;
     }
 
-    /* XXXX IP6 proposal 118 */
-    tor_addr_from_ipv4h(&addr, status->addr);
-
-    if (no_serverdesc_fetching && (
-       connection_get_by_type_addr_port_purpose(
-         CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_SERVERDESC)
-    || connection_get_by_type_addr_port_purpose(
-         CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_EXTRAINFO)
-    )) {
-      ++n_busy;
-      continue;
-    }
-
-    if (no_microdesc_fetching && connection_get_by_type_addr_port_purpose(
-      CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_MICRODESC)
-    ) {
+    if (router_is_already_dir_fetching_rs(status,
+                                          no_serverdesc_fetching,
+                                          no_microdesc_fetching)) {
       ++n_busy;
       continue;
     }
 
     is_overloaded = status->last_dir_503_at + DIR_503_TIMEOUT > now;
 
-    if ((!fascistfirewall ||
-         fascist_firewall_allows_address_or(&addr, status->or_port)))
+    /* Clients use IPv6 addresses if the server has one and the client
+     * prefers IPv6.
+     * Add the router if its preferred address and port are reachable.
+     * If we don't get any routers, we'll try again with the non-preferred
+     * 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))
       smartlist_add(is_trusted ? trusted_tunnel :
                     is_overloaded ? overloaded_tunnel : tunnel, (void*)node);
-    else if (!fascistfirewall ||
-             fascist_firewall_allows_address_dir(&addr, status->dir_port))
+    else if (skip_dir ||
+             fascist_firewall_allows_rs(status, 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)) {
@@ -1593,19 +1776,15 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
   smartlist_free(overloaded_direct);
   smartlist_free(overloaded_tunnel);
 
-  if (result == NULL && try_excluding && !options->StrictNodes && n_excluded
-      && !n_busy) {
-    /* If we got no result, and we are excluding nodes, and StrictNodes is
-     * not set, try again without excluding nodes. */
-    try_excluding = 0;
-    n_excluded = 0;
-    n_busy = 0;
-    goto retry_without_exclude;
-  }
+  RETRY_ALTERNATE_IP_VERSION(retry_search);
+
+  RETRY_WITHOUT_EXCLUDE(retry_search);
 
   if (n_busy_out)
     *n_busy_out = n_busy;
 
+  router_picked_poor_directory_log(result ? result->rs : NULL);
+
   return result ? result->rs : NULL;
 }
 
@@ -1656,22 +1835,25 @@ 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;
 
   if (!sourcelist)
     return NULL;
 
- retry_without_exclude:
+ retry_search:
 
   direct = smartlist_new();
   tunnel = smartlist_new();
   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);
+
   SMARTLIST_FOREACH_BEGIN(sourcelist, const dir_server_t *, d)
     {
       int is_overloaded =
           d->fake_status.last_dir_503_at + DIR_503_TIMEOUT > now;
-      tor_addr_t addr;
       if (!d->is_running) continue;
       if ((type & d->type) == 0)
         continue;
@@ -1687,35 +1869,29 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
         continue;
       }
 
-      /* XXXX IP6 proposal 118 */
-      tor_addr_from_ipv4h(&addr, d->addr);
-
-      if (no_serverdesc_fetching) {
-        if (connection_get_by_type_addr_port_purpose(
-            CONN_TYPE_DIR, &addr, d->dir_port, DIR_PURPOSE_FETCH_SERVERDESC)
-         || connection_get_by_type_addr_port_purpose(
-             CONN_TYPE_DIR, &addr, d->dir_port, DIR_PURPOSE_FETCH_EXTRAINFO)) {
-          //log_debug(LD_DIR, "We have an existing connection to fetch "
-          //           "descriptor from %s; delaying",d->description);
-          ++n_busy;
-          continue;
-        }
-      }
-      if (no_microdesc_fetching) {
-        if (connection_get_by_type_addr_port_purpose(
-             CONN_TYPE_DIR, &addr, d->dir_port, DIR_PURPOSE_FETCH_MICRODESC)) {
-          ++n_busy;
-          continue;
-        }
+      if (router_is_already_dir_fetching_ds(d, no_serverdesc_fetching,
+                                            no_microdesc_fetching)) {
+        ++n_busy;
+        continue;
       }
 
-      if (d->or_port &&
-          (!fascistfirewall ||
-           fascist_firewall_allows_address_or(&addr, d->or_port)))
+      /* Clients use IPv6 addresses if the server has one and the client
+       * prefers IPv6.
+       * Add the router if its preferred address and port are reachable.
+       * If we don't get any routers, we'll try again with the non-preferred
+       * 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_dir_server(d, FIREWALL_OR_CONNECTION,
+                                             try_ip_pref))
         smartlist_add(is_overloaded ? overloaded_tunnel : tunnel, (void*)d);
-      else if (!fascistfirewall ||
-               fascist_firewall_allows_address_dir(&addr, d->dir_port))
+      else if (skip_dir ||
+               fascist_firewall_allows_dir_server(d, FIREWALL_DIR_CONNECTION,
+                                                  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);
 
@@ -1742,19 +1918,14 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
   smartlist_free(overloaded_direct);
   smartlist_free(overloaded_tunnel);
 
-  if (result == NULL && try_excluding && !options->StrictNodes && n_excluded
-      && !n_busy) {
-    /* If we got no result, and we are excluding nodes, and StrictNodes is
-     * not set, try again without excluding nodes. */
-    try_excluding = 0;
-    n_excluded = 0;
-    n_busy = 0;
-    goto retry_without_exclude;
-  }
+  RETRY_ALTERNATE_IP_VERSION(retry_search);
+
+  RETRY_WITHOUT_EXCLUDE(retry_search);
+
+  router_picked_poor_directory_log(result);
 
   if (n_busy_out)
     *n_busy_out = n_busy;
-
   return result;
 }
 
@@ -1824,8 +1995,12 @@ routerlist_add_node_and_family(smartlist_t *sl, const routerinfo_t *router)
 void
 router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid,
                                       int need_uptime, int need_capacity,
-                                      int need_guard, int need_desc)
-{ /* XXXX MOVE */
+                                      int need_guard, int need_desc,
+                                      int pref_addr)
+{
+  const int check_reach = !router_skip_or_reachability(get_options(),
+                                                       pref_addr);
+  /* XXXX MOVE */
   SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) {
     if (!node->is_running ||
         (!node->is_valid && !allow_invalid))
@@ -1836,6 +2011,11 @@ router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid,
       continue;
     if (node_is_unreliable(node, need_uptime, need_capacity, need_guard))
       continue;
+    /* Choose a node with an OR address that matches the firewall rules */
+    if (check_reach && !fascist_firewall_allows_node(node,
+                                                     FIREWALL_OR_CONNECTION,
+                                                     pref_addr))
+      continue;
 
     smartlist_add(sl, (void *)node);
   } SMARTLIST_FOREACH_END(node);
@@ -2297,6 +2477,10 @@ node_sl_choose_by_bandwidth(const smartlist_t *sl,
  * If <b>CRN_NEED_DESC</b> is set in flags, we only consider nodes that
  * have a routerinfo or microdescriptor -- that is, enough info to be
  * used to build a circuit.
+ * If <b>CRN_PREF_ADDR</b> is set in flags, we only consider nodes that
+ * have an address that is preferred by the ClientPreferIPv6ORPort setting
+ * (regardless of this flag, we exclude nodes that aren't allowed by the
+ * firewall, including ClientUseIPv4 0 and fascist_firewall_use_ipv6() == 0).
  */
 const node_t *
 router_choose_random_node(smartlist_t *excludedsmartlist,
@@ -2309,6 +2493,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
   const int allow_invalid = (flags & CRN_ALLOW_INVALID) != 0;
   const int weight_for_exit = (flags & CRN_WEIGHT_AS_EXIT) != 0;
   const int need_desc = (flags & CRN_NEED_DESC) != 0;
+  const int pref_addr = (flags & CRN_PREF_ADDR) != 0;
 
   smartlist_t *sl=smartlist_new(),
     *excludednodes=smartlist_new();
@@ -2334,7 +2519,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
 
   router_add_running_nodes_to_smartlist(sl, allow_invalid,
                                         need_uptime, need_capacity,
-                                        need_guard, need_desc);
+                                        need_guard, need_desc, pref_addr);
   log_debug(LD_CIRC,
            "We found %d running nodes.",
             smartlist_len(sl));
@@ -2363,7 +2548,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
   choice = node_sl_choose_by_bandwidth(sl, rule);
 
   smartlist_free(sl);
-  if (!choice && (need_uptime || need_capacity || need_guard)) {
+  if (!choice && (need_uptime || need_capacity || need_guard || pref_addr)) {
     /* try once more -- recurse but with fewer restrictions. */
     log_info(LD_CIRC,
              "We couldn't find any live%s%s%s routers; falling back "
@@ -2371,7 +2556,8 @@ router_choose_random_node(smartlist_t *excludedsmartlist,
              need_capacity?", fast":"",
              need_uptime?", stable":"",
              need_guard?", guard":"");
-    flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD);
+    flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD|
+                CRN_PREF_ADDR);
     choice = router_choose_random_node(
                      excludedsmartlist, excludedset, flags);
   }

+ 4 - 1
src/or/routerlist.h

@@ -61,7 +61,8 @@ void router_reset_status_download_failures(void);
 int routers_have_same_or_addrs(const routerinfo_t *r1, const routerinfo_t *r2);
 void router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid,
                                            int need_uptime, int need_capacity,
-                                           int need_guard, int need_desc);
+                                           int need_guard, int need_desc,
+                                           int pref_addr);
 
 const routerinfo_t *routerlist_find_my_routerinfo(void);
 uint32_t router_get_advertised_bandwidth(const routerinfo_t *router);
@@ -245,6 +246,8 @@ MOCK_DECL(STATIC was_router_added_t, extrainfo_insert,
 MOCK_DECL(STATIC void, initiate_descriptor_downloads,
           (const routerstatus_t *source, int purpose, smartlist_t *digests,
            int lo, int hi, int pds_flags));
+STATIC int router_is_already_dir_fetching(const tor_addr_port_t *ap,
+                                          int serverdesc, int microdesc);
 
 #endif
 

+ 221 - 5
src/test/test_entrynodes.c

@@ -9,14 +9,16 @@
 
 #include "or.h"
 #include "test.h"
+
+#include "config.h"
 #include "entrynodes.h"
-#include "routerparse.h"
 #include "nodelist.h"
-#include "util.h"
+#include "policies.h"
 #include "routerlist.h"
+#include "routerparse.h"
 #include "routerset.h"
 #include "statefile.h"
-#include "config.h"
+#include "util.h"
 
 #include "test_helpers.h"
 
@@ -70,6 +72,14 @@ fake_network_setup(const struct testcase_t *testcase)
   return dummy_state;
 }
 
+static or_options_t mocked_options;
+
+static const or_options_t *
+mock_get_options(void)
+{
+  return &mocked_options;
+}
+
 /** Test choose_random_entry() with none of our routers being guard nodes. */
 static void
 test_choose_random_entry_no_guards(void *arg)
@@ -78,6 +88,14 @@ test_choose_random_entry_no_guards(void *arg)
 
   (void) arg;
 
+  MOCK(get_options, mock_get_options);
+
+  /* Check that we get a guard if it passes preferred
+   * address settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
   /* Try to pick an entry even though none of our routers are guards. */
   chosen_entry = choose_random_entry(NULL);
 
@@ -86,8 +104,55 @@ test_choose_random_entry_no_guards(void *arg)
      can't find a proper entry guard. */
   tt_assert(chosen_entry);
 
+  /* And with the other IP version active */
+  mocked_options.ClientUseIPv6 = 1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
+  /* And with the preference on auto */
+  mocked_options.ClientPreferIPv6ORPort = -1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
+  /* Check that we don't get a guard if it doesn't pass mandatory address
+   * settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 0;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
+  chosen_entry = choose_random_entry(NULL);
+
+  /* If we don't allow IPv4 at all, we don't get a guard*/
+  tt_assert(!chosen_entry);
+
+  /* Check that we get a guard if it passes allowed but not preferred address
+   * settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientUseIPv6 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
+  /* Check that we get a guard if it passes preferred address settings when
+   * they're auto */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = -1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
+  /* And with IPv6 active */
+  mocked_options.ClientUseIPv6 = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
  done:
-  ;
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  UNMOCK(get_options);
 }
 
 /** Test choose_random_entry() with only one of our routers being a
@@ -101,17 +166,78 @@ test_choose_random_entry_one_possible_guard(void *arg)
 
   (void) arg;
 
+  MOCK(get_options, mock_get_options);
+
   /* Set one of the nodes to be a guard. */
   our_nodelist = nodelist_get_list();
   the_guard = smartlist_get(our_nodelist, 4); /* chosen by fair dice roll */
   the_guard->is_possible_guard = 1;
 
+  /* Check that we get the guard if it passes preferred
+   * address settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
   /* Pick an entry. Make sure we pick the node we marked as guard. */
   chosen_entry = choose_random_entry(NULL);
   tt_ptr_op(chosen_entry, OP_EQ, the_guard);
 
+  /* And with the other IP version active */
+  mocked_options.ClientUseIPv6 = 1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_ptr_op(chosen_entry, OP_EQ, the_guard);
+
+  /* And with the preference on auto */
+  mocked_options.ClientPreferIPv6ORPort = -1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_ptr_op(chosen_entry, OP_EQ, the_guard);
+
+  /* Check that we don't get a guard if it doesn't pass mandatory address
+   * settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 0;
+  mocked_options.ClientPreferIPv6ORPort = 0;
+
+  chosen_entry = choose_random_entry(NULL);
+
+  /* If we don't allow IPv4 at all, we don't get a guard*/
+  tt_assert(!chosen_entry);
+
+  /* Check that we get a node if it passes allowed but not preferred
+   * address settings */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientUseIPv6 = 1;
+  mocked_options.ClientPreferIPv6ORPort = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+
+  /* We disable the guard check and the preferred address check at the same
+   * time, so we can't be sure we get the guard */
+  tt_assert(chosen_entry);
+
+  /* Check that we get a node if it is allowed but not preferred when settings
+   * are auto */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = -1;
+
+  chosen_entry = choose_random_entry(NULL);
+
+  /* We disable the guard check and the preferred address check at the same
+   * time, so we can't be sure we get the guard */
+  tt_assert(chosen_entry);
+
+  /* and with IPv6 active */
+  mocked_options.ClientUseIPv6 = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
  done:
-  ;
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  UNMOCK(get_options);
 }
 
 /** Helper to conduct tests for populate_live_entry_guards().
@@ -624,6 +750,93 @@ test_entry_is_live(void *arg)
   ; /* XXX */
 }
 
+#define TEST_IPV4_ADDR "123.45.67.89"
+#define TEST_IPV6_ADDR "[1234:5678:90ab:cdef::]"
+
+static void
+test_node_preferred_orport(void *arg)
+{
+  (void)arg;
+  tor_addr_t ipv4_addr;
+  const uint16_t ipv4_port = 4444;
+  tor_addr_t ipv6_addr;
+  const uint16_t ipv6_port = 6666;
+  routerinfo_t node_ri;
+  node_t node;
+  tor_addr_port_t ap;
+
+  /* Setup options */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  /* We don't test ClientPreferIPv6ORPort here, because it's used in
+   * nodelist_set_consensus to setup node.ipv6_preferred, which we set
+   * directly. */
+  MOCK(get_options, mock_get_options);
+
+  /* Setup IP addresses */
+  tor_addr_parse(&ipv4_addr, TEST_IPV4_ADDR);
+  tor_addr_parse(&ipv6_addr, TEST_IPV6_ADDR);
+
+  /* Setup node_ri */
+  memset(&node_ri, 0, sizeof(node_ri));
+  node_ri.addr = tor_addr_to_ipv4h(&ipv4_addr);
+  node_ri.or_port = ipv4_port;
+  tor_addr_copy(&node_ri.ipv6_addr, &ipv6_addr);
+  node_ri.ipv6_orport = ipv6_port;
+
+  /* Setup node */
+  memset(&node, 0, sizeof(node));
+  node.ri = &node_ri;
+
+  /* Check the preferred address is IPv4 if we're only using IPv4, regardless
+   * of whether we prefer it or not */
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientUseIPv6 = 0;
+  node.ipv6_preferred = 0;
+  node_get_pref_orport(&node, &ap);
+  tt_assert(tor_addr_eq(&ap.addr, &ipv4_addr));
+  tt_assert(ap.port == ipv4_port);
+
+  node.ipv6_preferred = 1;
+  node_get_pref_orport(&node, &ap);
+  tt_assert(tor_addr_eq(&ap.addr, &ipv4_addr));
+  tt_assert(ap.port == ipv4_port);
+
+  /* Check the preferred address is IPv4 if we're using IPv4 and IPv6, but
+   * don't prefer the IPv6 address */
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientUseIPv6 = 1;
+  node.ipv6_preferred = 0;
+  node_get_pref_orport(&node, &ap);
+  tt_assert(tor_addr_eq(&ap.addr, &ipv4_addr));
+  tt_assert(ap.port == ipv4_port);
+
+  /* Check the preferred address is IPv6 if we prefer it and
+   * ClientUseIPv6 is 1, regardless of ClientUseIPv4 */
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientUseIPv6 = 1;
+  node.ipv6_preferred = 1;
+  node_get_pref_orport(&node, &ap);
+  tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
+  tt_assert(ap.port == ipv6_port);
+
+  mocked_options.ClientUseIPv4 = 0;
+  node_get_pref_orport(&node, &ap);
+  tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
+  tt_assert(ap.port == ipv6_port);
+
+  /* Check the preferred address is IPv6 if we don't prefer it, but
+   * ClientUseIPv4 is 0 */
+  mocked_options.ClientUseIPv4 = 0;
+  mocked_options.ClientUseIPv6 = 1;
+  node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport(&mocked_options);
+  node_get_pref_orport(&node, &ap);
+  tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
+  tt_assert(ap.port == ipv6_port);
+
+ done:
+  UNMOCK(get_options);
+}
+
 static const struct testcase_setup_t fake_network = {
   fake_network_setup, fake_network_cleanup
 };
@@ -654,6 +867,9 @@ struct testcase_t entrynodes_tests[] = {
   { "entry_is_live",
     test_entry_is_live,
     TT_FORK, &fake_network, NULL },
+  { "node_preferred_orport",
+    test_node_preferred_orport,
+    0, NULL, NULL },
   END_OF_TESTCASES
 };
 

+ 536 - 0
src/test/test_policy.c

@@ -1129,6 +1129,538 @@ test_policies_getinfo_helper_policies(void *arg)
 #undef TEST_IPV4_ADDR
 #undef TEST_IPV6_ADDR
 
+#define TEST_IPV4_ADDR_STR "1.2.3.4"
+#define TEST_IPV6_ADDR_STR "[1002::4567]"
+#define REJECT_IPv4_FINAL_STR "reject 0.0.0.0/0:*"
+#define REJECT_IPv6_FINAL_STR "reject [::]/0:*"
+
+#define OTHER_IPV4_ADDR_STR "6.7.8.9"
+#define OTHER_IPV6_ADDR_STR "[afff::]"
+
+/** Run unit tests for fascist_firewall_allows_address */
+static void
+test_policies_fascist_firewall_allows_address(void *arg)
+{
+  (void)arg;
+  tor_addr_t ipv4_addr, ipv6_addr, r_ipv4_addr, r_ipv6_addr;
+  tor_addr_t n_ipv4_addr, n_ipv6_addr;
+  const uint16_t port = 1234;
+  smartlist_t *policy = NULL;
+  smartlist_t *e_policy = NULL;
+  addr_policy_t *item = NULL;
+  int malformed_list = 0;
+
+  /* Setup the options and the items in the policies */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  MOCK(get_options, mock_get_options);
+
+  policy = smartlist_new();
+  item = router_parse_addr_policy_item_from_string("accept "
+                                                   TEST_IPV4_ADDR_STR ":*",
+                                                   ADDR_POLICY_ACCEPT,
+                                                   &malformed_list);
+  tt_assert(item);
+  tt_assert(!malformed_list);
+  smartlist_add(policy, item);
+  item = router_parse_addr_policy_item_from_string("accept "
+                                                   TEST_IPV6_ADDR_STR,
+                                                   ADDR_POLICY_ACCEPT,
+                                                   &malformed_list);
+  tt_assert(item);
+  tt_assert(!malformed_list);
+  smartlist_add(policy, item);
+  /* Normally, policy_expand_unspec would do this for us */
+  item = router_parse_addr_policy_item_from_string(REJECT_IPv4_FINAL_STR,
+                                                   ADDR_POLICY_ACCEPT,
+                                                   &malformed_list);
+  tt_assert(item);
+  tt_assert(!malformed_list);
+  smartlist_add(policy, item);
+  item = router_parse_addr_policy_item_from_string(REJECT_IPv6_FINAL_STR,
+                                                   ADDR_POLICY_ACCEPT,
+                                                   &malformed_list);
+  tt_assert(item);
+  tt_assert(!malformed_list);
+  smartlist_add(policy, item);
+  item = NULL;
+
+  e_policy = smartlist_new();
+
+  /*
+  char *polstr = policy_dump_to_string(policy, 1, 1);
+  printf("%s\n", polstr);
+  tor_free(polstr);
+   */
+
+  /* Parse the addresses */
+  tor_addr_parse(&ipv4_addr, TEST_IPV4_ADDR_STR);
+  tor_addr_parse(&ipv6_addr, TEST_IPV6_ADDR_STR);
+  tor_addr_parse(&r_ipv4_addr, OTHER_IPV4_ADDR_STR);
+  tor_addr_parse(&r_ipv6_addr, OTHER_IPV6_ADDR_STR);
+  tor_addr_make_null(&n_ipv4_addr, AF_INET);
+  tor_addr_make_null(&n_ipv6_addr, AF_INET6);
+
+  /* Test the function's address matching with IPv4 and IPv6 on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
+  mock_options.UseBridges = 0;
+
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
+  /* Preferring IPv4 */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 1, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 1, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 1, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 1, 0)
+            == 0);
+
+  /* Preferring IPv6 */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 1, 1)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 1, 1)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 1, 1)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 1, 1)
+            == 0);
+
+  /* Test the function's address matching with UseBridges on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
+  mock_options.UseBridges = 1;
+
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
+  /* Preferring IPv4 */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 1, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 1, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 1, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 1, 0)
+            == 0);
+
+  /* Preferring IPv6 */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 1, 1)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 1, 1)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 1, 1)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 1, 1)
+            == 0);
+
+  /* bridge clients always use IPv6, regardless of ClientUseIPv6 */
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 0;
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
+  /* Test the function's address matching with IPv4 on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 0;
+  mock_options.UseBridges = 0;
+
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
+  /* Test the function's address matching with IPv6 on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 0;
+  mock_options.ClientUseIPv6 = 1;
+  mock_options.UseBridges = 0;
+
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
+  /* Test the function's address matching with ClientUseIPv4 0.
+   * This means "use IPv6" regardless of the other settings. */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 0;
+  mock_options.ClientUseIPv6 = 0;
+  mock_options.UseBridges = 0;
+
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
+  /* Test the function's address matching for unusual inputs */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
+  mock_options.UseBridges = 1;
+
+  /* NULL and tor_addr_is_null addresses are rejected */
+  tt_assert(fascist_firewall_allows_address(NULL, port, policy, 0, 0) == 0);
+  tt_assert(fascist_firewall_allows_address(&n_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&n_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
+  /* zero ports are rejected */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, 0, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, 0, policy, 0, 0)
+            == 0);
+
+  /* NULL and empty policies accept everything */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, NULL, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, NULL, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, e_policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, e_policy, 0, 0)
+            == 1);
+
+ done:
+  addr_policy_free(item);
+  addr_policy_list_free(policy);
+  addr_policy_list_free(e_policy);
+  UNMOCK(get_options);
+}
+
+#undef REJECT_IPv4_FINAL_STR
+#undef REJECT_IPv6_FINAL_STR
+#undef OTHER_IPV4_ADDR_STR
+#undef OTHER_IPV6_ADDR_STR
+
+#define TEST_IPV4_OR_PORT  1234
+#define TEST_IPV4_DIR_PORT 2345
+#define TEST_IPV6_OR_PORT  61234
+#define TEST_IPV6_DIR_PORT 62345
+
+/** Run unit tests for fascist_firewall_choose_address */
+static void
+test_policies_fascist_firewall_choose_address(void *arg)
+{
+  (void)arg;
+  tor_addr_port_t ipv4_or_ap, ipv4_dir_ap, ipv6_or_ap, ipv6_dir_ap;
+  tor_addr_port_t n_ipv4_ap, n_ipv6_ap;
+
+  /* Setup the options */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  MOCK(get_options, mock_get_options);
+
+  /* Parse the addresses */
+  tor_addr_parse(&ipv4_or_ap.addr, TEST_IPV4_ADDR_STR);
+  ipv4_or_ap.port = TEST_IPV4_OR_PORT;
+  tor_addr_parse(&ipv4_dir_ap.addr, TEST_IPV4_ADDR_STR);
+  ipv4_dir_ap.port = TEST_IPV4_DIR_PORT;
+
+  tor_addr_parse(&ipv6_or_ap.addr, TEST_IPV6_ADDR_STR);
+  ipv6_or_ap.port = TEST_IPV6_OR_PORT;
+  tor_addr_parse(&ipv6_dir_ap.addr, TEST_IPV6_ADDR_STR);
+  ipv6_dir_ap.port = TEST_IPV6_DIR_PORT;
+
+  tor_addr_make_null(&n_ipv4_ap.addr, AF_INET);
+  n_ipv4_ap.port = 0;
+  tor_addr_make_null(&n_ipv6_ap.addr, AF_INET6);
+  n_ipv6_ap.port = 0;
+
+  /* Choose an address with IPv4 and IPv6 on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
+  mock_options.UseBridges = 0;
+
+  /* Preferring IPv4 */
+  mock_options.ClientPreferIPv6ORPort = 0;
+  mock_options.ClientPreferIPv6DirPort = 0;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Auto (Preferring IPv4) */
+  mock_options.ClientPreferIPv6ORPort = -1;
+  mock_options.ClientPreferIPv6DirPort = -1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Preferring IPv6 */
+  mock_options.ClientPreferIPv6ORPort = 1;
+  mock_options.ClientPreferIPv6DirPort = 1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
+
+  /* Preferring IPv4 OR / IPv6 Dir */
+  mock_options.ClientPreferIPv6ORPort = 0;
+  mock_options.ClientPreferIPv6DirPort = 1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
+
+  /* Preferring IPv6 OR / IPv4 Dir */
+  mock_options.ClientPreferIPv6ORPort = 1;
+  mock_options.ClientPreferIPv6DirPort = 0;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Choose an address with UseBridges on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.UseBridges = 1;
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
+
+  /* Preferring IPv4 */
+  mock_options.ClientPreferIPv6ORPort = 0;
+  mock_options.ClientPreferIPv6DirPort = 0;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Auto:
+   * - bridge clients prefer the configured bridge OR address,
+   * - other clients prefer IPv4 OR by default,
+   * - all clients prefer IPv4 Dir by default.
+   */
+  mock_options.ClientPreferIPv6ORPort = -1;
+  mock_options.ClientPreferIPv6DirPort = -1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Preferring IPv6 */
+  mock_options.ClientPreferIPv6ORPort = 1;
+  mock_options.ClientPreferIPv6DirPort = 1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
+
+
+  /* In the default configuration (Auto / IPv6 off), bridge clients should
+   * still use IPv6, and only prefer it for bridges configured with an IPv6
+   * address, regardless of ClientUseIPv6. */
+  mock_options.ClientUseIPv6 = 0;
+  mock_options.ClientPreferIPv6ORPort = -1;
+  mock_options.ClientPreferIPv6DirPort = -1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Choose an address with IPv4 on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 0;
+  mock_options.UseBridges = 0;
+
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Choose an address with IPv6 on */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 0;
+  mock_options.ClientUseIPv6 = 1;
+  mock_options.UseBridges = 0;
+
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
+
+  /* Choose an address with ClientUseIPv4 0.
+   * This means "use IPv6" regardless of the other settings. */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 0;
+  mock_options.ClientUseIPv6 = 0;
+  mock_options.UseBridges = 0;
+
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
+
+  /* Choose from unusual inputs */
+  memset(&mock_options, 0, sizeof(or_options_t));
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
+  mock_options.UseBridges = 1;
+
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &n_ipv6_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&n_ipv4_ap, &ipv6_or_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&n_ipv4_ap, &n_ipv6_ap, 0,
+                                                 FIREWALL_OR_CONNECTION, 0)
+            == NULL);
+
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &n_ipv6_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&n_ipv4_ap, &ipv6_dir_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&n_ipv4_ap, &n_ipv6_ap, 0,
+                                                 FIREWALL_DIR_CONNECTION, 0)
+            == NULL);
+
+ done:
+  UNMOCK(get_options);
+}
+
+#undef TEST_IPV4_ADDR_STR
+#undef TEST_IPV6_ADDR_STR
+#undef TEST_IPV4_OR_PORT
+#undef TEST_IPV4_DIR_PORT
+#undef TEST_IPV6_OR_PORT
+#undef TEST_IPV6_DIR_PORT
+
 struct testcase_t policy_tests[] = {
   { "router_dump_exit_policy_to_string", test_dump_exit_policy_to_string, 0,
     NULL, NULL },
@@ -1139,6 +1671,10 @@ struct testcase_t policy_tests[] = {
   { "reject_interface_address", test_policies_reject_interface_address, 0,
     NULL, NULL },
   { "reject_port_address", test_policies_reject_port_address, 0, NULL, NULL },
+  { "fascist_firewall_allows_address",
+    test_policies_fascist_firewall_allows_address, 0, NULL, NULL },
+  { "fascist_firewall_choose_address",
+    test_policies_fascist_firewall_choose_address, 0, NULL, NULL },
   END_OF_TESTCASES
 };
 

+ 73 - 0
src/test/test_routerlist.c

@@ -11,6 +11,7 @@
 #define TOR_UNIT_TESTING
 #include "or.h"
 #include "config.h"
+#include "connection.h"
 #include "container.h"
 #include "directory.h"
 #include "dirvote.h"
@@ -371,6 +372,77 @@ test_router_pick_directory_server_impl(void *arg)
     policies_free_all();
 }
 
+connection_t *mocked_connection = NULL;
+
+/* Mock connection_get_by_type_addr_port_purpose by returning
+ * mocked_connection. */
+static connection_t *
+mock_connection_get_by_type_addr_port_purpose(int type,
+                                              const tor_addr_t *addr,
+                                              uint16_t port, int purpose)
+{
+  (void)type;
+  (void)addr;
+  (void)port;
+  (void)purpose;
+
+  return mocked_connection;
+}
+
+#define TEST_ADDR_STR "127.0.0.1"
+#define TEST_DIR_PORT 12345
+
+static void
+test_routerlist_router_is_already_dir_fetching(void *arg)
+{
+  (void)arg;
+  tor_addr_port_t test_ap, null_addr_ap, zero_port_ap;
+
+  /* Setup */
+  tor_addr_parse(&test_ap.addr, TEST_ADDR_STR);
+  test_ap.port = TEST_DIR_PORT;
+  tor_addr_make_null(&null_addr_ap.addr, AF_INET6);
+  null_addr_ap.port = TEST_DIR_PORT;
+  tor_addr_parse(&zero_port_ap.addr, TEST_ADDR_STR);
+  zero_port_ap.port = 0;
+  MOCK(connection_get_by_type_addr_port_purpose,
+       mock_connection_get_by_type_addr_port_purpose);
+
+  /* Test that we never get 1 from a NULL connection */
+  mocked_connection = NULL;
+  tt_assert(router_is_already_dir_fetching(&test_ap, 1, 1) == 0);
+  tt_assert(router_is_already_dir_fetching(&test_ap, 1, 0) == 0);
+  tt_assert(router_is_already_dir_fetching(&test_ap, 0, 1) == 0);
+  /* We always expect 0 in these cases */
+  tt_assert(router_is_already_dir_fetching(&test_ap, 0, 0) == 0);
+  tt_assert(router_is_already_dir_fetching(NULL, 1, 1) == 0);
+  tt_assert(router_is_already_dir_fetching(&null_addr_ap, 1, 1) == 0);
+  tt_assert(router_is_already_dir_fetching(&zero_port_ap, 1, 1) == 0);
+
+  /* Test that we get 1 with a connection in the appropriate circumstances */
+  mocked_connection = connection_new(CONN_TYPE_DIR, AF_INET);
+  tt_assert(router_is_already_dir_fetching(&test_ap, 1, 1) == 1);
+  tt_assert(router_is_already_dir_fetching(&test_ap, 1, 0) == 1);
+  tt_assert(router_is_already_dir_fetching(&test_ap, 0, 1) == 1);
+
+  /* Test that we get 0 even with a connection in the appropriate
+   * circumstances */
+  tt_assert(router_is_already_dir_fetching(&test_ap, 0, 0) == 0);
+  tt_assert(router_is_already_dir_fetching(NULL, 1, 1) == 0);
+  tt_assert(router_is_already_dir_fetching(&null_addr_ap, 1, 1) == 0);
+  tt_assert(router_is_already_dir_fetching(&zero_port_ap, 1, 1) == 0);
+
+ done:
+  /* If a connection is never set up, connection_free chokes on it. */
+  buf_free(mocked_connection->inbuf);
+  buf_free(mocked_connection->outbuf);
+  tor_free(mocked_connection);
+  UNMOCK(connection_get_by_type_addr_port_purpose);
+}
+
+#undef TEST_ADDR_STR
+#undef TEST_DIR_PORT
+
 #define NODE(name, flags) \
   { #name, test_routerlist_##name, (flags), NULL, NULL }
 #define ROUTER(name,flags) \
@@ -379,6 +451,7 @@ test_router_pick_directory_server_impl(void *arg)
 struct testcase_t routerlist_tests[] = {
   NODE(initiate_descriptor_downloads, 0),
   NODE(launch_descriptor_downloads, 0),
+  NODE(router_is_already_dir_fetching, TT_FORK),
   ROUTER(pick_directory_server_impl, TT_FORK),
   END_OF_TESTCASES
 };