Opened 7 years ago

Closed 7 years ago

#2580 closed defect (fixed)

TorCtl crashes when connected to the ORPort

Reported by: atagar Owned by: mikeperry
Priority: Low Milestone:
Component: Torctl Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor:

Description

Hi, gt_jk reports that when you connect arm (or in theory just torctl should do the trick) to the orport rather than the control port it gives the following stack trace:

Traceback (most recent call last):

File "/home/jklumpp0/tmp/arm/src/TorCtl/TorCtl.py", line 673, in _loop

isEvent, reply = self._read_reply()

File "/home/jklumpp0/tmp/arm/src/TorCtl/TorCtl.py", line 819, in _read_reply

line = self._s.readline()

File "/home/jklumpp0/tmp/arm/src/TorCtl/TorUtil.py", line 203, in readline

s = self._s.recv(128)

error: [Errno 104] Connection reset by peer
Terminated

11:58 < gt_jk> with arm can I monitor a remote Tor controlport?
11:58 < Runa> no
11:59 < gt_jk> ah - is it possible to monitor with another tool?
11:59 < Runa> not that I know of
11:59 < katmagic> arm -i ${remote_host}:${remote_port}
11:59 < Runa> katmagic: oh :)
11:59 < gt_jk> katmagic: I'm getting a traceback doing that
12:00 < gt_jk> katmagic: telnetting to the port, I can access it
12:00 < katmagic> Did you set ControlListenAddress to a non-loopback address?
12:01 < gt_jk> didn't set that at all
12:01 < gt_jk> do I need to specify the address or could I have it looked up?
12:01 < katmagic> You can use '0.0.0.0' to listen on all interfaces.
12:02 < katmagic> Otherwise you'll need to find the IP address of the interface you want to listen on.
12:04 < gt_jk> katmagic: still occurring
12:04 < katmagic> Did you restart Tor?
12:04 < gt_jk> si
12:04 < katmagic> Hmm.
12:05 < gt_jk> wait
12:05 < gt_jk> nm
12:05 < gt_jk> i'm retarded
12:05 < katmagic> lol
12:05 < gt_jk> good call, thanks
12:05 < gt_jk> (i was using wrong port)
12:06 < katmagic> That's often problematic.

...

13:13 < atagar> gt_jk: arm should never provide a traceback - could you please stick it in pastebin?
13:17 < gt_jk> atagar: http://pastebin.com/ZEkTYj3V
13:18 < gt_jk> atagar: it happened because I tried connecting to 9001 not 9051
13:18 < gt_jk> atagar: ORPort rather than control
13:20 < atagar> Ick, that's within TorCtl. I'll file a ticket for it with mikeperry - thanks!
13:21 < gt_jk> anytime, thank you
13:22 < mikeperry> bleh. looks like the control port connection died unexpectedly w/ an error we did not catch
13:24 < mikeperry> not clear why we don't catch it
13:24 < mikeperry> "error" must not be a proper python Exception

Child Tickets

Change History (9)

comment:1 Changed 7 years ago by nickm

(The type is socket.error, even if it's showing up as "error", I think.)

So the fix is -- to catch socket.error and raise something else instead? To document that network errors can get thrown as socket.error?

comment:2 Changed 7 years ago by mikeperry

Points: 1

Perhaps we should catch socket.error and throw ProtocolError. But are we sure that all socket.errors count as ProtocolError? I could be convinced of this.

comment:3 Changed 7 years ago by nickm

Not necessarily all socket.errors. In fact, probably most don't.

ECONNRESET is a special case, in fact: it happens when the other side closes the connection, or the connection explodes, or such. That *could* be a protocol violation, or it could mean that the Tor dies, or a lot of stuff. Tor isn't supposed to close control connections without being told to do so, so you could plausibly call it a protocolerror, but it might make sense to treat it however we currently handle closed connections.

comment:4 Changed 7 years ago by atagar

Status: newneeds_review

comment:5 Changed 7 years ago by mikeperry

So then this solves the more specific issue of properly catching the exception early on when you first ask for the PROTOCOLINFO? Did you test this against non-exception things like socket.error? (Ex: attempting to connect to a closed control port?)

comment:6 in reply to:  5 ; Changed 7 years ago by atagar

Replying to mikeperry:

So then this solves the more specific issue of properly catching the exception early on when you first ask for the PROTOCOLINFO?

This solves uncaught exceptions when querying the PROTOCOLINFO and closing connection.

Did you test this against non-exception things like socket.error? (Ex: attempting to connect to a closed control port?)

Done, TorCtl.connect() to a closed port still hits the socket.error results in:
Connection refused. Is the ControlPort enabled?

comment:7 in reply to:  6 Changed 7 years ago by mikeperry

Replying to atagar:

Replying to mikeperry:

So then this solves the more specific issue of properly catching the exception early on when you first ask for the PROTOCOLINFO?

This solves uncaught exceptions when querying the PROTOCOLINFO and closing connection.

Did you test this against non-exception things like socket.error? (Ex: attempting to connect to a closed control port?)

Done, TorCtl.connect() to a closed port still hits the socket.error results in:
Connection refused. Is the ControlPort enabled?

Ok, cool. I missed the outer catch block just looking at the diff. This looks good to merge then.

comment:8 Changed 7 years ago by mikeperry

Merged.

comment:9 Changed 7 years ago by mikeperry

Resolution: fixed
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.