April 2020 - tor-commits - lists.torproject.org

[tor/master] net: Remove an extra space in address.h
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit e9d04b05c6ba3930d55dc6c7e512e082ff67ebfb Author: teor <teor(a)torproject.org> Date: Wed Apr 15 09:52:25 2020 +1000 net: Remove an extra space in address.h --- src/lib/net/address.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/net/address.h b/src/lib/net/address.h index f1c223310..186f1fe86 100644 --- a/src/lib/net/address.h +++ b/src/lib/net/address.h @@ -221,7 +221,7 @@ char *tor_addr_to_str_dup(const tor_addr_t *addr) ATTR_MALLOC; const char *fmt_addr_impl(const tor_addr_t *addr, int decorate); const char *fmt_addrport(const tor_addr_t *addr, uint16_t port); -const char * fmt_addr32(uint32_t addr); +const char *fmt_addr32(uint32_t addr); MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family, tor_addr_t *addr));

1 0

[tor/master] relay: Log the address in circuit protocol warnings
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit 44f71e08c414f6c7aad6304e24be90d5d320c95b Author: teor <teor(a)torproject.org> Date: Wed Apr 15 09:55:09 2020 +1000 relay: Log the address in circuit protocol warnings Always log the address family in extend protocol warnings. If SafeLogging is 0, also log the address and port. Diagnostics for 33817. --- src/feature/relay/circuitbuild_relay.c | 12 ++++++++++-- src/test/test_circuitbuild.c | 19 ++++++++++--------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index dd38a2825..05146f1b6 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -130,16 +130,24 @@ static int circuit_extend_addr_port_helper(const struct tor_addr_port_t *ap, int log_level) { + /* It's safe to print the family. But we don't want to print the address, + * unless specifically configured to do so. (Zero addresses aren't sensitive, + * But some internal addresses might be.)*/ + if (!tor_addr_port_is_valid_ap(ap, 0)) { log_fn(log_level, LD_PROTOCOL, - "Client asked me to extend to zero destination port or addr."); + "Client asked me to extend to a zero destination port or " + "%s address '%s'.", + fmt_addr_family(&ap->addr), safe_str(fmt_addrport_ap(ap))); return -1; } if (tor_addr_is_internal(&ap->addr, 0) && !get_options()->ExtendAllowPrivateAddresses) { log_fn(log_level, LD_PROTOCOL, - "Client asked me to extend to a private address."); + "Client asked me to extend to a private %s address '%s'.", + fmt_addr_family(&ap->addr), + safe_str(fmt_and_decorate_addr(&ap->addr))); return -1; } diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 061f39937..ab5c9c993 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -521,21 +521,21 @@ test_circuit_extend_lspec_valid(void *arg) /* IPv4 addr or port are 0, these should fail */ tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or unspecified address '[scrubbed]'.\n"); mock_clean_saved_logs(); tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or IPv4 address '[scrubbed]'.\n"); mock_clean_saved_logs(); tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); ec->orport_ipv4.port = VALID_PORT; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or IPv4 address '[scrubbed]'.\n"); mock_clean_saved_logs(); ec->orport_ipv4.port = 0; @@ -546,7 +546,8 @@ test_circuit_extend_lspec_valid(void *arg) fake_options->ExtendAllowPrivateAddresses = 0; tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); - expect_log_msg("Client asked me to extend to a private address.\n"); + expect_log_msg("Client asked me to extend " + "to a private IPv4 address '[scrubbed]'.\n"); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0; @@ -1002,8 +1003,8 @@ test_circuit_extend(void *arg) tt_int_op(circuit_extend(cell, circ), OP_EQ, -1); tt_int_op(mock_extend_cell_parse_calls, OP_EQ, 1); - expect_log_msg("Client asked me to extend to " - "zero destination port or addr.\n"); + expect_log_msg("Client asked me to extend to a zero destination port " + "or unspecified address '[scrubbed]'.\n"); mock_clean_saved_logs(); mock_extend_cell_parse_calls = 0;

1 0

[tor/master] net: Add fmt_addrport_ap() and fmt_addr_family()
by nickm＠torproject.org 29 Apr '20

29 Apr '20

commit a72e017e7f69581ceb005d05ce8033a6fd05626e Author: teor <teor(a)torproject.org> Date: Wed Apr 15 09:52:57 2020 +1000 net: Add fmt_addrport_ap() and fmt_addr_family() Add fmt_addrport_ap(), a macro that takes a tor_addr_port_t, and uses it to call fmt_addrport(). Add fmt_addr_family(), a function that returns a string constant describing the address family. Utility functions for 33817. --- src/lib/net/address.c | 33 +++++++++++++++++++++++++++++++++ src/lib/net/address.h | 2 ++ 2 files changed, 35 insertions(+) diff --git a/src/lib/net/address.c b/src/lib/net/address.c index 5fe9abca1..9a90093ae 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -1206,6 +1206,39 @@ fmt_addr32(uint32_t addr) return buf; } +/** Return a string representing the family of addr. + * + * This string is a string constant, and must not be freed. + * This function is thread-safe. + */ +const char * +fmt_addr_family(const tor_addr_t *addr) +{ + static int default_bug_once = 0; + + IF_BUG_ONCE(!addr) + return "NULL pointer"; + + switch (tor_addr_family(addr)) { + case AF_INET6: + return "IPv6"; + case AF_INET: + return "IPv4"; + case AF_UNIX: + return "UNIX socket"; + case AF_UNSPEC: + return "unspecified"; + default: + if (!default_bug_once) { + log_warn(LD_BUG, "Called with unknown address family %d", + (int)tor_addr_family(addr)); + default_bug_once = 1; + } + return "unknown"; + } + //return "(unreachable code)"; +} + /** Convert the string in src to a tor_addr_t addr. The string * may be an IPv4 address, or an IPv6 address surrounded by square brackets. * diff --git a/src/lib/net/address.h b/src/lib/net/address.h index 186f1fe86..611d1ca1e 100644 --- a/src/lib/net/address.h +++ b/src/lib/net/address.h @@ -221,7 +221,9 @@ char *tor_addr_to_str_dup(const tor_addr_t *addr) ATTR_MALLOC; const char *fmt_addr_impl(const tor_addr_t *addr, int decorate); const char *fmt_addrport(const tor_addr_t *addr, uint16_t port); +#define fmt_addrport_ap(ap) fmt_addrport(&(ap)->addr, (ap)->port) const char *fmt_addr32(uint32_t addr); +const char *fmt_addr_family(const tor_addr_t *addr); MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family, tor_addr_t *addr));

1 0

[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