[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #14135 [meek]: Incorrect SocksListener temporary error check
#14135: Incorrect SocksListener temporary error check
------------------------+-----------------
Reporter: adam-p | Owner: dcf
Type: defect | Status: new
Priority: minor | Milestone:
Component: meek | Version:
Resolution: | Keywords:
Actual Points: | Parent ID:
Points: |
------------------------+-----------------
Comment (by dcf):
Thanks for looking at this.
Currently, the code works like this:
{{{
non-net.Error: try again
net.Error, Temporary: try again
net.Error, non-Temporary: quit
}}}
The patch changes it to be this:
{{{
non-net.Error: quit
net.Error, Temporary: try again
net.Error, non-Temporary: quit
}}}
The problem is that at least once in the past, we wanted to try again on a
non-net.Error. See this commit message:
https://gitweb.torproject.org/pluggable-
transports/goptlib.git/commit/?id=3030f080eecf72b0e896236fca5fabd245c00bdb
Before that commit (and with the patch in this ticket), you can kill a
client process by sending something unexpected in the SOCKS request, for
example:
{{{
socat -v - SOCKS4A:127.0.0.1:1.1.1.1:2222,socksport=<meek-client
port>,socksuser=bogus
}}}
The error you get is this, which comes from lower level goptlib code, is
not a net.Error:
{{{
2015/01/09 12:40:59 no equals sign in "bogus"
}}}
We can agree that we shouldn't kill the process in this case. But there
are a few ways we could do it. One way is to continue the way that it
works now, and document that errors that are not net.Errors should be
treated as Temporary. Another way is for AcceptSocks to intercept the
error returned by [https://gitweb.torproject.org/pluggable-
transports/goptlib.git/tree/socks.go?id=99ea2c51f294fbd4cb016d96acde1b4288fc63e7#n158
readSocks4aConnect] and, if it is not an net.Error, transform it into a
Temporary net.Error. Failure to read/parse the connect message is arguably
a network error.
I think I would prefer a patch to do the second thing. Basically
everywhere in readSocks4aConnect that returns fmt.Errorf, return a
Temporary net.Error instead. The errors from underlying network calls
should be returned without alteration. We should make sure that errors in
other functions are consistent, and also see what the standard library
does in similar cases (i.e., how does it construct Temporary errors).
The upstream for goptlib is https://gitweb.torproject.org/pluggable-
transports/goptlib.git/ BTW.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/14135#comment:1>
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