March 2018 - tor-commits - lists.torproject.org

[tor/master] fixup! Refactor: suppress duplicated functions from router.c and encapsulate NODE_DESC_BUF_LEN constant.
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 216bc353d3c724e8348408800f389c0c76c2bbc6 Author: George Kadianakis <desnacked(a)riseup.net> Date: Sun Mar 25 20:13:00 2018 +0300 fixup! Refactor: suppress duplicated functions from router.c and encapsulate NODE_DESC_BUF_LEN constant. --- changes/ticket25432 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changes/ticket25432 b/changes/ticket25432 index 2179770f6..21ca20134 100644 --- a/changes/ticket25432 +++ b/changes/ticket25432 @@ -1,6 +1,6 @@ o Code simplification and refactoring: - Merge functions used for describing nodes and suppress the functions that do not allocate memory for the output buffer string. - - NODE_DESC_BUF_LEN constant and format_node_description() function + NODE_DESC_BUF_LEN constant and format_node_description() function cannot be used externally from router.c module anymore. - - Closes ticket 25432. Patch by valentecaio. + Closes ticket 25432. Patch by valentecaio.

1 0

[tor/master] Merge remote-tracking branch 'asn-github/t-25432'
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit cb083b5d3e21307b2dd762a1e891b71a9bd69eda Merge: 794a25f8c 216bc353d Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 14:43:33 2018 -0400 Merge remote-tracking branch 'asn-github/t-25432' changes/ticket25432 | 6 +++ src/or/dirserv.c | 4 +- src/or/or.h | 4 +- src/or/router.c | 104 ++++++++++++++++++++-------------------------------- src/or/router.h | 18 --------- 5 files changed, 49 insertions(+), 87 deletions(-)

1 0

