Re: [tor-bugs] #23577 [Core Tor/Tor]: Add rendezvous point IPv6 address to client introduce cells

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#23577: Add rendezvous point IPv6 address to client introduce cells
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  (none)
     Type:  enhancement                          |         Status:
                                                 |  reopened
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  prop224, tor-hs, single-onion,       |  Actual Points:  2
  ipv6, review-group-25                          |
Parent ID:  #23493                               |         Points:  1
 Reviewer:  dgoulet                              |        Sponsor:
                                                 |  SponsorV-can
-------------------------------------------------+-------------------------

Comment (by teor):

 Replying to [comment:23 dgoulet]:
 > The patch to `get_lspecs_from_node()` for IPv6 support doesn't add the
 link specifier to the list so this it is just memleaking and the client is
 actually not sending it to the service as it is right now. This needs to
 be added:
 >
 > {{{
 >     if (node_has_ipv6_orport(node)) {
 >       [...]
 >       smartlist_add(lspecs, ls);
 >     }
 > }}}
 >
 > Second thing I'm a little bit worried about is this:
 >
 > {{{
 > -  setup_introduce1_data(ip, rend_circ->build_state->chosen_exit,
 > -                        subcredential, &intro1_data);
 > -  /* If we didn't get any link specifiers, it's because our extend info
 was
 > +  const node_t *exit_node =
 build_state_get_exit_node(rend_circ->build_state);
 > +  setup_introduce1_data(ip, exit_node, subcredential, &intro1_data);
 > }}}
 >
 > A client rendezvous circuit is often cannibalized from an existing
 circuit so I *think* it could be possible that we pick the rend circuit
 and then from that point up to using it for rendezvous, the `node_t` could
 disappear from our state.

 Cannibalisation always adds an extra node for the end point ("exit"), so
 this can't happen the way you describe.
 But it can happen that we add the end point, launch or extend the circuit,
 get a new consensus and delete the node, and then the circuit completes.
 This should be a very rare race condition.

 I think the correct thing to do here is fail the circuit if the returned
 node is NULL.
 We shouldn't be using it if it has been remove from the consensus.
 And falling back to the extend info is complex and leaks info about which
 consensus we have (if there is an IPv6 address, you use a node, if not,
 you used an extend info).

 > Thus, I do think we should check `exit_node` here else it will lead to a
 strong `tor_assert()`.

 Yes, let's check here and fail the function.

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

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs