Split "can reach ipv4 orport" from "can reach ipv6 orport".

I've managed to keep this change mainly contained to our
self-testing module.  The changes here are:

  * There are two different variables for tracking "is our orport
    reachable".

  * We have a new function that says whether we can skip a single
    family's orport reachability test; the old function for this now
    tells whether we can skip _all_ orport reachability testing.

    (The name, router_should_skip_orport_reachability_test, is not
    so good.  I will rename it later if I can think of a good
    replacement.)

  * The function that launches orport reachability tests now only
    launches the ones that haven't completed.

  * The function that notes that we're reachable on an ORPort now
    takes a family.

  * Various log messages are cleaned up.

changes/ticket34067

@@ -0,0 +1,4 @@
+  o Major features (relay self-testing, IPv6):
+    - Relays now track their IPv6 ORPort separately from the reachability of
+      their IPv4 ORPort.  They will not publish a descriptor unless _both_
+      ports appear to be externally reachable. Closes ticket 34067.

src/feature/relay/circuitbuild_relay.c

@@ -542,7 +542,11 @@ onionskin_answer(struct or_circuit_t *circ,
     /* record that we could process create cells from a non-local conn
      * that we didn't initiate; presumably this means that create cells
      * can reach us too. */
-    router_orport_found_reachable();
+    tor_addr_t remote_addr;
+    if (channel_get_addr_if_possible(circ->p_chan, &remote_addr)) {
+      int family = tor_addr_family(&remote_addr);
+      router_orport_found_reachable(family);
+    }
   }
 
   return 0;

src/feature/relay/relay_periodic.c

@@ -202,19 +202,45 @@ reachability_warnings_callback(time_t now, const or_options_t *options)
     /* every 20 minutes, check and complain if necessary */
     const routerinfo_t *me = router_get_my_routerinfo();
     if (me && !router_should_skip_orport_reachability_check(options)) {
-      char *address = tor_dup_ip(me->addr);
+      /* We need to warn that one or more of our ORPorts isn't reachable.
-      if (address) {
+       * Determine which, and give a reasonable warning. */
+      char *address4 = tor_dup_ip(me->addr);
+      char *address6 = tor_addr_to_str_dup(&me->ipv6_addr);
+      bool v4_ok =
+        router_should_skip_orport_reachability_check_family(options,AF_INET);
+      bool v6_ok =
+        router_should_skip_orport_reachability_check_family(options,AF_INET6);
+      if (address4 || address6) {
+        char *where4=NULL, *where6=NULL;
+        if (!v4_ok)
+          tor_asprintf(&where4, "%s:%d", address4, me->or_port);
+        if (!v6_ok)
+          tor_asprintf(&where6, "[%s]:%d", address6, me->or_port);
+        const char *opt_and = (!v4_ok && !v6_ok) ? "and" : "";
+
         log_warn(LD_CONFIG,
-                 "Your server (%s:%d) has not managed to confirm that "
+                 "Your server has not managed to confirm reachability for "
-                 "its ORPort is reachable. Relays do not publish descriptors "
+                 "its ORPort(s) at %s%s%s. Relays do not publish descriptors "
                  "until their ORPort and DirPort are reachable. Please check "
                  "your firewalls, ports, address, /etc/hosts file, etc.",
-                 address, me->or_port);
+                 where4?where4:"",
-        control_event_server_status(LOG_WARN,
+                 opt_and,
-                                    "REACHABILITY_FAILED ORADDRESS=%s:%d",
+                 where6?where6:"");
-                                    address, me->or_port);
+        tor_free(where4);
-        tor_free(address);
+        tor_free(where6);
+        if (!v4_ok) {
+          control_event_server_status(LOG_WARN,
+                                      "REACHABILITY_FAILED ORADDRESS=%s:%d",
+                                      address4, me->or_port);
+        }
+        if (!v6_ok) {
+          control_event_server_status(LOG_WARN,
+                                      "REACHABILITY_FAILED ORADDRESS=[%s]:%d",
+                                      address6, me->ipv6_orport);
+        }
       }
