- tor-commits - lists.torproject.org

[tor/master] test/circuitbuild: Disable some tests when ALL_BUGS_ARE_FATAL
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 063505446f5b85dd11ee8c9b2089f58fd3bf7427 Author: teor <teor(a)torproject.org> Date: Wed Apr 15 19:07:58 2020 +1000 test/circuitbuild: Disable some tests when ALL_BUGS_ARE_FATAL Some tests use IF_BUG_ONCE(), which is fatal when ALL_BUGS_ARE_FATAL, after the fixes in 33917. Also run "make autostyle" on these changes. Part of 33817. --- src/test/test_circuitbuild.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 668d9869d..5d09ba557 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -219,6 +219,7 @@ test_circuit_extend_state_valid(void *arg) expect_log_msg("Got an extend cell, but running as a client. Closing.\n"); mock_clean_saved_logs(); +#ifndef ALL_BUGS_ARE_FATAL /* Circuit must be non-NULL */ tor_capture_bugs_(1); server = 1; @@ -228,6 +229,7 @@ test_circuit_extend_state_valid(void *arg) "!(ASSERT_PREDICT_UNLIKELY_(!circ))"); tor_end_capture_bugs_(); mock_clean_saved_logs(); +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* n_chan and n_hop are NULL, this should succeed */ server = 1; @@ -314,6 +316,7 @@ test_circuit_extend_add_ed25519(void *arg) setup_full_capture_of_logs(LOG_INFO); +#ifndef ALL_BUGS_ARE_FATAL /* The extend cell must be non-NULL */ tor_capture_bugs_(1); tt_int_op(circuit_extend_add_ed25519_helper(NULL), OP_EQ, -1); @@ -322,6 +325,7 @@ test_circuit_extend_add_ed25519(void *arg) "!(ASSERT_PREDICT_UNLIKELY_(!ec))"); tor_end_capture_bugs_(); mock_clean_saved_logs(); +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* The node id must be non-zero */ memcpy(old_ec, ec, sizeof(extend_cell_t)); @@ -495,6 +499,7 @@ test_circuit_extend_lspec_valid(void *arg) setup_full_capture_of_logs(LOG_INFO); +#ifndef ALL_BUGS_ARE_FATAL /* Extend cell must be non-NULL */ tor_capture_bugs_(1); tt_int_op(circuit_extend_lspec_valid_helper(NULL, circ), OP_EQ, -1); @@ -521,6 +526,7 @@ test_circuit_extend_lspec_valid(void *arg) tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 2); tor_end_capture_bugs_(); mock_clean_saved_logs(); +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* IPv4 and IPv6 addr and port are all zero, this should fail */ tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); @@ -619,6 +625,7 @@ test_circuit_extend_lspec_valid(void *arg) tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); +#ifndef ALL_BUGS_ARE_FATAL /* If we pass the private address check, but don't have the right * OR circuit magic number, we trigger another bug */ tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); @@ -650,10 +657,12 @@ test_circuit_extend_lspec_valid(void *arg) tor_end_capture_bugs_(); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0; +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* Now set the right magic */ or_circ->base_.magic = OR_CIRCUIT_MAGIC; +#ifndef ALL_BUGS_ARE_FATAL /* If we pass the OR circuit magic check, but don't have p_chan, * we trigger another bug */ fake_options->ExtendAllowPrivateAddresses = 1; @@ -680,6 +689,7 @@ test_circuit_extend_lspec_valid(void *arg) tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); ec->orport_ipv4.port = 0x0000; +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* Now let's fake a p_chan and the addresses */ tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); @@ -855,6 +865,7 @@ test_circuit_open_connection_for_extend(void *arg) setup_full_capture_of_logs(LOG_INFO); +#ifndef ALL_BUGS_ARE_FATAL /* Circuit must be non-NULL */ mock_circuit_close_calls = 0; mock_channel_connect_calls = 0; @@ -895,6 +906,7 @@ 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(); +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* Succeed, but don't try to open a connection */ mock_circuit_close_calls = 0; @@ -1067,6 +1079,7 @@ test_circuit_extend(void *arg) setup_full_capture_of_logs(LOG_INFO); +#ifndef ALL_BUGS_ARE_FATAL /* Circuit must be non-NULL */ tor_capture_bugs_(1); tt_int_op(circuit_extend(cell, NULL), OP_EQ, -1); @@ -1093,6 +1106,7 @@ test_circuit_extend(void *arg) tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 2); tor_end_capture_bugs_(); mock_clean_saved_logs(); +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* Clients can't extend */ server = 0; @@ -1138,6 +1152,7 @@ test_circuit_extend(void *arg) PUBLIC_IPV4); mock_extend_cell_parse_cell_out.orport_ipv4.port = VALID_PORT; +#ifndef ALL_BUGS_ARE_FATAL tor_capture_bugs_(1); tt_int_op(circuit_extend(cell, circ), OP_EQ, -1); tt_int_op(mock_extend_cell_parse_calls, OP_EQ, 1); @@ -1147,6 +1162,7 @@ test_circuit_extend(void *arg) tor_end_capture_bugs_(); mock_clean_saved_logs(); mock_extend_cell_parse_calls = 0; +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* Now add the right magic and a p_chan. */ or_circ->base_.magic = OR_CIRCUIT_MAGIC; @@ -1292,6 +1308,7 @@ test_onionskin_answer(void *arg) setup_full_capture_of_logs(LOG_INFO); +#ifndef ALL_BUGS_ARE_FATAL /* Circuit must be non-NULL */ tor_capture_bugs_(1); tt_int_op(onionskin_answer(NULL, created_cell, @@ -1335,6 +1352,7 @@ test_onionskin_answer(void *arg) "!(ASSERT_PREDICT_UNLIKELY_(!rend_circ_nonce))"); tor_end_capture_bugs_(); mock_clean_saved_logs(); +#endif /* !defined(ALL_BUGS_ARE_FATAL) */ /* Also, the keys length must be CPATH_KEY_MATERIAL_LEN, but we can't catch * asserts in unit tests. */

