- tor-commits - lists.torproject.org

[tor/master] relay/circuitbuild: Consider IPv6-only extends valid
by nickm＠torproject.org 30 Apr '20

30 Apr '20

commit bad1181b5d1bef55e060d64a9d4ef9278619495b Author: teor <teor(a)torproject.org> Date: Wed Apr 15 11:34:12 2020 +1000 relay/circuitbuild: Consider IPv6-only extends valid Allow extend cells with IPv6-only link specifiers. Warn and fail if both IPv4 and IPv6 are invalid. Also warn if the IPv4 or IPv6 addresses are unexpectedly internal, but continue with the valid address. Part of 33817. --- src/feature/relay/circuitbuild_relay.c | 50 +++++++++---- src/test/test_circuitbuild.c | 125 ++++++++++++++++++++++++++++++++- 2 files changed, 161 insertions(+), 14 deletions(-) diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c index 05146f1b6..080781f71 100644 --- a/src/feature/relay/circuitbuild_relay.c +++ b/src/feature/relay/circuitbuild_relay.c @@ -123,11 +123,14 @@ circuit_extend_add_ed25519_helper(struct extend_cell_t *ec) * and are allowed by the current ExtendAllowPrivateAddresses config. * * If they are valid, return 0. - * Otherwise, if they are invalid, log a warning at log_level, - * and return -1. + * Otherwise, if they are invalid, return -1. + * If log_zero_addrs is true, log warnings about zero addresses at + * log_level. If log_internal_addrs is true, log warnings about + * internal addresses at log_level. */ static int circuit_extend_addr_port_helper(const struct tor_addr_port_t *ap, + bool log_zero_addrs, bool log_internal_addrs, int log_level) { /* It's safe to print the family. But we don't want to print the address, @@ -135,19 +138,23 @@ circuit_extend_addr_port_helper(const struct tor_addr_port_t *ap, * 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 a zero destination port or " - "%s address '%s'.", - fmt_addr_family(&ap->addr), safe_str(fmt_addrport_ap(ap))); + if (log_zero_addrs) { + log_fn(log_level, LD_PROTOCOL, + "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 %s address '%s'.", - fmt_addr_family(&ap->addr), - safe_str(fmt_and_decorate_addr(&ap->addr))); + if (log_internal_addrs) { + log_fn(log_level, LD_PROTOCOL, + "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; } @@ -174,9 +181,28 @@ circuit_extend_lspec_valid_helper(const struct extend_cell_t *ec, return -1; } - if (circuit_extend_addr_port_helper(&ec->orport_ipv4, - LOG_PROTOCOL_WARN) < 0) { + /* 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); + /* We need at least one valid address */ + if (!ipv4_valid && !ipv6_valid) { + /* Now, log the invalid addresses at protocol warning level */ + circuit_extend_addr_port_helper(&ec->orport_ipv4, true, true, + LOG_PROTOCOL_WARN); + circuit_extend_addr_port_helper(&ec->orport_ipv6, true, true, + LOG_PROTOCOL_WARN); + /* And fail */ return -1; + } else if (!ipv4_valid) { + /* Always log unexpected internal addresses, but go on to use the other + * valid address */ + circuit_extend_addr_port_helper(&ec->orport_ipv4, false, true, + LOG_PROTOCOL_WARN); + } else if (!ipv6_valid) { + circuit_extend_addr_port_helper(&ec->orport_ipv6, false, true, + LOG_PROTOCOL_WARN); } IF_BUG_ONCE(circ->magic != OR_CIRCUIT_MAGIC) { diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 932248084..a8d6323e4 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -474,6 +474,9 @@ mock_get_options(void) #define PUBLIC_IPV4 "1.2.3.4" #define INTERNAL_IPV4 "0.0.0.1" +#define PUBLIC_IPV6 "1234::cdef" +#define INTERNAL_IPV6 "::1" + #define VALID_PORT 0x1234 /* Test the different cases in circuit_extend_lspec_valid_helper(). */ @@ -519,7 +522,7 @@ test_circuit_extend_lspec_valid(void *arg) tor_end_capture_bugs_(); mock_clean_saved_logs(); - /* IPv4 addr or port are 0, these should fail */ + /* 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); expect_log_msg("Client asked me to extend to a zero destination port " "or unspecified address '[scrubbed]'.\n"); @@ -527,35 +530,103 @@ test_circuit_extend_lspec_valid(void *arg) /* Now ask for the actual address in the logs */ fake_options->SafeLogging_ = SAFELOG_SCRUB_NONE; + + /* IPv4 port is 0, IPv6 addr and port are both zero, this should fail */ 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 a zero destination port " "or IPv4 address '1.2.3.4:0'.\n"); mock_clean_saved_logs(); - tor_addr_make_null(&ec->orport_ipv4.addr, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + /* IPv4 addr is 0, IPv6 addr and port are both zero, this should fail */ 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 a zero destination port " "or IPv4 address '0.0.0.0:4660'.\n"); mock_clean_saved_logs(); ec->orport_ipv4.port = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); /* IPv4 addr is internal, and port is valid. + * (IPv6 addr and port are both zero.) + * Result depends on ExtendAllowPrivateAddresses. */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + + 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 IPv4 address '0.0.0.1'.\n"); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Now do the same tests, but for IPv6 */ + + /* IPv6 port is 0, IPv4 addr and port are both zero, this should fail */ + tor_addr_parse(&ec->orport_ipv6.addr, PUBLIC_IPV6); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + expect_log_msg("Client asked me to extend to a zero destination port " + "or IPv6 address '[1234::cdef]:0'.\n"); + mock_clean_saved_logs(); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* IPv6 addr is 0, IPv4 addr and port are both zero, this should fail */ + ec->orport_ipv6.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 a zero destination port " + "or IPv6 address '[::]:4660'.\n"); + mock_clean_saved_logs(); + ec->orport_ipv4.port = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* IPv6 addr is internal, and port is valid. + * (IPv4 addr and port are both zero.) + * Result depends on ExtendAllowPrivateAddresses. */ + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; + + 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 IPv6 address '[::1]'.\n"); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Both addresses are internal. * Result depends on ExtendAllowPrivateAddresses. */ tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; 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 IPv4 address '0.0.0.1'.\n"); + expect_log_msg("Client asked me to extend " + "to a private IPv6 address '[::1]'.\n"); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); /* 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); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; fake_options->ExtendAllowPrivateAddresses = 1; + tor_capture_bugs_(1); tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); @@ -564,6 +635,21 @@ test_circuit_extend_lspec_valid(void *arg) tor_end_capture_bugs_(); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0; + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Fail again, but this time only set an IPv4 address. */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + fake_options->ExtendAllowPrivateAddresses = 1; + tor_capture_bugs_(1); + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, -1); + /* Since we're using IF_BUG_ONCE(), expect 0-1 bug logs */ + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_GE, 0); + tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_LE, 1); + tor_end_capture_bugs_(); + mock_clean_saved_logs(); + fake_options->ExtendAllowPrivateAddresses = 0; /* Now set the right magic */ or_circ->base_.magic = OR_CIRCUIT_MAGIC; @@ -625,6 +711,41 @@ test_circuit_extend_lspec_valid(void *arg) tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); mock_clean_saved_logs(); + /* Now let's check that we warn, but succeed, when only one address is + * private */ + tor_addr_parse(&ec->orport_ipv4.addr, INTERNAL_IPV4); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, PUBLIC_IPV6); + ec->orport_ipv6.port = VALID_PORT; + fake_options->ExtendAllowPrivateAddresses = 0; + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + expect_log_msg("Client asked me to extend " + "to a private IPv4 address '0.0.0.1'.\n"); + mock_clean_saved_logs(); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Now with private IPv6 */ + tor_addr_parse(&ec->orport_ipv4.addr, PUBLIC_IPV4); + ec->orport_ipv4.port = VALID_PORT; + tor_addr_parse(&ec->orport_ipv6.addr, INTERNAL_IPV6); + ec->orport_ipv6.port = VALID_PORT; + fake_options->ExtendAllowPrivateAddresses = 0; + + tt_int_op(circuit_extend_lspec_valid_helper(ec, circ), OP_EQ, 0); + expect_log_msg("Client asked me to extend " + "to a private IPv6 address '[::1]'.\n"); + mock_clean_saved_logs(); + tor_addr_port_make_null_ap(&ec->orport_ipv4, AF_INET); + tor_addr_port_make_null_ap(&ec->orport_ipv6, AF_INET6); + + /* Now reset to public IPv4 and IPv6 */ + 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; + /* Fail on matching non-zero identities */ memset(&ec->ed_pubkey, 0xEE, sizeof(ec->ed_pubkey)); memset(&p_chan->ed25519_identity, 0xEE, sizeof(p_chan->ed25519_identity));

1 0

[tor/master] test/circuitbuild: Show bad addresses in some logs
by nickm＠torproject.org 30 Apr '20

30 Apr '20

commit 7cef02ec1fa76f593dfb9bd53f76922225696934 Author: teor <teor(a)torproject.org> Date: Wed Apr 15 10:07:26 2020 +1000 test/circuitbuild: Show bad addresses in some logs Disable SafeLogging for some extend tests, so we can check the actual addresses. Part of 33817. --- src/test/test_circuitbuild.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index ab5c9c993..932248084 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -525,17 +525,19 @@ test_circuit_extend_lspec_valid(void *arg) "or unspecified address '[scrubbed]'.\n"); mock_clean_saved_logs(); + /* Now ask for the actual address in the logs */ + fake_options->SafeLogging_ = SAFELOG_SCRUB_NONE; 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 a zero destination port " - "or IPv4 address '[scrubbed]'.\n"); + "or IPv4 address '1.2.3.4:0'.\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 a zero destination port " - "or IPv4 address '[scrubbed]'.\n"); + "or IPv4 address '0.0.0.0:4660'.\n"); mock_clean_saved_logs(); ec->orport_ipv4.port = 0; @@ -547,7 +549,7 @@ 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 IPv4 address '[scrubbed]'.\n"); + "to a private IPv4 address '0.0.0.1'.\n"); mock_clean_saved_logs(); fake_options->ExtendAllowPrivateAddresses = 0;

1 0

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

30 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 30 Apr '20

30 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 30 Apr '20

30 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 30 Apr '20

30 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 30 Apr '20

30 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 30 Apr '20

30 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 30 Apr '20

30 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 30 Apr '20

30 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