[tor/maint-0.3.3] relay: Avoid connecting to down relays
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit f29d158330a8c6bcc91b71a888db741766135aaf Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Feb 6 15:40:17 2018 -0500 relay: Avoid connecting to down relays If we failed to connect at the TCP level to a relay, note it down and refuse to connect again for another 60 seconds. Fixes #24767 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug24767 | 5 ++ src/or/connection_or.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) diff --git a/changes/bug24767 b/changes/bug24767 new file mode 100644 index 000000000..56fbe51a9 --- /dev/null +++ b/changes/bug24767 @@ -0,0 +1,5 @@ + o Major bugfixes (relay, connection): + - Refuse to connect again to a relay from which we failed previously with + a connection refused, timeout or error (at the TCP level). The relay + won't be retried for 60 seconds after the failure occured. Fixes bug + 24767; bugfix on 0.0.6. diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 272a086a3..976889ad0 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1122,6 +1122,216 @@ connection_or_group_set_badness_(smartlist_t *group, int force) } SMARTLIST_FOREACH_END(or_conn); } +/* Lifetime of a connection failure. After that, we'll retry. This is in + * seconds. */ +#define OR_CONNECT_FAILURE_LIFETIME 60 +/* The interval to use with when to clean up the failure cache. */ +#define OR_CONNECT_FAILURE_CLEANUP_INTERVAL 60 + +/* When is the next time we have to cleanup the failure map. We keep this + * because we clean it opportunistically. */ +static time_t or_connect_failure_map_next_cleanup_ts = 0; + +/* OR connection failure entry data structure. It is kept in the connection + * failure map defined below and indexed by OR identity digest, address and + * port. + * + * We need to identify a connection failure with these three values because we + * want to avoid to wrongfully blacklist a relay if someone is trying to + * extend to a known identity digest but with the wrong IP/port. For instance, + * it can happen if a relay changed its port but the client still has an old + * descriptor with the old port. We want to stop connecting to that + * IP/port/identity all together, not only the relay identity. */ +typedef struct or_connect_failure_entry_t { + HT_ENTRY(or_connect_failure_entry_t) node; + /* Identity digest of the connection where it is connecting to. */ + uint8_t identity_digest[DIGEST_LEN]; + /* This is the connection address from the base connection_t. After the + * connection is checked for canonicity, the base address should represent + * what we know instead of where we are connecting to. This is what we need + * so we can correlate known relays within the consensus. */ + tor_addr_t addr; + uint16_t port; + /* Last time we were unable to connect. */ + time_t last_failed_connect_ts; +} or_connect_failure_entry_t; + +/* Map where we keep connection failure entries. They are indexed by addr, + * port and identity digest. */ +static HT_HEAD(or_connect_failure_ht, or_connect_failure_entry_t) + or_connect_failures_map = HT_INITIALIZER(); + +/* Helper: Hashtable equal function. Return 1 if equal else 0. */ +static int +or_connect_failure_ht_eq(const or_connect_failure_entry_t *a, + const or_connect_failure_entry_t *b) +{ + return fast_memeq(a->identity_digest, b->identity_digest, DIGEST_LEN) && + tor_addr_eq(&a->addr, &b->addr) && + a->port == b->port; +} + +/* Helper: Return the hash for the hashtable of the given entry. For this + * table, it is a combination of address, port and identity digest. */ +static unsigned int +or_connect_failure_ht_hash(const or_connect_failure_entry_t *entry) +{ + size_t offset = 0, addr_size; + const void *addr_ptr; + /* Largest size is IPv6 and IPv4 is smaller so it is fine. */ + uint8_t data[16 + sizeof(uint16_t) + DIGEST_LEN]; + + /* Get the right address bytes depending on the family. */ + switch (tor_addr_family(&entry->addr)) { + case AF_INET: + addr_size = 4; + addr_ptr = &entry->addr.addr.in_addr.s_addr; + break; + case AF_INET6: + addr_size = 16; + addr_ptr = &entry->addr.addr.in6_addr.s6_addr; + break; + default: + tor_assert_nonfatal_unreached(); + return 0; + } + + memcpy(data, addr_ptr, addr_size); + offset += addr_size; + memcpy(data + offset, entry->identity_digest, DIGEST_LEN); + offset += DIGEST_LEN; + set_uint16(data + offset, entry->port); + offset += sizeof(uint16_t); + + return (unsigned int) siphash24g(data, offset); +} + +HT_PROTOTYPE(or_connect_failure_ht, or_connect_failure_entry_t, node, + or_connect_failure_ht_hash, or_connect_failure_ht_eq) + +HT_GENERATE2(or_connect_failure_ht, or_connect_failure_entry_t, node, + or_connect_failure_ht_hash, or_connect_failure_ht_eq, + 0.6, tor_reallocarray_, tor_free_) + +/* Initialize a given connect failure entry with the given identity_digest, + * addr and port. All field are optional except ocf. */ +static void +or_connect_failure_init(const char *identity_digest, const tor_addr_t *addr, + uint16_t port, or_connect_failure_entry_t *ocf) +{ + tor_assert(ocf); + if (identity_digest) { + memcpy(ocf->identity_digest, identity_digest, + sizeof(ocf->identity_digest)); + } + if (addr) { + tor_addr_copy(&ocf->addr, addr); + } + ocf->port = port; +} + +/* Return a newly allocated connection failure entry. It is initialized with + * the given or_conn data. This can't fail. */ +static or_connect_failure_entry_t * +or_connect_failure_new(const or_connection_t *or_conn) +{ + or_connect_failure_entry_t *ocf = tor_malloc_zero(sizeof(*ocf)); + or_connect_failure_init(or_conn->identity_digest, &or_conn->real_addr, + TO_CONN(or_conn)->port, ocf); + return ocf; +} + +/* Return a connection failure entry matching the given or_conn. NULL is + * returned if not found. */ +static or_connect_failure_entry_t * +or_connect_failure_find(const or_connection_t *or_conn) +{ + or_connect_failure_entry_t lookup; + tor_assert(or_conn); + or_connect_failure_init(or_conn->identity_digest, &TO_CONN(or_conn)->addr, + TO_CONN(or_conn)->port, &lookup); + return HT_FIND(or_connect_failure_ht, &or_connect_failures_map, &lookup); +} + +/* Note down in the connection failure cache that a failure occurred on the + * given or_conn. */ +static void +note_or_connect_failed(const or_connection_t *or_conn) +{ + or_connect_failure_entry_t *ocf = NULL; + + tor_assert(or_conn); + + ocf = or_connect_failure_find(or_conn); + if (ocf == NULL) { + ocf = or_connect_failure_new(or_conn); + HT_INSERT(or_connect_failure_ht, &or_connect_failures_map, ocf); + } + ocf->last_failed_connect_ts = approx_time(); +} + +/* Cleanup the connection failure cache and remove all entries below the + * given cutoff. */ +static void +or_connect_failure_map_cleanup(time_t cutoff) +{ + or_connect_failure_entry_t **ptr, **next, *entry; + + for (ptr = HT_START(or_connect_failure_ht, &or_connect_failures_map); + ptr != NULL; ptr = next) { + entry = *ptr; + if (entry->last_failed_connect_ts <= cutoff) { + next = HT_NEXT_RMV(or_connect_failure_ht, &or_connect_failures_map, ptr); + tor_free(entry); + } else { + next = HT_NEXT(or_connect_failure_ht, &or_connect_failures_map, ptr); + } + } +} + +/* Return true iff the given OR connection can connect to its destination that + * is the triplet identity_digest, address and port. + * + * The or_conn MUST have gone through connection_or_check_canonicity() so the + * base address is properly set to what we know or doesn't know. */ +static int +should_connect_to_relay(const or_connection_t *or_conn) +{ + time_t now, cutoff; + time_t connect_failed_since_ts = 0; + or_connect_failure_entry_t *ocf; + + tor_assert(or_conn); + + now = approx_time(); + cutoff = now - OR_CONNECT_FAILURE_LIFETIME; + + /* Opportunistically try to cleanup the failure cache. We do that at regular + * interval so it doesn't grow too big. */ + if (or_connect_failure_map_next_cleanup_ts <= now) { + or_connect_failure_map_cleanup(cutoff); + or_connect_failure_map_next_cleanup_ts = + now + OR_CONNECT_FAILURE_CLEANUP_INTERVAL; + } + + /* Look if we have failed previously to the same destination as this + * OR connection. */ + ocf = or_connect_failure_find(or_conn); + if (ocf) { + connect_failed_since_ts = ocf->last_failed_connect_ts; + } + /* If we do have an unable to connect timestamp and it is below cutoff, we + * can connect. Or we have never failed before so let it connect. */ + if (connect_failed_since_ts > cutoff) { + goto no_connect; + } + + /* Ok we can connect! */ + return 1; + no_connect: + return 0; +} + /** conn is in the 'connecting' state, and it failed to complete * a TCP connection. Send notifications appropriately. * @@ -1135,6 +1345,7 @@ connection_or_connect_failed(or_connection_t *conn, control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, reason); if (!authdir_mode_tests_reachability(get_options())) control_event_bootstrap_prob_or(msg, reason, conn); + note_or_connect_failed(conn); } /** conn got an error in connection_handle_read_impl() or @@ -1225,6 +1436,19 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, conn->chan = chan; chan->conn = conn; connection_or_init_conn_from_address(conn, &addr, port, id_digest, ed_id, 1); + + /* We have a proper OR connection setup, now check if we can connect to it + * that is we haven't had a failure earlier. This is to avoid to try to + * constantly connect to relays that we think are not reachable. */ + if (!should_connect_to_relay(conn)) { + log_info(LD_GENERAL, "Can't connect to identity %s at %s:%u because we " + "failed earlier. Refusing.", + hex_str(id_digest, DIGEST_LEN), fmt_addr(&TO_CONN(conn)->addr), + TO_CONN(conn)->port); + connection_free_(TO_CONN(conn)); + return NULL; + } + connection_or_change_state(conn, OR_CONN_STATE_CONNECTING); control_event_or_conn_status(conn, OR_CONN_EVENT_LAUNCHED, 0);

