Clients should check IPv4 and IPv6 subnets when choosing circuit paths

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

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

changed milestone to %Tor: 0.4.0.x-final

added 034-removed-20180328 034-triage-20180328 component::core tor/tor intro ipv6 milestone::Tor: 0.4.0.x-final owner::neel parent::24403 priority::medium resolution::implemented reviewer::dgoulet security severity::normal status::closed sybil tor-dirauth type::enhancement labels

#7193 (moved) also depends on this.

Trac:
Parent: #7193 (moved) to #24403 (moved)

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

Trac:
Milestone: Tor: unspecified to Tor: 0.3.4.x-final

Trac:
Keywords: tor-dirauth security sybil deleted, security, tor-dirauth, sybil, 034-triage-20180328 added

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

Trac:
Keywords: N/A deleted, 034-removed-20180328 added

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

Trac:
Milestone: Tor: 0.3.4.x-final to Tor: unspecified

Trac:
Owner: N/A to neel
Status: new to assigned
Cc: tyseom to tyseom, neel@neelc.org

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

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?

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

Trac:
Owner: neel to N/A

Trac:
Status: assigned to new

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

Trac:
Owner: N/A to neel
Status: new to assigned

Replying to neel:

Can I assume this bug is fixed as #15518 (moved) 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.

PR is here: https://github.com/torproject/tor/pull/308

Trac:
Status: assigned to needs_review

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 (moved).

Trac:
Milestone: Tor: unspecified to Tor: 0.3.6.x-final
Type: defect to enhancement
Status: needs_review to needs_revision

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.

Trac:
Status: needs_revision to needs_review

Trac:
Reviewer: N/A to catalyst

Trac:
Reviewer: catalyst to dgoulet

I've created a more recent branch with this. The branch is was based on has a unit test issue with expensive hardening on. I pushed a fixup commit to address the make check-spaces issue.

Branch: ticket24393_036_01 PR: https://github.com/torproject/tor/pull/460

Once CI passes, this lgtm; Needs an autosquash!

Trac:
Status: needs_review to merge_ready

Squashed and merged to master. Thanks!

Trac:
Status: merge_ready to closed
Resolution: N/A to implemented

Tor 0.3.6.x has been renamed to 0.4.0.x.

Trac:
Milestone: Tor: 0.3.6.x-final to Tor: 0.4.0.x-final

closed

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