Merge branch 'ticket40071_045_01_squashed' into master

6 files changed, 101 insertions, 55 deletions

diff --git a/changes/ticket40071 b/changes/ticket40071
new file mode 100644
index 0000000000..1e294a68e7
--- /dev/null
+++ b/changes/ticket40071
@@ -0,0 +1,7 @@
+  o Minor bugfixes (relay, address):
+    - Don't trigger an IP change if no new valid IP can be found. Fixes bug
+      40071; bugfix on 0.4.5.1-alpha.
+    - When attempting to discover our IP, don't launch a descriptor fetch
+      anymore but rather a simple test circuit since the address discovery is
+      through the NETINFO cell now from the authorities. Fixes bug 40071; bugfix
+      on 0.4.5.1-alpha.
diff --git a/src/core/or/circuitlist.h b/src/core/or/circuitlist.h
index fd7e22e4c0..3178e6cd0d 100644
--- a/src/core/or/circuitlist.h
+++ b/src/core/or/circuitlist.h
@@ -114,7 +114,9 @@
 #define CIRCUIT_PURPOSE_S_HSDIR_POST 20
 #define CIRCUIT_PURPOSE_S_HS_MAX_ 20
 
-/** A testing circuit; not meant to be used for actual traffic. */
+/** A testing circuit; not meant to be used for actual traffic. It is used for
+ * bandwidth measurement, reachability test and address discovery from an
+ * authority using the NETINFO cell. */
 #define CIRCUIT_PURPOSE_TESTING 21
 /** A controller made this circuit and Tor should not use it. */
 #define CIRCUIT_PURPOSE_CONTROLLER 22
diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c
index 3f6e31bc3a..a1a348edb9 100644
--- a/src/feature/nodelist/routerlist.c
+++ b/src/feature/nodelist/routerlist.c
@@ -65,6 +65,9 @@
 #include "app/config/config.h"
 #include "core/mainloop/connection.h"
 #include "core/mainloop/mainloop.h"
+#include "core/or/circuitlist.h"
+#include "core/or/circuituse.h"
+#include "core/or/extendinfo.h"
 #include "core/or/policies.h"
 #include "feature/client/bridges.h"
 #include "feature/control/control_events.h"
@@ -137,8 +140,6 @@ static int signed_desc_digest_is_recognized(signed_descriptor_t *desc);
 static const char *signed_descriptor_get_body_impl(
                                               const signed_descriptor_t *desc,
                                               int with_annotations);
-static void launch_dummy_descriptor_download_as_needed(time_t now,
-                                   const or_options_t *options);
 
 /****************************************************************************/
 
@@ -2306,7 +2307,6 @@ update_all_descriptor_downloads(time_t now)
     return;
   update_router_descriptor_downloads(now);
   update_microdesc_downloads(now);
-  launch_dummy_descriptor_download_as_needed(now, get_options());
 }
 
 /** Clear all our timeouts for fetching v3 directory stuff, and then
@@ -2760,54 +2760,6 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote,
   smartlist_free(no_longer_old);
 }
 
-/** How often should we launch a server/authority request to be sure of getting
- * a guess for our IP? */
-/*XXXX+ this info should come from netinfo cells or something, or we should
- * do this only when we aren't seeing incoming data. see bug 652. */
-#define DUMMY_DOWNLOAD_INTERVAL (20*60)
-
-/** As needed, launch a dummy router descriptor fetch to see if our
- * address has changed. */
-static void
-launch_dummy_descriptor_download_as_needed(time_t now,
-                                           const or_options_t *options)
-{
-  static time_t last_dummy_download = 0;
-  bool have_addr;
-  tor_addr_t addr_out;
-
-  /* This dummy fetch only matter for relays. */
-  if (!server_mode(options)) {
-    return;
-  }
-
-  /* Lookup the address cache to learn if we have a good usable address. We
-   * still force relays to have an IPv4 so that alone is enough to learn if we
-   * need a lookup. In case we don't have one, we might want to attempt a
-   * dummy fetch to learn our address as a suggestion from an authority. */
-  have_addr = relay_find_addr_to_publish(options, AF_INET,
-                                         RELAY_FIND_ADDR_CACHE_ONLY,
-                                         &addr_out);
-
-  /* XXXX+ we could be smarter here; see notes on bug 652. */
-  /* If we're a server that doesn't have an address, we rely on directory
-   * fetches to learn when our address changes.  So if we haven't tried to get
-   * any routerdescs in a long time, try a dummy fetch now. */
-  if (!have_addr &&
-      last_descriptor_download_attempted + DUMMY_DOWNLOAD_INTERVAL < now &&
-      last_dummy_download + DUMMY_DOWNLOAD_INTERVAL < now) {
-    last_dummy_download = now;
-    /* XX/teor - do we want an authority here, because they are less likely
-     * to give us the wrong address? (See #17782)
-     * I'm leaving the previous behaviour intact, because I don't like
-     * the idea of some relays contacting an authority every 20 minutes. */
-    directory_get_from_dirserver(DIR_PURPOSE_FETCH_SERVERDESC,
-                                 ROUTER_PURPOSE_GENERAL, "authority.z",
-                                 PDS_RETRY_IF_NO_SERVERS,
-                                 DL_WANT_ANY_DIRSERVER);
-  }
-}
-
 /** Launch downloads for router status as needed. */
 void
 update_router_descriptor_downloads(time_t now)
diff --git a/src/feature/relay/relay_find_addr.c b/src/feature/relay/relay_find_addr.c
index 43b958d563..9c2c8b281c 100644
--- a/src/feature/relay/relay_find_addr.c
+++ b/src/feature/relay/relay_find_addr.c
@@ -12,10 +12,16 @@
 #include "app/config/resolve_addr.h"
 
 #include "core/mainloop/mainloop.h"
