Give a more appropriate "I'm not an HTTP proxy" message when we get an HTTP request on the control port
Right now if you accidentally configure your applications to use the control port as an HTTP proxy or a SOCKS proxy, the applications don't get any useful feedback, and the log messages aren't too helpful. They might tell you that something weird happened, but they won't say what.
When possible, then, the control port should detect HTTP and SOCKS requests. It should respond to HTTP request with a "Tor isn't an HTTP Proxy" message to the HTTP request-maker, and should in both cases log the fact that the control port was used as the wrong kind of proxy.
Activity
changed milestone to %Tor: 0.3.2.x-final
Now checks for GET, POST and CONNECT requests on the control port and responds/logs appropriately.
My Git branch with the fix: http://repo.or.cz/w/tor/neena.git/shortlog/refs/heads/fix-1667
Trac:
Cc: N/A to neenaoffline@gmail.com
Status: reopened to needs_review
I think we want to send back an HTTP response, not a raw string, right?
We don't want to do this unless the GET/POST/CONNECT/DELETE/whatever comes as the first thing on the control port; I think this patch might make a GET/POST/CONNECT get treated as HTTP at any point on the control port.
Connection_mark_and_flush() seems right here; we don't want to close the connection until it is done flushing.
We usually write multiline strings as "a string on one line\n" "followed by a string on another line\n".
Let's try to get something like this into 0.2.4.x?
Trac:
Status: needs_review to needs_revisionTBB no longer includes an HTTP proxy.
According to the message which Tor's SOCKSPort emits when someone attempts to use it as an HTTP proxy/server, you need to send a larger response in order to ensure that Internet Explorer will display it.
Now the control port will silently eat commands with certain names. It should send a reply.
This still might send an HTTP-like response even if the HTTP-request-like line isn't the first thing send on a connection.
strcasecmpshould be the wrong function. Perhaps you wantstrcasecmpstart?Trac:
Status: needs_review to needs_revisionReplying to rransom:
According to the message which Tor's SOCKSPort emits when someone attempts to use it as an HTTP proxy/server, you need to send a larger response in order to ensure that Internet Explorer will display it.
Looked at it. It helped me understand a lot of things. Also, used the same message, with a few modifications.
Now the control port will silently eat commands with certain names. It should send a reply.
This still might send an HTTP-like response even if the HTTP-request-like line isn't the first thing send on a connection.
I misunderstood something the last time. Should be right now.
strcasecmpshould be the wrong function. Perhaps you wantstrcasecmpstart?Used a switch-case, like the "Tor is not an HTTP Proxy" code instead.
Trac:
Status: needs_revision to needs_reviewReplying to neena:
Replying to rransom:
strcasecmpshould be the wrong function. Perhaps you wantstrcasecmpstart?Used a switch-case, like the "Tor is not an HTTP Proxy" code instead.
This would be fine except that almost every controller we care about starts its control connections by sending “
PROTOCOLINFO\r\n”. You really do need to use something likestrcasecmpstart(defined in src/common/util.h and .c; hopefully I'm spelling its name correctly) to compare the first chunk of data with “GET”, “POST”, etc.. (The SOCKSPort code only looks at the first byte because the first byte sent on a SOCKS connection must be the SOCKS version number.)Other than that, looks good!
Replying to rransom:
This would be fine except that almost every controller we care about starts its control connections by sending “
PROTOCOLINFO\r\n”. You really do need to use something likestrcasecmpstart(defined in src/common/util.h and .c; hopefully I'm spelling its name correctly) to compare the first chunk of data with “GET”, “POST”, etc.. (The SOCKSPort code only looks at the first byte because the first byte sent on a SOCKS connection must be the SOCKS version number.)Made the change. I had (incorrectly) assumed that TC connections begin with an AUTHENTICATE command. So, I decided to use the switch-case in the "not an HTTP Proxy" code instead of strcasecmpstart.
Trac:
Owner: N/A to neena
Status: needs_review to accepted-
Hm. Actually, the strcmpstart() stuff in the last couple of commits isn't right yet. strcmpstart can fail if it gets something other than a NUL-terminated string, and neither buf->data nor the output of inspect_evbuffer() is guaranteed to be NUL-terminated. Also, if you want N characters from inspect_evbuffer, you need to tell it to give you N characters: right now, it is only guaranteed to give one character.
Trac:
Status: needs_review to needs_revision Replying to nickm:
Hm. Actually, the strcmpstart() stuff in the last couple of commits isn't right yet. strcmpstart can fail if it gets something other than a NUL-terminated string, and neither buf->data nor the output of inspect_evbuffer() is guaranteed to be NUL-terminated. Also, if you want N characters from inspect_evbuffer, you need to tell it to give you N characters: right now, it is only guaranteed to give one character.
Fixed. I think.
Trac:
Status: needs_revision to needs_reviewmentioned in issue #6060 (moved)
moved to tpo/core/tor#1667 (closed)
mentioned in issue tpo/core/tor#6060 (closed)
