Opened 7 years ago

Closed 2 years ago

#6354 closed enhancement (worksforme)

Tor control protocol should support pre-auth PING command to check if socket is not closed

Reported by: gsathya Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: needs-proposal tor-client
Cc: atagar Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

PING should work even w/o any auth.
More information - https://trac.torproject.org/projects/tor/ticket/6234

Child Tickets

Change History (16)

comment:1 Changed 7 years ago by gsathya

Component: TorctlTor Client

comment:2 Changed 7 years ago by gsathya

Oops i messed this up - Is this the right component? (Someone change the owner. I'm not sure how)

comment:3 Changed 7 years ago by arma

Owner: aagbsn deleted
Status: newassigned

comment:4 Changed 7 years ago by atagar

Priority: normalminor

More context: it would be nice for us to have an active method to test 'is this controller connection alive'. The only trouble with using a GETINFO query or something else is that unauthenticated controller connections detach after getting a message other than AUTHENTICATE or PROTOCOLINFO (and we can only query PROTOCOLINFO once).

Personally I've felt that this auto-detachment was a poorly done hack. As I understand, it comes from an old attack against tor where a user's browser was tricked into sending garbage along with some malicious controller commands to the local control port. This hack defeated the attack by making that garbage data disconnect the socket (probably from the days before authentication). That said, this was all before my time. :)

Lowering the priority since this is merely a nice-to-have addition, and isn't blocking anything for stem.

comment:5 Changed 7 years ago by arma

Status: assignednew
Summary: PING msg to check if socket is not closedTor control protocol should support pre-auth PING command to check if socket is not closed

I'm not sure whether this is a good idea for Tor; but I fixed up the title to be clearer about what you wanted.

comment:6 Changed 7 years ago by arma

(The reason I'm not sure we want it is because adding more pre-auth controller commands means increasing the attack surface area -- exactly the reason we added the auth stuff in the first place.)

The reason to do it on Tor's side is so each controller doesn't have to figure out how to do what you want on their own. The reason not to is because it makes Tor more complex / less safe.

comment:7 Changed 7 years ago by nickm

I'm with arma here; preauth commands are scary. The right way IMO to find out if a non-authorized connection is still alive is to try to authenticate to it.

comment:8 Changed 7 years ago by nickm

Milestone: Tor: unspecified

comment:9 Changed 7 years ago by nickm

Keywords: needs-proposal added

comment:10 Changed 7 years ago by arma

Cc: atagar added

Is this ticket still something that gsathya / atagar want? Or did they find a workaround on their side?

I think if we can avoid doing this in Tor, we should.

comment:11 Changed 7 years ago by atagar

Is this ticket still something that gsathya / atagar want?

It would be nice, but isn't important to me. The history behind this ticket is that stem's method to check if we still have a live tor connection is a bit of a hack. It checks (a) if we've closed the socket on our side or (b) we tried to send a message but the following recv() told us the socket was closed...

https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/socket.py#l137

In short, we can't tell if we have a working control connection except by using it, and we can't use it prior to authentication without losing it.

Sathyanarayanan submitted a patch to do is_alive() checks by making a GETINFO query, but I didn't like this due the pre-auth issue. I then proposed a PING method and hence this ticket.

My preference would actually be to both add a PING method and stop overzealously disconnecting the socket prior to authentication, but if we do neither it's not a big whoop.

comment:12 Changed 7 years ago by rransom

Why do you even need to ‘test 'is this controller connection alive'’? That really shouldn't be necessary, especially before authenticating.

comment:13 Changed 7 years ago by atagar

Why do you even need to ‘test 'is this controller connection alive'’?

The real question is "Why do stem's users need to ‘test 'is this controller connection alive'’?" - I don't need it within stem itself at the moment.

In arm I check if tor is alive or not by looking for a 'heartbeat'. We're subscribed to BW events so I can say "If I haven't gotten a BW event in the last ten seconds then something is wrong". This is used to detect if tor crashes, or if it stops becoming responsive to the control port (for instance, is swapping and about to trigger the OOM killer). Other stem users would likely want a function to explicitly check if tor's alive rather than subscribe to BW events.

I'm a little confused at this pushback. At work it's routine for services to have a ping health check so remote monitors can see if things are ok or not. Though again, I don't care very strongly about this.

comment:14 Changed 7 years ago by nickm

Keywords: tor-client added

comment:15 Changed 7 years ago by nickm

Component: Tor ClientTor

comment:16 Changed 2 years ago by nickm

Resolution: worksforme
Severity: Normal
Status: newclosed

We already allow a single PROTOCOLINFO command before authentication; I think that should meet the needs described here. But please reopen if I misunderstand.

Note: See TracTickets for help on using tickets.