1 0

[tor/maint-0.3.3] test: Add unittest for the OR connection failure cache
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit ab16f1e2a10ee1b95118ec266ba454697e9af012 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Feb 14 13:40:42 2018 +0200 test: Add unittest for the OR connection failure cache Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/connection_or.c | 5 ++- src/or/connection_or.h | 5 +++ src/or/nodelist.c | 4 +- src/or/nodelist.h | 2 +- src/test/test_connection.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 5 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 976889ad0..267463312 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -28,6 +28,7 @@ * part of a subclass (channel_tls_t). */ #define TOR_CHANNEL_INTERNAL_ +#define CONNECTION_OR_PRIVATE #include "channel.h" #include "channeltls.h" #include "circuitbuild.h" @@ -1255,7 +1256,7 @@ or_connect_failure_find(const or_connection_t *or_conn) /* Note down in the connection failure cache that a failure occurred on the * given or_conn. */ -static void +STATIC void note_or_connect_failed(const or_connection_t *or_conn) { or_connect_failure_entry_t *ocf = NULL; @@ -1294,7 +1295,7 @@ or_connect_failure_map_cleanup(time_t cutoff) * * The or_conn MUST have gone through connection_or_check_canonicity() so the * base address is properly set to what we know or doesn't know. */ -static int +STATIC int should_connect_to_relay(const or_connection_t *or_conn) { time_t now, cutoff; diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 7c1dced63..158eb1fda 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -120,6 +120,11 @@ int connection_or_single_set_badness_(time_t now, int force); void connection_or_group_set_badness_(smartlist_t *group, int force); +#ifdef CONNECTION_OR_PRIVATE +STATIC int should_connect_to_relay(const or_connection_t *or_conn); +STATIC void note_or_connect_failed(const or_connection_t *or_conn); +#endif + #ifdef TOR_UNIT_TESTS extern int certs_cell_ed25519_disabled_for_testing; #endif diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 391b31d68..7303539c8 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -161,8 +161,8 @@ init_nodelist(void) } /** As node_get_by_id, but returns a non-const pointer */ -node_t * -node_get_mutable_by_id(const char *identity_digest) +MOCK_IMPL(node_t *, +node_get_mutable_by_id,(const char *identity_digest)) { node_t search, *node; if (PREDICT_UNLIKELY(the_nodelist == NULL)) diff --git a/src/or/nodelist.h b/src/or/nodelist.h index dc20eaf0a..043d7b341 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -16,7 +16,7 @@ tor_assert((n)->ri || (n)->rs); \ } STMT_END -node_t *node_get_mutable_by_id(const char *identity_digest); +MOCK_DECL(node_t *, node_get_mutable_by_id,(const char *identity_digest)); MOCK_DECL(const node_t *, node_get_by_id, (const char *identity_digest)); node_t *node_get_mutable_by_ed25519_id(const ed25519_public_key_t *ed_id); MOCK_DECL(const node_t *, node_get_by_ed25519_id, diff --git a/src/test/test_connection.c b/src/test/test_connection.c index 33f453b8b..dc0f6860d 100644 --- a/src/test/test_connection.c +++ b/src/test/test_connection.c @@ -5,6 +5,7 @@ #define CONNECTION_PRIVATE #define MAIN_PRIVATE +#define CONNECTION_OR_PRIVATE #include "or.h" #include "test.h" @@ -13,9 +14,11 @@ #include "hs_common.h" #include "main.h" #include "microdesc.h" +#include "nodelist.h" #include "networkstatus.h" #include "rendcache.h" #include "directory.h" +#include "connection_or.h" #include "test_connection.h" #include "test_helpers.h" @@ -776,6 +779,99 @@ test_conn_download_status(void *arg) /* the teardown function removes all the connections in the global list*/; } +static node_t test_node; + +static node_t * +mock_node_get_mutable_by_id(const char *digest) +{ + (void) digest; + static routerinfo_t node_ri; + memset(&node_ri, 0, sizeof(node_ri)); + + test_node.ri = &node_ri; + memset(test_node.identity, 'c', sizeof(test_node.identity)); + + tor_addr_t ipv4_addr; + tor_addr_parse(&ipv4_addr, "18.0.0.1"); + node_ri.addr = tor_addr_to_ipv4h(&ipv4_addr); + node_ri.or_port = 1; + + return &test_node; +} + +static const node_t * +mock_node_get_by_id(const char *digest) +{ + (void) digest; + memset(test_node.identity, 'c', sizeof(test_node.identity)); + return &test_node; +} + +/* Test whether we correctly track failed connections between relays. */ +static void +test_failed_orconn_tracker(void *arg) +{ + (void) arg; + + int can_connect; + time_t now = 1281533250; /* 2010-08-11 13:27:30 UTC */ + (void) now; + + update_approx_time(now); + + /* Prepare the OR connection that will be used in this test */ + or_connection_t or_conn; + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.real_addr, "18.0.0.1")); + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.base_.addr, "18.0.0.1")); + or_conn.base_.port = 1; + memset(or_conn.identity_digest, 'c', sizeof(or_conn.identity_digest)); + + /* Check whether we can connect with an empty failure cache: + * this should succeed */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Now add the destination to the failure cache */ + note_or_connect_failed(&or_conn); + + /* Check again: now it shouldn't connect */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 0); + + /* Move time forward and check again: the cache should have been cleared and + * now it should connect */ + now += 3600; + update_approx_time(now); + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Now mock the node_get_*by_id() functions to start using the node subsystem + * optimization. */ + MOCK(node_get_by_id, mock_node_get_by_id); + MOCK(node_get_mutable_by_id, mock_node_get_mutable_by_id); + + /* Since we just started using the node subsystem it will allow connections + * now */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Mark it as failed */ + note_or_connect_failed(&or_conn); + + /* Check that it shouldn't connect now */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 0); + + /* Move time forward and check again: now it should connect */ + now += 3600; + update_approx_time(now); + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + done: + ; +} + #define CONNECTION_TESTCASE(name, fork, setup) \ { #name, test_conn_##name, fork, &setup, NULL } @@ -792,6 +888,7 @@ struct testcase_t connection_tests[] = { CONNECTION_TESTCASE_ARG(download_status, TT_FORK, test_conn_download_status_st, FLAV_NS), //CONNECTION_TESTCASE(func_suffix, TT_FORK, setup_func_pair), + { "failed_orconn_tracker", test_failed_orconn_tracker, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/maint-0.3.3] Merge remote-tracking branch 'dgoulet/bug24767_033_03' into maint-0.3.3
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit e0bbef48bf414574a9e1342accfd2fe0d2a5343e Merge: 3cc382b93 ab16f1e2a Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 14:21:04 2018 -0400 Merge remote-tracking branch 'dgoulet/bug24767_033_03' into maint-0.3.3 changes/bug24767 | 5 + src/or/connection_or.c | 225 +++++++++++++++++++++++++++++++++++++++++++++ src/or/connection_or.h | 5 + src/or/nodelist.c | 4 +- src/or/nodelist.h | 2 +- src/test/test_connection.c | 97 +++++++++++++++++++ 6 files changed, 335 insertions(+), 3 deletions(-)

