[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

132 Browser Test Service - Reviewer-Friendly Patch



OK, a 1200 line diff is probably not the smartest way of inviting someone to 
review my work so far. ;-)

This time I've only included the changes required to existing files in Tor.

The complete patch introduces two new files which are available for separate 
review at:

http://roberthogan.net/stuff/testservice.c
http://roberthogan.net/stuff/testservice.h

There is also an updated proposal available at:

http://roberthogan.net/stuff/132-browser-check-tor-service.txt

Demos:
  http://www.youtube.com/profile_videos?user=mwenge2

Areas I know that need work:
  - The text and presentation of the html pages.
  - A test image with a much lighter footprint.
  - Fold the http_ functions into those used by dirserver.c. (Not sure that this 
would save an awful lot in linecount though.)
  - A test page for javascript, plugins and the like.
  - More comments

Potential issues:
  - The Proxy Test uses a randomly generated IP as the 'location' of the test 
image. In theory the random IP may have a HTTP service and my be serving an 
image with a name such as /CC3B118B056F3A81.png, in which case the test would 
produce a false positive. My implementation assumes that this possibility is 
vanishingly small. I also assume that the possibility of a genuine user request 
using the randomly generated IP while the Proxy Test is in progress is also 
sufficiently remote.



Index: src/or/config.c
===================================================================
--- src/or/config.c	(revision 16872)
+++ src/or/config.c	(working copy)
@@ -302,6 +302,8 @@
   V(StrictEntryNodes,            BOOL,     "0"),
   V(StrictExitNodes,             BOOL,     "0"),
   OBSOLETE("SysLog"),
+  V(TestServicePort,             UINT,     "0"),
+  V(TestServiceListenAddress,    LINELIST, NULL),
   V(TestSocks,                   BOOL,     "0"),
   OBSOLETE("TestVia"),
   V(TrackHostExits,              CSV,      NULL),
@@ -2830,6 +2832,11 @@
   if (options->NatdPort == 0 && options->NatdListenAddress != NULL)
     REJECT("NatdPort must be defined if NatdListenAddress is defined.");
 
+  if (options->TestServicePort == 0 &&
+      options->TestServiceListenAddress != NULL)
+    REJECT("TestServicePort must be defined if "
+           "TestServiceListenAddress is defined.");
+
   /* Don't gripe about SocksPort 0 with SocksListenAddress set; a standard
    * configuration does this. */
 
Index: src/or/connection_edge.c
===================================================================
--- src/or/connection_edge.c	(revision 16872)
+++ src/or/connection_edge.c	(working copy)
@@ -130,6 +130,15 @@
       }
       return 0;
     case AP_CONN_STATE_OPEN:
+      /* If this is a test request we intercept and respond ourselves */
+      if (testservice_istestrequest(TO_CONN(conn))) {
+        /* (We already sent an end cell if possible) */
+        connection_edge_end(conn, END_STREAM_REASON_INTERNAL);
+        connection_mark_for_close(TO_CONN(conn));
+        return -1;
+      }
+      if (conn->_base.purpose == TEST_PURPOSE_AP)
+        return 0;
     case EXIT_CONN_STATE_OPEN:
       if (connection_edge_package_raw_inbuf(conn, package_partial) < 0) {
         /* (We already sent an end cell if possible) */
@@ -1839,6 +1848,25 @@
     return -1;
   } /* else socks handshake is done, continue processing */
 
