node: Move reentry set to use a digestmap_t

Any lookup now will be certain and not probabilistic as the bloomfilter.

Closes #40269

Signed-off-by: David Goulet <dgoulet@torproject.org>

4 files changed, 57 insertions, 23 deletions

diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c
index b40fa3e567..4727a0ad66 100644
--- a/src/core/or/connection_edge.c
+++ b/src/core/or/connection_edge.c
@@ -4062,7 +4062,7 @@ connection_exit_connect(edge_connection_t *edge_conn)
    * case of an attack so this is a small price to pay. */
   if (!connection_edge_is_rendezvous_stream(edge_conn) &&
       !network_reentry_is_allowed() &&
-      nodelist_reentry_probably_contains(&conn->addr, conn->port)) {
+      nodelist_reentry_contains(&conn->addr, conn->port)) {
     log_info(LD_EXIT, "%s:%d tried to connect back to a known relay address. "
                       "Closing.", escaped_safe_str_client(conn->address),
              conn->port);
diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c
index 22b93762e7..9d56f2c3aa 100644
--- a/src/feature/nodelist/nodelist.c
+++ b/src/feature/nodelist/nodelist.c
@@ -136,7 +136,7 @@ typedef struct nodelist_t {
 
   /* Set of addresses + port that belong to nodes we know and that we don't
    * allow network re-entry towards them. */
-  addr_port_set_t *reentry_set;
+  digestmap_t *reentry_set;
 
   /* The valid-after time of the last live consensus that initialized the
    * nodelist.  We use this to detect outdated nodelists that need to be
@@ -488,6 +488,42 @@ node_add_to_address_set(const node_t *node)
   }
 }
 
+/** Build a construction for the reentry set consisting of an address and port
+ * pair.
+ *
+ * If the given address is _not_ AF_INET or AF_INET6, then the item is an
+ * array of 0s.
+ *
+ * Return a pointer to a static buffer containing the item. Next call to this
+ * function invalidates its previous content. */
+static char *
+build_addr_port_item(const tor_addr_t *addr, const uint16_t port)
+{
+  /* At most 16 bytes are put in this (IPv6) and then 2 bytes for the port
+   * which is within the maximum of 20 bytes (DIGEST_LEN). */
+  static char data[DIGEST_LEN];
+
+  memset(data, 0, sizeof(data));
+  switch (tor_addr_family(addr)) {
+  case AF_INET:
+    memcpy(data, &addr->addr.in_addr.s_addr, 4);
+    break;
+  case AF_INET6:
+    memcpy(data, &addr->addr.in6_addr.s6_addr, 16);
+    break;
+  case AF_UNSPEC:
+    /* Leave the 0. */
+    break;
+  default:
+    /* LCOV_EXCL_START */
+    tor_fragile_assert();
+    /* LCOV_EXCL_STOP */
+  }
+
+  memcpy(data + 16, &port, sizeof(port));
+  return data;
+}
+
 /** Add the given address into the nodelist address set. */
 void
 nodelist_add_addr_to_address_set(const tor_addr_t *addr,
@@ -501,10 +537,12 @@ nodelist_add_addr_to_address_set(const tor_addr_t *addr,
   }
   address_set_add(the_nodelist->node_addrs, addr);
   if (or_port != 0) {
-    addr_port_set_add(the_nodelist->reentry_set, addr, or_port);
+    digestmap_set(the_nodelist->reentry_set,
+                  build_addr_port_item(addr, or_port), (void*) 1);
   }
   if (dir_port != 0) {
-    addr_port_set_add(the_nodelist->reentry_set, addr, dir_port);
+    digestmap_set(the_nodelist->reentry_set,
+                  build_addr_port_item(addr, dir_port), (void*) 1);
   }
 }
 
@@ -525,7 +563,7 @@ nodelist_probably_contains_address(const tor_addr_t *addr)
 /** Return true if <b>addr</b> is the address of some node in the nodelist and
  * corresponds also to the given port. If not, probably return false. */
 bool
-nodelist_reentry_probably_contains(const tor_addr_t *addr, uint16_t port)
+nodelist_reentry_contains(const tor_addr_t *addr, uint16_t port)
 {
   if (BUG(!addr) || BUG(!port))
     return false;
@@ -533,8 +571,8 @@ nodelist_reentry_probably_contains(const tor_addr_t *addr, uint16_t port)
   if (!the_nodelist || !the_nodelist->reentry_set)
     return false;
 
-  return addr_port_set_probably_contains(the_nodelist->reentry_set,
-                                         addr, port);
+  return digestmap_get(the_nodelist->reentry_set,
+                       build_addr_port_item(addr, port)) != NULL;
 }
 
 /** Add <b>ri</b> to an appropriate node in the nodelist.  If we replace an
@@ -669,15 +707,12 @@ nodelist_set_consensus(networkstatus_t *ns)
                             get_estimated_address_per_node();
   estimated_addresses += (get_n_authorities(V3_DIRINFO | BRIDGE_DIRINFO) *
                           get_estimated_address_per_node());
+  /* Clear our sets because we will repopulate them with what this new
+   * consensus contains. */
   address_set_free(the_nodelist->node_addrs);
-  addr_port_set_free(the_nodelist->reentry_set);
   the_nodelist->node_addrs = address_set_new(estimated_addresses);
-  /* Times two here is for both the ORPort and DirPort. We double it again in
-   * order to minimize as much as possible the false positive when looking up
-   * this set. Reason is that Exit streams that are legitimate but end up a
-   * false positive against this set will thus be considered reentry and be
-   * rejected which means a bad UX. */
-  the_nodelist->reentry_set = addr_port_set_new(estimated_addresses * 2 * 2);
+  digestmap_free(the_nodelist->reentry_set, NULL);
+  the_nodelist->reentry_set = digestmap_new();
 
   SMARTLIST_FOREACH_BEGIN(ns->routerstatus_list, routerstatus_t *, rs) {
     node_t *node = node_get_or_create(rs->identity_digest);
@@ -904,7 +939,7 @@ nodelist_free_all(void)
 
   address_set_free(the_nodelist->node_addrs);
   the_nodelist->node_addrs = NULL;
-  addr_port_set_free(the_nodelist->reentry_set);
+  digestmap_free(the_nodelist->reentry_set, NULL);
   the_nodelist->reentry_set = NULL;
 
   tor_free(the_nodelist);
diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h
index 4c4ee6fe83..bc09731ce2 100644
--- a/src/feature/nodelist/nodelist.h
+++ b/src/feature/nodelist/nodelist.h
@@ -35,8 +35,7 @@ node_t *nodelist_add_microdesc(microdesc_t *md);
 void nodelist_set_consensus(networkstatus_t *ns);
 void nodelist_ensure_freshness(networkstatus_t *ns);
 int nodelist_probably_contains_address(const tor_addr_t *addr);
-bool nodelist_reentry_probably_contains(const tor_addr_t *addr,
-                                        uint16_t port);
+bool nodelist_reentry_contains(const tor_addr_t *addr, uint16_t port);
 void nodelist_add_addr_to_address_set(const tor_addr_t *addr,
                                       uint16_t or_port, uint16_t dir_port);
 
diff --git a/src/test/test_address_set.c b/src/test/test_address_set.c
index 6d9fab67ab..4e55d71ff4 100644
--- a/src/test/test_address_set.c
+++ b/src/test/test_address_set.c
@@ -210,8 +210,8 @@ test_exit_no_reentry(void *arg)
   nodelist_set_consensus(dummy_ns);
 
   /* The address set is empty. Try it anyway */
-  tt_assert(!nodelist_reentry_probably_contains(&addr_v4, 244));
-  tt_assert(!nodelist_reentry_probably_contains(&addr_v6, 244));
+  tt_assert(!nodelist_reentry_contains(&addr_v4, 244));
+  tt_assert(!nodelist_reentry_contains(&addr_v6, 244));
 
   /* Now let's populate the network */
   md = tor_malloc_zero(sizeof(*md));
@@ -238,12 +238,12 @@ test_exit_no_reentry(void *arg)
 
   /* First let's try an address that is on the no-reentry list, but with a
      different port */
-  tt_assert(!nodelist_reentry_probably_contains(&addr_v4, 666));
-  tt_assert(!nodelist_reentry_probably_contains(&addr_v6, 444));
+  tt_assert(!nodelist_reentry_contains(&addr_v4, 666));
+  tt_assert(!nodelist_reentry_contains(&addr_v6, 444));
 
   /* OK now let's try with the right address and right port */
-  tt_assert(nodelist_reentry_probably_contains(&addr_v4, 444));
-  tt_assert(nodelist_reentry_probably_contains(&addr_v6, 666));
+  tt_assert(nodelist_reentry_contains(&addr_v4, 444));
+  tt_assert(nodelist_reentry_contains(&addr_v6, 666));
 
  done:
   routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md);