commit 2f3b4e38888116f434297fb45ac093acd2d01e55 Author: David Goulet dgoulet@torproject.org Date: Tue Jun 23 10:43:39 2020 -0400

addr: Refactor is_local_addr() to support IPv6

Series of changes:

1. Rename function to reflect the namespace of the file.

2. Use the new last resolved cache instead of the unused last_resolved_addr_v4 (which is also removed in this commit).

3. Make the entire code base use the new resolved_addr_is_local() function.

You will notice that this function uses /24 to differentiate subnets where the rest of tor uses /16 (including documentation of EnforceDistinctSubnets). Ticket #40009 has been opened for that.

But that the moment, the function keeps looking at /24.

Part of #33233

Signed-off-by: David Goulet dgoulet@torproject.org --- src/app/config/resolve_addr.c | 56 +++++++++++++++++++++++++---------------- src/app/config/resolve_addr.h | 2 +- src/core/or/channeltls.c | 6 ++--- src/feature/dircache/dircache.c | 2 +- src/test/test_channeltls.c | 32 +++++++++++------------ 5 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/src/app/config/resolve_addr.c b/src/app/config/resolve_addr.c index 1e861217d..c73519f14 100644 --- a/src/app/config/resolve_addr.c +++ b/src/app/config/resolve_addr.c @@ -44,9 +44,6 @@ /** Last resolved addresses. */ static tor_addr_t last_resolved_addrs[IDX_SIZE];