+  /* If this is a request for a proxy test image or a dns test image
+     then mark the conn as TEST_PURPOSE_AP so that no circuit is built
+     for it and testservice.c can serve the image itself. If it is a
+     request for a Connectivity Test image we want a circuit to build
+     first.*/
+  if (testservice_istestaddress(socks->address)) {
+    char buf[256];
+    memset(buf,0,SOCKS4_NETWORK_LEN);
+    buf[1] = SOCKS4_GRANTED;
+    /* leave version, destport, destip zero */
+    connection_write_to_buf(buf, SOCKS4_NETWORK_LEN, TO_CONN(conn));
+    conn->socks_request->has_finished = 1;
+    conn->_base.state = AP_CONN_STATE_OPEN;
+    conn->_base.purpose = TEST_PURPOSE_AP;
+    return 0;
+  }
+
+
+
   if (hostname_is_noconnect_address(socks->address))
   {
     control_event_stream_status(conn, STREAM_EVENT_NEW, 0);
Index: src/or/or.h
===================================================================
--- src/or/or.h	(revision 16872)
+++ src/or/or.h	(working copy)
@@ -207,7 +207,9 @@
 #define CONN_TYPE_AP_NATD_LISTENER 14
 /** Type for sockets listening for DNS requests. */
 #define CONN_TYPE_AP_DNS_LISTENER 15
-#define _CONN_TYPE_MAX 15
+#define CONN_TYPE_TEST_SERVICE_LISTENER 16
+#define CONN_TYPE_TEST 17
+#define _CONN_TYPE_MAX 17
 /* !!!! If _CONN_TYPE_MAX is ever over 15, we must grow the type field in
  * connection_t. */
 
@@ -305,6 +307,13 @@
 #define DIR_CONN_STATE_SERVER_WRITING 6
 #define _DIR_CONN_STATE_MAX 6
 
+#define _TEST_CONN_STATE_MIN 1
+/** State for connection at test service server: waiting for HTTP request. */
+#define TEST_CONN_STATE_SERVER_COMMAND_WAIT 1
+/** State for connection at test service server: sending HTTP response. */
+#define TEST_CONN_STATE_SERVER_WRITING 2
+#define _TEST_CONN_STATE_MAX 2
+
 /** True iff the purpose of <b>conn</b> means that it's a server-side
  * directory connection. */
 #define DIR_CONN_IS_SERVER(conn) ((conn)->purpose == DIR_PURPOSE_SERVER)
@@ -376,6 +385,12 @@
    (p)==DIR_PURPOSE_UPLOAD_VOTE ||              \
    (p)==DIR_PURPOSE_UPLOAD_SIGNATURES)
 
+#define _TEST_PURPOSE_MIN 1
+/** Purpose for connection at a test service server. */
+#define TEST_PURPOSE_SERVER 1
+#define TEST_PURPOSE_AP 2
+#define _TEST_PURPOSE_MAX 2
+
 #define _EXIT_PURPOSE_MIN 1
 /** This exit stream wants to do an ordinary connect. */
 #define EXIT_PURPOSE_CONNECT 1
@@ -790,7 +805,8 @@
 typedef struct socks_request_t socks_request_t;
 
 /* Values for connection_t.magic: used to make sure that downcasts (casts from
-* connection_t to foo_connection_t) are safe. */
+* connection_t to foo_connection_t) are safe. These values are arbitrary. They
+  are not calculated or derived, just invented.*/
 #define BASE_CONNECTION_MAGIC 0x7C3C304Eu
 #define OR_CONNECTION_MAGIC 0x7D31FF03u
 #define EDGE_CONNECTION_MAGIC 0xF0374013u
@@ -820,7 +836,7 @@
                    * *_CONNECTION_MAGIC. */
 
   uint8_t state; /**< Current state of this connection. */
-  unsigned int type:4; /**< What kind of connection is this? */
+  unsigned int type:5; /**< What kind of connection is this? */
   unsigned int purpose:5; /**< Only used for DIR and EXIT types currently. */
 
   /* The next fields are all one-bit booleans. Some are only applicable to
@@ -2102,6 +2118,8 @@
   config_line_t *DirListenAddress;
   /** Addresses to bind for listening for control connections. */
   config_line_t *ControlListenAddress;
+  /** Addresses to bind for listening for control connections. */
+  config_line_t *TestServiceListenAddress;
   /** Local address to bind outbound sockets */
   char *OutboundBindAddress;
   /** Directory server only: which versions of
@@ -2119,6 +2137,7 @@
   int TransPort;
   int NatdPort; /**< Port to listen on for transparent natd connections. */
   int ControlPort; /**< Port to listen on for control connections. */
+  int TestServicePort; /**< Port to listen on for control connections. */
   config_line_t *ControlSocket; /**< List of Unix Domain Sockets to listen on
                                  * for control connections. */
   int DirPort; /**< Port to listen on for directory connections. */
