132 Browser Test Service - Reviewer-Friendly Patch
Robert Hogan
robert at roberthogan.net
Fri Sep 12 18:25:29 UTC 2008
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
More information about the tor-dev
mailing list