1 0

[tor/master] Merge branch 'maint-0.3.3'
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit 794a25f8c11971cf71f472820c2d3cd6a0f29b94 Merge: a3c1d78c2 e0bbef48b Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 14:23:06 2018 -0400 Merge branch 'maint-0.3.3' changes/bug24767 | 5 + src/or/connection_or.c | 225 +++++++++++++++++++++++++++++++++++++++++++++ src/or/connection_or.h | 5 + src/or/nodelist.c | 4 +- src/or/nodelist.h | 2 +- src/test/test_connection.c | 97 +++++++++++++++++++ 6 files changed, 335 insertions(+), 3 deletions(-)

1 0

[tor/master] Merge remote-tracking branch 'dgoulet/bug24767_033_03' into maint-0.3.3
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit e0bbef48bf414574a9e1342accfd2fe0d2a5343e Merge: 3cc382b93 ab16f1e2a Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 28 14:21:04 2018 -0400 Merge remote-tracking branch 'dgoulet/bug24767_033_03' into maint-0.3.3 changes/bug24767 | 5 + src/or/connection_or.c | 225 +++++++++++++++++++++++++++++++++++++++++++++ src/or/connection_or.h | 5 + src/or/nodelist.c | 4 +- src/or/nodelist.h | 2 +- src/test/test_connection.c | 97 +++++++++++++++++++ 6 files changed, 335 insertions(+), 3 deletions(-)

1 0

