From 417d778652770a8f4b0b6f8e5d8e139e193b8b1e Mon Sep 17 00:00:00 2001 From: Ravi Chandra Padmala Date: Tue, 21 Feb 2012 08:52:03 +0530 Subject: [PATCH 1/6] Respond meaningfully to HTTP requests on the control port. Fix #1667 (Squashed with bufferevents portions removed, by nickm) --- changes/bug1667 | 4 ++++ src/or/buffers.c | 25 +++++++++++++++++++++++++ src/or/buffers.h | 2 ++ src/or/control.c | 38 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+) create mode 100644 changes/bug1667 diff --git a/changes/bug1667 b/changes/bug1667 new file mode 100644 index 0000000000..195993d6d5 --- /dev/null +++ b/changes/bug1667 @@ -0,0 +1,4 @@ + o Minor features: + - If the control port is used as the HTTP proxy, responds with + a meaningful "This is the Tor control port" message, and logs + the event. Fixes bug 1667. diff --git a/src/or/buffers.c b/src/or/buffers.c index 3692ed4d08..af1b67e091 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -2022,6 +2022,31 @@ parse_socks_client(const uint8_t *data, size_t datalen, return -1; } +/** Return true if cmd looks like a HTTP (proxy) request. */ +int +peek_buf_has_http_command(buf_t *buf) +{ + if (peek_buf_startswith(buf, "CONNECT ") || + peek_buf_startswith(buf, "DELETE ") || + peek_buf_startswith(buf, "GET ") || + peek_buf_startswith(buf, "POST ") || + peek_buf_startswith(buf, "PUT " )) + return 1; + return 0; +} + +/** Return 1 iff buf starts with cmd. cmd must be a null + * terminated string */ +int +peek_buf_startswith(buf_t *buf, const char *cmd) +{ + int clen = strlen(cmd); + if (buf->datalen >= clen) + if (!strncasecmp((buf->head)->data, cmd, (size_t) clen)) + return 1; + return 0; +} + /** Return 1 iff buf looks more like it has an (obsolete) v0 controller * command on it than any valid v1 controller command. */ int diff --git a/src/or/buffers.h b/src/or/buffers.h index 23b58a571a..5650beabc6 100644 --- a/src/or/buffers.h +++ b/src/or/buffers.h @@ -53,6 +53,8 @@ int fetch_from_buf_socks_client(buf_t *buf, int state, char **reason); int fetch_from_buf_line(buf_t *buf, char *data_out, size_t *data_len); int peek_buf_has_control0_command(buf_t *buf); +int peek_buf_startswith(buf_t *buf, const char *cmd); +int peek_buf_has_http_command(buf_t *buf); int fetch_ext_or_command_from_buf(buf_t *buf, ext_or_cmd_t **out); diff --git a/src/or/control.c b/src/or/control.c index 9454a7ab67..3e31f17248 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -4882,6 +4882,12 @@ peek_connection_has_control0_command(connection_t *conn) return peek_buf_has_control0_command(conn->inbuf); } +static int +peek_connection_has_http_command(connection_t *conn) +{ + return peek_buf_has_http_command(conn->inbuf); +} + /** Called when data has arrived on a v1 control connection: Try to fetch * commands from conn->inbuf, and execute them. */ @@ -4921,6 +4927,38 @@ connection_control_process_inbuf(control_connection_t *conn) return 0; } + /* If the user has the HTTP proxy port and the control port confused. */ + if (conn->_base.state == CONTROL_CONN_STATE_NEEDAUTH && + peek_connection_has_http_command(TO_CONN(conn))) { + connection_write_str_to_buf("HTTP/1.0 501 Tor ControlPort is not a proxy" +"\r\nContent-Type: text/html; charset=iso-8859-1\r\n\r\n" +"\n" +"\n" +"Tor's ControlPort is not proxy\n" +"\n" +"\n" +"

Tor's ControlPort is not a proxy

\n" +"

\n" +"It appears you have configured your web browser to use Tor's control port" +" as an HTTP proxy.\n" +"This is not correct: Tor's default SOCKS proxy port is 9050.\n" +"Please configure your client accordingly.\n" +"

