[tor-bugs] #23827 [Core Tor/Tor]: Clients/Relays: Use IPv6 Addresses from microdesc consensus

[Tor Bug Tracker & Wiki]

#23827: Clients/Relays: Use IPv6 Addresses from microdesc consensus
-----------------------------------+------------------------------------
 Reporter:  teor                   |          Owner:  teor
     Type:  enhancement            |         Status:  needs_review
 Priority:  Medium                 |      Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor           |        Version:
 Severity:  Normal                 |     Resolution:
 Keywords:  ipv6, review-group-27  |  Actual Points:  1
Parent ID:  #20916                 |         Points:  0.5
 Reviewer:  dgoulet                |        Sponsor:  SponsorV-can
-----------------------------------+------------------------------------
Changes (by teor):

 * status:  needs_revision => needs_review
 * sponsor:   => SponsorV-can


Comment:

 Replying to [comment:10 dgoulet]:
 > * (Nitpick) For some more safety, I would `const` this variable:
 >
 > {{{
 > networkstatus_t *cons = networkstatus_get_reasonably_live_consensus(
 > }}}

 Fixed in
 {{{
 [bug23827_tree 35f350a8f7] fixup! fixup! Add
 networkstatus_consensus_has_ipv6() and unit tests
 }}}

 > Apart from that lgtm;
 >
 > Although a question, `node_awaiting_ipv6()` doesn't check for the
 descriptor (`node->ri`) anymore and the only trace for a reason of this I
 can find is in the commit message:
 >
 > {{{
 >     If node_is_a_configured_bridge(), stop waiting for its IPv6 address
 in
 >     a microdescriptor. The previous check for ri was inaccurate.
 > }}}

 There is also the changes file entry for that commit:

 {{{
     - Make IPv6-only clients wait for microdescs for relays, even if we
 were
       previously using descriptors (or were using them as a bridge) and
 have
       a cached descriptor for them.
 }}}

 > So was it specifically done before because of bridges and turned out to
 be inaccurate?

 It was specifically done because of bridges, but it would also stop
 waiting for mds if we had cached descriptors. It works in 0.3.2 and
 earlier, because we always check ri, rs, and md. But then we would
 sometimes chose the wrong address. (Remember that bridge bug?)

 It won't work after we merge #23975, because we check which directory
 documents we can use, then look for the address in them.

 > Or because now `node_has_ipv6_addr()` before takes care of looking at
 the descriptor if one?

 `node_has_ipv6_addr()` has similar issues with looking everywhere, and
 choosing the wrong address.

 After we merge #23975, `node_has_ipv6_addr()` will also check which
 directory documents we can use, then look for the address in them.

 I edited the comment to explain better in:
 {{{
 [bug23827_tree 270a604297] fixup! squash! Stop waiting for microdescs if
 the consensus supports IPv6 ORPorts
 }}}

 I also updated the commit message when I rebased and squashed.
 The rebase was complex, because we squashed my branch bug23826-23828
 before merging it.

 Please merge my branch bug23827-v2, which has the same code as
 bug23827_tree.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/23827#comment:11>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online