Make node_get_pref_ipv6_orport() check addresses in the right order

node_get_pref_ipv6_orport() and node_get_prim_orport() check addresses in the following order:

• routerinfo / descriptor (modified by rewrite_bridge_address_for_node())
• routerstatus / consensus
• microdesc

Clients, bridge clients, and relays usually get the correct address from node_get_pref_ipv6_orport() and node_get_prim_orport(), whether they are using microdescriptors or not.

But there are a few cases when this breaks:

• bridge clients should only check routerinfo for configured bridges
• all clients and relays that use microdescs should never check full descriptors (and vice versa, except for the bridge case)
• all clients and relays that use full descriptors should ignore the IPv6 address in the descriptor
• all clients and relays that use microdescriptors should ignore the IPv6 address in the microdescriptor, when using a consensus method that puts IPv6 addresses in the microdesc consensus, otherwise they should use the microdescriptor

changed milestone to %Tor: unspecified

added 034-removed-20180328 034-triage-20180328 actualpoints::2 component::core tor/tor ipv6 milestone::Tor: unspecified parent::27248 points::1 priority::medium review-group-29 reviewer::nickm severity::normal status::new type::defect version::tor 0.2.8.1-alpha labels

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

These are all mine

These have code in bug20916, but it needs to be chopped out and refactored after the other children of #20916 (moved) merge

Trac:
Status: assigned to needs_revision

I wrote a draft branch bug23975_tree, which uses the new consensus methods from bug23826-23828.

It still needs revision to fix some unit test failures, and it needs a changes file.

This branch also obsoletes ReachableDirAddresses and removes all IPv6 DirPort code. Removing it was easier than trying to fix it.

Trac:
Actualpoints: N/A to 1
Keywords: regression deleted, N/A added

We should probably fix the onion key functions as well when we do this, and any other functions that implement a similar pattern of directory document checks. This depends on #23966 (moved).

Maybe there's even a way to make these generic?

This branch rapidly got out of control. I want to split it into:

• a branch that avoids using microdesc IPv6 addresses when the consensus method is high enough
• a branch that gets rid of IPv6 DirPorts
• a branch that deals with the bridge stuff
• a branch that does any other stuff

Making tor stop looking at unused directory documents is now #24731 (moved). This includes the bridge routerinfo checks.

Removing unused IPv6 DirPort code is now #24732 (moved).

Trac:
Sponsor: N/A to SponsorV-can
Summary: Make node_get_*_orport() check addresses in the right order to Make node_get_pref_ipv6_orport() check addresses in the right order
Actualpoints: 1 to 2

Please see my branch bug23975-redux.

It makes sure we ignore microdescs when the consensus has IPv6 ORPorts. And it refactors some of the address functions so they depend on the revised code.

It also contains the cherry-picked commit from #24736 (moved) branch bug24736_028, because it is required for the unit tests to pass.

Trac:
Status: needs_revision to needs_review

Trac:
Keywords: N/A deleted, review-group-28 added

Looks plausible! It's good to see so much complexity removed here.

I think we already merged #24736 (moved), so I'll assume we rebase before the merge to drop 189127acf2014907eac747e25d32668741dc2c5e ?

Let's add a comment to document SET_IPV6_AP. We should especially document that it conditionally returns, possibly by renaming it: macros that alter control flow should make it obvious.

Other than that, I think I like this. How is the test coverage for all the functions in policies.c that we modify?

Edit: fix ticket number (teor)

Trac:
Status: needs_review to needs_revision
Reviewer: N/A to nickm

Trac:
Keywords: review-group-28 deleted, review-group-29 added

The 0.3.3 feature freeze deadline has passed. We can do the remaining work in this ticket in 0.3.4. (The old code works, so deferring it is ok.)

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

Trac:
Keywords: N/A deleted, 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 needs_revision, tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if somebody does the necessary revision.

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

When #25691 (moved) merges, we should rebase this branch on top of it, and use the new has_preferred_descriptor() function. (Or perhaps create a similar function that actually returns some of the fields.)

Not on any roadmap yet.

Trac:
Status: needs_revision to assigned
Owner: teor to N/A

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

#27248 (moved) would fix this, and many similar issues.

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

Removing sponsor V as we do not have more time to include this tickets in the sponsor.

Removing sponsor from tickets that we do not have time to fit in the remain of this sponsorship.

Trac:
Sponsor: SponsorV-can to N/A

Change tickets that are assigned to nobody to "new".

Trac:
Status: assigned to new

changed time estimate to 8h

added 16h of time spent

mentioned in issue #24001 (moved)

mentioned in issue #24535 (moved)

mentioned in issue #24731 (moved)

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

mentioned in issue tpo/core/tor#24535 (closed)