\n" +"

\n" +"See " + "https://www.torproject.org/documentation.html for more " + "information.\n" +"\n" +"

\n" +"\n" +"\n", conn); + log_notice(LD_CONTROL, "Received HTTP request on ControlPort"); + connection_mark_and_flush(TO_CONN(conn)); + return 0; + } + again: while (1) { size_t last_idx; From acf65544bb0cb0473d1f846985ae2807123fbbbd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 21 Jun 2017 10:35:35 -0400 Subject: [PATCH 2/6] Fix compilation on 1667 code. --- src/or/buffers.c | 2 +- src/or/control.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index af1b67e091..060f58c89a 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -2040,7 +2040,7 @@ peek_buf_has_http_command(buf_t *buf) int peek_buf_startswith(buf_t *buf, const char *cmd) { - int clen = strlen(cmd); + size_t clen = strlen(cmd); if (buf->datalen >= clen) if (!strncasecmp((buf->head)->data, cmd, (size_t) clen)) return 1; diff --git a/src/or/control.c b/src/or/control.c index 3e31f17248..ee9a3ae29b 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -4928,7 +4928,7 @@ connection_control_process_inbuf(control_connection_t *conn) } /* If the user has the HTTP proxy port and the control port confused. */ - if (conn->_base.state == CONTROL_CONN_STATE_NEEDAUTH && + if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH && peek_connection_has_http_command(TO_CONN(conn))) { connection_write_str_to_buf("HTTP/1.0 501 Tor ControlPort is not a proxy" "\r\nContent-Type: text/html; charset=iso-8859-1\r\n\r\n" From ed4bc554503d1a18cb6764943eea98aaf43bf2da Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 21 Jun 2017 11:10:58 -0400 Subject: [PATCH 3/6] Replace peek_buf_startswith() with a safe version It's not okay to assume that the data in a buf_t is contiguous in the first chunk. --- src/or/buffers.c | 17 ++++++++++------- src/or/buffers.h | 5 +++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 060f58c89a..b071725474 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -2024,7 +2024,7 @@ parse_socks_client(const uint8_t *data, size_t datalen, /** Return true if cmd looks like a HTTP (proxy) request. */ int -peek_buf_has_http_command(buf_t *buf) +peek_buf_has_http_command(const buf_t *buf) { if (peek_buf_startswith(buf, "CONNECT ") || peek_buf_startswith(buf, "DELETE ") || @@ -2036,15 +2036,18 @@ peek_buf_has_http_command(buf_t *buf) } /** Return 1 iff buf starts with cmd. cmd must be a null - * terminated string */ + * terminated string, of no more than PEEK_BUF_STARTSWITH_MAX bytes. */ int -peek_buf_startswith(buf_t *buf, const char *cmd) +peek_buf_startswith(const buf_t *buf, const char *cmd) { + char tmp[PEEK_BUF_STARTSWITH_MAX]; size_t clen = strlen(cmd); - if (buf->datalen >= clen) - if (!strncasecmp((buf->head)->data, cmd, (size_t) clen)) - return 1; - return 0; + if (BUG(clen > sizeof(tmp))) + return 0; + if (buf->datalen < clen) + return 0; + peek_from_buf(tmp, clen, buf); + return fast_memeq(tmp, cmd, clen); } /** Return 1 iff buf looks more like it has an (obsolete) v0 controller diff --git a/src/or/buffers.h b/src/or/buffers.h index 5650beabc6..d884084385 100644 --- a/src/or/buffers.h +++ b/src/or/buffers.h @@ -53,8 +53,9 @@ int fetch_from_buf_socks_client(buf_t *buf, int state, char **reason); int fetch_from_buf_line(buf_t *buf, char *data_out, size_t *data_len); int peek_buf_has_control0_command(buf_t *buf); -int peek_buf_startswith(buf_t *buf, const char *cmd); -int peek_buf_has_http_command(buf_t *buf); +#define PEEK_BUF_STARTSWITH_MAX 16 +int peek_buf_startswith(const buf_t *buf, const char *cmd); +int peek_buf_has_http_command(const buf_t *buf); int fetch_ext_or_command_from_buf(buf_t *buf, ext_or_cmd_t **out); From aafeffe02af5b3c8306fe36eec00baaabc8e68eb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 21 Jun 2017 11:15:44 -0400 Subject: [PATCH 4/6] Fix the changes file --- changes/bug1667 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changes/bug1667 b/changes/bug1667 index 195993d6d5..368f9e35b2 100644 --- a/changes/bug1667 +++ b/changes/bug1667 @@ -1,4 +1,4 @@ - o Minor features: + o Minor features (control port): - If the control port is used as the HTTP proxy, responds with - a meaningful "This is the Tor control port" message, and logs - the event. Fixes bug 1667. + a meaningful "This is the Tor control port" message, and log + the event. Closes ticket 1667. Patch from Ravi Chandra Padmala. From 6595f55020e4a73e2ccf7e87dcea4c1f0362f4aa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 21 Jun 2017 11:20:33 -0400 Subject: [PATCH 5/6] unit tests for peek_buf_startswith() --- src/test/test_buffers.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/test/test_buffers.c b/src/test/test_buffers.c index 07114a8571..3989a45480 100644 --- a/src/test/test_buffers.c +++ b/src/test/test_buffers.c @@ -838,10 +838,37 @@ test_buffers_find_contentlen(void *arg) ; } +static void +test_buffer_peek_startswith(void *arg) +{ + (void)arg; + buf_t *buf; + buf = buf_new(); + tt_ptr_op(buf, OP_NE, NULL); + + tt_assert(peek_buf_startswith(buf, "")); + tt_assert(! peek_buf_startswith(buf, "X")); + + write_to_buf("Tor", 3, buf); + + tt_assert(peek_buf_startswith(buf, "")); + tt_assert(peek_buf_startswith(buf, "T")); + tt_assert(peek_buf_startswith(buf, "To")); + tt_assert(peek_buf_startswith(buf, "Tor")); + tt_assert(! peek_buf_startswith(buf, "Top")); + tt_assert(! peek_buf_startswith(buf, "For")); + tt_assert(! peek_buf_startswith(buf, "Tork")); + tt_assert(! peek_buf_startswith(buf, "Torpor")); + + done: + buf_free(buf); +} + struct testcase_t buffer_tests[] = { { "basic", test_buffers_basic, TT_FORK, NULL, NULL }, { "copy", test_buffer_copy, TT_FORK, NULL, NULL }, { "pullup", test_buffer_pullup, TT_FORK, NULL, NULL }, + { "startswith", test_buffer_peek_startswith, 0, NULL, NULL }, { "ext_or_cmd", test_buffer_ext_or_cmd, TT_FORK, NULL, NULL }, { "allocation_tracking", test_buffer_allocation_tracking, TT_FORK, NULL, NULL }, From 03b6cfd5911740471eac13e82f678fe50b4d18f9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 5 Jul 2017 11:01:17 -0400 Subject: [PATCH 6/6] Extract "not an HTTP proxy" messages. --- src/or/buffers.c | 54 +++++++++++++++++++++++++----------------------- src/or/control.c | 51 ++++++++++++++++++++++++--------------------- 2 files changed, 55 insertions(+), 50 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index b071725474..aff113ff6a 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1478,6 +1478,32 @@ socks_request_set_socks5_error(socks_request_t *req, req->reply[3] = 0x01; // ATYP field. } +const char SOCKS_PROXY_IS_NOT_AN_HTTP_PROXY_MSG[] = + "HTTP/1.0 501 Tor is not an HTTP Proxy\r\n" + "Content-Type: text/html; charset=iso-8859-1\r\n\r\n" + "\n" + "\n" + "Tor is not an HTTP Proxy\n" + "\n" + "\n" + "

