Opened 8 years ago

Closed 7 years ago

#5547 closed project (fixed)

Add support for resolving destination names to IPv6 and exiting to IPv6 destinations

Reported by: karsten Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: SponsorF20121101 needs-proposal tor-relay
Cc: nickm, karsten, ln5 Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Item 24 from org/sponsors/SponsorF/Year2 is: "Support for resolving destination names to IPv6 and exiting to IPv6 destinations."

Summarizing phase 5 of the IPv6 roadmap, this deliverable consists of the following substeps:

  • We'll need to get our DNS resolver to support IPv6 addresses, and our clients to decide which address to report to the client and which to use.
  • Transport IPv6 traffic and exit to IPv6 servers. The issues to solve here are exit policies; formulating an approach similar to the notion of topologically close in IPv4 (same /16) to IPv6, unless it doesn't make sense; and implementing the specified enhancements to RELAY_BEGIN cells from tor-spec.
  • We need to extend TorDNSEL/TorBEL and the part of ExoneraTor that processes the TorDNSEL/TorBEL output.
  • We also need to update VisiTor to handle IPv6 addresses in web server logs and compare them to exit lists. (There are zero known VisiTor users, so this should be considered optional.)

Anything else we need to do here?

Scheduling this ticket for November, not for July, because we'll be busy making clients talk to private and public bridges (phases 3 and 4 of the IPv4 roadmap) until June.

Child Tickets

Change History (25)

comment:1 Changed 8 years ago by ln5

Cc: karsten added; ln5 removed
Owner: set to ln5
Status: newassigned

Reassign to me after meeting in #tor-dev.

comment:2 Changed 8 years ago by karsten

Adding a few more notes from the #tor-dev meeting:

  • Linus says that exiting to IPv6 is orthogonal to the IPv6 work we're doing for sponsor G until June. But the November milestone is still the better choice for this deliverable, because we can't do all the IPv6 work at once. He says he can work on it after sponsor G.
  • Next steps are to review existing proposals (which ones?) which can happen before June and to start coding which will probably happen after June. We should come up with a more precise schedule after June.

comment:3 Changed 7 years ago by karsten

Owner: changed from ln5 to nickm

comment:4 Changed 7 years ago by karsten

Keywords: SponsorF20121101 added
Milestone: Sponsor F: November 1, 2012

Switching from using milestones to keywords for sponsor deliverables. See #6365 for details.

comment:5 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-final

comment:6 Changed 7 years ago by nickm

Keywords: needs-proposal added

comment:7 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:8 Changed 7 years ago by nickm

Component: Tor RelayTor

comment:9 Changed 7 years ago by nickm

Status: assignedneeds_review

I have a branch called "ipv6_exits_WILL_BE_REBASED" at my public repository at https://gitweb.torproject.org/nickm/tor.git .

I've tested and debugged it a bit; it needs review, and it needs testing on a private network on a machine with actual ipv6 connectivity.

comment:10 Changed 7 years ago by karsten

Cc: ln5 added

Linus, can you maybe help out, either by reviewing/testing code, or by providing a machine with actual IPv6 connectivity?

comment:11 Changed 7 years ago by nickm

For now, any testing needs to be on a private network on a machine with IPv6, since this requires a new consensus method from authorities to support it.

To test, make sure any exit relay that needs to have IPv6 has the "IPv6Exit" option defined. For now, clients can only get IPv6 when they're using SOCKS5 on a SocksPort with the "IPv6Traffic" flag set on it.

comment:12 in reply to:  11 Changed 7 years ago by ln5

Replying to nickm:

For now, any testing needs to be on a private network on a machine with IPv6, since this requires a new consensus method from authorities to support it.

The testing network described in
https://trac.torproject.org/projects/tor/wiki/org/roadmaps/Tor/IPv6/PrivateIPv6TestingNetwork
was set up for the exact same reason and can be revived. Not earlier than Monday though.

comment:13 Changed 7 years ago by andrea

[Beginning code review of ipv6_exits_WILL_BE_REBASED branch now]

In d39684896e93aefca1d1c738556c2943eae7ed29, it doesn't look like the cache actually becomes per-circuit, just that it now passes a circuit parameter around. This is intentional to allow for future implementation, I presume?

comment:14 Changed 7 years ago by andrea

9a7d1c27432f04bab21b0cbb5ddfc41bc53d8096 looks good and I agree that having
parsed-cell data structures like this is better than the current ad-hocery,
but I don't quite see the connection to IPv6. Might have made more sense in
its own branch. I presume from this that tor_addr_port_split() already
parses IPv6 addresses?

comment:15 Changed 7 years ago by andrea

In 2245ef768094646e22c01caa4420f49fcf6c432e, where does/will the
TAPMP_EXTENDED_STAR flag come from? It looks like the new flags
argument to tor_addr_parse_mask_ports() is always being set to 0
so far. Also, I think comments should be added to the default
config file documenting this syntax; it's convenient but strikes
me as quite counterintuitive, in that my brain wants to read things
like '*4' and '*6' as regular expressions.

comment:16 Changed 7 years ago by andrea

I know this is a bit of an abstract concern, since we're unlikely to be
worried about any protocols other than IPv4 and IPv6 at any point in
the near future, but it bothers me a bit that we now have two protocols
and in some places we're dealing with them by ad-hoc code that effectively
embeds knowledge of what protocols we support in a hardwired way. Things
like the expansion of AF_UNSPEC into AF_INET and AF_INET6 wildcards in
policy_expand_unspec() of commit 8e33d77ee1cc73fc5638caff886d16a1a7779646
feel like they should be more table-driven or something, I guess.
Similar comments for exit_policy_remove_redundancies() in that commit.