1 0

[tor/master] relay/circuitbuild: Re-use IPv6 connections for circuits
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 16f3f6a1afe5dcd75536039029f51392d05ce153 Author: teor <teor(a)torproject.org> Date: Wed Apr 15 13:04:33 2020 +1000 relay/circuitbuild: Re-use IPv6 connections for circuits Search for existing connections using the remote IPv4 and IPv6 addresses. Part of 33817. --- src/core/or/channel.c | 57 ++++++++++++++++++++++------------ src/core/or/channel.h | 3 +- src/core/or/circuitbuild.c | 16 +++++++--- src/feature/relay/circuitbuild_relay.c | 13 +++++++- src/lib/net/address.h | 4 +++ src/test/test_channel.c | 37 +++++++++++++++------- src/test/test_circuitbuild.c | 6 ++-- 7 files changed, 96 insertions(+), 40 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 89804826a..93245ce81 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -85,8 +85,10 @@ /* Static function prototypes */ -static int channel_matches_target_addr_for_extend(channel_t *chan, - const tor_addr_t *target); +static int channel_matches_target_addr_for_extend( + channel_t *chan, + const tor_addr_t *target_ipv4_addr, + const tor_addr_t *target_ipv6_addr); /* Global lists of channels */ @@ -2365,9 +2367,9 @@ channel_is_better(channel_t *a, channel_t *b) * Get a channel to extend a circuit. * * Given the desired relay identity, pick a suitable channel to extend a - * circuit to the target address requsted by the client. Search for an - * existing channel for the requested endpoint. Make sure the channel is - * usable for new circuits, and matches the target address. + * circuit to the target IPv4 or IPv6 address requsted by the client. Search + * for an existing channel for the requested endpoint. Make sure the channel + * is usable for new circuits, and matches one of the target addresses. * * Try to return the best channel. But if there is no good channel, set * *msg_out to a message describing the channel's state and our next action, @@ -2377,7 +2379,8 @@ channel_is_better(channel_t *a, channel_t *b) MOCK_IMPL(channel_t *, channel_get_for_extend,(const char *rsa_id_digest, const ed25519_public_key_t *ed_id, - const tor_addr_t *target_addr, + const tor_addr_t *target_ipv4_addr, + const tor_addr_t *target_ipv6_addr, const char **msg_out, int *launch_out)) { @@ -2409,11 +2412,15 @@ channel_get_for_extend,(const char *rsa_id_digest, continue; } + const int matches_target = + channel_matches_target_addr_for_extend(chan, + target_ipv4_addr, + target_ipv6_addr); /* Never return a non-open connection. */ if (!CHANNEL_IS_OPEN(chan)) { /* If the address matches, don't launch a new connection for this * circuit. */ - if (channel_matches_target_addr_for_extend(chan, target_addr)) + if (matches_target) ++n_inprogress_goodaddr; continue; } @@ -2424,22 +2431,21 @@ channel_get_for_extend,(const char *rsa_id_digest, continue; } - /* Never return a non-canonical connection using a recent link protocol - * if the address is not what we wanted. + /* If the connection is using a recent link protocol, only return canonical + * connections, when the address is one of the addresses we wanted. * * The channel_is_canonical_is_reliable() function asks the lower layer - * if we should trust channel_is_canonical(). The below is from the - * comments of the old circuit_or_get_for_extend() and applies when + * if we should trust channel_is_canonical(). It only applies when * the lower-layer transport is channel_tls_t. * - * (For old link protocols, we can't rely on is_canonical getting + * For old link protocols, we can't rely on is_canonical getting * set properly if we're talking to the right address, since we might * have an out-of-date descriptor, and we will get no NETINFO cell to - * tell us about the right address.) + * tell us about the right address. */ if (!channel_is_canonical(chan) && channel_is_canonical_is_reliable(chan) && - !channel_matches_target_addr_for_extend(chan, target_addr)) { + !matches_target) { ++n_noncanonical; continue; } @@ -3302,20 +3308,33 @@ channel_matches_extend_info(channel_t *chan, extend_info_t *extend_info) } /** - * Check if a channel matches a given target address; return true iff we do. + * Check if a channel matches the given target IPv4 or IPv6 addresses. + * If either address matches, return true. If neither address matches, + * return false. + * + * Both addresses can't be NULL. * * This function calls into the lower layer and asks if this channel thinks - * it matches a given target address for circuit extension purposes. + * it matches the target addresses for circuit extension purposes. */ int channel_matches_target_addr_for_extend(channel_t *chan, - const tor_addr_t *target) + const tor_addr_t *target_ipv4_addr, + const tor_addr_t *target_ipv6_addr) { tor_assert(chan); tor_assert(chan->matches_target); - tor_assert(target); - return chan->matches_target(chan, target); + IF_BUG_ONCE(!target_ipv4_addr && !target_ipv6_addr) + return 0; + + if (target_ipv4_addr && chan->matches_target(chan, target_ipv4_addr)) + return 1; + + if (target_ipv6_addr && chan->matches_target(chan, target_ipv6_addr)) + return 1; + + return 0; } /** diff --git a/src/core/or/channel.h b/src/core/or/channel.h index f86e77992..4968c8714 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -661,7 +661,8 @@ channel_t * channel_connect(const tor_addr_t *addr, uint16_t port, MOCK_DECL(channel_t *, channel_get_for_extend,( const char *rsa_id_digest, const struct ed25519_public_key_t *ed_id, - const tor_addr_t *target_addr, + const tor_addr_t *target_ipv4_addr, + const tor_addr_t *target_ipv6_addr, const char **msg_out, int *launch_out)); diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index ce0f9618f..0381a4dc3 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -559,11 +559,17 @@ circuit_handle_first_hop(origin_circuit_t *circ) fmt_addrport(&firsthop->extend_info->addr, firsthop->extend_info->port)); - n_chan = channel_get_for_extend(firsthop->extend_info->identity_digest, - &firsthop->extend_info->ed_identity, - &firsthop->extend_info->addr, - &msg, - &should_launch); + /* We'll cleanup this code in #33220, when we add an IPv6 address to + * extend_info_t. */ + const bool addr_is_ipv4 = + (tor_addr_family(&firsthop->extend_info->addr) == AF_INET); + n_chan = channel_get_for_extend( + firsthop->extend_info->identity_digest, + &firsthop->extend_info->ed_identity, + addr_is_ipv4 ? &firsthop->extend_info->addr : NULL, + addr_is_ipv4 ? NULL : &firsthop->extend_info->addr, + &msg, + &should_launch); if (!n_chan) { /* not currently connected in a useful way. */ diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 7d3d58977..a926a1d81 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -330,9 +330,20 @@ circuit_extend(struct cell_t *cell, struct circuit_t *circ) if (circuit_extend_lspec_valid_helper(&ec, circ) < 0) return -1; + /* 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 */ + return -1; + } + n_chan = channel_get_for_extend((const char*)ec.node_id, &ec.ed_pubkey, - &ec.orport_ipv4.addr, + ipv4_valid ? &ec.orport_ipv4.addr : NULL, + ipv6_valid ? &ec.orport_ipv6.addr : NULL, &msg, &should_launch); diff --git a/src/lib/net/address.h b/src/lib/net/address.h index 611d1ca1e..651d6bc71 100644 --- a/src/lib/net/address.h +++ b/src/lib/net/address.h @@ -104,6 +104,10 @@ int tor_addr_from_sockaddr(tor_addr_t *a, const struct sockaddr *sa, uint16_t *port_out); void tor_addr_make_unspec(tor_addr_t *a); void tor_addr_make_null(tor_addr_t *a, sa_family_t family); +#define tor_addr_port_make_null(addr, port, family) \ + (void)(tor_addr_make_null(addr, family), (port) = 0) +#define tor_addr_port_make_null_ap(ap, family) \ + tor_addr_port_make_null(&(ap)->addr, (ap)->port, family) char *tor_sockaddr_to_str(const struct sockaddr *sa); /** Return an in6_addr* equivalent to a, or NULL if a is not diff --git a/src/test/test_channel.c b/src/test/test_channel.c index f7efbd7ab..849cc497f 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -1326,7 +1326,7 @@ test_channel_for_extend(void *arg) channel_t *ret_chan = NULL; char digest[DIGEST_LEN]; ed25519_public_key_t ed_id; - tor_addr_t addr; + tor_addr_t ipv4_addr, ipv6_addr; const char *msg; int launch; time_t now = time(NULL); @@ -1336,6 +1336,9 @@ test_channel_for_extend(void *arg) memset(digest, 'A', sizeof(digest)); memset(&ed_id, 'B', sizeof(ed_id)); + tor_addr_make_null(&ipv4_addr, AF_INET); + tor_addr_make_null(&ipv6_addr, AF_INET6); + chan1 = new_fake_channel(); tt_assert(chan1); /* Need to be registered to get added to the id map. */ @@ -1366,7 +1369,8 @@ test_channel_for_extend(void *arg) tt_ptr_op(channel_find_by_remote_identity(digest, &ed_id), OP_EQ, chan1); /* The expected result is chan2 because it is older than chan1. */ - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(ret_chan); tt_ptr_op(ret_chan, OP_EQ, chan2); tt_int_op(launch, OP_EQ, 0); @@ -1374,7 +1378,8 @@ test_channel_for_extend(void *arg) /* Switch that around from previous test. */ chan2->timestamp_created = chan1->timestamp_created + 1; - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(ret_chan); tt_ptr_op(ret_chan, OP_EQ, chan1); tt_int_op(launch, OP_EQ, 0); @@ -1383,7 +1388,8 @@ test_channel_for_extend(void *arg) /* Same creation time, num circuits will be used and they both have 0 so the * channel 2 should be picked due to how channel_is_better() works. */ chan2->timestamp_created = chan1->timestamp_created; - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(ret_chan); tt_ptr_op(ret_chan, OP_EQ, chan1); tt_int_op(launch, OP_EQ, 0); @@ -1394,7 +1400,8 @@ test_channel_for_extend(void *arg) /* Condemned the older channel. */ chan1->state = CHANNEL_STATE_CLOSING; - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(ret_chan); tt_ptr_op(ret_chan, OP_EQ, chan2); tt_int_op(launch, OP_EQ, 0); @@ -1403,7 +1410,8 @@ test_channel_for_extend(void *arg) /* Make the older channel a client one. */ channel_mark_client(chan1); - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(ret_chan); tt_ptr_op(ret_chan, OP_EQ, chan2); tt_int_op(launch, OP_EQ, 0); @@ -1413,8 +1421,9 @@ test_channel_for_extend(void *arg) /* Non matching ed identity with valid digest. */ ed25519_public_key_t dumb_ed_id; memset(&dumb_ed_id, 0, sizeof(dumb_ed_id)); - ret_chan = channel_get_for_extend(digest, &dumb_ed_id, &addr, &msg, - &launch); + ret_chan = channel_get_for_extend(digest, &dumb_ed_id, + &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Not connected. Connecting."); tt_int_op(launch, OP_EQ, 1); @@ -1423,7 +1432,8 @@ test_channel_for_extend(void *arg) test_chan_should_match_target = 1; chan1->state = CHANNEL_STATE_OPENING; chan2->state = CHANNEL_STATE_OPENING; - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Connection in progress; waiting."); tt_int_op(launch, OP_EQ, 0); @@ -1432,7 +1442,8 @@ test_channel_for_extend(void *arg) /* Mark channel 1 as bad for circuits. */ channel_mark_bad_for_new_circs(chan1); - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(ret_chan); tt_ptr_op(ret_chan, OP_EQ, chan2); tt_int_op(launch, OP_EQ, 0); @@ -1442,7 +1453,8 @@ test_channel_for_extend(void *arg) /* Mark both channels as unusable. */ channel_mark_bad_for_new_circs(chan1); channel_mark_bad_for_new_circs(chan2); - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. " " Launching a new one."); @@ -1453,7 +1465,8 @@ test_channel_for_extend(void *arg) /* Non canonical channels. */ test_chan_should_match_target = 0; test_chan_canonical_should_be_reliable = 1; - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. " " Launching a new one."); diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index a8d6323e4..668d9869d 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -993,13 +993,15 @@ static channel_t *mock_channel_get_for_extend_nchan = NULL; static channel_t * mock_channel_get_for_extend(const char *rsa_id_digest, const ed25519_public_key_t *ed_id, - const tor_addr_t *target_addr, + const tor_addr_t *target_ipv4_addr, + const tor_addr_t *target_ipv6_addr, const char **msg_out, int *launch_out) { (void)rsa_id_digest; (void)ed_id; - (void)target_addr; + (void)target_ipv4_addr; + (void)target_ipv6_addr; /* channel_get_for_extend() requires non-NULL arguments */ tt_ptr_op(msg_out, OP_NE, NULL);

