[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #23819 [Core Tor/Tor]: Support IPv6 link-local interface addresses



#23819: Support IPv6 link-local interface addresses
-----------------------------+------------------------------------
 Reporter:  Zakhar           |          Owner:  (none)
     Type:  enhancement      |         Status:  needs_revision
 Priority:  Medium           |      Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor     |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:  ipv6 link-local  |  Actual Points:  1
Parent ID:                   |         Points:  1
 Reviewer:                   |        Sponsor:
-----------------------------+------------------------------------

Comment (by teor):

 Replying to [comment:6 Zakhar]:
 > So here is a new patch.
 >
 > I hope it is more compliant with coding standards.
 >
 > == Here is what I did: ==
 > - removed a tab (spotted by make check-spaces)
 > - split the assignment in 2 lines to stick to max line length
 > - (Note since nothing is said in the "coding style" if the assignment
 operator is to be at the end of the first line, or beginning of the
 second, I tried to find other examples and followed them: assignment
 operator at the end of first line, just before LF)
 > - provided a change description, categorized as "Minor feature".
 > - The change ticket describes itself as "Initial support"... because we
 know there is more work to do on link-local addresses!

 Thanks!

 > == What I didn't do: ==
 > - Documentation: I didn't change the ''man''/documentation. Rationale:
 in the ''man'', I couldn't find any explanation on how to specify ipv6
 addresses: enclosing them with square brackets, semi colons,
 abbreviations, etc... That is because it is "the standard way" to do it,
 hence it does not need documentation. Since we follow here "the standard
 way" to specify a link-local address, I didn't feel the urge to add
 anything in the documentation.

 We recently documented the format we expect for IPv6:

 https://gitweb.torproject.org/tor.git/commit/doc/tor.1.txt?id=29e98d16d2efade887367c3c81fb0bbdeba943ef

 Please rebase this patch to master, then document the changes to the IPv6
 format.
 (We won't accept new features in 0.2.9, we are only accepting security
 bugfixes.)

 > - Code: I left the in6_full as my original patch for the moment (more
 below)
 >
 > - Indeed link-local addresses are tricky! Probably, it would be best to
 open a discussion on the mailing list about the correct way to handle
 them.

 tor-dev@xxxxxxxxxxxxxxxxxxxx is the appropriate list.

 You might also find the proposals process useful:
 https://gitweb.torproject.org/torspec.git/tree/proposals/001-process.txt

 > ...
 > - To my opinion, the proper modification would probably be to create a
 new structure instead of tor_addr_t, like: tor_local_addr_t, when we use
 an address locally (binding for instance). This tor_local_addr_t will
 definitely needs 20 bytes for ipv6, whereas the previous tor_addr_t do not
 need that since we are routing.
 > - Doing that properly would probably be a '''Major''' architecture
 change in tor: distinguishing between local stuff, and routed stuff, and
 assigning each to their own addresses.
 > - '''The question you asked''' in your code review about knowing whether
 it is a AF_INET6/full or AF_INET6/partial is only for local stuff since
 for routing there is no scope_id. For local addresses, the simpler is to
 consider they always have a scope, be it zero when not needed.

 I think using tor_local_addr_t would be a good way of making this change,
 and it makes sure the address family is specified.

 > - Also note, about this question, that you could always use in6_full
 when the family is AF_INET6, in6_full includes in6_addr, and in6_addr is
 at he same offset in the union. Thus, the other solution is to drop
 in6_addr and use in6_full instead everywhere. Doing so, the family can
 serve as a flag again, as it is best practice for unions to have a flag.

 No, this would waste too much RAM, particularly on clients. (Tor stores a
 tor_addr_t for each of 7000 relays.)

 > - '''Alternate hack:''' if bumping the tor_addr_t to 20 bytes is too
 much, and you don't want to undertake any '''Major''' architecture change,
 BSD/OSX is using a (horrible) hack to specify link_local address. Instead
 of having fe80::3%17 (17 as an example, being the interface number as seen
 by the local system), they use: fe80:17::3. The second 16bits word is used
 to store the scope_id.

 No, we don't do hacks like this. They are hard to maintain, and lead to
 subtle bugs.

 > == What is left to be done ==
 > - Print the full ipv6 link-local address including the interface in the
 message to report the status of the bind operation (currently the patch
 does not address that, and the message is printed out without the
 interface part)

 This is an important part of the patch, otherwise we can't diagnose
 errors.

 > - Test on systems other than Linux! Since getaddrinfo conforms to
 POSIX.1-2001 and POSIX.1-2008, the patch should work also on BSD and OSX.
 The Win32 documentation shows an API with the same parameters and the same
 ipv6 structure including ip6 scope_id, so I presume it might work, but I
 have no way to test that.

 I can do this for macOS, and we have some BSD folk around.

 Windows testing is hard, but we don't have that many relays or relay
 testers on Windows, so as long as it doesn't break existing addresses, it
 should be ok.

 > == Questions: (the way forward) ==
 >
 > - Do you wish I deliver also the '''alternate hack''' patch so that you
 can choose/vote the patch you prefer?

 No, it is very unlikely it would be merged.

 > - Do you wish I open another 'low priority' ticket for what is still not
 implemented with link-local (such as printing nicely the interface part of
 the address)?

 No, that's an essential part of this ticket, otherwise we can't debug
 errors.
 But you could create a git branch and put it in a separate commit.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/23819#comment:7>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs