[tor-commits] [tor/master] Refactor begin cell parsing into its own function, with tests.
nickm at torproject.org
nickm at torproject.org
Thu Nov 15 19:47:52 UTC 2012
commit 462ebb270a10f02573b1847649db45b94c0e0fc3
Author: Nick Mathewson <nickm at torproject.org>
Date: Mon Oct 22 11:28:37 2012 -0400
Refactor begin cell parsing into its own function, with tests.
Add 'flags' argument to begin cells, per proposal 208.
---
src/or/connection_edge.c | 105 ++++++++++++++-----
src/or/connection_edge.h | 15 +++
src/test/include.am | 1 +
src/test/test.c | 2 +
src/test/test_cell_formats.c | 234 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 330 insertions(+), 27 deletions(-)
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index f548576..a840b43 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -8,6 +8,7 @@
* \file connection_edge.c
* \brief Handle edge streams.
**/
+#define CONNECTION_EDGE_PRIVATE
#include "or.h"
#include "addressmap.h"
@@ -2059,6 +2060,64 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply,
return;
}
+/* DOCDOC */
+/* static */ int
+begin_cell_parse(const cell_t *cell, begin_cell_t *bcell,
+ uint8_t *end_reason_out)
+{
+ relay_header_t rh;
+ const uint8_t *body, *nul;
+
+ memset(bcell, 0, sizeof(*bcell));
+ *end_reason_out = END_STREAM_REASON_MISC;
+
+ relay_header_unpack(&rh, cell->payload);
+ if (rh.length > RELAY_PAYLOAD_SIZE) {
+ return -2; /*XXXX why not TORPROTOL? */
+ }
+
+ bcell->stream_id = rh.stream_id;
+
+ if (rh.command == RELAY_COMMAND_BEGIN_DIR) {
+ bcell->is_begindir = 1;
+ return 0;
+ } else if (rh.command != RELAY_COMMAND_BEGIN) {
+ log_warn(LD_BUG, "Got an unexpected command %d", (int)rh.command);
+ *end_reason_out = END_STREAM_REASON_INTERNAL;
+ return -1;
+ }
+
+ body = cell->payload + RELAY_HEADER_SIZE;
+ nul = memchr(body, 0, rh.length);
+ if (! nul) {
+ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+ "Relay begin cell has no \\0. Closing.");
+ *end_reason_out = END_STREAM_REASON_TORPROTOCOL;
+ return -1;
+ }
+
+ if (tor_addr_port_split(LOG_PROTOCOL_WARN,
+ (char*)(body),
+ &bcell->address,&bcell->port)<0) {
+ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+ "Unable to parse addr:port in relay begin cell. Closing.");
+ *end_reason_out = END_STREAM_REASON_TORPROTOCOL;
+ return -1;
+ }
+ if (bcell->port == 0) {
+ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+ "Missing port in relay begin cell. Closing.");
+ tor_free(bcell->address);
+ *end_reason_out = END_STREAM_REASON_TORPROTOCOL;
+ return -1;
+ }
+ if (body + rh.length >= nul + 4)
+ bcell->flags = ntohl(get_uint32(nul+1));
+
+ return 0;
+}
+
+
/** A relay 'begin' or 'begin_dir' cell has arrived, and either we are
* an exit hop for the circuit, or we are the origin and it is a
* rendezvous begin.
@@ -2082,10 +2141,13 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
{
edge_connection_t *n_stream;
relay_header_t rh;
- char *address=NULL;
- uint16_t port;
+ char *address = NULL;
+ uint16_t port = 0;
or_circuit_t *or_circ = NULL;
const or_options_t *options = get_options();
+ begin_cell_t bcell;
+ int r;
+ uint8_t end_reason=0;
assert_circuit_ok(circ);
if (!CIRCUIT_IS_ORIGIN(circ))
@@ -2109,31 +2171,20 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
return 0;
}
- if (rh.command == RELAY_COMMAND_BEGIN) {
- if (!memchr(cell->payload+RELAY_HEADER_SIZE, 0, rh.length)) {
- log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
- "Relay begin cell has no \\0. Closing.");
- relay_send_end_cell_from_edge(rh.stream_id, circ,
- END_STREAM_REASON_TORPROTOCOL, NULL);
- return 0;
- }
- if (tor_addr_port_split(LOG_PROTOCOL_WARN,
- (char*)(cell->payload+RELAY_HEADER_SIZE),
- &address,&port)<0) {
- log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
- "Unable to parse addr:port in relay begin cell. Closing.");
- relay_send_end_cell_from_edge(rh.stream_id, circ,
- END_STREAM_REASON_TORPROTOCOL, NULL);
- return 0;
- }
- if (port==0) {
- log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
- "Missing port in relay begin cell. Closing.");
- relay_send_end_cell_from_edge(rh.stream_id, circ,
- END_STREAM_REASON_TORPROTOCOL, NULL);
- tor_free(address);
- return 0;
- }
+ r = begin_cell_parse(cell, &bcell, &end_reason);
+ if (r < -1) {
+ return -1;
+ } else if (r == -1) {
+ tor_free(bcell.address);
+ relay_send_end_cell_from_edge(rh.stream_id, circ, end_reason, NULL);
+ return 0;
+ }
+
+ if (! bcell.is_begindir) {
+ /* Steal reference */
+ address = bcell.address;
+ port = bcell.port;
+
if (or_circ && or_circ->p_chan) {
if (!options->AllowSingleHopExits &&
(or_circ->is_first_hop ||
diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h
index b5938b3..f3d1038 100644
--- a/src/or/connection_edge.h
+++ b/src/or/connection_edge.h
@@ -91,5 +91,20 @@ int connection_edge_update_circuit_isolation(const entry_connection_t *conn,
int dry_run);
void circuit_clear_isolation(origin_circuit_t *circ);
+#ifdef CONNECTION_EDGE_PRIVATE
+/*DOCDOC*/
+typedef struct begin_cell_t {
+ char *address;
+ uint32_t flags;
+ uint16_t port;
+ uint16_t stream_id;
+ unsigned is_begindir : 1;
+} begin_cell_t;
+
+int begin_cell_parse(const cell_t *cell, begin_cell_t *bcell,
+ uint8_t *end_reason_out);
+#endif
+
+
#endif
diff --git a/src/test/include.am b/src/test/include.am
index bdfe498..075df36 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -14,6 +14,7 @@ src_test_AM_CPPFLAGS = -DSHARE_DATADIR="\"$(datadir)\"" \
src_test_test_SOURCES = \
src/test/test.c \
src/test/test_addr.c \
+ src/test/test_cell_formats.c \
src/test/test_containers.c \
src/test/test_crypto.c \
src/test/test_data.c \
diff --git a/src/test/test.c b/src/test/test.c
index 1eaa65c..fc3c36e 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -1983,6 +1983,7 @@ extern struct testcase_t pt_tests[];
extern struct testcase_t config_tests[];
extern struct testcase_t introduce_tests[];
extern struct testcase_t replaycache_tests[];
+extern struct testcase_t cell_format_tests[];
static struct testgroup_t testgroups[] = {
{ "", test_array },
@@ -1991,6 +1992,7 @@ static struct testgroup_t testgroups[] = {
{ "crypto/", crypto_tests },
{ "container/", container_tests },
{ "util/", util_tests },
+ { "cellfmt/", cell_format_tests },
{ "dir/", dir_tests },
{ "dir/md/", microdesc_tests },
{ "pt/", pt_tests },
diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c
new file mode 100644
index 0000000..2199a05
--- /dev/null
+++ b/src/test/test_cell_formats.c
@@ -0,0 +1,234 @@
+/* Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2012, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "orconfig.h"
+
+#define CONNECTION_EDGE_PRIVATE
+#include "or.h"
+#include "connection_edge.h"
+#include "relay.h"
+#include "test.h"
+
+#include <stdlib.h>
+#include <string.h>
+
+static void
+test_cfmt_relay_header(void *arg)
+{
+ relay_header_t rh;
+ const uint8_t hdr_1[RELAY_HEADER_SIZE] =
+ "\x03" "\x00\x00" "\x21\x22" "ABCD" "\x01\x03";
+ uint8_t hdr_out[RELAY_HEADER_SIZE];
+ (void)arg;
+
+ tt_int_op(sizeof(hdr_1), ==, RELAY_HEADER_SIZE);
+ relay_header_unpack(&rh, hdr_1);
+ tt_int_op(rh.command, ==, 3);
+ tt_int_op(rh.recognized, ==, 0);
+ tt_int_op(rh.stream_id, ==, 0x2122);
+ test_mem_op(rh.integrity, ==, "ABCD", 4);
+ tt_int_op(rh.length, ==, 0x103);
+
+ relay_header_pack(hdr_out, &rh);
+ test_mem_op(hdr_out, ==, hdr_1, RELAY_HEADER_SIZE);
+
+ done:
+ ;
+}
+
+static void
+make_relay_cell(cell_t *out, uint8_t command,
+ const void *body, size_t bodylen)
+{
+ relay_header_t rh;
+
+ memset(&rh, 0, sizeof(rh));
+ rh.stream_id = 5;
+ rh.command = command;
+ rh.length = bodylen;
+
+ out->command = CELL_RELAY;
+ out->circ_id = 10;
+ relay_header_pack(out->payload, &rh);
+
+ memcpy(out->payload + RELAY_HEADER_SIZE, body, bodylen);
+}
+
+static void
+test_cfmt_begin_cells(void *arg)
+{
+ cell_t cell;
+ begin_cell_t bcell;
+ uint8_t end_reason;
+ (void)arg;
+
+ /* Try begindir. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN_DIR, "", 0);
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_ptr_op(NULL, ==, bcell.address);
+ tt_int_op(0, ==, bcell.flags);
+ tt_int_op(0, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(1, ==, bcell.is_begindir);
+
+ /* A Begindir with extra stuff. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN_DIR, "12345", 5);
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_ptr_op(NULL, ==, bcell.address);
+ tt_int_op(0, ==, bcell.flags);
+ tt_int_op(0, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(1, ==, bcell.is_begindir);
+
+ /* A short but valid begin cell */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "a.b:9", 6);
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_str_op("a.b", ==, bcell.address);
+ tt_int_op(0, ==, bcell.flags);
+ tt_int_op(9, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(0, ==, bcell.is_begindir);
+ tor_free(bcell.address);
+
+ /* A significantly loner begin cell */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ {
+ const char c[] = "here-is-a-nice-long.hostname.com:65535";
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, c, strlen(c)+1);
+ }
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_str_op("here-is-a-nice-long.hostname.com", ==, bcell.address);
+ tt_int_op(0, ==, bcell.flags);
+ tt_int_op(65535, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(0, ==, bcell.is_begindir);
+ tor_free(bcell.address);
+
+ /* An IPv4 begin cell. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "18.9.22.169:80", 15);
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_str_op("18.9.22.169", ==, bcell.address);
+ tt_int_op(0, ==, bcell.flags);
+ tt_int_op(80, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(0, ==, bcell.is_begindir);
+ tor_free(bcell.address);
+
+ /* An IPv6 begin cell. Let's make sure we handle colons*/
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN,
+ "[2620::6b0:b:1a1a:0:26e5:480e]:80", 34);
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_str_op("[2620::6b0:b:1a1a:0:26e5:480e]", ==, bcell.address);
+ tt_int_op(0, ==, bcell.flags);
+ tt_int_op(80, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(0, ==, bcell.is_begindir);
+ tor_free(bcell.address);
+
+ /* a begin cell with extra junk but not enough for flags. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ {
+ const char c[] = "another.example.com:80\x00\x01\x02";
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, c, sizeof(c)-1);
+ }
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_str_op("another.example.com", ==, bcell.address);
+ tt_int_op(0, ==, bcell.flags);
+ tt_int_op(80, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(0, ==, bcell.is_begindir);
+ tor_free(bcell.address);
+
+ /* a begin cell with flags. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ {
+ const char c[] = "another.example.com:443\x00\x01\x02\x03\x04";
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, c, sizeof(c)-1);
+ }
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_str_op("another.example.com", ==, bcell.address);
+ tt_int_op(0x1020304, ==, bcell.flags);
+ tt_int_op(443, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(0, ==, bcell.is_begindir);
+ tor_free(bcell.address);
+
+ /* a begin cell with flags and even more cruft after that. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ {
+ const char c[] = "a-further.example.com:22\x00\xee\xaa\x00\xffHi mom";
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, c, sizeof(c)-1);
+ }
+ tt_int_op(0, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ tt_str_op("a-further.example.com", ==, bcell.address);
+ tt_int_op(0xeeaa00ff, ==, bcell.flags);
+ tt_int_op(22, ==, bcell.port);
+ tt_int_op(5, ==, bcell.stream_id);
+ tt_int_op(0, ==, bcell.is_begindir);
+ tor_free(bcell.address);
+
+ /* bad begin cell: impossible length. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "a.b:80", 7);
+ cell.payload[9] = 0x01; /* Set length to 510 */
+ cell.payload[10] = 0xfe;
+ {
+ relay_header_t rh;
+ relay_header_unpack(&rh, cell.payload);
+ tt_int_op(rh.length, ==, 510);
+ }
+ tt_int_op(-2, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+
+ /* Bad begin cell: no body. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "", 0);
+ tt_int_op(-1, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+
+ /* bad begin cell: no body. */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "", 0);
+ tt_int_op(-1, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+
+ /* bad begin cell: no colon */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "a.b", 4);
+ tt_int_op(-1, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+
+ /* bad begin cell: no ports */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "a.b:", 5);
+ tt_int_op(-1, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+
+ /* bad begin cell: bad port */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "a.b:xyz", 8);
+ tt_int_op(-1, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "a.b:100000", 11);
+ tt_int_op(-1, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+
+ /* bad begin cell: no nul */
+ memset(&bcell, 0x7f, sizeof(bcell));
+ make_relay_cell(&cell, RELAY_COMMAND_BEGIN, "a.b:80", 6);
+ tt_int_op(-1, ==, begin_cell_parse(&cell, &bcell, &end_reason));
+
+ done:
+ tor_free(bcell.address);
+}
+
+#define TEST(name, flags) \
+ { #name, test_cfmt_ ## name, flags, 0, NULL }
+
+struct testcase_t cell_format_tests[] = {
+ TEST(relay_header, 0),
+ TEST(begin_cells, 0),
+ END_OF_TESTCASES
+};
+
More information about the tor-commits
mailing list