[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