-/** Last value actually set by resolve_my_address. */ -static uint32_t last_resolved_addr_v4 = 0; - /** Copy the last resolved address of family into addr_out. * * If not last resolved address existed, the addr_out is a null address (use @@ -598,27 +595,44 @@ resolve_my_address_v4(int warn_severity, const or_options_t *options, /** Return true iff <b>addr</b> is judged to be on the same network as us, or * on a private network. */ -MOCK_IMPL(int, -is_local_addr, (const tor_addr_t *addr)) +MOCK_IMPL(bool, +resolved_addr_is_local, (const tor_addr_t *addr)) { - if (tor_addr_is_internal(addr, 0)) - return 1; - /* Check whether ip is on the same /24 as we are. */ - if (get_options()->EnforceDistinctSubnets == 0) - return 0; - if (tor_addr_family(addr) == AF_INET) { - uint32_t ip = tor_addr_to_ipv4h(addr); + const int family = tor_addr_family(addr); + const tor_addr_t *last_resolved_addr = &last_resolved_addrs[family]; + + /* Internal address is always local. */ + if (tor_addr_is_internal(addr, 0)) { + return true; + } + + /* Address is not local if we don't enforce subnet distinction. */ + if (get_options()->EnforceDistinctSubnets == 0) { + return false; + } + + switch (family) { + case AF_INET: + /* XXX: Why is this /24 and not /16 which the rest of tor does? Unknown + * reasons at the moment highlighted in ticket #40009. Because of that, we + * can't use addrs_in_same_network_family(). */

/* It's possible that this next check will hit before the first time - * resolve_my_address actually succeeds. (For clients, it is likely that - * resolve_my_address will never be called at all). In those cases, + * resolve_my_address_v4 actually succeeds. For clients, it is likely that + * resolve_my_address_v4 will never be called at all. In those cases, * last_resolved_addr_v4 will be 0, and so checking to see whether ip is - * on the same /24 as last_resolved_addr_v4 will be the same as checking - * whether it was on net 0, which is already done by tor_addr_is_internal. - */ - if ((last_resolved_addr_v4 & (uint32_t)0xffffff00ul) - == (ip & (uint32_t)0xffffff00ul)) - return 1; + * on the same /24 as last_resolved_addrs[AF_INET] will be the same as + * checking whether it was on net 0, which is already done by + * tor_addr_is_internal. */ + return tor_addr_compare_masked(addr, last_resolved_addr, 24, + CMP_SEMANTIC) == 0; + case AF_INET6: + /* Look at the /32 like addrs_in_same_network_family() does. */ + return tor_addr_compare_masked(addr, last_resolved_addr, 32, + CMP_SEMANTIC) == 0; + break; + default: + /* Unknown address type so not local. */ + return false; } - return 0; } diff --git a/src/app/config/resolve_addr.h b/src/app/config/resolve_addr.h index 17e940203..f435ab29a 100644 --- a/src/app/config/resolve_addr.h +++ b/src/app/config/resolve_addr.h @@ -22,7 +22,7 @@ bool find_my_address(const or_options_t *options, int family, void resolved_addr_get_last(int family, tor_addr_t *addr_out); void resolved_addr_reset_last(int family);

-MOCK_DECL(int, is_local_addr, (const tor_addr_t *addr)); +MOCK_DECL(bool, resolved_addr_is_local, (const tor_addr_t *addr));

#ifdef RESOLVE_ADDR_PRIVATE

diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index 395fbf345..3e7f25ae4 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -203,7 +203,7 @@ channel_tls_connect(const tor_addr_t *addr, uint16_t port, tlschan, (chan->global_identifier));

- if (is_local_addr(addr)) { + if (resolved_addr_is_local(addr)) { log_debug(LD_CHANNEL, "Marking new outgoing channel %"PRIu64 " at %p as local", (chan->global_identifier), chan); @@ -340,7 +340,7 @@ channel_tls_handle_incoming(or_connection_t *orconn) tlschan->conn = orconn; orconn->chan = tlschan;

- if (is_local_addr(&(TO_CONN(orconn)->addr))) { + if (resolved_addr_is_local(&(TO_CONN(orconn)->addr))) { log_debug(LD_CHANNEL, "Marking new incoming channel %"PRIu64 " at %p as local", (chan->global_identifier), chan); @@ -1353,7 +1353,7 @@ channel_tls_update_marks(or_connection_t *conn)

chan = TLS_CHAN_TO_BASE(conn->chan);

- if (is_local_addr(&(TO_CONN(conn)->addr))) { + if (resolved_addr_is_local(&(TO_CONN(conn)->addr))) { if (!channel_is_local(chan)) { log_debug(LD_CHANNEL, "Marking channel %"PRIu64 " at %p as local", diff --git a/src/feature/dircache/dircache.c b/src/feature/dircache/dircache.c index ca127720f..44dc46205 100644 --- a/src/feature/dircache/dircache.c +++ b/src/feature/dircache/dircache.c @@ -142,7 +142,7 @@ write_http_response_header_impl(dir_connection_t *conn, ssize_t length, if (type) { buf_add_printf(buf, "Content-Type: %s\r\n", type); } - if (!is_local_addr(&conn->base_.addr)) { + if (!resolved_addr_is_local(&conn->base_.addr)) { /* Don't report the source address for a nearby/private connection. * Otherwise we tend to mis-report in cases where incoming ports are * being forwarded to a Tor server running behind the firewall. */ diff --git a/src/test/test_channeltls.c b/src/test/test_channeltls.c index f4f5cb447..801800033 100644 --- a/src/test/test_channeltls.c +++ b/src/test/test_channeltls.c @@ -38,13 +38,13 @@ static or_connection_t * tlschan_connection_or_connect_mock( const char *digest, const ed25519_public_key_t *ed_id, channel_tls_t *tlschan); -static int tlschan_is_local_addr_mock(const tor_addr_t *addr); +static bool tlschan_resolved_addr_is_local_mock(const tor_addr_t *addr);

/* Fake close method */ static void tlschan_fake_close_method(channel_t *chan);

/* Flags controlling behavior of channeltls unit test mocks */ -static int tlschan_local = 0; +static bool tlschan_local = false; static const buf_t * tlschan_buf_datalen_mock_target = NULL; static size_t tlschan_buf_datalen_mock_size = 0;

@@ -67,9 +67,9 @@ test_channeltls_create(void *arg) test_addr.addr.in_addr.s_addr = htonl(0x01020304);

/* For this test we always want the address to be treated as non-local */ - tlschan_local = 0; - /* Install is_local_addr() mock */ - MOCK(is_local_addr, tlschan_is_local_addr_mock); + tlschan_local = false; + /* Install resolved_addr_is_local() mock */ + MOCK(resolved_addr_is_local, tlschan_resolved_addr_is_local_mock);

/* Install mock for connection_or_connect() */ MOCK(connection_or_connect, tlschan_connection_or_connect_mock); @@ -92,7 +92,7 @@ test_channeltls_create(void *arg) }

UNMOCK(connection_or_connect); - UNMOCK(is_local_addr); + UNMOCK(resolved_addr_is_local);

return; } @@ -116,9 +116,9 @@ test_channeltls_num_bytes_queued(void *arg) test_addr.addr.in_addr.s_addr = htonl(0x01020304);

/* For this test we always want the address to be treated as non-local */ - tlschan_local = 0; - /* Install is_local_addr() mock */ - MOCK(is_local_addr, tlschan_is_local_addr_mock); + tlschan_local = false; + /* Install resolved_addr_is_local() mock */ + MOCK(resolved_addr_is_local, tlschan_resolved_addr_is_local_mock);

/* Install mock for connection_or_connect() */ MOCK(connection_or_connect, tlschan_connection_or_connect_mock); @@ -178,7 +178,7 @@ test_channeltls_num_bytes_queued(void *arg) }

UNMOCK(connection_or_connect); - UNMOCK(is_local_addr); + UNMOCK(resolved_addr_is_local);

return; } @@ -201,9 +201,9 @@ test_channeltls_overhead_estimate(void *arg) test_addr.addr.in_addr.s_addr = htonl(0x01020304);

/* For this test we always want the address to be treated as non-local */ - tlschan_local = 0; - /* Install is_local_addr() mock */ - MOCK(is_local_addr, tlschan_is_local_addr_mock); + tlschan_local = false; + /* Install resolved_addr_is_local() mock */ + MOCK(resolved_addr_is_local, tlschan_resolved_addr_is_local_mock);

/* Install mock for connection_or_connect() */ MOCK(connection_or_connect, tlschan_connection_or_connect_mock); @@ -252,7 +252,7 @@ test_channeltls_overhead_estimate(void *arg) }

UNMOCK(connection_or_connect); - UNMOCK(is_local_addr); + UNMOCK(resolved_addr_is_local);

return; } @@ -321,8 +321,8 @@ tlschan_fake_close_method(channel_t *chan) return; }

-static int -tlschan_is_local_addr_mock(const tor_addr_t *addr) +static bool +tlschan_resolved_addr_is_local_mock(const tor_addr_t *addr) { tt_ptr_op(addr, OP_NE, NULL);