comment:17 Changed 7 years ago by andrea

The rest of it all looks okay to me, assuming that it does indeed work. One thing that concerns me, though, is that it seems to only be dealing with *exiting to* IPv6 destinations and clients passing IPv6 addresses into the SOCKS5 port. I can think of two other 'easy' cases I didn't see handled (correct me if I'm wrong): transparent proxying of IPv6 clients, and hidden service on an IPv6 address.

comment:18 Changed 7 years ago by andrea

Then I can think of one harder one: a multiprotocol network where relays are allowed to support only a subset of all protocols means the connectivity graph among relays is no longer complete. This complicates path selection. I didn't see anything here about that. We should be able to handle stuff like, for example, an IPv6 only client and an IPv4 only exit, but the client can still use that exit because at least one of the entry guard or the middle node for the circuit has both IPv4 and IPv6 support.

comment:19 Changed 7 years ago by andrea

I think that's all I have now.

comment:20 Changed 7 years ago by ln5

router_compare_to_my_exit_policy() fails unless compare_tor_addr_to_short_policy() returns ADDR_POLICY_ACCEPTED which it doesn't. It may return ADDR_POLICY_PROBABLY_ACCEPTED though, in which case exit to an IPv6 service actually does work. Yay.

I just learned about this "probably" business and don't know the correct way to handle this yet. Might be a hint for someone else though.

comment:21 Changed 7 years ago by ln5

Example of how to make it work (not correct!):

@@ -1387,7 +1393,7 @@ router_compare_to_my_exit_policy(const tor_addr_t *addr, uint16_t port)
     return get_options()->IPv6Exit &&
       desc_routerinfo->ipv6_exit_policy &&
       compare_tor_addr_to_short_policy(addr, port,
-               desc_routerinfo->ipv6_exit_policy) != ADDR_POLICY_ACCEPTED;
+               desc_routerinfo->ipv6_exit_policy) != ADDR_POLICY_PROBABLY_ACCEPTED;
   } else {
     return -1;
   }

comment:22 in reply to:  21 Changed 7 years ago by nickm

Replying to ln5:

Example of how to make it work (not correct!):

Yeah; that shouldn't actually be looking at our ipv6_exit_policy at all. If we do, then we'll be ignoring our actual IPv6 exit policy. I uploaded a different fix for this, to test.

comment:23 Changed 7 years ago by nickm

Acting on Andrea's reviews:

In d39684896e93aefca1d1c738556c2943eae7ed29, it doesn't look like the cache actually becomes per-circuit, just that it now passes a circuit parameter around. This is intentional to allow for future implementation, I presume?

Yes, that's right.

9a7d1c27432f04bab21b0cbb5ddfc41bc53d8096 looks good and I agree that having parsed-cell data structures like this is better than the current ad-hocery, but I don't quite see the connection to IPv6. Might have made more sense in its own branch. I presume from this that tor_addr_port_split() already parses IPv6 addresses?

The rationale for doing this on this branch was that BEGIN cells needed to begin taking a new 'flags' argument, and I wasn't confident in my ability to add it correctly without screwing anything up.

In 2245ef768094646e22c01caa4420f49fcf6c432e, where does/will the TAPMP_EXTENDED_STAR flag come from? It looks like the new flags argument to tor_addr_parse_mask_ports() is always being set to 0 so far. Also, I think comments should be added to the default config file documenting this syntax; it's convenient but strikes me as quite counterintuitive, in that my brain wants to read things like '*4' and '*6' as regular expressions.

I added it to router_parse_addr_policy_item_from_string in 8e33d77e. Editing the default config file is something we don't want to do too often, since it messes with debian packages, but we should do so when we're close to an 0.2.4.x-beta or -rc. I'll add a ticket to that effect.

It's not too late to come up with a better syntax! Is there anything you like more?

I know this is a bit of an abstract concern, since we're unlikely to be worried about any protocols other than IPv4 and IPv6 at any point in the near future, but it bothers me a bit that we now have two protocols and in some places we're dealing with them by ad-hoc code that effectively embeds knowledge of what protocols we support in a hardwired way. Things like the expansion of AF_UNSPEC into AF_INET and AF_INET6 wildcards in policy_expand_unspec() of commit 8e33d77ee1cc73fc5638caff886d16a1a7779646 feel like they should be more table-driven or something, I guess. Similar comments for exit_policy_remove_redundancies() in that commit.

I don't agree here; if we need to add IPv7 support, we will have plenty of warning, and the likelihood of an IPv7 hitting in our lifetime seems a little low. Although from an abstract POV I agree that "there are only 3 numbers in cs: 0, 1, and infinity", I think that this number is so likely to remain 2 that it is going to be safe to keep it.

I can think of two other 'easy' cases I didn't see handled (correct me if I'm wrong): transparent proxying of IPv6 clients, and hidden service on an IPv6 address.

Then I can think of one harder one: a multiprotocol network where relays are allowed to support only a subset of all protocols means the connectivity graph among relays is no longer complete

I think transparent proxying of IPv6 clients will be trivial to add after the nov15 deadline; it isn't much more code. Hidden service to an IPv6 address is orthogonal to this, but doable. A network where some relays have no IPv4 address has the problem you mention where the topology of the network would change; it's something to contemplate, but also orthogonal IMO.

comment:24 Changed 7 years ago by nickm

I fixed a bunch more issues on ipv6_exits_WILL_BE_REBASED, and (as promised) made a new squashed, rebased "ipv6_exits" branch.

So far the only difference in ipv6_exits is that I'm adding a changes file and a little documentation.

comment:25 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged. I opened other bugs for the pending issues, I believe.

Note: See TracTickets for help on using tickets.