ClientPreferIPv6ORPort not honoured when running with bridges
Since get_configured_bridge_by_routerinfo() returns the same bridge regardless of which routerinfo we pass to it, the first found (seemingly the one configured first in the config file).
One effect of this is that rewrite_node_address_for_bridge() will prefer whatever is listed first. Looking at ClientPreferIPv6 wouldn't make sense here since we come here once (only one routerinfo is received), unless we start calling it for every configured bridge. That doesn't seem like the right thing though.
Note that the idea of having ipv6_preferred as a property of the node_t is that we'd like to be able to change its value, per relay, when we learn about how we have been in touch with the relay.
Activity
changed milestone to %Tor: 0.2.4.x-final
Also, in proposed patch set for #5535 (moved) we never reset node_t->ipv6_preferred once it's been set (in rewrite_node_address_for_bridge()).
Replying to ln5:
Since get_configured_bridge_by_routerinfo() returns the same bridge regardless of which routerinfo we pass to it, the first found (seemingly the one configured first in the config file).
Please ignore that sentence. It's both confusing and pointless.
In 0.2.4.x, clients running with bridges need to set both ClientUseIPv6 and ClientPreferIPv6ORPort in order to connect over IPv6. This is regression from 0.2.3.x.
Based on the assumption that we want i) a Bridge line with an IPv6 address to imply ClientUseIPv6=1 and b) a Bridge line with an IPv6 address for a given bridge listed before an IPv4 address for the same bridge to imply ClientPreferIPv6ORPort=1, I suggest two changes:
-
Set and reset node_t.ipv6_preferred in rewrite_node_address_for_bridge() based on address family of the bridge entry (i.e. first configured IP address matching newly arrived router) and the existence of an IPv6 address in the routerinfo.
-
have node_get_pref_ipv6_orport() return an IPv6 address when UseBridges is set, not only when ClientUseIPv6 is set
Please review branch bug6757 in my public repo.
Trac:
Keywords: N/A deleted, ipv6 added
Status: new to needs_review
Version: N/A to Tor: 0.2.4.1-alpha
Component: Tor Bridge to Tor Client-
Needs more careful review than I can give it in the time before I need to head to the airport today.
Replying to ln5:
Based on the assumption that we want i) a Bridge line with an IPv6 address to imply ClientUseIPv6=1 and b) a Bridge line with an IPv6 address for a given bridge listed before an IPv4 address for the same bridge to imply ClientPreferIPv6ORPort=1, I suggest two changes:
Hm. I'm hoping that ClientUseIPv6 and ClientPreferIPv6ORPort in this context are meant bridge-specific, right? That is, if a bridge has an Ipv6 address listed first, we don't necessarily want to prefer IPv6 for all bridges.
Replying to nickm:
Replying to ln5:
Based on the assumption that we want i) a Bridge line with an IPv6 address to imply ClientUseIPv6=1 and b) a Bridge line with an IPv6 address for a given bridge listed before an IPv4 address for the same bridge to imply ClientPreferIPv6ORPort=1, I suggest two changes:
Hm. I'm hoping that ClientUseIPv6 and ClientPreferIPv6ORPort in this context are meant bridge-specific, right? That is, if a bridge has an Ipv6 address listed first, we don't necessarily want to prefer IPv6 for all bridges.
ClientUseIPv6 is not bridge-specific. It's meant to mean that a client might use IPv6 for connecting to first hop, be it a bridge or not. I don't think that a user should have to set that if she's configured an IPv6 address as a bridge.
With this patch, ClientPreferIPv6ORPort is not considered at all when it comes to picking address family for a bridge with both.
We might want to backport this part to 0.2.3.x:
node->ipv6_preferred = (tor_addr_family(&bridge->addr) == AF_INET6 && !tor_addr_is_null(&node->ri->ipv6_addr));since 0.2.3.x does
ri->ipv6_preferred = tor_addr_family(&bridge->addr) == AF_INET6;and will thus potentially prefer an address which is not in the descriptor of the bridge. I think that this can happen, f.ex. if the bridge withdraws its IPv6 address and uploads a new descriptor (matched on digest in get_configured_bridge_by_orports_digest()) and probably more.
Regardless, it seems like the safe and sane thing to do.
This patch has been tested this patch on a local test network with an assert triggering if we ever call node_ipv6_preferred() if we're a server.
I think this can be merged to master.
If you think that the part discussed in the previous comment should go into 0.2.3.x, please let me know and I'll prepare a patch and a changes file for 0.2.3.x.
Trac:
Status: needs_revision to needs_review-
Merged into master.
If you think that the part discussed in the previous comment should go into 0.2.3.x, please let me know and I'll prepare a patch and a changes file for 0.2.3.x.
I think it's okay to leave this one unfixed in 0.2.3; people who want to use a lot of IPv6 stuff should be running 0.2.4. Or is the impact worse than I'm seeing?
Replying to nickm:
Merged into master.
If you think that the part discussed in the previous comment should go into 0.2.3.x, please let me know and I'll prepare a patch and a changes file for 0.2.3.x.
I think it's okay to leave this one unfixed in 0.2.3; people who want to use a lot of IPv6 stuff should be running 0.2.4. Or is the impact worse than I'm seeing?
It's not too bad. Let's leave it.
Thanks.
Trac:
Status: needs_review to closed
Resolution: N/A to fixedmoved to tpo/core/tor#6757 (closed)
