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

[tor/release-0.4.3] relay: Double the size of the relay reentry set
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit 98590621bb49cbecd601085f7e03697f8783ff7b Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 1 09:04:50 2021 -0500 relay: Double the size of the relay reentry set This is to minimize false positive and thus deny reentry to Exit connections that were in reality not re-entering. Helps with overall UX. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/nodelist/nodelist.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 9d553ce1f5..22b93762e7 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -672,8 +672,12 @@ nodelist_set_consensus(networkstatus_t *ns) 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. */ - the_nodelist->reentry_set = addr_port_set_new(estimated_addresses * 2); + /* 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); SMARTLIST_FOREACH_BEGIN(ns->routerstatus_list, routerstatus_t *, rs) { node_t *node = node_get_or_create(rs->identity_digest);

1 0

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

03 Feb '21

commit c2cee6c780a1329a6906e91dc6f7854635727691 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 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 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; @@ -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 ri 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);

1 0

[tor/release-0.4.3] Merge branch 'maint-0.4.3' into release-0.4.3
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit 62cecfdc5f998afee580b7338325db803c69b827 Merge: 0968c40e36 0f8195406e Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Feb 3 08:51:37 2021 -0500 Merge branch 'maint-0.4.3' into release-0.4.3 src/core/or/connection_edge.c | 12 ++++++++++++ src/feature/nodelist/nodelist.c | 8 ++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)

1 0

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

03 Feb '21

commit 59f1a41a7fdc03f5008e859e7e56e9c159b8a8d9 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 b40fa3e567..895e73ac1b 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -4066,7 +4066,7 @@ connection_exit_connect(edge_connection_t *edge_conn) log_info(LD_EXIT, "%s:%d tried to connect back to a known relay address. " "Closing.", escaped_safe_str_client(conn->address), conn->port); - 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/maint-0.3.5] Remove unused addr_port_set code
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit 6f95cdf87ead5c9d47d384be41a2cffed07fc867 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 c2daf89dde..7ada4446c4 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 a7b7cb3f78..a505d31628 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/maint-0.3.5] Merge branch 'ticket40269_035_01' into maint-0.3.5
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit f322ea3fa8e86b9f284ed6df030c9744d731300e Merge: a3cef41fc3 6f95cdf87e Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Feb 3 09:11:09 2021 -0500 Merge branch 'ticket40269_035_01' into maint-0.3.5 src/core/or/address_set.c | 75 ----------------------------------------------- src/core/or/address_set.h | 15 ---------- 2 files changed, 90 deletions(-)

1 0

[tor/maint-0.4.3] relay: Add bloomfiter of relay address + {OR|Dir}Port
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit f26950fa7a077de4b6a9329af397fce74182b1e5 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jan 27 21:49:56 2021 -0500 relay: Add bloomfiter of relay address + {OR|Dir}Port In order to deny re-entry in the network, we now keep a bloomfilter of relay ORPort + address and authorities ORPort + address and DirPort + address combinations. So when an Exit stream is handled, we deny anything connecting back into the network on the ORPorts for relays and on the ORPort+DirPort for the authorities. Related to #2667 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/address_set.c | 74 +++++++++++++++++++++++++++++++++++ src/core/or/address_set.h | 20 +++++++++- src/feature/nodelist/dirlist.c | 36 +++++++++++++++++ src/feature/nodelist/dirlist.h | 2 + src/feature/nodelist/nodelist.c | 87 +++++++++++++++++++++++++++++++++++------ src/feature/nodelist/nodelist.h | 4 ++ 6 files changed, 209 insertions(+), 14 deletions(-) diff --git a/src/core/or/address_set.c b/src/core/or/address_set.c index 758fba4aac..c2daf89dde 100644 --- a/src/core/or/address_set.c +++ b/src/core/or/address_set.c @@ -69,3 +69,77 @@ 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 7a9e71628e..a7b7cb3f78 100644 --- a/src/core/or/address_set.h +++ b/src/core/or/address_set.h @@ -13,13 +13,14 @@ #include "lib/cc/torint.h" #include "lib/container/bloomfilt.h" +struct tor_addr_t; + /** * An address_set_t represents a set of tor_addr_t values. The implementation * is probabilistic: false negatives cannot occur but false positives are * possible. */ typedef struct bloomfilt_t address_set_t; -struct tor_addr_t; address_set_t *address_set_new(int max_addresses_guess); #define address_set_free(set) bloomfilt_free(set) @@ -28,4 +29,19 @@ 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); -#endif +/** + * 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) */ diff --git a/src/feature/nodelist/dirlist.c b/src/feature/nodelist/dirlist.c index 93baa6e4e0..25f769dd5a 100644 --- a/src/feature/nodelist/dirlist.c +++ b/src/feature/nodelist/dirlist.c @@ -49,6 +49,42 @@ static smartlist_t *trusted_dir_servers = NULL; * and all fallback directory servers. */ static smartlist_t *fallback_dir_servers = NULL; +/** Helper: From a given trusted directory entry, add the v4 or/and v6 address + * to the nodelist address set. */ +static void +add_trusted_dir_to_nodelist_addr_set(const dir_server_t *dir) +{ + tor_addr_t tmp_addr; + + tor_assert(dir); + tor_assert(dir->is_authority); + + /* Add IPv4 and then IPv6 if applicable. For authorities, we add the ORPort + * and DirPort so re-entry into the network back to them is not possible. */ + tor_addr_from_ipv4h(&tmp_addr, dir->addr); + nodelist_add_addr_to_address_set(&tmp_addr, dir->or_port, dir->dir_port); + if (!tor_addr_is_null(&dir->ipv6_addr)) { + /* IPv6 DirPort is not a thing yet for authorities. */ + nodelist_add_addr_to_address_set(&dir->ipv6_addr, dir->ipv6_orport, 0); + } +} + +/** Go over the trusted directory server list and add their address(es) to the + * nodelist address set. This is called every time a new consensus is set. */ +void +dirlist_add_trusted_dir_addresses(void) +{ + if (!trusted_dir_servers) { + return; + } + + SMARTLIST_FOREACH_BEGIN(trusted_dir_servers, const dir_server_t *, ent) { + if (ent->is_authority) { + add_trusted_dir_to_nodelist_addr_set(ent); + } + } SMARTLIST_FOREACH_END(ent); +} + /** Return the number of directory authorities whose type matches some bit set * in type */ int diff --git a/src/feature/nodelist/dirlist.h b/src/feature/nodelist/dirlist.h index 9fabd0a44a..9354769bcf 100644 --- a/src/feature/nodelist/dirlist.h +++ b/src/feature/nodelist/dirlist.h @@ -44,4 +44,6 @@ void dir_server_add(dir_server_t *ent); void clear_dir_servers(void); void dirlist_free_all(void); +void dirlist_add_trusted_dir_addresses(void); + #endif diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 8974d95db6..9d553ce1f5 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -134,6 +134,10 @@ typedef struct nodelist_t { /* Set of addresses that belong to nodes we believe in. */ address_set_t *node_addrs; + /* 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; + /* The valid-after time of the last live consensus that initialized the * nodelist. We use this to detect outdated nodelists that need to be * rebuilt using a newer consensus. */ @@ -446,27 +450,61 @@ node_addrs_changed(node_t *node) static void node_add_to_address_set(const node_t *node) { - if (!the_nodelist || !the_nodelist->node_addrs) + tor_addr_t tmp_addr; + + if (!the_nodelist || + !the_nodelist->node_addrs || !the_nodelist->reentry_set) return; - /* These various address sources can be redundant, but it's likely faster - * to add them all than to compare them all for equality. */ + /* These various address sources can be redundant, but it's likely faster to + * add them all than to compare them all for equality. + * + * For relays, we only add the ORPort in the addr+port set since we want to + * allow re-entry into the network to the DirPort so the self reachability + * test succeeds and thus the 0 value for the DirPort. */ if (node->rs) { - if (node->rs->addr) - address_set_add_ipv4h(the_nodelist->node_addrs, node->rs->addr); + if (node->rs->addr) { + tor_addr_from_ipv4h(&tmp_addr, node->rs->addr); + nodelist_add_addr_to_address_set(&tmp_addr, node->rs->or_port, 0); + } if (!tor_addr_is_null(&node->rs->ipv6_addr)) - address_set_add(the_nodelist->node_addrs, &node->rs->ipv6_addr); + nodelist_add_addr_to_address_set(&node->rs->ipv6_addr, + node->rs->ipv6_orport, 0); } if (node->ri) { - if (node->ri->addr) - address_set_add_ipv4h(the_nodelist->node_addrs, node->ri->addr); + if (node->ri->addr) { + tor_addr_from_ipv4h(&tmp_addr, node->ri->addr); + nodelist_add_addr_to_address_set(&tmp_addr, node->ri->or_port, 0); + } if (!tor_addr_is_null(&node->ri->ipv6_addr)) - address_set_add(the_nodelist->node_addrs, &node->ri->ipv6_addr); + nodelist_add_addr_to_address_set(&node->ri->ipv6_addr, + node->ri->ipv6_orport, 0); } if (node->md) { if (!tor_addr_is_null(&node->md->ipv6_addr)) - address_set_add(the_nodelist->node_addrs, &node->md->ipv6_addr); + nodelist_add_addr_to_address_set(&node->md->ipv6_addr, + node->md->ipv6_orport, 0); + } +} + +/** Add the given address into the nodelist address set. */ +void +nodelist_add_addr_to_address_set(const tor_addr_t *addr, + uint16_t or_port, uint16_t dir_port) +{ + if (BUG(!addr) || tor_addr_is_null(addr) || + (!tor_addr_is_v4(addr) && tor_addr_family(addr) != AF_INET6) || + !the_nodelist || !the_nodelist->node_addrs || + !the_nodelist->reentry_set) { + return; + } + address_set_add(the_nodelist->node_addrs, addr); + if (or_port != 0) { + addr_port_set_add(the_nodelist->reentry_set, addr, or_port); + } + if (dir_port != 0) { + addr_port_set_add(the_nodelist->reentry_set, addr, dir_port); } } @@ -484,6 +522,21 @@ nodelist_probably_contains_address(const tor_addr_t *addr) return address_set_probably_contains(the_nodelist->node_addrs, 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) +{ + if (BUG(!addr) || BUG(!port)) + return false; + + if (!the_nodelist || !the_nodelist->reentry_set) + return false; + + return addr_port_set_probably_contains(the_nodelist->reentry_set, + addr, port); +} + /** Add ri to an appropriate node in the nodelist. If we replace an * old routerinfo, and ri_old_out is not NULL, set *ri_old_out * to the previous routerinfo. @@ -612,10 +665,15 @@ nodelist_set_consensus(networkstatus_t *ns) node->rs = NULL); /* Conservatively estimate that every node will have 2 addresses. */ - const int estimated_addresses = smartlist_len(ns->routerstatus_list) * - get_estimated_address_per_node(); + int estimated_addresses = smartlist_len(ns->routerstatus_list) * + get_estimated_address_per_node(); + estimated_addresses += (get_n_authorities(V3_DIRINFO | BRIDGE_DIRINFO) * + get_estimated_address_per_node()); 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. */ + the_nodelist->reentry_set = addr_port_set_new(estimated_addresses * 2); SMARTLIST_FOREACH_BEGIN(ns->routerstatus_list, routerstatus_t *, rs) { node_t *node = node_get_or_create(rs->identity_digest); @@ -664,6 +722,9 @@ nodelist_set_consensus(networkstatus_t *ns) SMARTLIST_FOREACH_BEGIN(the_nodelist->nodes, node_t *, node) { node_add_to_address_set(node); } SMARTLIST_FOREACH_END(node); + /* Then, add all trusted configured directories. Some might not be in the + * consensus so make sure we know them. */ + dirlist_add_trusted_dir_addresses(); if (! authdir) { SMARTLIST_FOREACH_BEGIN(the_nodelist->nodes, node_t *, node) { @@ -839,6 +900,8 @@ nodelist_free_all(void) address_set_free(the_nodelist->node_addrs); the_nodelist->node_addrs = NULL; + addr_port_set_free(the_nodelist->reentry_set); + the_nodelist->reentry_set = NULL; tor_free(the_nodelist); } diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index c430f497d5..4c4ee6fe83 100644 --- a/src/feature/nodelist/nodelist.h +++ b/src/feature/nodelist/nodelist.h @@ -35,6 +35,10 @@ 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); +void nodelist_add_addr_to_address_set(const tor_addr_t *addr, + uint16_t or_port, uint16_t dir_port); void nodelist_remove_microdesc(const char *identity_digest, microdesc_t *md); void nodelist_remove_routerinfo(routerinfo_t *ri);

