[tor-commits] [tor/master] relay: Remove dirport reachability self test

nickm at torproject.org nickm at torproject.org
Tue Feb 23 13:43:25 UTC 2021


commit 38649b4f9522d5fc2b186d2a02cefb9abaefc519
Author: David Goulet <dgoulet at torproject.org>
Date:   Wed Feb 17 11:00:14 2021 -0500

    relay: Remove dirport reachability self test
    
    Regular relays are about to get their DirPort removed so that reachability
    test is not useful anymore
    
    Authorities will still use the DirPort but because network reentry towards
    their DirPort is now denied network wide, this test is not useful anymore and
    so it should simply be considered reachable at all time.
    
    Part of #40282
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/core/mainloop/mainloop.c       |   2 +-
 src/core/or/circuitbuild.c         |   2 +-
 src/core/or/circuituse.c           |   5 +-
 src/feature/dirclient/dirclient.c  |  23 -----
 src/feature/relay/relay_periodic.c |  18 +---
 src/feature/relay/selftest.c       | 172 ++++++++-----------------------------
 src/feature/relay/selftest.h       |  13 ++-
 src/feature/stats/predict_ports.c  |   2 -
 8 files changed, 45 insertions(+), 192 deletions(-)

diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c
index ba87e62af7..22e66b954d 100644
--- a/src/core/mainloop/mainloop.c
+++ b/src/core/mainloop/mainloop.c
@@ -1146,7 +1146,7 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs)
 
   if (server_mode(options) && !net_is_disabled() && !from_cache &&
       (have_completed_a_circuit() || !any_predicted_circuits(now)))
-   router_do_reachability_checks(1, 1);
+   router_do_reachability_checks();
 }
 
 /** Perform regular maintenance tasks for a single connection.  This
diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c
index 03af7e3e82..fa06d9c7a0 100644
--- a/src/core/or/circuitbuild.c
+++ b/src/core/or/circuitbuild.c
@@ -1069,7 +1069,7 @@ circuit_build_no_more_hops(origin_circuit_t *circ)
     clear_broken_connection_map(1);
     if (server_mode(options) &&
         !router_all_orports_seem_reachable(options)) {
-      router_do_reachability_checks(1, 1);
+      router_do_reachability_checks();
     }
   }
 
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c
index b00d24407a..a49d4b8889 100644
--- a/src/core/or/circuituse.c
+++ b/src/core/or/circuituse.c
@@ -1647,8 +1647,9 @@ circuit_testing_opened(origin_circuit_t *circ)
   } else if (circuit_enough_testing_circs()) {
     router_perform_bandwidth_test(NUM_PARALLEL_TESTING_CIRCS, time(NULL));
     have_performed_bandwidth_test = 1;
-  } else
-    router_do_reachability_checks(1, 0);
+  } else {
+    router_do_reachability_checks();
+  }
 }
 
 /** A testing circuit has failed to build. Take whatever stats we want. */
diff --git a/src/feature/dirclient/dirclient.c b/src/feature/dirclient/dirclient.c
index c5b0d19dd7..439936f83a 100644
--- a/src/feature/dirclient/dirclient.c
+++ b/src/feature/dirclient/dirclient.c
@@ -696,24 +696,6 @@ directory_choose_address_routerstatus(const routerstatus_t *status,
   return 0;
 }
 
-/** Return true iff <b>conn</b> is the client side of a directory connection
- * we launched to ourself in order to determine the reachability of our
- * dir_port. */
-static int
-directory_conn_is_self_reachability_test(dir_connection_t *conn)
-{
-  if (conn->requested_resource &&
-      !strcmpstart(conn->requested_resource,"authority")) {
-    const routerinfo_t *me = router_get_my_routerinfo();
-    if (me &&
-        router_digest_is_me(conn->identity_digest) &&
-        tor_addr_eq(&TO_CONN(conn)->addr, &me->ipv4_addr) &&
-        me->ipv4_dirport == conn->base_.port)
-      return 1;
-  }
-  return 0;
-}
-
 /** Called when we are unable to complete the client's request to a directory
  * server due to a network error: Mark the router as down and try again if
  * possible.
@@ -726,9 +708,6 @@ connection_dir_client_request_failed(dir_connection_t *conn)
      * failed. */
     entry_guard_failed(&conn->guard_state);
   }
