[tor-commits] [tor/master] addr: Refactor is_local_addr() to support IPv6

dgoulet at torproject.org dgoulet at torproject.org
Wed Jun 24 17:58:40 UTC 2020


commit 2f3b4e38888116f434297fb45ac093acd2d01e55
Author: David Goulet <dgoulet at 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 at 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);
 





More information about the tor-commits mailing list