February 2021 - tor-commits - lists.torproject.org

[tor/master] node: Move reentry set to use a digestmap_t
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit bd4a3f64a1f6802c97a7976ff9f80495ef3ba57d Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 12:20:39 2021 -0500 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(a)torproject.org> --- src/core/or/connection_edge.c | 2 +- src/feature/nodelist/nodelist.c | 63 ++++++++++++++++++++++++++++++++--------- src/feature/nodelist/nodelist.h | 3 +- src/test/test_address_set.c | 12 ++++---- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index da5431c47e..27a616fa24 100644 --- a/src/core/or/connection_edge.c +++ b/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_TORPROTOCOL); diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 6a6f277849..03b158e68d 100644 --- a/src/feature/nodelist/nodelist.c +++ b/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 addr 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 ri 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); diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index ada0a640a1..44b8918b06 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(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); diff --git a/src/test/test_address_set.c b/src/test/test_address_set.c index 55a0b106d2..a73fbab76a 100644 --- a/src/test/test_address_set.c +++ b/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);

1 0

[tor/maint-0.4.5] node: Move reentry set to use a digestmap_t
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit bd4a3f64a1f6802c97a7976ff9f80495ef3ba57d Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 12:20:39 2021 -0500 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(a)torproject.org> --- src/core/or/connection_edge.c | 2 +- src/feature/nodelist/nodelist.c | 63 ++++++++++++++++++++++++++++++++--------- src/feature/nodelist/nodelist.h | 3 +- src/test/test_address_set.c | 12 ++++---- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index da5431c47e..27a616fa24 100644 --- a/src/core/or/connection_edge.c +++ b/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_TORPROTOCOL); diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 6a6f277849..03b158e68d 100644 --- a/src/feature/nodelist/nodelist.c +++ b/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 addr 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 ri 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); diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index ada0a640a1..44b8918b06 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(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); diff --git a/src/test/test_address_set.c b/src/test/test_address_set.c index 55a0b106d2..a73fbab76a 100644 --- a/src/test/test_address_set.c +++ b/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);

1 0

[tor/maint-0.4.5] Remove unused addr_port_set code
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit 627e7d6625997da33b7e00ef33f71db8a8a20cdc Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 12:28:29 2021 -0500 Remove unused addr_port_set code Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/address_set.c | 75 ----------------------------------------------- src/core/or/address_set.h | 15 ---------- 2 files changed, 90 deletions(-) diff --git a/src/core/or/address_set.c b/src/core/or/address_set.c index fcddc55e9f..9bd3cc0f2d 100644 --- a/src/core/or/address_set.c +++ b/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)); -} diff --git a/src/core/or/address_set.h b/src/core/or/address_set.h index 2c78ab0576..b4d94b65a9 100644 --- a/src/core/or/address_set.h +++ b/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) */

1 0

[tor/master] relay: Send back CONNECTION_REFUSED on reentry
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit 387d1d8835df1bffa71e5157c41cb856c5f18d7d Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 12:45:32 2021 -0500 relay: Send back CONNECTION_REFUSED on reentry The TORPROTOCOL reason causes the client to close the circuit which is not what we want because other valid streams might be on it. Instead, CONNECTION_REFUSED will leave it open but will not allow more streams to be attached to it. The client then open a new circuit to the destination. Closes #40270 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/connection_edge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index da5431c47e..04005b29a6 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -4293,7 +4293,7 @@ connection_exit_connect(edge_connection_t *edge_conn) nodelist_reentry_probably_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_TORPROTOCOL); + connection_edge_end(edge_conn, END_STREAM_REASON_CONNECTREFUSED); circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn); connection_free(conn); return;

1 0

[tor/release-0.4.5] node: Move reentry set to use a digestmap_t
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit bd4a3f64a1f6802c97a7976ff9f80495ef3ba57d Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 12:20:39 2021 -0500 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(a)torproject.org> --- src/core/or/connection_edge.c | 2 +- src/feature/nodelist/nodelist.c | 63 ++++++++++++++++++++++++++++++++--------- src/feature/nodelist/nodelist.h | 3 +- src/test/test_address_set.c | 12 ++++---- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index da5431c47e..27a616fa24 100644 --- a/src/core/or/connection_edge.c +++ b/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_TORPROTOCOL); diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 6a6f277849..03b158e68d 100644 --- a/src/feature/nodelist/nodelist.c +++ b/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 addr 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 ri 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); diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index ada0a640a1..44b8918b06 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(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); diff --git a/src/test/test_address_set.c b/src/test/test_address_set.c index 55a0b106d2..a73fbab76a 100644 --- a/src/test/test_address_set.c +++ b/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);