@@ -2503,7 +2522,7 @@
     state->next_write = when;
 }
 
-#define MAX_SOCKS_REPLY_LEN 1024
+#define MAX_SOCKS_REPLY_LEN 2056
 #define MAX_SOCKS_ADDR_LEN 256
 
 /** Please open a TCP connection to this addr:port. */
@@ -2569,6 +2588,7 @@
                       const char *data, size_t data_len, int done);
 int move_buf_to_buf(buf_t *buf_out, buf_t *buf_in, size_t *buf_flushlen);
 int fetch_from_buf(char *string, size_t string_len, buf_t *buf);
+int buf_startswith(buf_t *buf, const char *string, size_t len);
 int fetch_var_cell_from_buf(buf_t *buf, var_cell_t **out, int linkproto);
 int fetch_from_buf_http(buf_t *buf,
                         char **headers_out, size_t max_headerlen,
@@ -4352,5 +4372,15 @@
                                    size_t intro_points_encoded_size);
 int rend_parse_client_keys(strmap_t *parsed_clients, const char *str);
 
+/********************************* testservice.c ************************/
+
+int testservice_istestrequest(connection_t *conn);
+int testservice_istestaddress(const char *address);
+int testservice_handlebrowserusingtorasproxy(const char *buf,
+                                              socks_request_t *req);
+int connection_testserv_process_inbuf(connection_t *conn);
+int connection_testserv_finished_flushing(connection_t *conn);
+
 #endif
 
+
Index: src/or/buffers.c
===================================================================
--- src/or/buffers.c	(revision 16872)
+++ src/or/buffers.c	(working copy)
@@ -962,6 +962,15 @@
   }
 }
 
+int
+buf_startswith(buf_t *buf, const char *string, size_t len)
+{
+  if (buf->datalen < len) return 0;
+  buf_pullup(buf, len, 0);
+  if (!memcmp(buf->head->data, string, len)) return 1;
+  return 0;
+}
+
 /** Remove <b>string_len</b> bytes from the front of <b>buf</b>, and store
  * them into <b>string</b>.  Return the new buffer size.  <b>string_len</b>
  * must be \<= the number of bytes on the buffer.
@@ -1574,7 +1583,11 @@
     case 'H': /* head */
     case 'P': /* put/post */
     case 'C': /* connect */
-      strlcpy(req->reply,
+      /* If this is a request for a test image served by testservice.c then
+         we will serve a warning image, otherwise serve the standard HTML
+         warning */
+      if (!testservice_handlebrowserusingtorasproxy(buf->head->data,req)) {
+        strlcpy(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"
@@ -1601,6 +1614,7 @@
 "</html>\n"
              , MAX_SOCKS_REPLY_LEN);
       req->replylen = strlen(req->reply)+1;
+      }
       /* fall through */
     default: /* version is not socks4 or socks5 */
       log_warn(LD_APP,
Index: src/or/connection.c
===================================================================
--- src/or/connection.c	(revision 16872)
+++ src/or/connection.c	(working copy)
@@ -54,9 +54,11 @@
       return "Transparent pf/netfilter listener";
     case CONN_TYPE_AP_NATD_LISTENER: return "Transparent natd listener";
     case CONN_TYPE_AP_DNS_LISTENER: return "DNS listener";
+    case CONN_TYPE_TEST_SERVICE_LISTENER: return "Test Service listener";
     case CONN_TYPE_AP: return "Socks";
     case CONN_TYPE_DIR_LISTENER: return "Directory listener";
     case CONN_TYPE_DIR: return "Directory";
+    case CONN_TYPE_TEST: return "Test";
     case CONN_TYPE_CPUWORKER: return "CPU worker";
     case CONN_TYPE_CONTROL_LISTENER: return "Control listener";
     case CONN_TYPE_CONTROL: return "Control";
@@ -82,6 +84,7 @@
     case CONN_TYPE_AP_NATD_LISTENER:
     case CONN_TYPE_AP_DNS_LISTENER:
     case CONN_TYPE_DIR_LISTENER:
+    case CONN_TYPE_TEST_SERVICE_LISTENER:
     case CONN_TYPE_CONTROL_LISTENER:
       if (state == LISTENER_STATE_READY)
         return "ready";
@@ -130,6 +133,12 @@
         case DIR_CONN_STATE_SERVER_WRITING: return "writing";
       }
       break;
+    case CONN_TYPE_TEST:
+      switch (state) {
+        case TEST_CONN_STATE_SERVER_COMMAND_WAIT: return "waiting for command";
+        case TEST_CONN_STATE_SERVER_WRITING: return "writing";
+      }
+      break;
     case CONN_TYPE_CPUWORKER:
       switch (state) {
         case CPUWORKER_STATE_IDLE: return "idle";
@@ -1169,6 +1178,10 @@
       conn->purpose = DIR_PURPOSE_SERVER;
       conn->state = DIR_CONN_STATE_SERVER_COMMAND_WAIT;
       break;
+    case CONN_TYPE_TEST:
+      conn->purpose = TEST_PURPOSE_SERVER;
+      conn->state = TEST_CONN_STATE_SERVER_COMMAND_WAIT;
+      break;
     case CONN_TYPE_CONTROL:
       conn->state = CONTROL_CONN_STATE_NEEDAUTH;
       break;
@@ -1475,6 +1488,12 @@
                       replaced_conns, new_conns, 0,
                       AF_INET)<0)
     return -1;
