Merge remote-tracking branch 'tor-gitlab/mr/21'

8 files changed, 241 insertions, 44 deletions

diff --git a/changes/ticket40022 b/changes/ticket40022
new file mode 100644
index 0000000000..aa7bb256e6
--- /dev/null
+++ b/changes/ticket40022
@@ -0,0 +1,4 @@
+  o Minor feature (relay):
+    - If a relay is unable to discover its address, attempt to learn it from the
+      NETINFO cell. Closes ticket 40022.
+
diff --git a/src/app/config/resolve_addr.c b/src/app/config/resolve_addr.c
index caca5a37d9..172f698549 100644
--- a/src/app/config/resolve_addr.c
+++ b/src/app/config/resolve_addr.c
@@ -42,7 +42,17 @@ typedef enum {
 } fn_address_ret_t;
 
 /** Last resolved addresses. */
-static tor_addr_t last_resolved_addrs[IDX_SIZE];
+static tor_addr_t last_resolved_addrs[] =
+  { TOR_ADDR_NULL, TOR_ADDR_NULL, TOR_ADDR_NULL };
+CTASSERT(ARRAY_LENGTH(last_resolved_addrs) == IDX_SIZE);
+
+/** Last suggested addresses.
+ *
+ * These addresses come from a NETINFO cell from a trusted relay (currently
+ * only authorities). We only use those in last resort. */
+static tor_addr_t last_suggested_addrs[] =
+  { TOR_ADDR_NULL, TOR_ADDR_NULL, TOR_ADDR_NULL };
+CTASSERT(ARRAY_LENGTH(last_suggested_addrs) == IDX_SIZE);
 
 static inline int
 af_to_idx(const int family)
@@ -60,6 +70,29 @@ af_to_idx(const int family)
   }
 }
 
+/** Copy the last suggested address of family into addr_out.
+ *
+ * If no last suggested address exists, the addr_out is a null address (use
+ * tor_addr_is_null() to confirm). */
+void
+resolved_addr_get_suggested(int family, tor_addr_t *addr_out)
+{
+  tor_addr_copy(addr_out, &last_suggested_addrs[af_to_idx(family)]);
+}
+
+/** Set the last suggested address into our cache. This is called when we get
+ * a new NETINFO cell from a trusted source. */
+void
+resolved_addr_set_suggested(const tor_addr_t *addr)
+{
+  if (BUG(tor_addr_family(addr) != AF_INET ||
+          tor_addr_family(addr) != AF_INET6)) {
+    return;
+  }
+  tor_addr_copy(&last_suggested_addrs[af_to_idx(tor_addr_family(addr))],
+                addr);
+}
+
 /** Copy the last resolved address of family into addr_out.
  *
  * If not last resolved address existed, the addr_out is a null address (use
@@ -427,7 +460,7 @@ get_address_from_orport(const or_options_t *options, int warn_severity,
   return FN_RET_OK;
 }
 
-/** @brief Update the last resolved address cache using the given address.
+/** @brief Set the last resolved address cache using the given address.
  *
  * A log notice is emitted if the given address has changed from before. Not
  * emitted on first resolve.
@@ -443,12 +476,14 @@ get_address_from_orport(const or_options_t *options, int warn_severity,
  * @param hostname_used Which hostname was used. If none were used, it is
  *                      NULL. (for logging and control port).
  */
-static void
-update_resolved_cache(const tor_addr_t *addr, const char *method_used,
-                      const char *hostname_used)
+void
+resolved_addr_set_last(const tor_addr_t *addr, const char *method_used,
+                       const char *hostname_used)
 {
   /** Have we done a first resolve. This is used to control logging. */
-  static bool have_resolved_once[IDX_SIZE] = { false, false, false };
+  static bool have_resolved_once[] = { false, false, false };
+  CTASSERT(ARRAY_LENGTH(have_resolved_once) == IDX_SIZE);
+
   bool *done_one_resolve;
   bool have_hostname = false;
   tor_addr_t *last_resolved;
@@ -619,7 +654,7 @@ find_my_address(const or_options_t *options, int family, int warn_severity,
   /*
    * Step 2: Update last resolved address cache and inform the control port.
    */
-  update_resolved_cache(&my_addr, method_used, hostname_used);
+  resolved_addr_set_last(&my_addr, method_used, hostname_used);
 
   if (method_out) {
     *method_out = method_used;
diff --git a/src/app/config/resolve_addr.h b/src/app/config/resolve_addr.h
index e6f8a72554..d9a3e28924 100644
--- a/src/app/config/resolve_addr.h
+++ b/src/app/config/resolve_addr.h
@@ -23,6 +23,11 @@ bool find_my_address(const or_options_t *options, int family,
 
 void resolved_addr_get_last(int family, tor_addr_t *addr_out);
 void resolved_addr_reset_last(int family);
+void resolved_addr_set_last(const tor_addr_t *addr, const char *method_used,
+                            const char *hostname_used);
+
+void resolved_addr_get_suggested(int family, tor_addr_t *addr_out);
+void resolved_addr_set_suggested(const tor_addr_t *addr);
 
 MOCK_DECL(bool, is_local_to_resolve_addr, (const tor_addr_t *addr));
 
diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c
index a51fbf1dd6..4db3730972 100644
--- a/src/core/or/channeltls.c
+++ b/src/core/or/channeltls.c
@@ -72,6 +72,7 @@
 #include "core/or/or_handshake_state_st.h"
 #include "feature/nodelist/routerinfo_st.h"
 #include "core/or/var_cell_st.h"
+#include "src/feature/relay/relay_find_addr.h"
 
 #include "lib/tls/tortls.h"
 #include "lib/tls/x509.h"
@@ -1688,6 +1689,85 @@ time_abs(time_t val)
   return (val < 0) ? -val : val;
 }
 
+/** Return true iff the channel can process a NETINFO cell. For this to return
+ * true, these channel conditions apply:
+ *
+ *    1. Link protocol is version 2 or higher (tor-spec.txt, NETINFO cells
+ *       section).
+ *
+ *    2. Underlying OR connection of the channel is either in v2 or v3
+ *       handshaking state.
+ */
+static bool
+can_process_netinfo_cell(const channel_tls_t *chan)
+{
+  /* NETINFO cells can only be negotiated on link protocol 2 or higher. */
+  if (chan->conn->link_proto < 2) {
+    log_fn(LOG_PROTOCOL_WARN, LD_OR,
+           "Received a NETINFO cell on %s connection; dropping.",
+           chan->conn->link_proto == 0 ? "non-versioned" : "a v1");
+    return false;
+  }
+
+  /* Can't process a NETINFO cell if the connection is not handshaking. */
+  if (chan->conn->base_.state != OR_CONN_STATE_OR_HANDSHAKING_V2 &&
+      chan->conn->base_.state != OR_CONN_STATE_OR_HANDSHAKING_V3) {
+    log_fn(LOG_PROTOCOL_WARN, LD_OR,
+           "Received a NETINFO cell on non-handshaking connection; dropping.");
+    return false;
+  }
+
+  /* Make sure we do have handshake state. */
+  tor_assert(chan->conn->handshake_state);
+  tor_assert(chan->conn->handshake_state->received_versions);
+
+  return true;
+}
+
+/** Mark the given channel endpoint as a client (which means either a tor
+ * client or a tor bridge).
+ *
+ * This MUST be done on an _unauthenticated_ channel. It is a mistake to mark
+ * an authenticated channel as a client.
+ *
+ * The following is done on the channel:
+ *
+ *    1. Marked as a client.
+ *    2. Type of circuit ID type is set.
+ *    3. The underlying OR connection is initialized with the address of the
+ *       endpoint.
+ */
+static void
+mark_channel_tls_endpoint_as_client(channel_tls_t *chan)
+{
+  /* Ending up here for an authenticated link is a mistake. */
+  if (BUG(chan->conn->handshake_state->authenticated)) {
+    return;
+  }
+
+  tor_assert(tor_digest_is_zero(
+            (const char*)(chan->conn->handshake_state->
+                authenticated_rsa_peer_id)));
+  tor_assert(fast_mem_is_zero(
+            (const char*)(chan->conn->handshake_state->
+                          authenticated_ed25519_peer_id.pubkey), 32));
+  /* If the client never authenticated, it's a tor client or bridge
+   * relay, and we must not use it for EXTEND requests (nor could we, as
+   * there are no authenticated peer IDs) */
+  channel_mark_client(TLS_CHAN_TO_BASE(chan));
+  channel_set_circid_type(TLS_CHAN_TO_BASE(chan), NULL,
+         chan->conn->link_proto < MIN_LINK_PROTO_FOR_WIDE_CIRC_IDS);
+
+  connection_or_init_conn_from_address(chan->conn,
+            &(chan->conn->base_.addr),
+            chan->conn->base_.port,
+            /* zero, checked above */
+            (const char*)(chan->conn->handshake_state->
+                          authenticated_rsa_peer_id),
+            NULL, /* Ed25519 ID: Also checked as zero */
+            0);
+}
+
 /**
  * Process a 'netinfo' cell
  *
@@ -1713,20 +1793,12 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
   tor_assert(chan);
   tor_assert(chan->conn);
 
-  if (chan->conn->link_proto < 2) {
-    log_fn(LOG_PROTOCOL_WARN, LD_OR,
-           "Received a NETINFO cell on %s connection; dropping.",
-           chan->conn->link_proto == 0 ? "non-versioned" : "a v1");
+  /* Make sure we can process a NETINFO cell. Link protocol and state
+   * validation is done to make sure of it. */
+  if (!can_process_netinfo_cell(chan)) {
     return;
   }
-  if (chan->conn->base_.state != OR_CONN_STATE_OR_HANDSHAKING_V2 &&
-      chan->conn->base_.state != OR_CONN_STATE_OR_HANDSHAKING_V3) {
-    log_fn(LOG_PROTOCOL_WARN, LD_OR,
-           "Received a NETINFO cell on non-handshaking connection; dropping.");
-    return;
-  }
-  tor_assert(chan->conn->handshake_state &&
-             chan->conn->handshake_state->received_versions);
+
   started_here = connection_or_nonopen_was_started_here(chan->conn);
   identity_digest = chan->conn->identity_digest;
 
@@ -1741,30 +1813,13 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
         return;
       }
     } else {
-      /* we're the server.  If the client never authenticated, we have
-         some housekeeping to do.*/
+      /* We're the server. If the client never authenticated, we have some
+       * housekeeping to do.
+       *
+       * It's a tor client or bridge relay, and we must not use it for EXTEND
+       * requests (nor could we, as there are no authenticated peer IDs) */
       if (!(chan->conn->handshake_state->authenticated)) {
-        tor_assert(tor_digest_is_zero(
-                  (const char*)(chan->conn->handshake_state->
-                      authenticated_rsa_peer_id)));
-        tor_assert(fast_mem_is_zero(
-                  (const char*)(chan->conn->handshake_state->
-                                authenticated_ed25519_peer_id.pubkey), 32));
-        /* If the client never authenticated, it's a tor client or bridge
-         * relay, and we must not use it for EXTEND requests (nor could we, as
-         * there are no authenticated peer IDs) */
-        channel_mark_client(TLS_CHAN_TO_BASE(chan));
-        channel_set_circid_type(TLS_CHAN_TO_BASE(chan), NULL,
-               chan->conn->link_proto < MIN_LINK_PROTO_FOR_WIDE_CIRC_IDS);
-
-        connection_or_init_conn_from_address(chan->conn,
-                  &(chan->conn->base_.addr),
-                  chan->conn->base_.port,
-                  /* zero, checked above */
-                  (const char*)(chan->conn->handshake_state->
-                                authenticated_rsa_peer_id),
-                  NULL, /* Ed25519 ID: Also checked as zero */
-                  0);
+        mark_channel_tls_endpoint_as_client(chan);
       }
     }
   }