Tor is not an HTTP Proxy

\n" + "

\n" + "It appears you have configured your web browser to use Tor as " + "an HTTP proxy.\n\n" + "This is not correct: Tor is a SOCKS proxy, not an HTTP proxy.\n" + "Please configure your client accordingly.\n" + "

\n" + "

\n" + "See " + "https://www.torproject.org/documentation.html for more " + "information.\n" + "\n" + "

\n" + "\n" + "\n"; + /** Implementation helper to implement fetch_from_*_socks. Instead of looking * at a buffer's contents, we look at the datalen bytes of data in * data. Instead of removing data from the buffer, we set @@ -1834,32 +1860,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, case 'H': /* head */ case 'P': /* put/post */ case 'C': /* connect */ - strlcpy((char*)req->reply, -"HTTP/1.0 501 Tor is not an HTTP Proxy\r\n" -"Content-Type: text/html; charset=iso-8859-1\r\n\r\n" -"\n" -"\n" -"Tor is not an HTTP Proxy\n" -"\n" -"\n" -"

Tor is not an HTTP Proxy

\n" -"

\n" -"It appears you have configured your web browser to use Tor as an HTTP proxy." -"\n" -"This is not correct: Tor is a SOCKS proxy, not an HTTP proxy.\n" -"Please configure your client accordingly.\n" -"