+  if (retry_listeners(CONN_TYPE_TEST_SERVICE_LISTENER,
+                      options->TestServiceListenAddress,
+                      options->TestServicePort, "127.0.0.1",
+                      replaced_conns, new_conns, 0,
+                      AF_INET)<0)
+    return -1;
   if (retry_listeners(CONN_TYPE_CONTROL_LISTENER,
                       options->ControlListenAddress,
                       options->ControlPort, "127.0.0.1",
@@ -1923,6 +1942,8 @@
     case CONN_TYPE_AP_TRANS_LISTENER:
     case CONN_TYPE_AP_NATD_LISTENER:
       return connection_handle_listener_read(conn, CONN_TYPE_AP);
+    case CONN_TYPE_TEST_SERVICE_LISTENER:
+      return connection_handle_listener_read(conn, CONN_TYPE_TEST);
     case CONN_TYPE_DIR_LISTENER:
       return connection_handle_listener_read(conn, CONN_TYPE_DIR);
     case CONN_TYPE_CONTROL_LISTENER:
@@ -2602,6 +2623,7 @@
       conn->type == CONN_TYPE_AP_TRANS_LISTENER ||
       conn->type == CONN_TYPE_AP_DNS_LISTENER ||
       conn->type == CONN_TYPE_AP_NATD_LISTENER ||
+      conn->type == CONN_TYPE_TEST_SERVICE_LISTENER ||
       conn->type == CONN_TYPE_DIR_LISTENER ||
       conn->type == CONN_TYPE_CONTROL_LISTENER)
     return 1;
@@ -2769,6 +2791,8 @@
     case CONN_TYPE_AP:
       return connection_edge_process_inbuf(TO_EDGE_CONN(conn),
                                            package_partial);
+    case CONN_TYPE_TEST:
+      return connection_testserv_process_inbuf(conn);
     case CONN_TYPE_DIR:
       return connection_dir_process_inbuf(TO_DIR_CONN(conn));
     case CONN_TYPE_CPUWORKER:
@@ -2822,6 +2846,8 @@
     case CONN_TYPE_AP:
     case CONN_TYPE_EXIT:
       return connection_edge_finished_flushing(TO_EDGE_CONN(conn));
+    case CONN_TYPE_TEST:
+      return connection_testserv_finished_flushing(conn);
     case CONN_TYPE_DIR:
       return connection_dir_finished_flushing(TO_DIR_CONN(conn));
     case CONN_TYPE_CPUWORKER:
@@ -3014,7 +3040,8 @@
       tor_assert(edge_conn->socks_request);
       if (conn->state == AP_CONN_STATE_OPEN) {
         tor_assert(edge_conn->socks_request->has_finished != 0);
-        if (!conn->marked_for_close) {
+        if ((!conn->marked_for_close) &&
+            (!conn->purpose == TEST_PURPOSE_AP)){
           tor_assert(edge_conn->cpath_layer);
           assert_cpath_layer_ok(edge_conn->cpath_layer);
         }
@@ -3025,6 +3052,7 @@
                  conn->purpose == EXIT_PURPOSE_RESOLVE);
     }
   } else if (conn->type == CONN_TYPE_DIR) {
+  } else if (conn->type == CONN_TYPE_TEST) {
   } else {
     /* Purpose is only used for dir and exit types currently */
     tor_assert(!conn->purpose);
@@ -3039,6 +3067,7 @@
     case CONN_TYPE_DIR_LISTENER:
     case CONN_TYPE_CONTROL_LISTENER:
     case CONN_TYPE_AP_DNS_LISTENER:
+    case CONN_TYPE_TEST_SERVICE_LISTENER:
       tor_assert(conn->state == LISTENER_STATE_READY);
       break;
     case CONN_TYPE_OR:
@@ -3057,6 +3086,12 @@
       tor_assert(conn->state <= _AP_CONN_STATE_MAX);
       tor_assert(TO_EDGE_CONN(conn)->socks_request);
       break;
+    case CONN_TYPE_TEST:
+      tor_assert(conn->state >= _TEST_CONN_STATE_MIN);
+      tor_assert(conn->state <= _TEST_CONN_STATE_MAX);
+      tor_assert(conn->purpose >= _TEST_PURPOSE_MIN);
+      tor_assert(conn->purpose <= _TEST_PURPOSE_MAX);
+      break;
     case CONN_TYPE_DIR:
       tor_assert(conn->state >= _DIR_CONN_STATE_MIN);
       tor_assert(conn->state <= _DIR_CONN_STATE_MAX);
Index: src/or/Makefile.am
===================================================================
--- src/or/Makefile.am	(revision 16872)
+++ src/or/Makefile.am	(working copy)
@@ -20,7 +20,7 @@
 	networkstatus.c onion.c policies.c \
 	reasons.c relay.c rendcommon.c rendclient.c rendmid.c \
 	rendservice.c rephist.c router.c routerlist.c routerparse.c \
-	eventdns.c \
+	eventdns.c testservice.c \
 	tor_main.c
 
 AM_CPPFLAGS = -DSHARE_DATADIR="\"$(datadir)\"" \
@@ -42,7 +42,7 @@
 	networkstatus.c onion.c policies.c \
 	reasons.c relay.c rendcommon.c rendclient.c rendmid.c \
 	rendservice.c rephist.c router.c routerlist.c routerparse.c \
-	eventdns.c \
+	eventdns.c testservice.c\
 	test_data.c test.c
 
 test_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ \
Index: src/common/log.h
===================================================================
--- src/common/log.h	(revision 16872)
+++ src/common/log.h	(working copy)
@@ -90,9 +90,11 @@
 #define LD_EXIT     LD_EDGE
 /** Bandwidth accounting. */
 #define LD_ACCT     (1u<<17)
+/** Browser Test Service. */
+#define LD_TESTSERV (1u<<18)
 
 /** Number of logging domains in the code. */
-#define N_LOGGING_DOMAINS 18
+#define N_LOGGING_DOMAINS 19
 
 typedef uint32_t log_domain_mask_t;
 
Index: doc/spec/control-spec.txt
===================================================================
--- doc/spec/control-spec.txt	(revision 16872)
+++ doc/spec/control-spec.txt	(working copy)
@@ -1424,6 +1424,17 @@
        {Controllers may want to warn their users when this occurs: it
        usually indicates a misconfigured application.}
 
+     TESTSERVICE_REQUEST
+      "TYPE=QuotedString"
+      "RESOURCE=QuotedString"
+       A browser has connected to the test service and requested the
+       resource specified in 'RESOURCE'. RESOURCE can be the main test page
+       itself, or one of the test images it links to.
+
+       {Controllers may want to inform their users of this event: it
+        will assure them that they are connecting to the test page
+        served by their Tor client.}
+
   Actions for STATUS_SERVER can be as follows:
 
      EXTERNAL_ADDRESS