Merge branch 'tor-gitlab/mr/289' into maint-0.4.5

src/core/or/address_set.c

@@ -15,7 +15,6 @@
 #include "lib/net/address.h"
 #include "lib/container/bloomfilt.h"
 #include "lib/crypt_ops/crypto_rand.h"
-#include "siphash.h"
 
 /** Wrap our hash function to have the signature that the bloom filter
  * needs. */
@@ -69,77 +68,3 @@ address_set_probably_contains(const address_set_t *set,
 {
   return bloomfilt_probably_contains(set, addr);
 }
-
-/* Length of the item is an address (IPv4 or IPv6) and a 2 byte port. We use
- * 16 bytes for the address here (IPv6) since we do not know which family
- * the given address in the item thus in the case of IPv4, the extra bytes
- * are simply zeroes to accomodate. */
-#define BLOOMFILT_ADDR_PORT_ITEM_LEN (16 + sizeof(uint16_t))
-
-/** Build an item for the bloomfilter 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 const uint8_t *
-build_addr_port_item(const tor_addr_t *addr, const uint16_t port)
-{
-  static uint8_t data[BLOOMFILT_ADDR_PORT_ITEM_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;
-}
-
-/** Return a hash value for the given item that the bloomfilter will use. */
-static uint64_t
-bloomfilt_addr_port_hash(const struct sipkey *key,
-                         const void *item)
-{
-  return siphash24(item, BLOOMFILT_ADDR_PORT_ITEM_LEN, key);
-}
-
-/** Allocate and return an addr_port_set_t, suitable for holding up to
- * max_address_guess distinct values. */
-addr_port_set_t *
-addr_port_set_new(int max_addresses_guess)
-{
-  uint8_t k[BLOOMFILT_KEY_LEN];
-  crypto_rand((void*)k, sizeof(k));
-  return bloomfilt_new(max_addresses_guess, bloomfilt_addr_port_hash, k);
-}
-
-/** Add an address and port pair to the given set. */
-void
-addr_port_set_add(addr_port_set_t *set, const tor_addr_t *addr, uint16_t port)
-{
-  bloomfilt_add(set, build_addr_port_item(addr, port));
-}
-
-/** Return true if the given address and port pair are in the set. Of course,
- * this is a bloomfilter and thus in rare occasion, a false positive happens
- * thus the "probably". */
-bool
-addr_port_set_probably_contains(const addr_port_set_t *set,
-                                const tor_addr_t *addr, uint16_t port)
-{
-  return !!bloomfilt_probably_contains(set, build_addr_port_item(addr, port));
-}

src/core/or/address_set.h

@@ -29,19 +29,4 @@ void address_set_add_ipv4h(address_set_t *set, uint32_t addr);
 int address_set_probably_contains(const address_set_t *set,
                                   const struct tor_addr_t *addr);
 
-/**
- * An addr_port_set_t represents a set of tor_addr_t values with a uint16_t
- * port value. The implementation is probabilistic: false negatives cannot
- * occur but false positives are possible.
- */
-typedef struct bloomfilt_t addr_port_set_t;
-
-addr_port_set_t *addr_port_set_new(int max_addresses_guess);
-#define addr_port_set_free(s) bloomfilt_free(s)
-void addr_port_set_add(addr_port_set_t *set,
-                       const struct tor_addr_t *addr, uint16_t port);
-bool addr_port_set_probably_contains(const addr_port_set_t *set,
-                                     const struct tor_addr_t *addr,
-                                     uint16_t port);
-
 #endif /* !defined(TOR_ADDRESS_SET_H) */

src/core/or/connection_edge.c

@@ -4290,7 +4290,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 tried to connect back to a known relay address. "
                       "Closing.", connection_describe(conn));
     connection_edge_end(edge_conn, END_STREAM_REASON_CONNECTREFUSED);

src/feature/nodelist/nodelist.c

@@ -137,7 +137,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
@@ -485,6 +485,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,
@@ -498,10 +534,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);
   }
 }
 
@@ -522,7 +560,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;
@@ -530,8 +568,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
@@ -667,15 +705,12 @@ nodelist_set_consensus(const 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);
@@ -902,7 +937,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);

src/feature/nodelist/nodelist.h

@@ -35,8 +35,7 @@ node_t *nodelist_add_microdesc(microdesc_t *md);
 void nodelist_set_consensus(const networkstatus_t *ns);
 void nodelist_ensure_freshness(const 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);
 

src/test/test_address_set.c

@@ -215,8 +215,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));
@@ -243,12 +243,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);