1 0

[tor/master] log/util_bug: Make IF_BUG_ONCE() support ALL_BUGS_ARE_FATAL
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 6c458d2d6eaed20e48b1e66bebd1ec0466838e33 Author: teor <teor(a)torproject.org> Date: Wed Apr 15 17:45:29 2020 +1000 log/util_bug: Make IF_BUG_ONCE() support ALL_BUGS_ARE_FATAL ... and DISABLE_ASSERTS_IN_UNIT_TESTS. Make all of tor's assertion macros support the ALL_BUGS_ARE_FATAL and DISABLE_ASSERTS_IN_UNIT_TESTS debugging modes. Implements these modes for IF_BUG_ONCE(). (It used to log a non-fatal warning, regardless of the debugging mode.) Fixes bug 33917; bugfix on 0.2.9.1-alpha. --- changes/bug33917 | 5 +++++ src/lib/log/util_bug.h | 23 ++++++++++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/changes/bug33917 b/changes/bug33917 new file mode 100644 index 000000000..6a8daa9e2 --- /dev/null +++ b/changes/bug33917 @@ -0,0 +1,5 @@ + o Minor bugfixes (logging, testing): + - Make all of tor's assertion macros support the ALL_BUGS_ARE_FATAL and + DISABLE_ASSERTS_IN_UNIT_TESTS debugging modes. Implements these modes + for IF_BUG_ONCE(). (It used to log a non-fatal warning, regardless of + the debugging mode.) Fixes bug 33917; bugfix on 0.2.9.1-alpha. diff --git a/src/lib/log/util_bug.h b/src/lib/log/util_bug.h index ae3d125a0..6b27b36f0 100644 --- a/src/lib/log/util_bug.h +++ b/src/lib/log/util_bug.h @@ -142,6 +142,8 @@ #define ALL_BUGS_ARE_FATAL #endif +/** Define ALL_BUGS_ARE_FATAL if you want Tor to crash when any problem comes + * up, so you can get a coredump and track things down. */ #ifdef ALL_BUGS_ARE_FATAL #define tor_assert_nonfatal_unreached() tor_assert(0) #define tor_assert_nonfatal(cond) tor_assert((cond)) @@ -154,6 +156,9 @@ (tor_assertion_failed_(SHORT_FILE__,__LINE__,__func__,"!("#cond")",NULL), \ tor_abort_(), 1) \ : 0) +#ifndef COCCI +#define IF_BUG_ONCE(cond) if (BUG(cond)) +#endif #elif defined(TOR_UNIT_TESTS) && defined(DISABLE_ASSERTS_IN_UNIT_TESTS) #define tor_assert_nonfatal_unreached() STMT_NIL #define tor_assert_nonfatal(cond) ((void)(cond)) @@ -164,6 +169,9 @@ #define tor_assert_nonfatal_unreached_once() STMT_NIL #define tor_assert_nonfatal_once(cond) ((void)(cond)) #define BUG(cond) (ASSERT_PREDICT_UNLIKELY_(cond) ? 1 : 0) +#ifndef COCCI +#define IF_BUG_ONCE(cond) if (BUG(cond)) +#endif #else /* Normal case, !ALL_BUGS_ARE_FATAL, !DISABLE_ASSERTS_IN_UNIT_TESTS */ #define tor_assert_nonfatal_unreached() STMT_BEGIN \ tor_bug_occurred_(SHORT_FILE__, __LINE__, __func__, NULL, 0, NULL); \ @@ -200,7 +208,6 @@ (ASSERT_PREDICT_UNLIKELY_(cond) ? \ (tor_bug_occurred_(SHORT_FILE__,__LINE__,__func__,"!("#cond")",0,NULL),1) \ : 0) -#endif /* defined(ALL_BUGS_ARE_FATAL) || ... */ #ifndef COCCI #ifdef __GNUC__ @@ -232,7 +239,7 @@ #define IF_BUG_ONCE_VARNAME__(a) \ IF_BUG_ONCE_VARNAME_(a) -/** This macro behaves as 'if (bug(x))', except that it only logs its +/** This macro behaves as 'if (BUG(x))', except that it only logs its * warning once, no matter how many times it triggers. */ @@ -240,9 +247,15 @@ IF_BUG_ONCE__(ASSERT_PREDICT_UNLIKELY_(cond), \ IF_BUG_ONCE_VARNAME__(__LINE__)) -/** Define this if you want Tor to crash when any problem comes up, - * so you can get a coredump and track things down. */ -// #define tor_fragile_assert() tor_assert_unreached(0) +#endif /* defined(ALL_BUGS_ARE_FATAL) || ... */ + +/** In older code, we used tor_fragile_assert() to mark optional failure + * points. At these points, we could make some debug builds fail. + * (But release builds would continue.) + * + * To get the same behaviour in recent tor versions, define + * ALL_BUGS_ARE_FATAL, and use any non-fatal assertion or *BUG() macro. + */ #define tor_fragile_assert() tor_assert_nonfatal_unreached_once() void tor_assertion_failed_(const char *fname, unsigned int line,