-  if (directory_conn_is_self_reachability_test(conn)) {
-    return; /* this was a test fetch. don't retry. */
-  }
   if (!entry_list_is_constrained(get_options()))
     router_set_status(conn->identity_digest, 0); /* don't try this one again */
   if (conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC ||
@@ -2516,8 +2495,6 @@ handle_response_fetch_desc(dir_connection_t *conn,
     SMARTLIST_FOREACH(which, char *, cp, tor_free(cp));
     smartlist_free(which);
   }
-  if (directory_conn_is_self_reachability_test(conn))
-    router_dirport_found_reachable();
 
   return 0;
 }
diff --git a/src/feature/relay/relay_periodic.c b/src/feature/relay/relay_periodic.c
index a917d90f1a..478d746dca 100644
--- a/src/feature/relay/relay_periodic.c
+++ b/src/feature/relay/relay_periodic.c
@@ -164,9 +164,7 @@ check_for_reachability_bw_callback(time_t now, const or_options_t *options)
       (have_completed_a_circuit() || !any_predicted_circuits(now)) &&
       !net_is_disabled()) {
     if (get_uptime() < TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) {
-      router_do_reachability_checks(1, dirport_reachability_count==0);
-      if (++dirport_reachability_count > 5)
-        dirport_reachability_count = 0;
+      router_do_reachability_checks();
       return EARLY_CHECK_REACHABILITY_INTERVAL;
     } else {
       /* If we haven't checked for 12 hours and our bandwidth estimate is
@@ -264,20 +262,6 @@ reachability_warnings_callback(time_t now, const or_options_t *options)
       tor_free(address4);
       tor_free(address6);
     }
-
-    if (me && !router_dirport_seems_reachable(options)) {
-      char *address4 = tor_addr_to_str_dup(&me->ipv4_addr);
-      log_warn(LD_CONFIG,
-               "Your server (%s:%d) has not managed to confirm that its "
-               "DirPort is reachable. Relays do not publish descriptors "
-               "until their ORPort and DirPort are reachable. Please check "
-               "your firewalls, ports, address, /etc/hosts file, etc.",
-               address4, me->ipv4_dirport);
-      control_event_server_status(LOG_WARN,
-                                  "REACHABILITY_FAILED DIRADDRESS=%s:%d",
-                                  address4, me->ipv4_dirport);
-      tor_free(address4);
-    }
   }
 
   return TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT;
diff --git a/src/feature/relay/selftest.c b/src/feature/relay/selftest.c
index 1b438b0330..66e5db34dd 100644
--- a/src/feature/relay/selftest.c
+++ b/src/feature/relay/selftest.c
@@ -47,15 +47,12 @@
 
 static bool have_orport_for_family(int family);
 static void inform_testing_reachability(const tor_addr_t *addr,
-                                        uint16_t port,
-                                        bool is_dirport);
+                                        uint16_t port);
 
 /** 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;
 
 /** Has informed_testing_reachable logged a message about testing our IPv4
  * ORPort? */
@@ -63,18 +60,14 @@ static bool have_informed_testing_or_port_ipv4 = false;
 /** Has informed_testing_reachable logged a message about testing our IPv6
  * ORPort? */
 static bool have_informed_testing_or_port_ipv6 = false;
-/** Has informed_testing_reachable logged a message about testing our
- * DirPort? */
-static bool have_informed_testing_dir_port = false;
 
 /** Forget what we have learned about our reachability status. */
 void
 router_reset_reachability(void)
 {
-  can_reach_or_port_ipv4 = can_reach_or_port_ipv6 = can_reach_dir_port = false;
+  can_reach_or_port_ipv4 = can_reach_or_port_ipv6 = false;
   have_informed_testing_or_port_ipv4 =
-    have_informed_testing_or_port_ipv6 =
-    have_informed_testing_dir_port = false;
+    have_informed_testing_or_port_ipv6 = false;
 }
 
 /** Return 1 if we won't do reachability checks, because:
@@ -135,29 +128,20 @@ router_orport_seems_reachable(const or_options_t *options,
   return true;
 }
 
-/** Return 0 if we need to do a DirPort reachability check, because:
- *   - no reachability check has been done yet, or
- *   - we've initiated reachability checks, but none have succeeded.
- *  Return 1 if we don't need to do a DirPort reachability check, because:
- *   - we've seen a successful reachability check, or
- *   - there is no DirPort set, or
- *   - AssumeReachable is set, or
- *   - the network is disabled.
- */
+/** Relays no long open a DirPort except authorities. In both cases, no
+ * reachability self test is done anymore since network re-entry towards an
+ * authority DirPort is not allowed. Thus, consider it always reachable. */
 int
 router_dirport_seems_reachable(const or_options_t *options)
 {
-  int reach_checks_disabled = router_reachability_checks_disabled(options) ||
-                              !options->DirPort_set;
-  return reach_checks_disabled ||
-         can_reach_dir_port;
+  (void) options;
+  return 1;
 }
 
-/** See if we currently believe our ORPort or DirPort to be
- * unreachable. If so, return 1 else return 0.
- */
+/** See if we currently believe our ORPort to be unreachable. If so, return 1
+ * else return 0. */
 static int
-router_should_check_reachability(int test_or, int test_dir)
+router_should_check_reachability(void)
 {
   const routerinfo_t *me = router_get_my_routerinfo();
   const or_options_t *options = get_options();
@@ -170,15 +154,13 @@ router_should_check_reachability(int test_or, int test_dir)
       options->StrictNodes) {
     /* If we've excluded ourself, and StrictNodes is set, we can't test
      * ourself. */
-    if (test_or || test_dir) {
 #define SELF_EXCLUDED_WARN_INTERVAL 3600
-      static ratelim_t warning_limit=RATELIM_INIT(SELF_EXCLUDED_WARN_INTERVAL);
-      log_fn_ratelim(&warning_limit, LOG_WARN, LD_CIRC,
-                 "Can't perform self-tests for this relay: we have "
-                 "listed ourself in ExcludeNodes, and StrictNodes is set. "
-                 "We cannot learn whether we are usable, and will not "
-                 "be able to advertise ourself.");
-    }
+    static ratelim_t warning_limit=RATELIM_INIT(SELF_EXCLUDED_WARN_INTERVAL);
+    log_fn_ratelim(&warning_limit, LOG_WARN, LD_CIRC,
+                   "Can't perform self-tests for this relay: we have "
+                   "listed ourself in ExcludeNodes, and StrictNodes is set. "
+                   "We cannot learn whether we are usable, and will not "
+                   "be able to advertise ourself.");
     return 0;
   }
   return 1;
@@ -278,7 +260,7 @@ router_do_orport_reachability_checks(const routerinfo_t *me,
       /* Only log if we are actually doing a reachability test to learn if our
        * ORPort is reachable. Else, this prints a log notice if we are simply
        * opening a bandwidth testing circuit even though we are reachable. */
-      inform_testing_reachability(&ap->addr, ap->port, false);
+      inform_testing_reachability(&ap->addr, ap->port);
     }
 
     circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei,
@@ -289,53 +271,15 @@ router_do_orport_reachability_checks(const routerinfo_t *me,
   }
 }
 
-/** Launch a self-testing circuit, and ask an exit to connect to our DirPort.
- * <b>me</b> is our own routerinfo.
- *
- * Relays don't advertise IPv6 DirPorts, so this function only supports IPv4.
- *
- * See router_do_reachability_checks() for details. */
-static void
-router_do_dirport_reachability_checks(const routerinfo_t *me)
-{
-  tor_addr_port_t my_dirport;
-  tor_addr_copy(&my_dirport.addr, &me->ipv4_addr);
-  my_dirport.port = me->ipv4_dirport;
-
-  /* If there is already a pending connection, don't open another one. */
-  if (!connection_get_by_type_addr_port_purpose(
-                  CONN_TYPE_DIR,
-                  &my_dirport.addr, my_dirport.port,
-                  DIR_PURPOSE_FETCH_SERVERDESC)) {
-    /* ask myself, via tor, for my server descriptor. */
-    directory_request_t *req =
-      directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC);
-    directory_request_set_dir_addr_port(req, &my_dirport);
-    directory_request_set_directory_id_digest(req,
-                                              me->cache_info.identity_digest);
-    /* ask via an anon circuit, connecting to our dirport. */
-    directory_request_set_indirection(req, DIRIND_ANON_DIRPORT);
-    directory_request_set_resource(req, "authority.z");
-    directory_initiate_request(req);
-    directory_request_free(req);
-
-    inform_testing_reachability(&my_dirport.addr, my_dirport.port, true);
-  }
-}
-
-/** Some time has passed, or we just got new directory information.
- * See if we currently believe our ORPort or DirPort to be
- * unreachable. If so, launch a new test for it.
- *
- * For ORPort, we simply try making a circuit that ends at ourselves.
- * Success is noticed in onionskin_answer().
+/** Some time has passed, or we just got new directory information. See if we
+ * currently believe our ORPort to be unreachable. If so, launch a new test
+ * for it.
  *
- * For DirPort, we make a connection via Tor to our DirPort and ask
- * for our own server descriptor.
- * Success is noticed in connection_dir_client_reached_eof().
+ * For ORPort, we simply try making a circuit that ends at ourselves.  Success
+ * is noticed in onionskin_answer().
  */
 void
