router_new_address_suggestion is not IPv6 aware

I see this in the debug log when using an IPv6 bridge. I censored the addresses but they are well-formed IPv6 addresses in the log.

Jan 08 12:49:16.000 [debug] connection_dir_client_reached_eof(): Received response from directory server '2600:3c01::...:9001': 200 "
OK" (purpose: 6)
Jan 08 12:49:16.000 [debug] router_new_address_suggestion(): Malformed X-Your-Address-Is header "2001:470::...". Ignoring.

changed milestone to %Tor: 0.2.3.x-final

added component::core tor/tor ipv6 milestone::Tor: 0.2.3.x-final owner::ln5 parent::3563 priority::low resolution::fixed status::closed tor-client type::defect version::tor 0.2.3.10-alpha labels

Trac:
Priority: minor to normal
Component: - Select a component to Tor Client

Yes, this needs fixing. I have a note about this being harmless for the current use case (but no explanation why). Thanks for creating a ticket.

Trac:
Status: new to accepted

It would be nice to have this fixed in 0.2.3.x, but it is not essetial.

Trac:
Priority: normal to minor
Milestone: N/A to Tor: 0.2.3.x-final

Trac:
Parent: N/A to #3563 (moved)

Branch bug4875 in my public repo fixes this. Tested on a client and a bridge relay.

Trac:
Status: accepted to needs_review

Note that bug4875 is based on bug4561_4.

Since #4875 (moved) depends on #4561 (moved), either #4561 (moved) needs to go into 0.2.3.x, or this needs to go into 0.2.4.x, or the two branches need to be decoupled.

Which commit is the 4875 fix? I see a couple whose commit messages are just "first take" -- are they the ones I should be looking at?

Replying to nickm:

Which commit is the 4875 fix? I see a couple whose commit messages are just "first take" -- are they the ones I should be looking at?

Please see 1ff25a3e. Sorry about the mess here.

Let me know if you think that I should backport it to pre-#4561.

re the patch contents:

• Let's just always use strlcpy, not strcpy. OpenBSD spews warnings when you use strcpy, and the performance/convenience advantage just isn't worth it.
• I don't like {AF_UNSPEC,{0}} as a literal initializer. If we want to have something like that, it ought to be a macro in address.h , so we use it everywhere and change it in one place only if the representation of tor_addr_t changes.
• It needs a real commit message and a changes file.
• I think that cherry-picking it back onto something pre-#4561 makes sense : when I tried "git cherry-pick 1ff25a3e", there was only one (trivial) conflict.
• Can you tell me a little about the "XXX ipv6" instances? It looks like part of this patch makes parts of the code work with IPv6, but other parts of the patch assume that everything is still IPv4? Or do I understand wrong?

Please see branch bug4875_2 based on master.

• Using strlcpy now.
• Created TOR_ADDR_NULL.
• changes file added and commit message improved.
• Now based on master.
• Re "XXXX ipv6", yes this patch doesn't fix everything "all the way down". Functions or parts of functions involved in this patch which still don't handle IPv6 are marked with "XXXX ipv6". For example, router_new_address_suggestion() uses resolve_my_address() which still doesn't know about IPv6.

Looks good. will review again in the morning, then probably merge.

Still looks good; merged and closing.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Trac:
Keywords: ipv6 deleted, ipv6 tor-client added

Trac:
Component: Tor Client to Tor

closed

moved to tpo/core/tor#4875 (closed)