Validate address more carefully when checking self-reachability

Previously, we would treat *any* incoming circuit on a non-local
channel as meaning that our ORPort was reachable.  With this patch,
we make sure that the address that the peer _says_ we have is the
same as the one we're trying to advertise right now.

Closes 20165. Bugfix on 4f5192b2803c706 in 0.1.0.1-rc, when
reachability self-tests were first introduced.

3 files changed, 43 insertions, 7 deletions

diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c
index ad20e143be..64f3c341ae 100644
--- a/src/feature/relay/circuitbuild_relay.c
+++ b/src/feature/relay/circuitbuild_relay.c
@@ -588,13 +588,23 @@ onionskin_answer(struct or_circuit_t *circ,
   if ((!channel_is_local(circ->p_chan)
        || get_options()->ExtendAllowPrivateAddresses)
       && !channel_is_outgoing(circ->p_chan)) {
-    /* 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. */
-    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);
+    /* Okay, it's a create cell from a non-local connection
+     * that we didn't initiate. Presumably this means that create cells
+     * can reach us too. But what address can they reach us on? */
+    const tor_addr_t *my_supposed_addr = &circ->p_chan->addr_according_to_peer;
+    if (router_addr_is_my_published_addr(my_supposed_addr)) {
+      /* Great, this create cell came on connection where the peer says
+       * that the our address is an address we're actually advertising!
+       * That should mean that we're reachable.  But before we finally
+       * declare ourselves reachable, make sure that the address listed
+       * by the peer is the same family as the peer is actually using.
+       */
+      tor_addr_t remote_addr;
+      int family = tor_addr_family(my_supposed_addr);
+      if (channel_get_addr_if_possible(circ->p_chan, &remote_addr) &&
+          tor_addr_family(&remote_addr) == family) {
+        router_orport_found_reachable(family);
+      }
     }
   }
 
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index 675b977ade..3fcf0d616b 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -1729,6 +1729,31 @@ router_is_me(const routerinfo_t *router)
   return router_digest_is_me(router->cache_info.identity_digest);
 }
 
+/**
+ * Return true if we are a server, and if @a addr is an address we are
+ * currently publishing (or trying to publish) in our descriptor.
+ * Return false otherwise.
+ **/
+bool
+router_addr_is_my_published_addr(const tor_addr_t *addr)
+{
+  IF_BUG_ONCE(!addr)
+    return false;
+
+  const routerinfo_t *me = router_get_my_routerinfo();
+  if (!me)
+    return false;
+
+  switch (tor_addr_family(addr)) {
+  case AF_INET:
+    return tor_addr_eq(addr, &me->ipv4_addr);
+  case AF_INET6:
+    return tor_addr_eq(addr, &me->ipv6_addr);
+  default:
+    return false;
+  }
+}
+
 /** Return a routerinfo for this OR, rebuilding a fresh one if
  * necessary.  Return NULL on error, or if called on an OP. */
 MOCK_IMPL(const routerinfo_t *,
diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h
index 89b4a479a4..f71ada8eb7 100644
--- a/src/feature/relay/router.h
+++ b/src/feature/relay/router.h
@@ -100,6 +100,7 @@ int router_digest_is_me(const char *digest);
 const uint8_t *router_get_my_id_digest(void);
 int router_extrainfo_digest_is_me(const char *digest);
 int router_is_me(const routerinfo_t *router);
+bool router_addr_is_my_published_addr(const tor_addr_t *addr);
 int router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e);
 int router_rebuild_descriptor(int force);
 char *router_dump_router_to_string(routerinfo_t *router,