+#include "core/or/circuitlist.h"
+#include "core/or/circuituse.h"
+#include "core/or/extendinfo.h"
 
 #include "feature/control/control_events.h"
 #include "feature/dircommon/dir_connection_st.h"
 #include "feature/nodelist/dirlist.h"
+#include "feature/nodelist/node_select.h"
+#include "feature/nodelist/nodelist.h"
+#include "feature/nodelist/routerstatus_st.h"
 #include "feature/relay/relay_find_addr.h"
 #include "feature/relay/router.h"
 #include "feature/relay/routermode.h"
@@ -151,3 +157,65 @@ relay_has_address_set(int family)
   return relay_find_addr_to_publish(get_options(), family,
                                     RELAY_FIND_ADDR_CACHE_ONLY, &addr);
 }
+
+/** How often should we launch a circuit to an authority to be sure of getting
+ * a guess for our IP? */
+#define DUMMY_DOWNLOAD_INTERVAL (20*60)
+
+void
+relay_addr_learn_from_dirauth(void)
+{
+  static time_t last_dummy_circuit = 0;
+  const or_options_t *options = get_options();
+  time_t now = time(NULL);
+  bool have_addr;
+  tor_addr_t addr_out;
+
+  /* This dummy circuit only matter for relays. */
+  if (BUG(!server_mode(options))) {
+    return;
+  }
+
+  /* Lookup the address cache to learn if we have a good usable address. We
+   * still force relays to have an IPv4 so that alone is enough to learn if we
+   * need a lookup. In case we don't have one, we might want to attempt a
+   * dummy circuit to learn our address as a suggestion from an authority. */
+  have_addr = relay_find_addr_to_publish(options, AF_INET,
+                                         RELAY_FIND_ADDR_CACHE_ONLY,
+                                         &addr_out);
+
+  /* If we're a relay or bridge for which we were unable to discover our
+   * public address, we rely on learning our address from a directory
+   * authority from the NETINFO cell. */
+  if (!have_addr && last_dummy_circuit + DUMMY_DOWNLOAD_INTERVAL < now) {
+    last_dummy_circuit = now;
+
+    const routerstatus_t *rs = router_pick_trusteddirserver(V3_DIRINFO, 0);
+    if (BUG(!rs)) {
+      /* We should really always have trusted directories configured at this
+       * stage. They are loaded early either from default list or the one
+       * given in the configuration file. */
+      return;
+    }
+    const node_t *node = node_get_by_id(rs->identity_digest);
+    if (BUG(!node)) {
+      /* If there is a routerstatus_t, there is a node_t thus this should
+       * never fail. */
+      return;
+    }
+    extend_info_t *ei = extend_info_from_node(node, 1);
+    if (BUG(!ei)) {
+      return;
+    }
+
+    log_debug(LD_GENERAL, "Attempting dummy testing circuit to an authority "
+                          "in order to learn our address.");
+
+    /* Launch a one-hop testing circuit to a trusted authority so we can learn
+     * our address through the NETINFO cell. */
+    circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei,
+                                  CIRCLAUNCH_IS_INTERNAL |
+                                  CIRCLAUNCH_ONEHOP_TUNNEL);
+    extend_info_free(ei);
+  }
+}
diff --git a/src/feature/relay/relay_find_addr.h b/src/feature/relay/relay_find_addr.h
index 3d30946b05..34890cd34e 100644
--- a/src/feature/relay/relay_find_addr.h
+++ b/src/feature/relay/relay_find_addr.h
@@ -24,6 +24,8 @@ MOCK_DECL(bool, relay_find_addr_to_publish,
 
 bool relay_has_address_set(int family);
 
+void relay_addr_learn_from_dirauth(void);
+
 #ifdef RELAY_FIND_ADDR_PRIVATE
 
 #endif /* RELAY_FIND_ADDR_PRIVATE */
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index 29103ed6c6..3aa9561f4f 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -2071,7 +2071,9 @@ router_build_fresh_unsigned_routerinfo,(routerinfo_t **ri_out))
 
   /* Tor requires a relay to have an IPv4 so bail if we can't find it. */
   if (!have_v4) {
-    log_warn(LD_CONFIG, "Don't know my address while generating descriptor");
+    log_info(LD_CONFIG, "Don't know my address while generating descriptor. "
+                        "Launching circuit to authority to learn it.");
+    relay_addr_learn_from_dirauth();
     result = TOR_ROUTERINFO_ERROR_NO_EXT_ADDR;
     goto err;
   }
@@ -2679,8 +2681,21 @@ check_descriptor_ipaddress_changed(time_t now)
 
     /* Ignore returned value because we want to notice not only an address
      * change but also if an address is lost (current == UNSPEC). */
-    find_my_address(get_options(), family, LOG_INFO, &current, &method,
-                    &hostname);
+    bool found = find_my_address(get_options(), family, LOG_INFO, &current,
+                                 &method, &hostname);
+    if (!found) {
+      /* Address was possibly not found because it is simply not configured or
+       * discoverable. Fallback to our cache, which includes any suggestion
+       * sent by a trusted directory server. */
+      found = relay_find_addr_to_publish(get_options(), family,
+                                         RELAY_FIND_ADDR_CACHE_ONLY,
+                                         &current);
+    }
+
+    /* The "current" address might be UNSPEC meaning it was not discovered nor
+     * found in our current cache. If we had an address before and we have
+     * none now, we consider this an IP change since it appears the relay lost
+     * its address. */
 
     if (!tor_addr_eq(previous, &current)) {
       char *source;