commit 462ebb270a10f02573b1847649db45b94c0e0fc3 Author: Nick Mathewson nickm@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 +}; +