\n" -"

\n" -"See " - "https://www.torproject.org/documentation.html for more " - "information.\n" -"\n" -"

\n" -"\n" -"\n" - , MAX_SOCKS_REPLY_LEN); + strlcpy((char*)req->reply, SOCKS_PROXY_IS_NOT_AN_HTTP_PROXY_MSG, + MAX_SOCKS_REPLY_LEN); req->replylen = strlen((char*)req->reply)+1; /* fall through */ default: /* version is not socks4 or socks5 */ diff --git a/src/or/control.c b/src/or/control.c index ee9a3ae29b..20a0b0b89e 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -4888,6 +4888,32 @@ peek_connection_has_http_command(connection_t *conn) return peek_buf_has_http_command(conn->inbuf); } +const char CONTROLPORT_IS_NOT_AN_HTTP_PROXY_MSG[] = + "HTTP/1.0 501 Tor ControlPort is not an HTTP proxy" + "\r\nContent-Type: text/html; charset=iso-8859-1\r\n\r\n" + "\n" + "\n" + "Tor's ControlPort is not an HTTP proxy\n" + "\n" + "\n" + "

Tor's ControlPort is not an HTTP proxy

\n" + "

\n" + "It appears you have configured your web browser to use Tor's control port" + " as an HTTP proxy.\n" + "This is not correct: Tor's default SOCKS proxy port is 9050.\n" + "Please configure your client accordingly.\n" + "

\n" + "

\n" + "See " + "https://www.torproject.org/documentation.html for more " + "information.\n" + "\n" + "

\n" + "\n" + "\n"; + /** Called when data has arrived on a v1 control connection: Try to fetch * commands from conn->inbuf, and execute them. */ @@ -4930,30 +4956,7 @@ connection_control_process_inbuf(control_connection_t *conn) /* If the user has the HTTP proxy port and the control port confused. */ if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH && peek_connection_has_http_command(TO_CONN(conn))) { - connection_write_str_to_buf("HTTP/1.0 501 Tor ControlPort is not a proxy" -"\r\nContent-Type: text/html; charset=iso-8859-1\r\n\r\n" -"\n" -"\n" -"Tor's ControlPort is not proxy\n" -"\n" -"\n" -"

Tor's ControlPort is not a proxy

\n" -"

\n" -"It appears you have configured your web browser to use Tor's control port" -" as an HTTP proxy.\n" -"This is not correct: Tor's default SOCKS proxy port is 9050.\n" -"Please configure your client accordingly.\n" -"

\n" -"

\n" -"See " - "https://www.torproject.org/documentation.html for more " - "information.\n" -"\n" -"

\n" -"\n" -"\n", conn); + connection_write_str_to_buf(CONTROLPORT_IS_NOT_AN_HTTP_PROXY_MSG, conn); log_notice(LD_CONTROL, "Received HTTP request on ControlPort"); connection_mark_and_flush(TO_CONN(conn)); return 0;