Opened 3 years ago

Closed 3 years ago

#21585 closed defect (implemented)

Check code that uses consensus membership to find clients

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: triaged-out-20170308, tor-relay is-client refactor client-detection technical-debt
Cc: Actual Points:
Parent ID: #22805 Points: 1
Reviewer: Sponsor:


In #21406, we identify clients by checking peer authentication information, as CREATE_FAST has been phased out.

Most code that needs to know if the other end of a connection is a client uses membership in the consensus, but this is unreliable, as relays drop out of the consensus.

So we should check code that uses node_t to identify clients, and switch it over to using this flag instead.

This might mean creating an accessor function that takes an or_connection_t and returns whether the channel is a client.

Child Tickets

Change History (8)

comment:1 Changed 3 years ago by nickm

Keywords: triaged-out-20170308 added
Milestone: Tor: 0.3.1.x-finalTor: unspecified

Deferring all 0.3.1 tickets with status == new, owner == nobody, sponsor == nobody, points > 0.5, and priority < high.

I'd still take patches for most of these -- there's just nobody currently lined up to work on them in this timeframe.

comment:2 Changed 3 years ago by mikeperry

#16861 added such a consensus check to update the is_client flag, btw.

comment:3 Changed 3 years ago by nickm

Keywords: tor-relay is-client refactor client-detection added

comment:4 Changed 3 years ago by teor

Keywords: technical-debt added
Parent ID: #22805

This also includes the is_first_hop flag on an or_circuit_t, which is set using CREATE_FAST

Last edited 3 years ago by teor (previous) (diff)

comment:5 Changed 3 years ago by nickm

The patch in the parent ticket kills off the is_first_hop flag.

I have looked over all the uses of node_get_by_id() to make sure we aren't using it for client-testing without an appropriate wrapper [connection_or_digest_is_known_relay]. I also looked at the routerstatus-lookup functions.

There is a problem with the router_get_by_id_digest() function -- it is used too much, when some other test would be more correct.

comment:6 Changed 3 years ago by nickm

There is also a problem with connection_or_digest_is_known_relay() -- it is used redundantly with channel_is_client() in a few places.

comment:7 Changed 3 years ago by nickm

The parent ticket now includes a fixes for the router_get_by_id_digest() issues too. I've opened #23423 to track the connection_or_digest_is_known_relay() issue

comment:8 Changed 3 years ago by nickm

Resolution: implemented
Status: newclosed
Note: See TracTickets for help on using tickets.