[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 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()?

 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?

 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.

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