1 0

[tor/master] channel: Make channel_matches_target_addr_for_extend() static
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit f8f688b3097d4aeaabc3f91d644c28540c8c9548 Author: teor <teor(a)torproject.org> Date: Wed Apr 15 12:08:46 2020 +1000 channel: Make channel_matches_target_addr_for_extend() static It isn't used outside channel.c. Part of 33817. --- src/core/or/channel.c | 5 +++++ src/core/or/channel.h | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 75054aa0c..89804826a 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -83,6 +83,11 @@ #include "core/or/cell_queue_st.h" +/* Static function prototypes */ + +static int channel_matches_target_addr_for_extend(channel_t *chan, + const tor_addr_t *target); + /* Global lists of channels */ /* All channel_t instances */ diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 49331d5d5..f86e77992 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -737,8 +737,6 @@ int channel_is_outgoing(channel_t *chan); void channel_mark_client(channel_t *chan); void channel_clear_client(channel_t *chan); int channel_matches_extend_info(channel_t *chan, extend_info_t *extend_info); -int channel_matches_target_addr_for_extend(channel_t *chan, - const tor_addr_t *target); unsigned int channel_num_circuits(channel_t *chan); MOCK_DECL(void,channel_set_circid_type,(channel_t *chan, crypto_pk_t *identity_rcvd,

1 0

[tor/master] relay/circuitbuild: Report IPv6 addresses in a debug log
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit ec5f4f3c5a5aa4d69b2867ba41bc83ba1e6c888a Author: teor <teor(a)torproject.org> Date: Wed Apr 15 13:07:10 2020 +1000 relay/circuitbuild: Report IPv6 addresses in a debug log Part of 33817. --- src/feature/relay/circuitbuild_relay.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 080781f71..7d3d58977 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -337,9 +337,13 @@ circuit_extend(struct cell_t *cell, struct circuit_t *circ) &should_launch); if (!n_chan) { - log_debug(LD_CIRC|LD_OR,"Next router (%s): %s.", - fmt_addrport(&ec.orport_ipv4.addr,ec.orport_ipv4.port), - msg?msg:"????"); + /* We can't use fmt_addr*() twice in the same function call, + * because it uses a static buffer. */ + log_debug(LD_CIRC|LD_OR, "Next router IPv4 (%s): %s.", + fmt_addrport_ap(&ec.orport_ipv4), + msg ? msg : "????"); + log_debug(LD_CIRC|LD_OR, "Next router IPv6 (%s).", + fmt_addrport_ap(&ec.orport_ipv6)); circuit_open_connection_for_extend(&ec, circ, should_launch);

1 0

[tor/master] relay: Add IP version tests for circuit extends
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit cab05a84cd1504bc4929207a1f33b266744e5dca Author: teor <teor(a)torproject.org> Date: Wed Apr 29 20:10:14 2020 +1000 relay: Add IP version tests for circuit extends Add IPv4-only and IPv6-only tests for circuit_open_connection_for_extend(). Part of 33817. --- src/feature/relay/router.c | 4 ++-- src/feature/relay/router.h | 3 ++- src/test/test_circuitbuild.c | 48 +++++++++++++++++++++++++++++++++++++------- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 89e232ccc..b0a56012d 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -1491,8 +1491,8 @@ router_get_advertised_ipv6_or_ap(const or_options_t *options, } /** Returns true if this router has an advertised IPv6 ORPort. */ -bool -router_has_advertised_ipv6_orport(const or_options_t *options) +MOCK_IMPL(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); diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index c3a93cc0a..1a262a679 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -68,7 +68,8 @@ 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); +MOCK_DECL(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 a26109ed8..5ae7dbf97 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -30,6 +30,7 @@ #include "feature/client/entrynodes.h" #include "feature/nodelist/nodelist.h" #include "feature/relay/circuitbuild_relay.h" +#include "feature/relay/router.h" #include "feature/relay/routermode.h" #include "feature/nodelist/node_st.h" @@ -848,11 +849,27 @@ mock_channel_connect_for_circuit(const tor_addr_t *addr, return mock_channel_connect_nchan; } -/* Test the different cases in circuit_open_connection_for_extend(). */ +static bool +mock_router_has_advertised_ipv6_orport(const or_options_t *options) +{ + (void)options; + return 1; +} + +/* Test the different cases in circuit_open_connection_for_extend(). + * Chooses different IP addresses depending on the first character in arg: + * - 4: IPv4 + * - 6: IPv6 + * - d: IPv4 and IPv6 (dual-stack) + */ static void test_circuit_open_connection_for_extend(void *arg) { - (void)arg; + const char ip_version = ((const char *)arg)[0]; + const bool use_ipv4 = (ip_version == '4' || ip_version == 'd'); + const bool use_ipv6 = (ip_version == '6' || ip_version == 'd'); + tor_assert(use_ipv4 || use_ipv6); + extend_cell_t *ec = tor_malloc_zero(sizeof(extend_cell_t)); circuit_t *circ = tor_malloc_zero(sizeof(circuit_t)); channel_t *fake_n_chan = tor_malloc_zero(sizeof(channel_t)); @@ -867,6 +884,9 @@ test_circuit_open_connection_for_extend(void *arg) mock_channel_connect_calls = 0; mock_channel_connect_nchan = NULL; + MOCK(router_has_advertised_ipv6_orport, + mock_router_has_advertised_ipv6_orport); + setup_full_capture_of_logs(LOG_INFO); #ifndef ALL_BUGS_ARE_FATAL @@ -929,10 +949,14 @@ test_circuit_open_connection_for_extend(void *arg) #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; + if (use_ipv4) { + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + ec->orport_ipv4.port = VALID_PORT; + } + if (use_ipv6) { + 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; @@ -1001,6 +1025,8 @@ test_circuit_open_connection_for_extend(void *arg) or_options_free(fake_options); mocked_options = NULL; + UNMOCK(router_has_advertised_ipv6_orport); + tor_free(ec); tor_free(circ->n_hop); tor_free(circ->n_chan_create_cell); @@ -1414,6 +1440,12 @@ test_onionskin_answer(void *arg) #define TEST_CIRCUIT(name, flags) \ { #name, test_circuit_ ## name, flags, NULL, NULL } +#ifndef COCCI +#define TEST_CIRCUIT_PASSTHROUGH(name, flags, arg) \ + { #name "/" arg, test_circuit_ ## name, flags, \ + &passthrough_setup, (void *)(arg) } +#endif + struct testcase_t circuitbuild_tests[] = { TEST_NEW_ROUTE_LEN(noexit, 0), TEST_NEW_ROUTE_LEN(safe_exit, 0), @@ -1425,7 +1457,9 @@ struct testcase_t circuitbuild_tests[] = { TEST_CIRCUIT(extend_state_valid, TT_FORK), TEST_CIRCUIT(extend_add_ed25519, TT_FORK), TEST_CIRCUIT(extend_lspec_valid, TT_FORK), - TEST_CIRCUIT(open_connection_for_extend, TT_FORK), + TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "4"), + TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "6"), + TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "dual-stack"), TEST_CIRCUIT(extend, TT_FORK), TEST(onionskin_answer, TT_FORK, NULL, NULL),

1 0

[tor/master] relay/circuitbuild: Refactor open connection for extend
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit a0b12f3cd40fc00c9bdbb1ff01b0d074673a7524 Author: teor <teor(a)torproject.org> Date: Thu Apr 16 08:13:02 2020 +1000 relay/circuitbuild: Refactor open connection for extend Re-use the newly created extend_info to launch the connection in circuit_open_connection_for_extend(). No behaviour change. Part of 33817. --- src/feature/relay/circuitbuild_relay.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index a926a1d81..d6ea22ca7 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -273,10 +273,11 @@ circuit_open_connection_for_extend(const struct extend_cell_t *ec, if (should_launch) { /* we should try to open a connection */ - channel_t *n_chan = channel_connect_for_circuit(&ec->orport_ipv4.addr, - ec->orport_ipv4.port, - (const char*)ec->node_id, - &ec->ed_pubkey); + channel_t *n_chan = channel_connect_for_circuit( + &circ->n_hop->addr, + circ->n_hop->port, + circ->n_hop->identity_digest, + &circ->n_hop->ed_identity); if (!n_chan) { log_info(LD_CIRC,"Launching n_chan failed. Closing circuit."); circuit_mark_for_close(circ, END_CIRC_REASON_CONNECTFAILED);

1 0

[tor/master] relay: Add tests for choosing extend IPs
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit df5ea297751d68dac7986f827494f2be87fe954f Author: teor <teor(a)torproject.org> Date: Wed Apr 29 21:25:10 2020 +1000 relay: Add tests for choosing extend IPs Part of 33817. --- src/test/test_circuitbuild.c | 100 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 93 insertions(+), 7 deletions(-) diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 5ae7dbf97..ed75d102b 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -821,6 +821,97 @@ test_circuit_extend_lspec_valid(void *arg) tor_free(p_chan); } +static bool router_has_ipv6_orport_result = false; +static int mock_router_ipv6_orport_calls = 0; +static bool +mock_router_has_advertised_ipv6_orport(const or_options_t *options) +{ + (void)options; + mock_router_ipv6_orport_calls++; + return router_has_ipv6_orport_result; +} + +/* Test the different cases in circuit_choose_ip_ap_for_extend(). */ +static void +test_circuit_choose_ip_ap_for_extend(void *arg) +{ + (void)arg; + tor_addr_port_t ipv4_ap; + tor_addr_port_t ipv6_ap; + + /* Set up valid addresses */ + tor_addr_parse(&ipv4_ap.addr, PUBLIC_IPV4); + ipv4_ap.port = VALID_PORT; + tor_addr_parse(&ipv6_ap.addr, PUBLIC_IPV6); + ipv6_ap.port = VALID_PORT; + + or_options_t *fake_options = options_new(); + MOCK(get_options, mock_get_options); + mocked_options = fake_options; + + MOCK(router_has_advertised_ipv6_orport, + mock_router_has_advertised_ipv6_orport); + router_has_ipv6_orport_result = true; + mock_router_ipv6_orport_calls = 0; + + /* No valid addresses */ + router_has_ipv6_orport_result = true; + mock_router_ipv6_orport_calls = 0; + tt_ptr_op(circuit_choose_ip_ap_for_extend(NULL, NULL), OP_EQ, NULL); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + router_has_ipv6_orport_result = false; + mock_router_ipv6_orport_calls = 0; + tt_ptr_op(circuit_choose_ip_ap_for_extend(NULL, NULL), OP_EQ, NULL); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + /* One valid address: IPv4 */ + router_has_ipv6_orport_result = true; + mock_router_ipv6_orport_calls = 0; + tt_ptr_op(circuit_choose_ip_ap_for_extend(&ipv4_ap, NULL), OP_EQ, &ipv4_ap); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + router_has_ipv6_orport_result = false; + mock_router_ipv6_orport_calls = 0; + tt_ptr_op(circuit_choose_ip_ap_for_extend(&ipv4_ap, NULL), OP_EQ, &ipv4_ap); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + /* One valid address: IPv6 */ + router_has_ipv6_orport_result = true; + mock_router_ipv6_orport_calls = 0; + tt_ptr_op(circuit_choose_ip_ap_for_extend(NULL, &ipv6_ap), OP_EQ, &ipv6_ap); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + router_has_ipv6_orport_result = false; + mock_router_ipv6_orport_calls = 0; + tt_ptr_op(circuit_choose_ip_ap_for_extend(NULL, &ipv6_ap), OP_EQ, NULL); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + /* Two valid addresses */ + const tor_addr_port_t *chosen_addr = NULL; + + router_has_ipv6_orport_result = true; + mock_router_ipv6_orport_calls = 0; + chosen_addr = circuit_choose_ip_ap_for_extend(&ipv4_ap, &ipv6_ap); + tt_assert(chosen_addr == &ipv4_ap || chosen_addr == &ipv6_ap); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + router_has_ipv6_orport_result = false; + mock_router_ipv6_orport_calls = 0; + tt_ptr_op(circuit_choose_ip_ap_for_extend(&ipv4_ap, &ipv6_ap), + OP_EQ, &ipv4_ap); + tt_int_op(mock_router_ipv6_orport_calls, OP_EQ, 1); + + done: + UNMOCK(get_options); + or_options_free(fake_options); + mocked_options = NULL; + + UNMOCK(router_has_advertised_ipv6_orport); + + tor_free(fake_options); +} + static int mock_circuit_close_calls = 0; static void mock_circuit_mark_for_close_(circuit_t *circ, int reason, @@ -849,13 +940,6 @@ mock_channel_connect_for_circuit(const tor_addr_t *addr, return mock_channel_connect_nchan; } -static bool -mock_router_has_advertised_ipv6_orport(const or_options_t *options) -{ - (void)options; - return 1; -} - /* Test the different cases in circuit_open_connection_for_extend(). * Chooses different IP addresses depending on the first character in arg: * - 4: IPv4 @@ -886,6 +970,7 @@ test_circuit_open_connection_for_extend(void *arg) MOCK(router_has_advertised_ipv6_orport, mock_router_has_advertised_ipv6_orport); + router_has_ipv6_orport_result = true; setup_full_capture_of_logs(LOG_INFO); @@ -1457,6 +1542,7 @@ struct testcase_t circuitbuild_tests[] = { TEST_CIRCUIT(extend_state_valid, TT_FORK), TEST_CIRCUIT(extend_add_ed25519, TT_FORK), TEST_CIRCUIT(extend_lspec_valid, TT_FORK), + TEST_CIRCUIT(choose_ip_ap_for_extend, 0), TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "4"), TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "6"), TEST_CIRCUIT_PASSTHROUGH(open_connection_for_extend, TT_FORK, "dual-stack"),

1 0

[tor/master] relay: Choose between IPv4 and IPv6 extends at random
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit c3e058dfac1cbc7cb0dee5cdb1bdc61c1dc0f4fa Author: teor <teor(a)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 ec, set up the - * circuit circ to wait for a new connection. If should_launch - * is true, open a new connection. (Otherwise, we are already waiting for a - * new connection to the same relay.) + * circuit circ to wait for a new connection. + * + * If should_launch 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 dirport 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);

1 0

[tor/master] relay: Refactor choosing extend IPs
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit bcec1ec071400464f7cefa4f0d493964984905ce Author: teor <teor(a)torproject.org> Date: Wed Apr 29 21:37:31 2020 +1000 relay: Refactor choosing extend IPs Flatten the logic in circuit_choose_ip_ap_for_extend(). Part of 33817. --- src/feature/relay/circuitbuild_relay.c | 45 +++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 2bf08547d..9420ea11a 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -250,8 +250,27 @@ STATIC const tor_addr_port_t * circuit_choose_ip_ap_for_extend(const tor_addr_port_t *ipv4_ap, const tor_addr_port_t *ipv6_ap) { - /* If we could make an IPv4 or an IPv6 connection, make an IPv6 connection - * at random, with probability 1 in N. + const bool ipv6_supported = router_has_advertised_ipv6_orport(get_options()); + + /* If IPv6 is not supported, we can't use the IPv6 address. */ + if (!ipv6_supported) { + ipv6_ap = NULL; + } + + /* If there is no IPv6 address, IPv4 is always supported. + * Until clients include IPv6 ORPorts, and most relays support IPv6, + * this is the most common case. */ + if (!ipv6_ap) { + return ipv4_ap; + } + + /* If there is no IPv4 address, return the (possibly NULL) IPv6 address. */ + if (!ipv4_ap) { + return ipv6_ap; + } + + /* Now we have an IPv4 and an IPv6 address, and IPv6 is supported. + * So 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) ... @@ -260,26 +279,12 @@ circuit_choose_ip_ap_for_extend(const tor_addr_port_t *ipv4_ap, */ #define IPV6_CONNECTION_ONE_IN_N 2 - /* IPv4 is always supported */ - const bool ipv6_supported = router_has_advertised_ipv6_orport(get_options()); - - if (ipv4_ap && ipv6_ap && 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) { - return ipv6_ap; - } else { - return ipv4_ap; - } - } else if (ipv6_ap && ipv6_supported) { - /* There's only one valid address: try to use it */ + bool choose_ipv6 = crypto_fast_rng_one_in_n(get_thread_fast_rng(), + IPV6_CONNECTION_ONE_IN_N); + if (choose_ipv6) { return ipv6_ap; - } else if (ipv4_ap) { - return ipv4_ap; } else { - /* IPv6-only extend, but IPv6 is not supported. */ - return NULL; + return ipv4_ap; } }

1 0