Browse Source

Merge branch 'neena-fix-1667'

Nick Mathewson 6 years ago
parent
commit
cd77ea782e
5 changed files with 131 additions and 26 deletions
  1. 4 0
      changes/bug1667
  2. 56 26
      src/or/buffers.c
  3. 3 0
      src/or/buffers.h
  4. 41 0
      src/or/control.c
  5. 27 0
      src/test/test_buffers.c

+ 4 - 0
changes/bug1667

@@ -0,0 +1,4 @@
+  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 log
+      the event. Closes ticket 1667. Patch from Ravi Chandra Padmala.

+ 56 - 26
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"
+  "<html>\n"
+  "<head>\n"
+  "<title>Tor is not an HTTP Proxy</title>\n"
+  "</head>\n"
+  "<body>\n"
+  "<h1>Tor is not an HTTP Proxy</h1>\n"
+  "<p>\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"
+  "</p>\n"
+  "<p>\n"
+  "See <a href=\"https://www.torproject.org/documentation.html\">"
+  "https://www.torproject.org/documentation.html</a> for more "
+  "information.\n"
+  "<!-- Plus this comment, to make the body response more than 512 bytes, so "
+  "     IE will be willing to display it. Comment comment comment comment "
+  "     comment comment comment comment comment comment comment comment.-->\n"
+  "</p>\n"
+  "</body>\n"
+  "</html>\n";
+
 /** Implementation helper to implement fetch_from_*_socks.  Instead of looking
  * at a buffer's contents, we look at the <b>datalen</b> bytes of data in
  * <b>data</b>. Instead of removing data from the buffer, we set
@@ -1826,32 +1852,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"
-"<html>\n"
-"<head>\n"
-"<title>Tor is not an HTTP Proxy</title>\n"
-"</head>\n"
-"<body>\n"
-"<h1>Tor is not an HTTP Proxy</h1>\n"
-"<p>\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"
-"</p>\n"
-"<p>\n"
-"See <a href=\"https://www.torproject.org/documentation.html\">"
-           "https://www.torproject.org/documentation.html</a> for more "
-           "information.\n"
-"<!-- Plus this comment, to make the body response more than 512 bytes, so "
-"     IE will be willing to display it. Comment comment comment comment "
-"     comment comment comment comment comment comment comment comment.-->\n"
-"</p>\n"
-"</body>\n"
-"</html>\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 */
@@ -2014,6 +2016,34 @@ parse_socks_client(const uint8_t *data, size_t datalen,
   return -1;
 }
 
+/** Return true if <b>cmd</b> looks like a HTTP (proxy) request. */
+int
+peek_buf_has_http_command(const 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 <b>buf</b> starts with <b>cmd</b>. <b>cmd</b> must be a null
+ * terminated string, of no more than PEEK_BUF_STARTSWITH_MAX bytes. */
+int
+peek_buf_startswith(const buf_t *buf, const char *cmd)
+{
+  char tmp[PEEK_BUF_STARTSWITH_MAX];
+  size_t clen = strlen(cmd);
+  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
  * command on it than any valid v1 controller command. */
 int

+ 3 - 0
src/or/buffers.h

@@ -53,6 +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);
+#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);
 

+ 41 - 0
src/or/control.c

@@ -4912,6 +4912,38 @@ 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);
+}
+
+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"
+  "<html>\n"
+  "<head>\n"
+  "<title>Tor's ControlPort is not an HTTP proxy</title>\n"
+  "</head>\n"
+  "<body>\n"
+  "<h1>Tor's ControlPort is not an HTTP proxy</h1>\n"
+  "<p>\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"
+  "</p>\n"
+  "<p>\n"
+  "See <a href=\"https://www.torproject.org/documentation.html\">"
+  "https://www.torproject.org/documentation.html</a> for more "
+  "information.\n"
+  "<!-- Plus this comment, to make the body response more than 512 bytes, so "
+  "     IE will be willing to display it. Comment comment comment comment "
+  "     comment comment comment comment comment comment comment comment.-->\n"
+  "</p>\n"
+  "</body>\n"
+  "</html>\n";
+
 /** Called when data has arrived on a v1 control connection: Try to fetch
  * commands from conn->inbuf, and execute them.
  */
@@ -4951,6 +4983,15 @@ 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(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;
+  }
+
  again:
   while (1) {
     size_t last_idx;

+ 27 - 0
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 },