+      tor_free(address4);
+      tor_free(address6);
     }
 
     if (me && !router_should_skip_dirport_reachability_check(options)) {

src/feature/relay/selftest.c

@@ -44,8 +44,12 @@
 #include "feature/relay/router.h"
 #include "feature/relay/selftest.h"
 
-/** Whether we can reach our ORPort from the outside. */
+static bool have_orport_for_family(int family);
-static bool can_reach_or_port = false;
+
+/** Whether we can reach our IPv4 ORPort from the outside. */
+static bool can_reach_or_port_ipv4 = false;
+/** Whether we can reach our IPv6 ORPort from the outside. */
+static bool can_reach_or_port_ipv6 = false;
 /** Whether we can reach our DirPort from the outside. */
 static bool can_reach_dir_port = false;
 
@@ -53,7 +57,7 @@ static bool can_reach_dir_port = false;
 void
 router_reset_reachability(void)
 {
-  can_reach_or_port = can_reach_dir_port = false;
+  can_reach_or_port_ipv4 = can_reach_or_port_ipv6 = can_reach_dir_port = false;
 }
 
 /** Return 1 if we won't do reachability checks, because:
@@ -75,13 +79,35 @@ router_reachability_checks_disabled(const or_options_t *options)
  *   - we've seen a successful reachability check, or
  *   - AssumeReachable is set, or
  *   - the network is disabled.
+
+ * If `family'`is AF_INET or AF_INET6, return true only when we should skip
+ * the given family's orport check (Because it's been checked, or because we
+ * aren't checking it.)  If `family` is 0, return true if we can skip _all_
+ * orport checks.
  */
 int
-router_should_skip_orport_reachability_check(const or_options_t *options)
+router_should_skip_orport_reachability_check_family(
+                                                const or_options_t *options,
+                                                int family)
 {
+  tor_assert_nonfatal(family == AF_INET || family == AF_INET6 || family == 0);
   int reach_checks_disabled = router_reachability_checks_disabled(options);
-  return reach_checks_disabled ||
+  if (reach_checks_disabled) {
-         can_reach_or_port;
+    return true;
+  }
+
+  if (family != AF_INET6) {
+    if (have_orport_for_family(AF_INET) && !can_reach_or_port_ipv4) {
+      return false;
+    }
+  }
+  if (family != AF_INET) {
+    if (have_orport_for_family(AF_INET6) && !can_reach_or_port_ipv6) {
+      return false;
+    }
+  }
+
+  return true;
 }
 
 /** Return 0 if we need to do a DirPort reachability check, because:
@@ -133,6 +159,28 @@ router_should_check_reachability(int test_or, int test_dir)
   return 1;
 }
 
+/**
+ * Return true if we have configured an ORPort for the given family that
+ * we would like to advertise.
+ *
+ * Like other self-testing functions, this function looks at our most
+ * recently built descriptor.
+ **/
+static bool
+have_orport_for_family(int family)
+{
+  const routerinfo_t *me = router_get_my_routerinfo();
+
+  if (!me)
+    return false;
+
+  tor_addr_port_t ap;
+  if (router_get_orport(me, &ap, family) < 0) {
+    return false;
+  }
+  return true;
+}
+
 /** Allocate and return a new extend_info_t that can be used to build
  * a circuit to or through the router <b>r</b>, using an address from
  * <b>family</b> (if available).
@@ -255,16 +303,22 @@ router_do_reachability_checks(int test_or, int test_dir)
 {
   const routerinfo_t *me = router_get_my_routerinfo();
   const or_options_t *options = get_options();
-  int orport_reachable = router_should_skip_orport_reachability_check(options);
+  int orport_reachable_v4 =
+    router_should_skip_orport_reachability_check_family(options, AF_INET);
+  int orport_reachable_v6 =
+    router_should_skip_orport_reachability_check_family(options, AF_INET6);
 
   if (router_should_check_reachability(test_or, test_dir)) {
-    if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) {
+    bool need_testing = !circuit_enough_testing_circs();
-      /* At the moment, tor relays believe that they are reachable when they
+    /* At the moment, tor relays believe that they are reachable when they
-       * receive any create cell on an inbound connection. We'll do separate
+     * receive any create cell on an inbound connection, if the address
-       * IPv4 and IPv6 reachability checks in #34067, and make them more
+     * family is correct.
-       * precise. */
+     */
-      router_do_orport_reachability_checks(me, AF_INET, orport_reachable);
+    if (test_or && (!orport_reachable_v4 || need_testing)) {
-      router_do_orport_reachability_checks(me, AF_INET6, orport_reachable);
+      router_do_orport_reachability_checks(me, AF_INET, orport_reachable_v4);
+    }
+    if (test_or && (!orport_reachable_v6 || need_testing)) {
+      router_do_orport_reachability_checks(me, AF_INET6, orport_reachable_v6);
     }
 
     if (test_dir && !router_should_skip_dirport_reachability_check(options)) {
@@ -341,34 +395,58 @@ inform_testing_reachability(void)
   return 1;
 }
 
-/** Annotate that we found our ORPort reachable. */
+/**
+ * Return true if this module knows of no reason why we shouldn't publish
+ * a server descriptor.
+ **/
+static bool
+ready_to_publish(const or_options_t *options)
+{
+  return options->PublishServerDescriptor_ != NO_DIRINFO &&
+    router_should_skip_dirport_reachability_check(options) &&
+    router_should_skip_orport_reachability_check(options);
+}
+
+/** Annotate that we found our ORPort reachable with a given address
+ * family. */
 void
-router_orport_found_reachable(void)
+router_orport_found_reachable(int family)
 {
   const routerinfo_t *me = router_get_my_routerinfo();
   const or_options_t *options = get_options();
-  if (!can_reach_or_port && me) {
+  bool *can_reach_ptr;
-    char *address = tor_dup_ip(me->addr);
+  if (family == AF_INET) {
-
+    can_reach_ptr = &can_reach_or_port_ipv4;
-    if (!address)
+  } else if (family == AF_INET6) {
+    can_reach_ptr = &can_reach_or_port_ipv6;
+  } else {
+    tor_assert_nonfatal_unreached();
+    return;
+  }
+  if (!*can_reach_ptr && me) {
+    tor_addr_port_t ap;
+    if (router_get_orport(me, &ap, family) < 0) {
       return;
+    }
+    char *address = tor_strdup(fmt_addrport_ap(&ap));
 
-    log_notice(LD_OR,"Self-testing indicates your ORPort is reachable from "
+    *can_reach_ptr = true;
+
+    log_notice(LD_OR,"Self-testing indicates your ORPort %s is reachable from "
                "the outside. Excellent.%s",
-               options->PublishServerDescriptor_ != NO_DIRINFO
+               address,
-               && router_should_skip_dirport_reachability_check(options) ?
+               ready_to_publish(options) ?
-                 " Publishing server descriptor." : "");
+               " Publishing server descriptor." : "");
-    can_reach_or_port = true;
+
     mark_my_descriptor_dirty("ORPort found reachable");
     /* This is a significant enough change to upload immediately,
      * at least in a test network */
     if (options->TestingTorNetwork == 1) {
       reschedule_descriptor_update_check();
     }
-    /* We'll add an IPv6 event in #34068. */
     control_event_server_status(LOG_NOTICE,
-                                "REACHABILITY_SUCCEEDED ORADDRESS=%s:%d",
+                                "REACHABILITY_SUCCEEDED ORADDRESS=%s",
-                                address, me->or_port);
+                                address);
     tor_free(address);
   }
 }
@@ -379,18 +457,19 @@ router_dirport_found_reachable(void)
 {
   const routerinfo_t *me = router_get_my_routerinfo();
   const or_options_t *options = get_options();
+
   if (!can_reach_dir_port && me) {
     char *address = tor_dup_ip(me->addr);
 
     if (!address)
       return;
 
+    can_reach_dir_port = true;
     log_notice(LD_DIRSERV,"Self-testing indicates your DirPort is reachable "
                "from the outside. Excellent.%s",
-               options->PublishServerDescriptor_ != NO_DIRINFO
+               ready_to_publish(options) ?
-               && router_should_skip_orport_reachability_check(options) ?
                " Publishing server descriptor." : "");
-    can_reach_dir_port = true;
+
     if (router_should_advertise_dirport(options, me->dir_port)) {
       mark_my_descriptor_dirty("DirPort found reachable");
       /* This is a significant enough change to upload immediately,

src/feature/relay/selftest.h

@@ -15,8 +15,11 @@
 #ifdef HAVE_MODULE_RELAY
 
 struct or_options_t;
-int router_should_skip_orport_reachability_check(
+#define router_should_skip_orport_reachability_check(opts) \
-                                         const struct or_options_t *options);
+  router_should_skip_orport_reachability_check_family((opts),0)
+int router_should_skip_orport_reachability_check_family(
+                                         const struct or_options_t *options,
+                                         int family);
 int router_should_skip_dirport_reachability_check(
                                          const struct or_options_t *options);
 
@@ -24,15 +27,17 @@ void router_do_reachability_checks(int test_or, int test_dir);
 void router_perform_bandwidth_test(int num_circs, time_t now);
 int inform_testing_reachability(void);
 
-void router_orport_found_reachable(void);
+void router_orport_found_reachable(int family);
 void router_dirport_found_reachable(void);
 
 void router_reset_reachability(void);
 
 #else /* !defined(HAVE_MODULE_RELAY) */
 
-#define router_should_skip_orport_reachability_check(opts) \
+#define router_should_skip_orport_reachability_check(opts)     \
   ((void)(opts), 0)
+#define router_should_skip_orport_reachability_check_family(opts, fam)  \
+  ((void)(opts), (void)(fam), 0)
 #define router_should_skip_dirport_reachability_check(opts) \
   ((void)(opts), 0)