[tor-bugs] #17604 [Tor]: Try to use only one canonical connection

[Tor Bug Tracker & Wiki]

#17604: Try to use only one canonical connection
-----------------------+------------------------------
 Reporter:  mikeperry  |          Owner:  mikeperry
     Type:  defect     |         Status:  needs_review
 Priority:  Medium     |      Milestone:
Component:  Tor        |        Version:
 Severity:  Normal     |     Resolution:
 Keywords:             |  Actual Points:
Parent ID:  #16861     |         Points:
  Sponsor:             |
-----------------------+------------------------------

Comment (by mikeperry):

 Replying to [comment:5 teor]:
 > This patch looks good overall.
 >
 > Just a few questions:
 >
 > channel_check_for_duplicates() says:
 > {{{
 > This function is similar to connection_or_set_bad_connections(),
 > and probably could be adapted to replace it, if it was modified to
 actually
 > take action on any of these connections.
 > }}}
 > Are we waiting to see what it logs before using it to replace
 connection_or_set_bad_connections()?

 I think so. That or a switch to a datagram transport, or some other wider
 effort to completely remove all of the connection_or layer.

 > Replying to [comment:4 mikeperry]:
 > > Oh, it also turns out that we're already vulnerable to the attack in
 comment:1, because all a rogue node has to do is list its rogue address in
 its NETINFO cells, and it gets marked canonical. It is only non-canonical
 connections that get their real_addr checked by
 channel_tls_matches_target_method(). Do we care about that? I did not
 change that behavior in this patch at all. I merely noted the issue with
 an XXX in the source.
 >
 > Can we check real_addr for all connections?
 > Will it take a long time to code up?
 > Does it impact performance?

 I think the main problem is that if we don't allow this netinfo mechanism,
 we need to find a different way for IPv6 connections to become
 'canonical'. If we do care about this (and maybe we do), I think it should
 probably be a different ticket to change this behavior. The right way to
 do it probably means checking that the netinfo cell stuff matches at least
 *something* from the descriptor. But maybe that will have other issues?
 Nick or Andrea probably need to chime in on that topic.

 > And a nitpick:
 >
 > In check_canonical_channels_callback:
 > * I think public_server_mode(options) is the standard way of saying
 `!options->BridgeRelay && server_mode(options)`. I think they do the same
 thing, but it might be worth checking.

 Fixed in another fixup commit.

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