Opened 2 years ago

Closed 2 years ago

#18348 closed defect (fixed)

Tor conflates IPv4 Dir port with IPv6 OR Port

Reported by: sysrqb Owned by:
Priority: Very High Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version: Tor: 0.2.8.1-alpha
Severity: Major Keywords:
Cc: teor Actual Points: small
Parent ID: Points: small
Reviewer: Sponsor:

Description

Since #17840 tor prefers IPv6 addresses for client connections when they're available. This is a significant improvement but is not always correct in the network as it is now. Unfortunately, this affects a relays dirconns, too. The primary problem arises when a relay attempt a descriptor upload/fetch with a directory authority with an IPv6 OR port.

Currently all configuration options allow configuring IPv6 OR ports, but none specify dir ports. When a client attempts a dir port connection, it implicitly assumes the dir port is listening on the same ip address as the OR port.

Currently most of the dir auths Dir ports are only listening on their ipv4 address, including the dir auths with ipv6 OR addresses. An easy (but not necessary correct) solution is Dir Auth Op configure their dirauths so they accept ipv6 connections on the dir port. A better solution is tor knows when a dir port is ipv4 or ipv6 and chooses the correct corresponding ip address.

Now, as a relay, in fascist_firewall_allows_dir_server() we choose the destination's ipv4 address. However, when we subsequently call directory_choose_address_routerstatus() we don't remember which address we prefer:

  } else {
    /* We use an IPv6 address if we have one and we prefer it.
     * Use the preferred address and port if they are reachable, otherwise,
     * use the alternate address and port (if any).
     */
    have_or = fascist_firewall_choose_address_rs(status,
                                                 FIREWALL_OR_CONNECTION, 0,
                                                 use_or_ap);
  }

  have_dir = fascist_firewall_choose_address_rs(status,
                                                FIREWALL_DIR_CONNECTION, 0,
                                                use_dir_ap);

Therefore directory_initiate_command_rend() uses the ipv6 address by default.

As an example (with additional debug messages):

Feb 19 16:57:33.000 [info] router_upload_dir_desc_to_dirservers: Uploading relay descriptor to directory authorities
Feb 19 16:57:33.000 [info] directory_post_to_dirservers: Uploading an extrainfo too (length 980)
Feb 19 16:57:33.000 [debug] directory_initiate_command_rend: anonymized 0, use_begindir 0.
Feb 19 16:57:33.000 [debug] directory_initiate_command_rend: Initiating server descriptor upload
Feb 19 16:57:33.000 [debug] connection_connect: Connecting to [scrubbed]:9131.
Feb 19 16:57:33.000 [debug] connection_connect_sockaddr: Connection to socket in progress (sock 32).
Feb 19 16:57:33.000 [debug] connection_add_impl: new conn type Directory, socket 32, address 128.31.0.39, n_conns 36.
Feb 19 16:57:33.000 [info] directory_post_to_dirservers: Uploading an extrainfo too (length 980)
Feb 19 16:57:33.000 [debug] directory_initiate_command_rend: anonymized 0, use_begindir 0.
Feb 19 16:57:33.000 [debug] directory_initiate_command_rend: Initiating server descriptor upload
Feb 19 16:57:33.000 [debug] connection_connect: Connecting to [scrubbed]:80.
Feb 19 16:57:33.000 [debug] connection_connect_sockaddr: Connection to socket in progress (sock 33).
Feb 19 16:57:33.000 [debug] connection_add_impl: new conn type Directory, socket 33, address 2001:858:2:2:aabb:0:563b:1526, n_conns 37.
...
Feb 19 16:57:33.000 [debug] conn_read_callback: socket 33 wants to read.
Feb 19 16:57:33.000 [debug] connection_handle_read_impl: Closing conn after error: Connection refused (61)
Feb 19 16:57:33.000 [info] connection_close_immediate: fd 33, type Directory, state connecting, 3298 bytes on outbuf.
Feb 19 16:57:33.000 [debug] conn_close_if_marked: Cleaning up connection (fd -1).
Feb 19 16:57:33.000 [info] connection_dir_request_failed: Setting dir 2001:858:2:2:aabb:0:563b:1526 as down after failed request.
Feb 19 16:57:33.000 [debug] router_set_status: Setting 86.59.21.38 as running: 0
Feb 19 16:57:33.000 [debug] router_set_status: Marking router $847B1F850344D7876491A54892F904934E4EB85D~tor26 at 86.59.21.38 as down.
Feb 19 16:57:33.000 [debug] connection_remove: removing socket -1 (type Directory), n_conns now 47