1 0

[tor/master] Remove unused addr_port_set code
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit 627e7d6625997da33b7e00ef33f71db8a8a20cdc Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 12:28:29 2021 -0500 Remove unused addr_port_set code Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/address_set.c | 75 ----------------------------------------------- src/core/or/address_set.h | 15 ---------- 2 files changed, 90 deletions(-) diff --git a/src/core/or/address_set.c b/src/core/or/address_set.c index fcddc55e9f..9bd3cc0f2d 100644 --- a/src/core/or/address_set.c +++ b/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)); -} diff --git a/src/core/or/address_set.h b/src/core/or/address_set.h index 2c78ab0576..b4d94b65a9 100644 --- a/src/core/or/address_set.h +++ b/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) */

1 0

[tor/master] Merge branch 'tor-gitlab/mr/289' into maint-0.4.5
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit ed373eaa8de41f22c28b5c203b85342e0fc48bee Merge: 387d1d8835 627e7d6625 Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 14:09:58 2021 -0500 Merge branch 'tor-gitlab/mr/289' into maint-0.4.5 src/core/or/address_set.c | 75 ----------------------------------------- src/core/or/address_set.h | 15 --------- src/core/or/connection_edge.c | 2 +- src/feature/nodelist/nodelist.c | 63 ++++++++++++++++++++++++++-------- src/feature/nodelist/nodelist.h | 3 +- src/test/test_address_set.c | 12 +++---- 6 files changed, 57 insertions(+), 113 deletions(-) diff --cc src/core/or/connection_edge.c index 04005b29a6,27a616fa24..a33c64fe19 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@@ -4290,10 -4290,10 +4290,10 @@@ connection_exit_connect(edge_connection * 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_TORPROTOCOL); + connection_edge_end(edge_conn, END_STREAM_REASON_CONNECTREFUSED); circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn); connection_free(conn); return;

1 0

[tor/master] Merge branch 'maint-0.4.5'
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit 6bde42b4de3fd1a3d7c8385a66f8e40b954e60f8 Merge: 385fda038f ed373eaa8d Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 14:10:06 2021 -0500 Merge branch 'maint-0.4.5' src/core/or/address_set.c | 75 ----------------------------------------- src/core/or/address_set.h | 15 --------- src/core/or/connection_edge.c | 4 +-- src/feature/nodelist/nodelist.c | 63 ++++++++++++++++++++++++++-------- src/feature/nodelist/nodelist.h | 3 +- src/test/test_address_set.c | 12 +++---- 6 files changed, 58 insertions(+), 114 deletions(-)

1 0

[tor/release-0.4.5] Merge branch 'tor-gitlab/mr/289' into maint-0.4.5
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit ed373eaa8de41f22c28b5c203b85342e0fc48bee Merge: 387d1d8835 627e7d6625 Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 14:09:58 2021 -0500 Merge branch 'tor-gitlab/mr/289' into maint-0.4.5 src/core/or/address_set.c | 75 ----------------------------------------- src/core/or/address_set.h | 15 --------- src/core/or/connection_edge.c | 2 +- src/feature/nodelist/nodelist.c | 63 ++++++++++++++++++++++++++-------- src/feature/nodelist/nodelist.h | 3 +- src/test/test_address_set.c | 12 +++---- 6 files changed, 57 insertions(+), 113 deletions(-) diff --cc src/core/or/connection_edge.c index 04005b29a6,27a616fa24..a33c64fe19 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@@ -4290,10 -4290,10 +4290,10 @@@ connection_exit_connect(edge_connection * 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_TORPROTOCOL); + connection_edge_end(edge_conn, END_STREAM_REASON_CONNECTREFUSED); circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn); connection_free(conn); return;

1 0

[tor/release-0.4.5] relay: Send back CONNECTION_REFUSED on reentry
by dgoulet＠torproject.org 01 Feb '21

01 Feb '21

commit 387d1d8835df1bffa71e5157c41cb856c5f18d7d Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 12:45:32 2021 -0500 relay: Send back CONNECTION_REFUSED on reentry The TORPROTOCOL reason causes the client to close the circuit which is not what we want because other valid streams might be on it. Instead, CONNECTION_REFUSED will leave it open but will not allow more streams to be attached to it. The client then open a new circuit to the destination. Closes #40270 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/connection_edge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index da5431c47e..04005b29a6 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -4293,7 +4293,7 @@ connection_exit_connect(edge_connection_t *edge_conn) nodelist_reentry_probably_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_TORPROTOCOL); + connection_edge_end(edge_conn, END_STREAM_REASON_CONNECTREFUSED); circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn); connection_free(conn); return;

1 0