[tor/master] relay: Avoid connecting to down relays
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit f29d158330a8c6bcc91b71a888db741766135aaf Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Feb 6 15:40:17 2018 -0500 relay: Avoid connecting to down relays If we failed to connect at the TCP level to a relay, note it down and refuse to connect again for another 60 seconds. Fixes #24767 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug24767 | 5 ++ src/or/connection_or.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) diff --git a/changes/bug24767 b/changes/bug24767 new file mode 100644 index 000000000..56fbe51a9 --- /dev/null +++ b/changes/bug24767 @@ -0,0 +1,5 @@ + o Major bugfixes (relay, connection): + - Refuse to connect again to a relay from which we failed previously with + a connection refused, timeout or error (at the TCP level). The relay + won't be retried for 60 seconds after the failure occured. Fixes bug + 24767; bugfix on 0.0.6. diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 272a086a3..976889ad0 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1122,6 +1122,216 @@ connection_or_group_set_badness_(smartlist_t *group, int force) } SMARTLIST_FOREACH_END(or_conn); } +/* Lifetime of a connection failure. After that, we'll retry. This is in + * seconds. */ +#define OR_CONNECT_FAILURE_LIFETIME 60 +/* The interval to use with when to clean up the failure cache. */ +#define OR_CONNECT_FAILURE_CLEANUP_INTERVAL 60 + +/* When is the next time we have to cleanup the failure map. We keep this + * because we clean it opportunistically. */ +static time_t or_connect_failure_map_next_cleanup_ts = 0; + +/* OR connection failure entry data structure. It is kept in the connection + * failure map defined below and indexed by OR identity digest, address and + * port. + * + * We need to identify a connection failure with these three values because we + * want to avoid to wrongfully blacklist a relay if someone is trying to + * extend to a known identity digest but with the wrong IP/port. For instance, + * it can happen if a relay changed its port but the client still has an old + * descriptor with the old port. We want to stop connecting to that + * IP/port/identity all together, not only the relay identity. */ +typedef struct or_connect_failure_entry_t { + HT_ENTRY(or_connect_failure_entry_t) node; + /* Identity digest of the connection where it is connecting to. */ + uint8_t identity_digest[DIGEST_LEN]; + /* This is the connection address from the base connection_t. After the + * connection is checked for canonicity, the base address should represent + * what we know instead of where we are connecting to. This is what we need + * so we can correlate known relays within the consensus. */ + tor_addr_t addr; + uint16_t port; + /* Last time we were unable to connect. */ + time_t last_failed_connect_ts; +} or_connect_failure_entry_t; + +/* Map where we keep connection failure entries. They are indexed by addr, + * port and identity digest. */ +static HT_HEAD(or_connect_failure_ht, or_connect_failure_entry_t) + or_connect_failures_map = HT_INITIALIZER(); + +/* Helper: Hashtable equal function. Return 1 if equal else 0. */ +static int +or_connect_failure_ht_eq(const or_connect_failure_entry_t *a, + const or_connect_failure_entry_t *b) +{ + return fast_memeq(a->identity_digest, b->identity_digest, DIGEST_LEN) && + tor_addr_eq(&a->addr, &b->addr) && + a->port == b->port; +} + +/* Helper: Return the hash for the hashtable of the given entry. For this + * table, it is a combination of address, port and identity digest. */ +static unsigned int +or_connect_failure_ht_hash(const or_connect_failure_entry_t *entry) +{ + size_t offset = 0, addr_size; + const void *addr_ptr; + /* Largest size is IPv6 and IPv4 is smaller so it is fine. */ + uint8_t data[16 + sizeof(uint16_t) + DIGEST_LEN]; + + /* Get the right address bytes depending on the family. */ + switch (tor_addr_family(&entry->addr)) { + case AF_INET: + addr_size = 4; + addr_ptr = &entry->addr.addr.in_addr.s_addr; + break; + case AF_INET6: + addr_size = 16; + addr_ptr = &entry->addr.addr.in6_addr.s6_addr; + break; + default: + tor_assert_nonfatal_unreached(); + return 0; + } + + memcpy(data, addr_ptr, addr_size); + offset += addr_size; + memcpy(data + offset, entry->identity_digest, DIGEST_LEN); + offset += DIGEST_LEN; + set_uint16(data + offset, entry->port); + offset += sizeof(uint16_t); + + return (unsigned int) siphash24g(data, offset); +} + +HT_PROTOTYPE(or_connect_failure_ht, or_connect_failure_entry_t, node, + or_connect_failure_ht_hash, or_connect_failure_ht_eq) + +HT_GENERATE2(or_connect_failure_ht, or_connect_failure_entry_t, node, + or_connect_failure_ht_hash, or_connect_failure_ht_eq, + 0.6, tor_reallocarray_, tor_free_) + +/* Initialize a given connect failure entry with the given identity_digest, + * addr and port. All field are optional except ocf. */ +static void +or_connect_failure_init(const char *identity_digest, const tor_addr_t *addr, + uint16_t port, or_connect_failure_entry_t *ocf) +{ + tor_assert(ocf); + if (identity_digest) { + memcpy(ocf->identity_digest, identity_digest, + sizeof(ocf->identity_digest)); + } + if (addr) { + tor_addr_copy(&ocf->addr, addr); + } + ocf->port = port; +} + +/* Return a newly allocated connection failure entry. It is initialized with + * the given or_conn data. This can't fail. */ +static or_connect_failure_entry_t * +or_connect_failure_new(const or_connection_t *or_conn) +{ + or_connect_failure_entry_t *ocf = tor_malloc_zero(sizeof(*ocf)); + or_connect_failure_init(or_conn->identity_digest, &or_conn->real_addr, + TO_CONN(or_conn)->port, ocf); + return ocf; +} + +/* Return a connection failure entry matching the given or_conn. NULL is + * returned if not found. */ +static or_connect_failure_entry_t * +or_connect_failure_find(const or_connection_t *or_conn) +{ + or_connect_failure_entry_t lookup; + tor_assert(or_conn); + or_connect_failure_init(or_conn->identity_digest, &TO_CONN(or_conn)->addr, + TO_CONN(or_conn)->port, &lookup); + return HT_FIND(or_connect_failure_ht, &or_connect_failures_map, &lookup); +} + +/* Note down in the connection failure cache that a failure occurred on the + * given or_conn. */ +static void +note_or_connect_failed(const or_connection_t *or_conn) +{ + or_connect_failure_entry_t *ocf = NULL; + + tor_assert(or_conn); + + ocf = or_connect_failure_find(or_conn); + if (ocf == NULL) { + ocf = or_connect_failure_new(or_conn); + HT_INSERT(or_connect_failure_ht, &or_connect_failures_map, ocf); + } + ocf->last_failed_connect_ts = approx_time(); +} + +/* Cleanup the connection failure cache and remove all entries below the + * given cutoff. */ +static void +or_connect_failure_map_cleanup(time_t cutoff) +{ + or_connect_failure_entry_t **ptr, **next, *entry; + + for (ptr = HT_START(or_connect_failure_ht, &or_connect_failures_map); + ptr != NULL; ptr = next) { + entry = *ptr; + if (entry->last_failed_connect_ts <= cutoff) { + next = HT_NEXT_RMV(or_connect_failure_ht, &or_connect_failures_map, ptr); + tor_free(entry); + } else { + next = HT_NEXT(or_connect_failure_ht, &or_connect_failures_map, ptr); + } + } +} + +/* Return true iff the given OR connection can connect to its destination that + * is the triplet identity_digest, address and port. + * + * The or_conn MUST have gone through connection_or_check_canonicity() so the + * base address is properly set to what we know or doesn't know. */ +static int +should_connect_to_relay(const or_connection_t *or_conn) +{ + time_t now, cutoff; + time_t connect_failed_since_ts = 0; + or_connect_failure_entry_t *ocf; + + tor_assert(or_conn); + + now = approx_time(); + cutoff = now - OR_CONNECT_FAILURE_LIFETIME; + + /* Opportunistically try to cleanup the failure cache. We do that at regular + * interval so it doesn't grow too big. */ + if (or_connect_failure_map_next_cleanup_ts <= now) { + or_connect_failure_map_cleanup(cutoff); + or_connect_failure_map_next_cleanup_ts = + now + OR_CONNECT_FAILURE_CLEANUP_INTERVAL; + } + + /* Look if we have failed previously to the same destination as this + * OR connection. */ + ocf = or_connect_failure_find(or_conn); + if (ocf) { + connect_failed_since_ts = ocf->last_failed_connect_ts; + } + /* If we do have an unable to connect timestamp and it is below cutoff, we + * can connect. Or we have never failed before so let it connect. */ + if (connect_failed_since_ts > cutoff) { + goto no_connect; + } + + /* Ok we can connect! */ + return 1; + no_connect: + return 0; +} + /** conn is in the 'connecting' state, and it failed to complete * a TCP connection. Send notifications appropriately. * @@ -1135,6 +1345,7 @@ connection_or_connect_failed(or_connection_t *conn, control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, reason); if (!authdir_mode_tests_reachability(get_options())) control_event_bootstrap_prob_or(msg, reason, conn); + note_or_connect_failed(conn); } /** conn got an error in connection_handle_read_impl() or @@ -1225,6 +1436,19 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, conn->chan = chan; chan->conn = conn; connection_or_init_conn_from_address(conn, &addr, port, id_digest, ed_id, 1); + + /* We have a proper OR connection setup, now check if we can connect to it + * that is we haven't had a failure earlier. This is to avoid to try to + * constantly connect to relays that we think are not reachable. */ + if (!should_connect_to_relay(conn)) { + log_info(LD_GENERAL, "Can't connect to identity %s at %s:%u because we " + "failed earlier. Refusing.", + hex_str(id_digest, DIGEST_LEN), fmt_addr(&TO_CONN(conn)->addr), + TO_CONN(conn)->port); + connection_free_(TO_CONN(conn)); + return NULL; + } + connection_or_change_state(conn, OR_CONN_STATE_CONNECTING); control_event_or_conn_status(conn, OR_CONN_EVENT_LAUNCHED, 0);

