[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #17604 [Tor]: Try to use only one canonical connection
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs