Opened 11 months ago

Last modified 3 hours ago

#24393 needs_review enhancement

Clients should check IPv4 and IPv6 subnets when choosing circuit paths

Reported by: teor Owned by: neel
Priority: Medium Milestone: Tor: 0.3.6.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: ipv6, intro, tor-dirauth, security, sybil, 034-triage-20180328, 034-removed-20180328
Cc: tyseom, neel@… Actual Points:
Parent ID: #24403 Points:
Reviewer: Sponsor:

Description

Currently, clients only check IPv4 subnets when choosing circuit paths. But they should probably also check IPv6 subnets as well.

Once #15518 is fixed, we can make them check both.

Child Tickets

TicketTypeStatusOwnerSummary
#15518defectclosedneelTor considers routers in the same IPv6 /16 to be "in the same subnet"

Change History (17)

comment:1 Changed 11 months ago by teor

Parent ID: #7193#24403

#7193 also depends on this.

comment:2 Changed 9 months ago by teor

Milestone: Tor: unspecifiedTor: 0.3.4.x-final

The 0.3.3 freeze deadline has passed, all these children of #24403 belong in 0.3.4

comment:3 Changed 7 months ago by nickm

Keywords: 034-triage-20180328 added

comment:4 Changed 7 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:5 Changed 6 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if time permits.

comment:6 Changed 8 weeks ago by neel

Cc: neel@… added
Owner: set to neel
Status: newassigned

comment:7 Changed 8 weeks ago by neel

Good catch! I have deleted the "bad" commits, and have pushed the updated code.

comment:8 in reply to:  7 Changed 8 weeks ago by teor

Replying to neel:

Good catch! I have deleted the "bad" commits, and have pushed the updated code.

Did you mean to comment on this ticket?

If so, which branch did you push to?

comment:9 Changed 8 weeks ago by neel

Well, this patch has not been started. The comment I posted was meant for #23588.

comment:10 Changed 8 weeks ago by neel

Owner: neel deleted

comment:11 Changed 8 weeks ago by neel

Status: assignednew

comment:12 Changed 6 weeks ago by neel

Owner: set to neel
Status: newassigned

Can I assume this bug is fixed as #15518 is committed already? If not, where in the codebase should I look?

comment:13 in reply to:  12 Changed 6 weeks ago by teor

Replying to neel:

Can I assume this bug is fixed as #15518 is committed already? If not, where in the codebase should I look?

You should look for places where addrs_in_same_network_family() is called.
Last time I looked, it was only called on IPv4 addresses.
We also need to check IPv6 addresses, if both nodes have them.

comment:15 Changed 5 weeks ago by neel

Status: assignedneeds_review

comment:16 Changed 5 weeks ago by teor

Milestone: Tor: unspecifiedTor: 0.3.6.x-final
Status: needs_reviewneeds_revision
Type: defectenhancement

This is new IPv6 code, and the code freeze for 0.3.5 is on Friday. Let's put it in 0.3.6.

I think the changes file for this ticket has the wrong release. 0.3.5.1-alpha isn't out yet.
Maybe this is really a new feature?

Can you add some unit tests?
They can be similar to the tests in #15518.

comment:17 Changed 3 hours ago by neel

Status: needs_revisionneeds_review

I have added tests for nodes_in_same_family(). I don't believe that tests for nodelist_add_node_and_family() are feasible because of smartlists.

Setting this as needs review.

Note: See TracTickets for help on using tickets.