1 0

[tor/master] test: Add unittest for the OR connection failure cache
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit ab16f1e2a10ee1b95118ec266ba454697e9af012 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Feb 14 13:40:42 2018 +0200 test: Add unittest for the OR connection failure cache Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/connection_or.c | 5 ++- src/or/connection_or.h | 5 +++ src/or/nodelist.c | 4 +- src/or/nodelist.h | 2 +- src/test/test_connection.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 5 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 976889ad0..267463312 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -28,6 +28,7 @@ * part of a subclass (channel_tls_t). */ #define TOR_CHANNEL_INTERNAL_ +#define CONNECTION_OR_PRIVATE #include "channel.h" #include "channeltls.h" #include "circuitbuild.h" @@ -1255,7 +1256,7 @@ or_connect_failure_find(const or_connection_t *or_conn) /* Note down in the connection failure cache that a failure occurred on the * given or_conn. */ -static void +STATIC void note_or_connect_failed(const or_connection_t *or_conn) { or_connect_failure_entry_t *ocf = NULL; @@ -1294,7 +1295,7 @@ or_connect_failure_map_cleanup(time_t cutoff) * * The or_conn MUST have gone through connection_or_check_canonicity() so the * base address is properly set to what we know or doesn't know. */ -static int +STATIC int should_connect_to_relay(const or_connection_t *or_conn) { time_t now, cutoff; diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 7c1dced63..158eb1fda 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -120,6 +120,11 @@ int connection_or_single_set_badness_(time_t now, int force); void connection_or_group_set_badness_(smartlist_t *group, int force); +#ifdef CONNECTION_OR_PRIVATE +STATIC int should_connect_to_relay(const or_connection_t *or_conn); +STATIC void note_or_connect_failed(const or_connection_t *or_conn); +#endif + #ifdef TOR_UNIT_TESTS extern int certs_cell_ed25519_disabled_for_testing; #endif diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 391b31d68..7303539c8 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -161,8 +161,8 @@ init_nodelist(void) } /** As node_get_by_id, but returns a non-const pointer */ -node_t * -node_get_mutable_by_id(const char *identity_digest) +MOCK_IMPL(node_t *, +node_get_mutable_by_id,(const char *identity_digest)) { node_t search, *node; if (PREDICT_UNLIKELY(the_nodelist == NULL)) diff --git a/src/or/nodelist.h b/src/or/nodelist.h index dc20eaf0a..043d7b341 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -16,7 +16,7 @@ tor_assert((n)->ri || (n)->rs); \ } STMT_END -node_t *node_get_mutable_by_id(const char *identity_digest); +MOCK_DECL(node_t *, node_get_mutable_by_id,(const char *identity_digest)); MOCK_DECL(const node_t *, node_get_by_id, (const char *identity_digest)); node_t *node_get_mutable_by_ed25519_id(const ed25519_public_key_t *ed_id); MOCK_DECL(const node_t *, node_get_by_ed25519_id, diff --git a/src/test/test_connection.c b/src/test/test_connection.c index 33f453b8b..dc0f6860d 100644 --- a/src/test/test_connection.c +++ b/src/test/test_connection.c @@ -5,6 +5,7 @@ #define CONNECTION_PRIVATE #define MAIN_PRIVATE +#define CONNECTION_OR_PRIVATE #include "or.h" #include "test.h" @@ -13,9 +14,11 @@ #include "hs_common.h" #include "main.h" #include "microdesc.h" +#include "nodelist.h" #include "networkstatus.h" #include "rendcache.h" #include "directory.h" +#include "connection_or.h" #include "test_connection.h" #include "test_helpers.h" @@ -776,6 +779,99 @@ test_conn_download_status(void *arg) /* the teardown function removes all the connections in the global list*/; } +static node_t test_node; + +static node_t * +mock_node_get_mutable_by_id(const char *digest) +{ + (void) digest; + static routerinfo_t node_ri; + memset(&node_ri, 0, sizeof(node_ri)); + + test_node.ri = &node_ri; + memset(test_node.identity, 'c', sizeof(test_node.identity)); + + tor_addr_t ipv4_addr; + tor_addr_parse(&ipv4_addr, "18.0.0.1"); + node_ri.addr = tor_addr_to_ipv4h(&ipv4_addr); + node_ri.or_port = 1; + + return &test_node; +} + +static const node_t * +mock_node_get_by_id(const char *digest) +{ + (void) digest; + memset(test_node.identity, 'c', sizeof(test_node.identity)); + return &test_node; +} + +/* Test whether we correctly track failed connections between relays. */ +static void +test_failed_orconn_tracker(void *arg) +{ + (void) arg; + + int can_connect; + time_t now = 1281533250; /* 2010-08-11 13:27:30 UTC */ + (void) now; + + update_approx_time(now); + + /* Prepare the OR connection that will be used in this test */ + or_connection_t or_conn; + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.real_addr, "18.0.0.1")); + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.base_.addr, "18.0.0.1")); + or_conn.base_.port = 1; + memset(or_conn.identity_digest, 'c', sizeof(or_conn.identity_digest)); + + /* Check whether we can connect with an empty failure cache: + * this should succeed */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Now add the destination to the failure cache */ + note_or_connect_failed(&or_conn); + + /* Check again: now it shouldn't connect */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 0); + + /* Move time forward and check again: the cache should have been cleared and + * now it should connect */ + now += 3600; + update_approx_time(now); + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Now mock the node_get_*by_id() functions to start using the node subsystem + * optimization. */ + MOCK(node_get_by_id, mock_node_get_by_id); + MOCK(node_get_mutable_by_id, mock_node_get_mutable_by_id); + + /* Since we just started using the node subsystem it will allow connections + * now */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Mark it as failed */ + note_or_connect_failed(&or_conn); + + /* Check that it shouldn't connect now */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 0); + + /* Move time forward and check again: now it should connect */ + now += 3600; + update_approx_time(now); + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + done: + ; +} + #define CONNECTION_TESTCASE(name, fork, setup) \ { #name, test_conn_##name, fork, &setup, NULL } @@ -792,6 +888,7 @@ struct testcase_t connection_tests[] = { CONNECTION_TESTCASE_ARG(download_status, TT_FORK, test_conn_download_status_st, FLAV_NS), //CONNECTION_TESTCASE(func_suffix, TT_FORK, setup_func_pair), + { "failed_orconn_tracker", test_failed_orconn_tracker, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/release-0.3.3] test: Add unittest for the OR connection failure cache
by nickm＠torproject.org 28 Mar '18