1 0

[tor/maint-0.4.3] test: Add test for exits blocking reentry to the network
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit 9eba65bd8b688497de139b57ac72e5b8a40bb728 Author: George Kadianakis <desnacked(a)riseup.net> Date: Fri Jan 29 18:21:30 2021 +0200 test: Add test for exits blocking reentry to the network Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/nodelist/dirlist.c | 4 +- src/feature/nodelist/dirlist.h | 2 +- src/test/test_address_set.c | 87 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/feature/nodelist/dirlist.c b/src/feature/nodelist/dirlist.c index 25f769dd5a..b4abffad67 100644 --- a/src/feature/nodelist/dirlist.c +++ b/src/feature/nodelist/dirlist.c @@ -71,8 +71,8 @@ add_trusted_dir_to_nodelist_addr_set(const dir_server_t *dir) /** Go over the trusted directory server list and add their address(es) to the * nodelist address set. This is called every time a new consensus is set. */ -void -dirlist_add_trusted_dir_addresses(void) +MOCK_IMPL(void, +dirlist_add_trusted_dir_addresses, (void)) { if (!trusted_dir_servers) { return; diff --git a/src/feature/nodelist/dirlist.h b/src/feature/nodelist/dirlist.h index 9354769bcf..527af35427 100644 --- a/src/feature/nodelist/dirlist.h +++ b/src/feature/nodelist/dirlist.h @@ -44,6 +44,6 @@ void dir_server_add(dir_server_t *ent); void clear_dir_servers(void); void dirlist_free_all(void); -void dirlist_add_trusted_dir_addresses(void); +MOCK_DECL(void, dirlist_add_trusted_dir_addresses, (void)); #endif diff --git a/src/test/test_address_set.c b/src/test/test_address_set.c index fb8408b3c3..6d9fab67ab 100644 --- a/src/test/test_address_set.c +++ b/src/test/test_address_set.c @@ -4,6 +4,7 @@ #include "core/or/or.h" #include "lib/crypt_ops/crypto_rand.h" #include "core/or/address_set.h" +#include "feature/nodelist/dirlist.h" #include "feature/nodelist/microdesc.h" #include "feature/nodelist/networkstatus.h" #include "feature/nodelist/nodelist.h" @@ -31,6 +32,12 @@ mock_networkstatus_get_latest_consensus_by_flavor(consensus_flavor_t f) return dummy_ns; } +static void +mock_dirlist_add_trusted_dir_addresses(void) +{ + return; +} + /* Number of address a single node_t can have. Default to the production * value. This is to control the size of the bloom filter. */ static int addr_per_node = 2; @@ -169,11 +176,91 @@ test_nodelist(void *arg) UNMOCK(get_estimated_address_per_node); } +/** Test that the no-reentry exit filter works as intended */ +static void +test_exit_no_reentry(void *arg) +{ + routerstatus_t *rs = NULL; microdesc_t *md = NULL; routerinfo_t *ri = NULL; + (void) arg; + + MOCK(networkstatus_get_latest_consensus, + mock_networkstatus_get_latest_consensus); + MOCK(networkstatus_get_latest_consensus_by_flavor, + mock_networkstatus_get_latest_consensus_by_flavor); + MOCK(get_estimated_address_per_node, + mock_get_estimated_address_per_node); + MOCK(dirlist_add_trusted_dir_addresses, + mock_dirlist_add_trusted_dir_addresses); + + dummy_ns = tor_malloc_zero(sizeof(*dummy_ns)); + dummy_ns->flavor = FLAV_MICRODESC; + dummy_ns->routerstatus_list = smartlist_new(); + + tor_addr_t addr_v4, addr_v6, dummy_addr; + tor_addr_parse(&addr_v4, "42.42.42.42"); + tor_addr_parse(&addr_v6, "1:2:3:4::"); + memset(&dummy_addr, 'A', sizeof(dummy_addr)); + + /* This will make the nodelist bloom filter very large + * (the_nodelist->node_addrs) so we will fail the contain test rarely. */ + addr_per_node = 1024; + + /* After this point the nodelist is populated with the directory authorities + * address and ports */ + 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)); + + /* Now let's populate the network */ + md = tor_malloc_zero(sizeof(*md)); + ri = tor_malloc_zero(sizeof(*ri)); + rs = tor_malloc_zero(sizeof(*rs)); + crypto_rand(rs->identity_digest, sizeof(rs->identity_digest)); + crypto_rand(md->digest, sizeof(md->digest)); + memcpy(rs->descriptor_digest, md->digest, DIGEST256_LEN); + + /* Setup the rs, ri and md addresses. */ + rs->addr = tor_addr_to_ipv4h(&addr_v4); + rs->or_port = 444; + tor_addr_parse(&rs->ipv6_addr, "1:2:3:4::"); + rs->ipv6_orport = 666; + ri->addr = tor_addr_to_ipv4h(&addr_v4); + tor_addr_parse(&ri->ipv6_addr, "1:2:3:4::"); + tor_addr_parse(&md->ipv6_addr, "1:2:3:4::"); + + /* Add the rs to the consensus becoming a node_t. */ + smartlist_add(dummy_ns->routerstatus_list, rs); + nodelist_set_consensus(dummy_ns); + + /* Now that the nodelist is populated let's do some retry attempts */ + + /* 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)); + + /* 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)); + + done: + routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md); + smartlist_clear(dummy_ns->routerstatus_list); + networkstatus_vote_free(dummy_ns); + UNMOCK(networkstatus_get_latest_consensus); + UNMOCK(networkstatus_get_latest_consensus_by_flavor); + UNMOCK(get_estimated_address_per_node); + UNMOCK(dirlist_add_trusted_dir_addresses); +} + struct testcase_t address_set_tests[] = { { "contains", test_contains, TT_FORK, NULL, NULL }, { "nodelist", test_nodelist, TT_FORK, NULL, NULL }, + { "exit_no_reentry", test_exit_no_reentry, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/maint-0.4.3] exit: Deny re-entry into the network
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit 93ac6ec4d3d19aaff7fed2ea97b0a30528da3767 Author: Roger Dingledine <arma(a)torproject.org> Date: Wed Jan 27 23:48:57 2021 -0500 exit: Deny re-entry into the network Exit relays now reject exit attempts to known relay addresses + ORPort and also to authorities on the ORPort and DirPort. Closes #2667 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket2667 | 4 ++++ src/core/or/connection_edge.c | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/changes/ticket2667 b/changes/ticket2667 new file mode 100644 index 0000000000..cc42286ef9 --- /dev/null +++ b/changes/ticket2667 @@ -0,0 +1,4 @@ + o Major feature (exit): + - Re-entry into the network is now denied at the Exit level to all relays' + ORPort and authorities' ORPort+DirPort. This is to help mitigate a series + of attacks. See ticket for more information. Closes ticket 2667. diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index 67a772be08..f9a9bbdb73 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -4035,6 +4035,31 @@ connection_exit_connect(edge_connection_t *edge_conn) return; } + /* Next, check for attempts to connect back into the Tor network. We don't + * want to allow these for the same reason we don't want to allow + * infinite-length circuits (see "A Practical Congestion Attack on Tor Using + * Long Paths", Usenix Security 2009). See also ticket 2667. + * + * The TORPROTOCOL reason is used instead of EXITPOLICY so client do NOT + * attempt to retry connecting onto another circuit that will also fail + * bringing considerable more load on the network if so. + * + * Since the address+port set here is a bloomfilter, in very rare cases, the + * check will create a false positive meaning that the destination could + * actually be legit and thus being denied exit. However, sending back a + * reason that makes the client retry results in much worst consequences in + * case of an attack so this is a small price to pay. */ + if (!connection_edge_is_rendezvous_stream(edge_conn) && + nodelist_reentry_probably_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); + connection_edge_end(edge_conn, END_STREAM_REASON_TORPROTOCOL); + circuit_detach_stream(circuit_get_by_edge_conn(edge_conn), edge_conn); + connection_free(conn); + return; + } + #ifdef HAVE_SYS_UN_H if (conn->socket_family != AF_UNIX) { #else

1 0

[tor/maint-0.4.3] Merge branch 'tor-gitlab/mr/284' into ticket2667_043_01
by dgoulet＠torproject.org 03 Feb '21

03 Feb '21

commit 705fd37875c4acd61037da6a2680678ae128e4a4 Merge: 79cb47cfc2 9eba65bd8b Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Jan 29 14:51:38 2021 -0500 Merge branch 'tor-gitlab/mr/284' into ticket2667_043_01 changes/ticket2667 | 4 ++ src/core/or/address_set.c | 74 +++++++++++++++++++++++++++++++++++ src/core/or/address_set.h | 18 ++++++++- src/core/or/connection_edge.c | 25 ++++++++++++ src/feature/nodelist/dirlist.c | 11 ++++-- src/feature/nodelist/nodelist.c | 87 +++++++++++++++++++++++++++++------------ src/feature/nodelist/nodelist.h | 6 ++- src/test/test_address_set.c | 80 +++++++++++++++++++++++++++++++++++++ 8 files changed, 274 insertions(+), 31 deletions(-) diff --cc src/feature/nodelist/nodelist.c index 6c7fd6d0f9,9d553ce1f5..5df8e63073 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@@ -633,14 -664,16 +665,17 @@@ nodelist_set_consensus(networkstatus_t SMARTLIST_FOREACH(the_nodelist->nodes, node_t *, node, node->rs = NULL); - /* Conservatively estimate that every node will have 2 addresses. */ + /* Conservatively estimate that every node will have 2 addresses (v4 and + * v6). Then we add the number of configured trusted authorities we have. */ int estimated_addresses = smartlist_len(ns->routerstatus_list) * get_estimated_address_per_node(); - estimated_addresses += (get_n_authorities(V3_DIRINFO & BRIDGE_DIRINFO) * + estimated_addresses += (get_n_authorities(V3_DIRINFO | BRIDGE_DIRINFO) * get_estimated_address_per_node()); 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. */ + the_nodelist->reentry_set = addr_port_set_new(estimated_addresses * 2); SMARTLIST_FOREACH_BEGIN(ns->routerstatus_list, routerstatus_t *, rs) { node_t *node = node_get_or_create(rs->identity_digest); diff --cc src/test/test_address_set.c index 829ecd79e8,6d9fab67ab..4c99008ae3 --- a/src/test/test_address_set.c +++ b/src/test/test_address_set.c @@@ -179,9 -174,87 +179,88 @@@ test_nodelist(void *arg UNMOCK(networkstatus_get_latest_consensus); UNMOCK(networkstatus_get_latest_consensus_by_flavor); UNMOCK(get_estimated_address_per_node); + UNMOCK(dirlist_add_trusted_dir_addresses); } + /** Test that the no-reentry exit filter works as intended */ + static void + test_exit_no_reentry(void *arg) + { + routerstatus_t *rs = NULL; microdesc_t *md = NULL; routerinfo_t *ri = NULL; + (void) arg; + + MOCK(networkstatus_get_latest_consensus, + mock_networkstatus_get_latest_consensus); + MOCK(networkstatus_get_latest_consensus_by_flavor, + mock_networkstatus_get_latest_consensus_by_flavor); + MOCK(get_estimated_address_per_node, + mock_get_estimated_address_per_node); + MOCK(dirlist_add_trusted_dir_addresses, + mock_dirlist_add_trusted_dir_addresses); + + dummy_ns = tor_malloc_zero(sizeof(*dummy_ns)); + dummy_ns->flavor = FLAV_MICRODESC; + dummy_ns->routerstatus_list = smartlist_new(); + + tor_addr_t addr_v4, addr_v6, dummy_addr; + tor_addr_parse(&addr_v4, "42.42.42.42"); + tor_addr_parse(&addr_v6, "1:2:3:4::"); + memset(&dummy_addr, 'A', sizeof(dummy_addr)); + + /* This will make the nodelist bloom filter very large + * (the_nodelist->node_addrs) so we will fail the contain test rarely. */ + addr_per_node = 1024; + + /* After this point the nodelist is populated with the directory authorities + * address and ports */ + 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)); + + /* Now let's populate the network */ + md = tor_malloc_zero(sizeof(*md)); + ri = tor_malloc_zero(sizeof(*ri)); + rs = tor_malloc_zero(sizeof(*rs)); + crypto_rand(rs->identity_digest, sizeof(rs->identity_digest)); + crypto_rand(md->digest, sizeof(md->digest)); + memcpy(rs->descriptor_digest, md->digest, DIGEST256_LEN); + + /* Setup the rs, ri and md addresses. */ + rs->addr = tor_addr_to_ipv4h(&addr_v4); + rs->or_port = 444; + tor_addr_parse(&rs->ipv6_addr, "1:2:3:4::"); + rs->ipv6_orport = 666; + ri->addr = tor_addr_to_ipv4h(&addr_v4); + tor_addr_parse(&ri->ipv6_addr, "1:2:3:4::"); + tor_addr_parse(&md->ipv6_addr, "1:2:3:4::"); + + /* Add the rs to the consensus becoming a node_t. */ + smartlist_add(dummy_ns->routerstatus_list, rs); + nodelist_set_consensus(dummy_ns); + + /* Now that the nodelist is populated let's do some retry attempts */ + + /* 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)); + + /* 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)); + + done: + routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md); + smartlist_clear(dummy_ns->routerstatus_list); + networkstatus_vote_free(dummy_ns); + UNMOCK(networkstatus_get_latest_consensus); + UNMOCK(networkstatus_get_latest_consensus_by_flavor); + UNMOCK(get_estimated_address_per_node); + UNMOCK(dirlist_add_trusted_dir_addresses); + } + struct testcase_t address_set_tests[] = { { "contains", test_contains, TT_FORK, NULL, NULL },

1 0