-router_do_reachability_checks(int test_or, int test_dir)
+router_do_reachability_checks(void)
 {
   const routerinfo_t *me = router_get_my_routerinfo();
   const or_options_t *options = get_options();
@@ -344,45 +288,34 @@ router_do_reachability_checks(int test_or, int test_dir)
   int orport_reachable_v6 =
     router_orport_seems_reachable(options, AF_INET6);
 
-  if (router_should_check_reachability(test_or, test_dir)) {
+  if (router_should_check_reachability()) {
     bool need_testing = !circuit_enough_testing_circs();
     /* At the moment, tor relays believe that they are reachable when they
      * receive any create cell on an inbound connection, if the address
      * family is correct.
      */
-    if (test_or && (!orport_reachable_v4 || need_testing)) {
+    if (!orport_reachable_v4 || need_testing) {
       router_do_orport_reachability_checks(me, AF_INET, orport_reachable_v4);
     }
-    if (test_or && (!orport_reachable_v6 || need_testing)) {
+    if (!orport_reachable_v6 || need_testing) {
       router_do_orport_reachability_checks(me, AF_INET6, orport_reachable_v6);
     }
-
-    if (test_dir && !router_dirport_seems_reachable(options)) {
-      router_do_dirport_reachability_checks(me);
-    }
   }
 }
 
 /** Log a message informing the user that we are testing a port for
  * reachability, if we have not already logged such a message.
  *
- * If @a is_dirport is true, then the port is a DirPort; otherwise it is an
- * ORPort.
- *
  * Calls to router_reset_reachability() will reset our view of whether we have
  * logged this message for a given port. */
 static void
-inform_testing_reachability(const tor_addr_t *addr,
-                            uint16_t port,
-                            bool is_dirport)
+inform_testing_reachability(const tor_addr_t *addr, uint16_t port)
 {
   if (!router_get_my_routerinfo())
     return;
 
   bool *have_informed_ptr;
-  if (is_dirport) {
-    have_informed_ptr = &have_informed_testing_dir_port;
-  } else if (tor_addr_family(addr) == AF_INET) {
+  if (tor_addr_family(addr) == AF_INET) {
     have_informed_ptr = &have_informed_testing_or_port_ipv4;
   } else {
     have_informed_ptr = &have_informed_testing_or_port_ipv6;
@@ -397,18 +330,16 @@ inform_testing_reachability(const tor_addr_t *addr,
   char addr_buf[TOR_ADDRPORT_BUF_LEN];
   strlcpy(addr_buf, fmt_addrport(addr, port), sizeof(addr_buf));
 
-  const char *control_addr_type = is_dirport ? "DIRADDRESS" : "ORADDRESS";
-  const char *port_type = is_dirport ? "DirPort" : "ORPort";
   const char *afname = fmt_af_family(tor_addr_family(addr));
 
   control_event_server_status(LOG_NOTICE,
-                              "CHECKING_REACHABILITY %s=%s",
-                              control_addr_type, addr_buf);
+                              "CHECKING_REACHABILITY ORADDRESS=%s",
+                              addr_buf);
 
-  log_notice(LD_OR, "Now checking whether %s %s %s is reachable... "
+  log_notice(LD_OR, "Now checking whether %s ORPort %s is reachable... "
              "(this may take up to %d minutes -- look for log "
              "messages indicating success)",
-             afname, port_type, addr_buf,
+             afname, addr_buf,
              TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT/60);
 
   *have_informed_ptr = true;
@@ -422,8 +353,7 @@ static bool
 ready_to_publish(const or_options_t *options)
 {
   return options->PublishServerDescriptor_ != NO_DIRINFO &&
-    router_dirport_seems_reachable(options) &&
-    router_all_orports_seem_reachable(options);
+         router_all_orports_seem_reachable(options);
 }
 
 /** Annotate that we found our ORPort reachable with a given address
@@ -477,40 +407,6 @@ router_orport_found_reachable(int family)
   }
 }
 
-/** Annotate that we found our DirPort reachable. */
-void
-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_addr_to_str_dup(&me->ipv4_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",
-               ready_to_publish(options) ?
-               " Publishing server descriptor." : "");
-
-    if (router_should_advertise_dirport(options, me->ipv4_dirport)) {
-      mark_my_descriptor_dirty("DirPort 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();
-      }
-    }
-    control_event_server_status(LOG_NOTICE,
-                                "REACHABILITY_SUCCEEDED DIRADDRESS=%s:%d",
-                                address, me->ipv4_dirport);
-    tor_free(address);
-  }
-}
-
 /** We have enough testing circuits open. Send a bunch of "drop"
  * cells down each of them, to exercise our bandwidth.
  *
diff --git a/src/feature/relay/selftest.h b/src/feature/relay/selftest.h
index e09c0e7898..3d7fa2f307 100644
--- a/src/feature/relay/selftest.h
+++ b/src/feature/relay/selftest.h
@@ -23,11 +23,10 @@ int router_orport_seems_reachable(
 int router_dirport_seems_reachable(
                                          const struct or_options_t *options);
 
-void router_do_reachability_checks(int test_or, int test_dir);
+void router_do_reachability_checks(void);
 void router_perform_bandwidth_test(int num_circs, time_t now);
 
 void router_orport_found_reachable(int family);
-void router_dirport_found_reachable(void);
 
 void router_reset_reachability(void);
 
@@ -41,10 +40,8 @@ void router_reset_reachability(void);
   ((void)(opts), 0)
 
 static inline void
-router_do_reachability_checks(int test_or, int test_dir)
+router_do_reachability_checks(void)
 {
-  (void)test_or;
-  (void)test_dir;
   tor_assert_nonfatal_unreached();
 }
 static inline void
@@ -55,16 +52,16 @@ router_perform_bandwidth_test(int num_circs, time_t now)
   tor_assert_nonfatal_unreached();
 }
 static inline int
-inform_testing_reachability(void)
+inform_testing_reachability(const tor_addr_t *addr, uint16_t port)
 {
+  (void) addr;
+  (void) port;
   tor_assert_nonfatal_unreached();
   return 0;
 }
 
 #define router_orport_found_reachable() \
   STMT_NIL
-#define router_dirport_found_reachable() \
-  STMT_NIL
 
 #define router_reset_reachability() \
   STMT_NIL
diff --git a/src/feature/stats/predict_ports.c b/src/feature/stats/predict_ports.c
index 57463952e7..b9c8a0662e 100644
--- a/src/feature/stats/predict_ports.c
+++ b/src/feature/stats/predict_ports.c
@@ -273,8 +273,6 @@ rep_hist_circbuilding_dormant(time_t now)
       (!router_all_orports_seem_reachable(options) ||
        !circuit_enough_testing_circs()))
     return 0;
-  if (!router_dirport_seems_reachable(options))
-    return 0;
 
   return 1;
 }





More information about the tor-commits mailing list