28 Mar '18

commit ab16f1e2a10ee1b95118ec266ba454697e9af012 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Feb 14 13:40:42 2018 +0200 test: Add unittest for the OR connection failure cache Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/connection_or.c | 5 ++- src/or/connection_or.h | 5 +++ src/or/nodelist.c | 4 +- src/or/nodelist.h | 2 +- src/test/test_connection.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 5 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 976889ad0..267463312 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -28,6 +28,7 @@ * part of a subclass (channel_tls_t). */ #define TOR_CHANNEL_INTERNAL_ +#define CONNECTION_OR_PRIVATE #include "channel.h" #include "channeltls.h" #include "circuitbuild.h" @@ -1255,7 +1256,7 @@ or_connect_failure_find(const or_connection_t *or_conn) /* Note down in the connection failure cache that a failure occurred on the * given or_conn. */ -static void +STATIC void note_or_connect_failed(const or_connection_t *or_conn) { or_connect_failure_entry_t *ocf = NULL; @@ -1294,7 +1295,7 @@ or_connect_failure_map_cleanup(time_t cutoff) * * The or_conn MUST have gone through connection_or_check_canonicity() so the * base address is properly set to what we know or doesn't know. */ -static int +STATIC int should_connect_to_relay(const or_connection_t *or_conn) { time_t now, cutoff; diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 7c1dced63..158eb1fda 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -120,6 +120,11 @@ int connection_or_single_set_badness_(time_t now, int force); void connection_or_group_set_badness_(smartlist_t *group, int force); +#ifdef CONNECTION_OR_PRIVATE +STATIC int should_connect_to_relay(const or_connection_t *or_conn); +STATIC void note_or_connect_failed(const or_connection_t *or_conn); +#endif + #ifdef TOR_UNIT_TESTS extern int certs_cell_ed25519_disabled_for_testing; #endif diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 391b31d68..7303539c8 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -161,8 +161,8 @@ init_nodelist(void) } /** As node_get_by_id, but returns a non-const pointer */ -node_t * -node_get_mutable_by_id(const char *identity_digest) +MOCK_IMPL(node_t *, +node_get_mutable_by_id,(const char *identity_digest)) { node_t search, *node; if (PREDICT_UNLIKELY(the_nodelist == NULL)) diff --git a/src/or/nodelist.h b/src/or/nodelist.h index dc20eaf0a..043d7b341 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -16,7 +16,7 @@ tor_assert((n)->ri || (n)->rs); \ } STMT_END -node_t *node_get_mutable_by_id(const char *identity_digest); +MOCK_DECL(node_t *, node_get_mutable_by_id,(const char *identity_digest)); MOCK_DECL(const node_t *, node_get_by_id, (const char *identity_digest)); node_t *node_get_mutable_by_ed25519_id(const ed25519_public_key_t *ed_id); MOCK_DECL(const node_t *, node_get_by_ed25519_id, diff --git a/src/test/test_connection.c b/src/test/test_connection.c index 33f453b8b..dc0f6860d 100644 --- a/src/test/test_connection.c +++ b/src/test/test_connection.c @@ -5,6 +5,7 @@ #define CONNECTION_PRIVATE #define MAIN_PRIVATE +#define CONNECTION_OR_PRIVATE #include "or.h" #include "test.h" @@ -13,9 +14,11 @@ #include "hs_common.h" #include "main.h" #include "microdesc.h" +#include "nodelist.h" #include "networkstatus.h" #include "rendcache.h" #include "directory.h" +#include "connection_or.h" #include "test_connection.h" #include "test_helpers.h" @@ -776,6 +779,99 @@ test_conn_download_status(void *arg) /* the teardown function removes all the connections in the global list*/; } +static node_t test_node; + +static node_t * +mock_node_get_mutable_by_id(const char *digest) +{ + (void) digest; + static routerinfo_t node_ri; + memset(&node_ri, 0, sizeof(node_ri)); + + test_node.ri = &node_ri; + memset(test_node.identity, 'c', sizeof(test_node.identity)); + + tor_addr_t ipv4_addr; + tor_addr_parse(&ipv4_addr, "18.0.0.1"); + node_ri.addr = tor_addr_to_ipv4h(&ipv4_addr); + node_ri.or_port = 1; + + return &test_node; +} + +static const node_t * +mock_node_get_by_id(const char *digest) +{ + (void) digest; + memset(test_node.identity, 'c', sizeof(test_node.identity)); + return &test_node; +} + +/* Test whether we correctly track failed connections between relays. */ +static void +test_failed_orconn_tracker(void *arg) +{ + (void) arg; + + int can_connect; + time_t now = 1281533250; /* 2010-08-11 13:27:30 UTC */ + (void) now; + + update_approx_time(now); + + /* Prepare the OR connection that will be used in this test */ + or_connection_t or_conn; + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.real_addr, "18.0.0.1")); + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.base_.addr, "18.0.0.1")); + or_conn.base_.port = 1; + memset(or_conn.identity_digest, 'c', sizeof(or_conn.identity_digest)); + + /* Check whether we can connect with an empty failure cache: + * this should succeed */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Now add the destination to the failure cache */ + note_or_connect_failed(&or_conn); + + /* Check again: now it shouldn't connect */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 0); + + /* Move time forward and check again: the cache should have been cleared and + * now it should connect */ + now += 3600; + update_approx_time(now); + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Now mock the node_get_*by_id() functions to start using the node subsystem + * optimization. */ + MOCK(node_get_by_id, mock_node_get_by_id); + MOCK(node_get_mutable_by_id, mock_node_get_mutable_by_id); + + /* Since we just started using the node subsystem it will allow connections + * now */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + /* Mark it as failed */ + note_or_connect_failed(&or_conn); + + /* Check that it shouldn't connect now */ + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 0); + + /* Move time forward and check again: now it should connect */ + now += 3600; + update_approx_time(now); + can_connect = should_connect_to_relay(&or_conn); + tt_int_op(can_connect, OP_EQ, 1); + + done: + ; +} + #define CONNECTION_TESTCASE(name, fork, setup) \ { #name, test_conn_##name, fork, &setup, NULL } @@ -792,6 +888,7 @@ struct testcase_t connection_tests[] = { CONNECTION_TESTCASE_ARG(download_status, TT_FORK, test_conn_download_status_st, FLAV_NS), //CONNECTION_TESTCASE(func_suffix, TT_FORK, setup_func_pair), + { "failed_orconn_tracker", test_failed_orconn_tracker, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0