commit c3e058dfac1cbc7cb0dee5cdb1bdc61c1dc0f4fa Author: teor <teor@torproject.org> Date: Wed Apr 29 15:56:40 2020 +1000 relay: Choose between IPv4 and IPv6 extends at random When an EXTEND2 cell has an IPv4 and an IPv6 address, choose one of them uniformly at random. Part of 33817. --- src/feature/relay/circuitbuild_relay.c | 66 +++++++++++++++++++++++++++++++--- src/feature/relay/router.c | 11 +++++- src/feature/relay/router.h | 1 + src/test/test_circuitbuild.c | 32 ++++++++++++++++- 4 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index d6ea22ca7..261fbc7e4 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -18,6 +18,8 @@ #include "orconfig.h" #include "feature/relay/circuitbuild_relay.h" +#include "lib/crypt_ops/crypto_rand.h" + #include "core/or/or.h" #include "app/config/config.h" @@ -36,6 +38,7 @@ #include "feature/nodelist/nodelist.h" +#include "feature/relay/router.h" #include "feature/relay/routermode.h" #include "feature/relay/selftest.h" @@ -237,9 +240,14 @@ circuit_extend_lspec_valid_helper(const struct extend_cell_t *ec, } /* When there is no open channel for an extend cell <b>ec</b>, set up the - * circuit <b>circ</b> to wait for a new connection. If <b>should_launch</b> - * is true, open a new connection. (Otherwise, we are already waiting for a - * new connection to the same relay.) + * circuit <b>circ</b> to wait for a new connection. + * + * If <b>should_launch</b> is true, open a new connection. (Otherwise, we are + * already waiting for a new connection to the same relay.) + * + * Check if IPv6 extends are supported by our current configuration. If they + * are, new connections may be made over IPv4 or IPv6. (IPv4 connections are + * always supported.) */ STATIC void circuit_open_connection_for_extend(const struct extend_cell_t *ec, @@ -258,13 +266,61 @@ circuit_open_connection_for_extend(const struct extend_cell_t *ec, return; } + /* Check the addresses, without logging */ + const int ipv4_valid = + (circuit_extend_addr_port_helper(&ec->orport_ipv4, false, false, 0) == 0); + const int ipv6_valid = + (circuit_extend_addr_port_helper(&ec->orport_ipv6, false, false, 0) == 0); + + IF_BUG_ONCE(!ipv4_valid && !ipv6_valid) { + /* circuit_extend_lspec_valid_helper() should have caught this */ + circuit_mark_for_close(circ, END_CIRC_REASON_CONNECTFAILED); + return; + } + + /* If we could make an IPv4 or an IPv6 connection, make an IPv6 connection + * at random, with probability 1 in N. + * 1 means "always IPv6 (and no IPv4)" + * 2 means "equal probability of IPv4 or IPv6" + * ... (and so on) ... + * (UINT_MAX - 1) means "almost always IPv4 (and almost never IPv6)" + * To disable IPv6, set ipv6_supported to 0. + */ +#define IPV6_CONNECTION_ONE_IN_N 2 + + const bool ipv6_supported = router_has_advertised_ipv6_orport(get_options()); + const tor_addr_port_t *chosen_ap = NULL; + + /* IPv4 is always supported */ + if (ipv4_valid && ipv6_valid && ipv6_supported) { + /* Choose between IPv4 and IPv6 at random */ + bool choose_ipv6 = crypto_fast_rng_one_in_n(get_thread_fast_rng(), + IPV6_CONNECTION_ONE_IN_N); + if (choose_ipv6) { + chosen_ap = &ec->orport_ipv6; + } else { + chosen_ap = &ec->orport_ipv4; + } + } else if (ipv6_valid && ipv6_supported) { + /* There's only one valid address: try to use it */ + chosen_ap = &ec->orport_ipv6; + } else if (ipv4_valid) { + chosen_ap = &ec->orport_ipv4; + } else { + /* An IPv6-only extend, but IPv6 is not supported */ + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Received IPv6-only extend, but we don't have an IPv6 ORPort."); + circuit_mark_for_close(circ, END_CIRC_REASON_CONNECTFAILED); + return; + } + circ->n_hop = extend_info_new(NULL /*nickname*/, (const char*)ec->node_id, &ec->ed_pubkey, NULL, /*onion_key*/ NULL, /*curve25519_key*/ - &ec->orport_ipv4.addr, - ec->orport_ipv4.port); + &chosen_ap->addr, + chosen_ap->port); circ->n_chan_create_cell = tor_memdup(&ec->create_cell, sizeof(ec->create_cell)); diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index a4a9c6a81..89e232ccc 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -1469,7 +1469,7 @@ router_get_advertised_ipv6_or_ap(const or_options_t *options, AF_INET6); if (!addr || port == 0) { - log_info(LD_CONFIG, "There is no advertised IPv6 ORPort."); + log_debug(LD_CONFIG, "There is no advertised IPv6 ORPort."); return; } @@ -1490,6 +1490,15 @@ router_get_advertised_ipv6_or_ap(const or_options_t *options, ipv6_ap_out->port = port; } +/** Returns true if this router has an advertised IPv6 ORPort. */ +bool +router_has_advertised_ipv6_orport(const or_options_t *options) +{ + tor_addr_port_t ipv6_ap; + router_get_advertised_ipv6_or_ap(options, &ipv6_ap); + return tor_addr_port_is_valid_ap(&ipv6_ap, 0); +} + /** Return the port that we should advertise as our DirPort; * this is one of three possibilities: * The one that is passed as <b>dirport</b> if the DirPort option is 0, or diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index d1b4ce5f8..c3a93cc0a 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -68,6 +68,7 @@ uint16_t router_get_active_listener_port_by_type_af(int listener_type, uint16_t router_get_advertised_or_port(const or_options_t *options); void router_get_advertised_ipv6_or_ap(const or_options_t *options, tor_addr_port_t *ipv6_ap_out); +bool router_has_advertised_ipv6_orport(const or_options_t *options); uint16_t router_get_advertised_or_port_by_af(const or_options_t *options, sa_family_t family); uint16_t router_get_advertised_dir_port(const or_options_t *options, diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 5d09ba557..a26109ed8 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -857,6 +857,10 @@ test_circuit_open_connection_for_extend(void *arg) circuit_t *circ = tor_malloc_zero(sizeof(circuit_t)); channel_t *fake_n_chan = tor_malloc_zero(sizeof(channel_t)); + or_options_t *fake_options = options_new(); + MOCK(get_options, mock_get_options); + mocked_options = fake_options; + MOCK(circuit_mark_for_close_, mock_circuit_mark_for_close_); mock_circuit_close_calls = 0; MOCK(channel_connect_for_circuit, mock_channel_connect_for_circuit); @@ -906,8 +910,30 @@ test_circuit_open_connection_for_extend(void *arg) tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 2); tor_end_capture_bugs_(); mock_clean_saved_logs(); + + /* Fail, because neither address is valid */ + mock_circuit_close_calls = 0; + mock_channel_connect_calls = 0; + tor_capture_bugs_(1); + circuit_open_connection_for_extend(ec, circ, 0); + /* Close the circuit, don't connect */ + tt_int_op(mock_circuit_close_calls, OP_EQ, 1); + tt_int_op(mock_channel_connect_calls, OP_EQ, 0); + /* Check state */ + tt_ptr_op(circ->n_hop, OP_EQ, NULL); + tt_ptr_op(circ->n_chan_create_cell, OP_EQ, NULL); + tt_int_op(circ->state, OP_EQ, 0); + /* Cleanup */ + tor_end_capture_bugs_(); + mock_clean_saved_logs(); #endif /* !defined(ALL_BUGS_ARE_FATAL) */ + /* Set up valid addresses */ + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, PUBLIC_IPV6); + ec->orport_ipv6.port = VALID_PORT; + /* Succeed, but don't try to open a connection */ mock_circuit_close_calls = 0; mock_channel_connect_calls = 0; @@ -948,7 +974,7 @@ test_circuit_open_connection_for_extend(void *arg) mock_circuit_close_calls = 0; mock_channel_connect_calls = 0; circuit_open_connection_for_extend(ec, circ, 1); - /* Try to connect, and succeed, leaving the circuit open */ + /* Connection attempt succeeded, leaving the circuit open */ tt_int_op(mock_circuit_close_calls, OP_EQ, 0); tt_int_op(mock_channel_connect_calls, OP_EQ, 1); /* Check state */ @@ -971,6 +997,10 @@ test_circuit_open_connection_for_extend(void *arg) UNMOCK(channel_connect_for_circuit); mock_channel_connect_calls = 0; + UNMOCK(get_options); + or_options_free(fake_options); + mocked_options = NULL; + tor_free(ec); tor_free(circ->n_hop); tor_free(circ->n_chan_create_cell);