Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#6364 closed enhancement (implemented)

Make sure the NETINFO cells include and use IPv6 addresses correctly

Reported by: ln5 Owned by: ln5
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: SponsorG20120930 ipv6 tor-client
Cc: Actual Points:
Parent ID: #4564 Points:
Reviewer: Sponsor:

Description


Child Tickets

Change History (11)

comment:1 Changed 7 years ago by ln5

Component: - Select a componentTor Client

comment:2 Changed 7 years ago by ln5

Milestone: Sponsor G: September 30, 2012
Type: defectenhancement

comment:3 Changed 7 years ago by karsten

Keywords: SponsorG20120930 added
Milestone: Sponsor G: September 30, 2012

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

comment:4 Changed 7 years ago by ln5

Keywords: ipv6 added
Status: newneeds_review

Branch bug6364 in my public repo contains an implementation for this.

We still log IPv4 addresses only. Should we print both addresses for
relays with two? I'm not sure.

comment:5 Changed 7 years ago by nickm

Looks ok. Shall I merge?

Is there a separate ticket for using NETINFO cells correctly? If so, the code in the NETINFO that fetches my_appent_addr is IPv4-only. (On the other hand, my_apparent_addr just isn't used.)

When you talk about printing both addresses, do you mean in command_process_netinfo_cell() or somewhere else?

comment:6 in reply to:  5 ; Changed 7 years ago by ln5

Replying to nickm:

Looks ok. Shall I merge?

Is there a separate ticket for using NETINFO cells correctly? If so, the code in the NETINFO that fetches my_appent_addr is IPv4-only. (On the other hand, my_apparent_addr just isn't used.)

There isn't, as far as i know. This branch is supposed to do that too.

Right, I looked at this line in tor-spec.txt and just let it be as it
was: "[As of 0.2.3.1-alpha, nodes use neither of these values.]"

Want me to either remove the IPv4 address fetching, add IPv6 fetching
or just keep ignoring it? Happy to do it either way.

When you talk about printing both addresses, do you mean in command_process_netinfo_cell() or somewhere else?

I mean in command_process_netinfo_cell().

comment:7 in reply to:  6 ; Changed 7 years ago by nickm

Replying to ln5:

Replying to nickm:

Looks ok. Shall I merge?

Is there a separate ticket for using NETINFO cells correctly? If so, the code in the NETINFO that fetches my_appent_addr is IPv4-only. (On the other hand, my_apparent_addr just isn't used.)

There isn't, as far as i know. This branch is supposed to do that too.

Right, I looked at this line in tor-spec.txt and just let it be as it
was: "[As of 0.2.3.1-alpha, nodes use neither of these values.]"

Want me to either remove the IPv4 address fetching, add IPv6 fetching
or just keep ignoring it? Happy to do it either way.

Either add IPv6 fetch; remove IPv4 fetch, or add a comment to the IPv4 fetch saying that it's broken to be IPv4-only and we need to fix that if we ever start using the info.

When you talk about printing both addresses, do you mean in command_process_netinfo_cell() or somewhere else?

I mean in command_process_netinfo_cell().

So, the only one we log is conn->_base.address, which I *think* is the one that means the "real" address of this router, yeah? If so that's ok.

comment:8 in reply to:  7 Changed 7 years ago by ln5

Replying to nickm:

Either add IPv6 fetch; remove IPv4 fetch, or add a comment to the IPv4 fetch saying that it's broken to be IPv4-only and we need to fix that if we ever start using the info.

Added IPv6 fetch, please see newly updated branch.

Is the cast to const char* safe?

When you talk about printing both addresses, do you mean in command_process_netinfo_cell() or somewhere else?

I mean in command_process_netinfo_cell().

So, the only one we log is conn->_base.address, which I *think* is the one that means the "real" address of this router, yeah? If so that's ok.

Yes, you're right. This is actually the other end of the
connection. I've seen IPv6 addresses in there. We're fine then.

comment:9 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-final
Resolution: implemented
Status: needs_reviewclosed

Okay, still seems okay. Merging.

comment:10 Changed 7 years ago by nickm

Keywords: tor-client added

comment:11 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.