@@ -1875,8 +1930,12 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
                        "NETINFO cell", "OR");
   }
 
-  /* XXX maybe act on my_apparent_addr, if the source is sufficiently
-   * trustworthy. */
+  /* Consider our apparent address as a possible suggestion for our address if
+   * we were unable to resolve it previously. The endpoint address is passed
+   * in order to make sure to never consider an address that is the same as
+   * our endpoint. */
+  relay_address_new_suggestion(&my_apparent_addr, &chan->conn->real_addr,
+                               identity_digest);
 
   if (! chan->conn->handshake_state->sent_netinfo) {
     /* If we were prepared to authenticate, but we never got an AUTH_CHALLENGE
diff --git a/src/feature/nodelist/dirlist.c b/src/feature/nodelist/dirlist.c
index bd647ab530..c1864faedf 100644
--- a/src/feature/nodelist/dirlist.c
+++ b/src/feature/nodelist/dirlist.c
@@ -250,6 +250,34 @@ router_digest_is_trusted_dir_type(const char *digest, dirinfo_type_t type)
   return 0;
 }
 
+/** Return true iff the given address matches a trusted directory that matches
+ * at least one bit of type.
+ *
+ * If type is NO_DIRINFO or ALL_DIRINFO, any authority is matched. */
+bool
+router_addr_is_trusted_dir_type(const tor_addr_t *addr, dirinfo_type_t type)
+{
+  int family = tor_addr_family(addr);
+
+  if (!trusted_dir_servers) {
+    return false;
+  }
+
+  SMARTLIST_FOREACH_BEGIN(trusted_dir_servers, dir_server_t *, ent) {
+    /* Ignore entries that don't match the given type. */
+    if (type != NO_DIRINFO && (type & ent->type) == 0) {
+      continue;
+    }
+    /* Match IPv4 or IPv6 address. */
+    if ((family == AF_INET && tor_addr_eq_ipv4h(addr, ent->addr)) ||
+        (family == AF_INET6 && tor_addr_eq(addr, &ent->ipv6_addr))) {
+      return true;
+    }
+  } SMARTLIST_FOREACH_END(ent);
+
+  return false;
+}
+
 /** Create a directory server at <b>address</b>:<b>port</b>, with OR identity
  * key <b>digest</b> which has DIGEST_LEN bytes.  If <b>address</b> is NULL,
  * add ourself.  If <b>is_authority</b>, this is a directory authority.  Return
diff --git a/src/feature/nodelist/dirlist.h b/src/feature/nodelist/dirlist.h
index 9201e76a9c..c9310ff357 100644
--- a/src/feature/nodelist/dirlist.h
+++ b/src/feature/nodelist/dirlist.h
@@ -25,6 +25,11 @@ int router_digest_is_fallback_dir(const char *digest);
 MOCK_DECL(dir_server_t *, trusteddirserver_get_by_v3_auth_digest,
           (const char *d));
 
+bool router_addr_is_trusted_dir_type(const tor_addr_t *addr,
+                                     dirinfo_type_t type);
+#define router_addr_is_trusted_dir(d) \
+  router_addr_is_trusted_dir_type((d), NO_DIRINFO)
+
 int router_digest_is_trusted_dir_type(const char *digest,
                                       dirinfo_type_t type);
 #define router_digest_is_trusted_dir(d) \
diff --git a/src/feature/relay/relay_find_addr.c b/src/feature/relay/relay_find_addr.c
index a51457ddbb..28b5985bb8 100644
--- a/src/feature/relay/relay_find_addr.c
+++ b/src/feature/relay/relay_find_addr.c
@@ -15,6 +15,7 @@
 
 #include "feature/control/control_events.h"
 #include "feature/dircommon/dir_connection_st.h"
+#include "feature/nodelist/dirlist.h"
 #include "feature/relay/relay_find_addr.h"
 #include "feature/relay/router.h"
 #include "feature/relay/routermode.h"
@@ -37,6 +38,62 @@ router_guess_address_from_dir_headers(uint32_t *guess)
   return -1;
 }
 
+/** Consider the address suggestion suggested_addr as a possible one to use as
+ * our address.
+ *
+ * This is called when a valid NETINFO cell is recevied containing a candidate
+ * for our address.
+ *
+ * The suggested address is ignored if it does NOT come from a trusted source.
+ * At the moment, we only look a trusted directory authorities.
+ *
+ * The suggested address is ignored if it is internal or it is the same as the
+ * given peer_addr which is the address from the endpoint that sent the
+ * NETINFO cell.
+ *
+ * The suggested address is set in our suggested address cache if everything
+ * passes. */
+void
+relay_address_new_suggestion(const tor_addr_t *suggested_addr,
+                             const tor_addr_t *peer_addr,
+                             const char *identity_digest)
+{
+  const or_options_t *options = get_options();
+
+  tor_assert(suggested_addr);
+  tor_assert(peer_addr);
+  tor_assert(identity_digest);
+
+  /* This should never be called on a non Tor relay. */
+  if (BUG(!server_mode(options))) {
+    return;
+  }
+
+  /* Is the peer a trusted source? Ignore anything coming from non trusted
+   * source. In this case, we only look at trusted directory authorities. */
+  if (!router_addr_is_trusted_dir(peer_addr) ||
+      !router_digest_is_trusted_dir(identity_digest)) {
+    return;
+  }
+
+  /* Ignore a suggestion that is an internal address or the same as the one
+   * the peer address. */
+  if (tor_addr_is_internal(suggested_addr, 0)) {
+    /* Do not believe anyone who says our address is internal. */
+    return;
+  }
+  if (tor_addr_eq(suggested_addr, peer_addr)) {
+    /* Do not believe anyone who says our address is their address. */
+    log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+           "A relay endpoint %s is telling us that their address is ours.",
+           fmt_addr(peer_addr));
+    return;
+  }
+
+  /* Save the suggestion in our cache. */
+  resolved_addr_set_suggested(suggested_addr);
+}
+
 /** A directory server <b>d_conn</b> told us our IP address is
  * <b>suggestion</b>.
  * If this address is different from the one we think we are now, and
diff --git a/src/feature/relay/relay_find_addr.h b/src/feature/relay/relay_find_addr.h
index ac51a977e6..6f298e6c79 100644
--- a/src/feature/relay/relay_find_addr.h
+++ b/src/feature/relay/relay_find_addr.h
@@ -15,6 +15,10 @@ MOCK_DECL(int, router_pick_published_address,
 void router_new_address_suggestion(const char *suggestion,
                                    const dir_connection_t *d_conn);
 
+void relay_address_new_suggestion(const tor_addr_t *suggested_addr,
+                                  const tor_addr_t *peer_addr,
+                                  const char *identity_digest);
+
 #ifdef RELAY_FIND_ADDR_PRIVATE
 
 #endif /* RELAY_FIND_ADDR_PRIVATE */