commit 44f71e08c414f6c7aad6304e24be90d5d320c95b Author: teor <teor@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;