commit 5c68a1efaa9511baf2a2af0a49946e0a2de9e246 Author: Nick Mathewson nickm@torproject.org Date: Thu Dec 6 00:21:24 2012 -0500

Don't check create cells too much when we're relaying them

We want to sanity-check our own create cells carefully, and other people's loosely. --- src/or/circuitbuild.c | 21 ++++++++++++--------- src/or/onion.c | 21 +++++++++++++++++---- src/or/onion.h | 1 + src/test/test_cell_formats.c | 3 ++- 4 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 43ad9f4..b7ab47f 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -55,7 +55,8 @@ static channel_t * channel_connect_for_circuit(const tor_addr_t *addr, uint16_t port, const char *id_digest); static int circuit_deliver_create_cell(circuit_t *circ, - const create_cell_t *create_cell); + const create_cell_t *create_cell, + int relayed); static int onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit); static crypt_path_t *onion_next_hop_in_cpath(crypt_path_t *cpath); static int onion_extend_cpath(origin_circuit_t *circ); @@ -474,7 +475,7 @@ circuit_n_chan_done(channel_t *chan, int status) } else { /* pull the create cell out of circ->n_chan_create_cell, and send it */ tor_assert(circ->n_chan_create_cell); - if (circuit_deliver_create_cell(circ, circ->n_chan_create_cell)<0) { + if (circuit_deliver_create_cell(circ, circ->n_chan_create_cell, 1)<0) { circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); continue; } @@ -491,14 +492,16 @@ circuit_n_chan_done(channel_t *chan, int status) * for the outgoing * circuit <b>circ</b>, and deliver a cell of type <b>cell_type</b> * (either CELL_CREATE or CELL_CREATE_FAST) with payload <b>payload</b> - * to this circuit. DOCDOC payload_len + * to this circuit. DOCDOC new arguments * Return -1 if we failed to find a suitable circid, else return 0. */ static int -circuit_deliver_create_cell(circuit_t *circ, const create_cell_t *create_cell) +circuit_deliver_create_cell(circuit_t *circ, const create_cell_t *create_cell, + int relayed) { cell_t cell; circid_t id; + int r;

tor_assert(circ); tor_assert(circ->n_chan); @@ -516,7 +519,9 @@ circuit_deliver_create_cell(circuit_t *circ, const create_cell_t *create_cell) circuit_set_n_circid_chan(circ, id, circ->n_chan);

memset(&cell, 0, sizeof(cell_t)); - if (create_cell_format(&cell, create_cell) < 0) { + r = relayed ? create_cell_format_relayed(&cell, create_cell) + : create_cell_format(&cell, create_cell); + if (r < 0) { log_warn(LD_CIRC,"Couldn't format create cell"); return -1; } @@ -657,7 +662,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) } cc.handshake_len = len;

- if (circuit_deliver_create_cell(TO_CIRCUIT(circ), &cc) < 0) + if (circuit_deliver_create_cell(TO_CIRCUIT(circ), &cc, 0) < 0) return - END_CIRC_REASON_RESOURCELIMIT;

circ->cpath->state = CPATH_STATE_AWAITING_KEYS; @@ -901,8 +906,6 @@ circuit_extend(cell_t *cell, circuit_t *circ) &ec.orport_ipv4.addr, ec.orport_ipv4.port);

- /* XXXX Make sure we can eventually deliver create cell with weird - * content */ circ->n_chan_create_cell = tor_memdup(&ec.create_cell, sizeof(ec.create_cell));

@@ -933,7 +936,7 @@ circuit_extend(cell_t *cell, circuit_t *circ) "n_chan is %s", channel_get_canonical_remote_descr(n_chan));

- if (circuit_deliver_create_cell(circ, &ec.create_cell) < 0) + if (circuit_deliver_create_cell(circ, &ec.create_cell, 1) < 0) return -1; return 0; } diff --git a/src/or/onion.c b/src/or/onion.c index 753ddcf..b42a66c 100644 --- a/src/or/onion.c +++ b/src/or/onion.c @@ -808,13 +808,14 @@ extended_cell_parse(extended_cell_t *cell_out,

/** Fill <b>cell_out</b> with a correctly formatted version of the * CREATE{,_FAST,2} cell in <b>cell_in</b>. Return 0 on success, -1 on - * failure. */ -int -create_cell_format(cell_t *cell_out, const create_cell_t *cell_in) + * failure. This is a cell we didn't originate if <b>relayed</b> is true. */ +static int +create_cell_format_impl(cell_t *cell_out, const create_cell_t *cell_in, + int relayed) { uint8_t *p; size_t space; - if (check_create_cell(cell_in, 0) < 0) + if (check_create_cell(cell_in, relayed) < 0) return -1;

memset(cell_out->payload, 0, sizeof(cell_out->payload)); @@ -848,6 +849,18 @@ create_cell_format(cell_t *cell_out, const create_cell_t *cell_in) return 0; }

+int +create_cell_format(cell_t *cell_out, const create_cell_t *cell_in) +{ + return create_cell_format_impl(cell_out, cell_in, 0); +} + +int +create_cell_format_relayed(cell_t *cell_out, const create_cell_t *cell_in) +{ + return create_cell_format_impl(cell_out, cell_in, 1); +} + /** Fill <b>cell_out</b> with a correctly formatted version of the * CREATED{,_FAST,2} cell in <b>cell_in</b>. Return 0 on success, -1 on * failure. */ diff --git a/src/or/onion.h b/src/or/onion.h index 36cb761..e408139 100644 --- a/src/or/onion.h +++ b/src/or/onion.h @@ -106,6 +106,7 @@ int extended_cell_parse(extended_cell_t *cell_out, uint8_t command, const uint8_t *payload_in, size_t payload_len);

int create_cell_format(cell_t *cell_out, const create_cell_t *cell_in); +int create_cell_format_relayed(cell_t *cell_out, const create_cell_t *cell_in); int created_cell_format(cell_t *cell_out, const created_cell_t *cell_in); int extend_cell_format(uint8_t *command_out, uint16_t *len_out, uint8_t *payload_out, const extend_cell_t *cell_in); diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c index bdbc555..932124c 100644 --- a/src/test/test_cell_formats.c +++ b/src/test/test_cell_formats.c @@ -651,7 +651,7 @@ test_cfmt_extend_cells(void *arg) tt_int_op(p2_cmd, ==, RELAY_COMMAND_EXTEND); tt_int_op(p2_len, ==, 26+TAP_ONIONSKIN_CHALLENGE_LEN); test_memeq(p2, p, RELAY_PAYLOAD_SIZE); - tt_int_op(0, ==, create_cell_format(&cell, cc)); + tt_int_op(0, ==, create_cell_format_relayed(&cell, cc));

/* Now let's do a minimal ntor EXTEND2 cell. */ memset(&ec, 0xff, sizeof(ec)); @@ -721,6 +721,7 @@ test_cfmt_extend_cells(void *arg) /* Now the handshake prologue */ "01050063"); test_memeq(p2+1+8+22+4, b, 99+20); + tt_int_op(0, ==, create_cell_format_relayed(&cell, cc));

/* == Now try parsing some junk */