(this issue is only in master, not in any released version)

To make matters worse (and the reason I found this), eventually after most of the ipv6-enabled dir auths are marked as down due to the connection being refused, relays later get this scary thing:

Feb 19 09:26:53.000 [warn] router_picked_poor_directory_log: Bug: Firewall denied all OR and Dir addresses for all relays when searching for a directo
ry. (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug: Node search initiated by. Stack trace: (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x1157ff8 <log_backtrace+0x48> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x10af99c <hid_serv_responsible_for_desc_id+0xebc> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x10a6ae6 <router_pick_trusteddirserver+0x76> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x1126333 <directory_get_from_dirserver+0x293> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x10ad4ba <launch_descriptor_downloads+0x4ba> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x10ad27a <launch_descriptor_downloads+0x27a> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x10adbcb <update_consensus_router_descriptor_downloads+0x6cb> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x10ac856 <update_all_descriptor_downloads+0x66> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x10602c8 <directory_info_has_arrived+0x48> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x1128a40 <connection_dir_reached_eof+0x1160> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x110810b <connection_handle_read+0xb3b> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x105f044 <connection_add_impl+0x214> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x801aa538e <event_base_loop+0x81e> at /usr/local/lib/libevent-2.0.so.5 (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x1060cc5 <do_main_loop+0x5c5> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x1062fcf <tor_main+0xdf> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x105ed49 <main+0x19> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)
Feb 19 09:26:53.000 [warn] Bug:     0x105ec41 <_start+0x1a1> at /usr/local/bin/tor (on Tor 0.2.8.1-alpha-dev 1f679d4ae11cd976)

Because we already asked the useful dir auths for descriptors and those requests are still outstanding, so we don't have any viable directories remaining. (Ignore the mention of hid_serv_responsible_for_desc_id+0xbfb, it is actually router_pick_trusteddirserver_impl()).

Child Tickets

Change History (7)

comment:1 Changed 2 years ago by nickm

Priority: MediumVery High
Severity: NormalMajor

comment:2 Changed 2 years ago by sysrqb

It looks like this should do it - I'm testing it now:

diff --git a/src/or/policies.c b/src/or/policies.c
index 179230b..5b3405b 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -413,6 +413,11 @@ fascist_firewall_allows_address(const tor_addr_t *addr,
     if (tor_addr_family(addr) == AF_INET6 &&
         (!fascist_firewall_use_ipv6(options) || (pref_only && !pref_ipv6)))
       return 0;
+  } else {
+    if (tor_addr_family(addr) == AF_INET)
+      return 1;
+    return 0;
   }
 
   return addr_policy_permits_tor_addr(addr, port,

Based on the comment for fascist_firewall_allows_address(), this was the intended functionality but I don't see where this happens:

 * If we are configured as a server, ignore any address family preference and
 * just use IPv4.

comment:3 Changed 2 years ago by sysrqb

Status: newneeds_review

So, yes and no. Yes, that patch does what's expected. teor, is this what you were planning or did you have a better patch? I don't think this is completely the correct answer, but it seems good enough for now.

But, on the other hand, no, this doesn't completely solve the problem where we exhaust all the dir auths and log a warn/bug message. I'll open a different ticket for that.

The above diff is the patch for review.

comment:4 in reply to:  description Changed 2 years ago by teor

Replying to sysrqb:

Since #17840 tor prefers IPv6 addresses for client connections when they're available. This is a significant improvement but is not always correct in the network as it is now. Unfortunately, this affects a relays dirconns, too. The primary problem arises when a relay attempt a descriptor upload/fetch with a directory authority with an IPv6 OR port.

Relays should never use IPv6. If they are, it's a bug.

Currently all configuration options allow configuring IPv6 OR ports, but none specify dir ports. When a client attempts a dir port connection, it implicitly assumes the dir port is listening on the same ip address as the OR port.

This is a deliberate implementation choice - we don't have IPv6 dirport entries or data structures.

Currently most of the dir auths Dir ports are only listening on their ipv4 address, including the dir auths with ipv6 OR addresses. An easy (but not necessary correct) solution is Dir Auth Op configure their dirauths so they accept ipv6 connections on the dir port.

Opened #18350 to encourage Directory Authorities to bind their existing DirPort to IPv6.

A better solution is tor knows when a dir port is ipv4 or ipv6 and chooses the correct corresponding ip address.

I think this is #6772, but for DirPorts. Let's handle it there, as the code is very similar.

Now, as a relay, in fascist_firewall_allows_dir_server() we choose the destination's ipv4 address. However, when we subsequently call directory_choose_address_routerstatus() we don't remember which address we prefer:

  } else {
    /* We use an IPv6 address if we have one and we prefer it.
     * Use the preferred address and port if they are reachable, otherwise,
     * use the alternate address and port (if any).
     */
    have_or = fascist_firewall_choose_address_rs(status,
                                                 FIREWALL_OR_CONNECTION, 0,
                                                 use_or_ap);
  }

  have_dir = fascist_firewall_choose_address_rs(status,
                                                FIREWALL_DIR_CONNECTION, 0,
                                                use_dir_ap);

Therefore directory_initiate_command_rend() uses the ipv6 address by default.

Oh dear, there's no way that should be happening. Relays should always allow and choose IPv4 addresses.

comment:5 in reply to:  3 Changed 2 years ago by teor

Actual Points: small
Points: small
Version: Tor: unspecifiedTor: 0.2.8.1-alpha

There are two interrelated issues here:

  • enabling IPv4 on all relays wasn't working correctly
  • using IPv4/IPv6 based on the configured bridge address wasn't working correctly

Please see my branch bug18348-v2 on https://github.com/teor2345/tor.git

Relays

Replying to sysrqb:

So, yes and no. Yes, that patch does what's expected. teor, is this what you were planning or did you have a better patch? I don't think this is completely the correct answer, but it seems good enough for now.

Your patch is OK - it enables IPv4 for all relays. But we want to allow relays to use IPv6 if they configure it (in addition to IPv4).

25543387ede5a4143d9ef4fdff2b34846ca788c6 prevents relays from disabling IPv4.
a4eddfff666226014545efd6f5bf390173c0fdfa refactors the code to make it clearer, and adds comments.

But, on the other hand, no, this doesn't completely solve the problem where we exhaust all the dir auths and log a warn/bug message. I'll open a different ticket for that.

In the interim, be16c16bdaae9ac1ebddbe755236e62de9011f01 downgrades one of those warnings to info level. It's non-fatal, and may be triggered when using bridges.

Bridges

To fix the bridge issue, c281c0365482891d6c3e71f85b2a6615faa5990b redesigns the node address checks to use node_ipv6_or/dir_preferred(). The routerstatus address checks then use the node checks, and fall back to fascist_firewall_prefer_ipv6_or/dirport() if there's no node.

(I thought we were doing this already when I changed how the bridge client code sets node->ipv6_preferred. But it turns out that we weren't checking node->ipv6_preferred at all.)

The other commits are refactoring and unit tests. The unit tests are more comprehensive now, and cover fascist_firewall_choose_address_rs/node(), including all the desired bridge and relay behaviours.

comment:6 Changed 2 years ago by nickm

Looks okay to me; merged it. Please do the right thing with the child ticket, then close this one if appropriate?

comment:7 Changed 2 years ago by teor

Resolution: fixed
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.