commit 7a032c5e4838651b62b99938d56e94b4a6fc6197 Author: George Kadianakis <desnacked@riseup.net> Date: Tue Jul 23 13:01:12 2019 +0300 Split connection_edge_process_relay_cell() in two functions. One function does the validation, the other does the handling. --- src/core/or/relay.c | 225 +++++++++++++++++++++++++++++----------------------- 1 file changed, 124 insertions(+), 101 deletions(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 9e691a02b..84fc58787 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1576,83 +1576,27 @@ process_sendme_cell(const relay_header_t *rh, const cell_t *cell, return 0; } -/** An incoming relay cell has arrived on circuit <b>circ</b>. If - * <b>conn</b> is NULL this is a control cell, else <b>cell</b> is - * destined for <b>conn</b>. - * - * If <b>layer_hint</b> is defined, then we're the origin of the - * circuit, and it specifies the hop that packaged <b>cell</b>. +/** A helper for connection_edge_process_relay_cell(): Actually handles the + * cell that we received on the connection. * - * Return -reason if you want to warn and tear down the circuit, else 0. + * The arguments are the same as in the parent function + * connection_edge_process_relay_cell(), plus the relay header <b>rh</b> as + * unpacked by the parent function, and <b>optimistic_data</b> as set by the + * parent function. */ -STATIC int -connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, - edge_connection_t *conn, - crypt_path_t *layer_hint) +static int +handle_relay_command(cell_t *cell, circuit_t *circ, + edge_connection_t *conn, + crypt_path_t *layer_hint, + relay_header_t *rh, + int optimistic_data) { - static int num_seen=0; - relay_header_t rh; unsigned domain = layer_hint?LD_APP:LD_EXIT; int reason; - int optimistic_data = 0; /* Set to 1 if we receive data on a stream - * that's in the EXIT_CONN_STATE_RESOLVING - * or EXIT_CONN_STATE_CONNECTING states. */ - - tor_assert(cell); - tor_assert(circ); - - relay_header_unpack(&rh, cell->payload); -// log_fn(LOG_DEBUG,"command %d stream %d", rh.command, rh.stream_id); - num_seen++; - log_debug(domain, "Now seen %d relay cells here (command %d, stream %d).", - num_seen, rh.command, rh.stream_id); - if (rh.length > RELAY_PAYLOAD_SIZE) { - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Relay cell length field too long. Closing circuit."); - return - END_CIRC_REASON_TORPROTOCOL; - } - - if (rh.stream_id == 0) { - switch (rh.command) { - case RELAY_COMMAND_BEGIN: - case RELAY_COMMAND_CONNECTED: - case RELAY_COMMAND_END: - case RELAY_COMMAND_RESOLVE: - case RELAY_COMMAND_RESOLVED: - case RELAY_COMMAND_BEGIN_DIR: - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay command %d with zero " - "stream_id. Dropping.", (int)rh.command); - return 0; - default: - ; - } - } - - /* Tell circpad that we've received a recognized cell */ - circpad_deliver_recognized_relay_cell_events(circ, rh.command, layer_hint); - - /* either conn is NULL, in which case we've got a control cell, or else - * conn points to the recognized stream. */ - if (conn && !connection_state_is_open(TO_CONN(conn))) { - if (conn->base_.type == CONN_TYPE_EXIT && - (conn->base_.state == EXIT_CONN_STATE_CONNECTING || - conn->base_.state == EXIT_CONN_STATE_RESOLVING) && - rh.command == RELAY_COMMAND_DATA) { - /* Allow DATA cells to be delivered to an exit node in state - * EXIT_CONN_STATE_CONNECTING or EXIT_CONN_STATE_RESOLVING. - * This speeds up HTTP, for example. */ - optimistic_data = 1; - } else if (rh.stream_id == 0 && rh.command == RELAY_COMMAND_DATA) { - log_warn(LD_BUG, "Somehow I had a connection that matched a " - "data cell with stream ID 0."); - } else { - return connection_edge_process_relay_cell_not_open( - &rh, cell, circ, conn, layer_hint); - } - } + tor_assert(rh); - switch (rh.command) { + switch (rh->command) { case RELAY_COMMAND_DROP: /* Already examined in circpad_deliver_recognized_relay_cell_events */ return 0; @@ -1661,7 +1605,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, return 0; case RELAY_COMMAND_PADDING_NEGOTIATED: if (circpad_handle_padding_negotiated(circ, cell, layer_hint) == 0) - circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length); + circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length); return 0; } @@ -1694,7 +1638,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, "Begin cell for known stream. Dropping."); return 0; } - if (rh.command == RELAY_COMMAND_BEGIN_DIR && + if (rh->command == RELAY_COMMAND_BEGIN_DIR && circ->purpose != CIRCUIT_PURPOSE_S_REND_JOINED) { /* Assign this circuit and its app-ward OR connection a unique ID, * so that we can measure download times. The local edge and dir @@ -1721,7 +1665,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, /* Consider sending a circuit-level SENDME cell. */ sendme_circuit_consider_sending(circ, layer_hint); - if (rh.stream_id == 0) { + if (rh->stream_id == 0) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay data cell with zero " "stream_id. Dropping."); return 0; @@ -1729,16 +1673,16 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); if (connection_half_edge_is_valid_data(ocirc->half_streams, - rh.stream_id)) { - circuit_read_valid_data(ocirc, rh.length); + rh->stream_id)) { + circuit_read_valid_data(ocirc, rh->length); log_info(domain, "data cell on circ %u valid on half-closed " - "stream id %d", ocirc->global_identifier, rh.stream_id); + "stream id %d", ocirc->global_identifier, rh->stream_id); } } log_info(domain,"data cell dropped, unknown stream (streamid %d).", - rh.stream_id); + rh->stream_id); return 0; } @@ -1753,13 +1697,13 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, return -END_CIRC_REASON_TORPROTOCOL; } /* Total all valid application bytes delivered */ - if (CIRCUIT_IS_ORIGIN(circ) && rh.length > 0) { - circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length); + if (CIRCUIT_IS_ORIGIN(circ) && rh->length > 0) { + circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length); } - stats_n_data_bytes_received += rh.length; + stats_n_data_bytes_received += rh->length; connection_buf_add((char*)(cell->payload + RELAY_HEADER_SIZE), - rh.length, TO_CONN(conn)); + rh->length, TO_CONN(conn)); #ifdef MEASUREMENTS_21206 /* Count number of RELAY_DATA cells received on a linked directory @@ -1780,20 +1724,20 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, return 0; case RELAY_COMMAND_END: - reason = rh.length > 0 ? + reason = rh->length > 0 ? get_uint8(cell->payload+RELAY_HEADER_SIZE) : END_STREAM_REASON_MISC; if (!conn) { if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); if (connection_half_edge_is_valid_end(ocirc->half_streams, - rh.stream_id)) { + rh->stream_id)) { - circuit_read_valid_data(ocirc, rh.length); + circuit_read_valid_data(ocirc, rh->length); log_info(domain, "end cell (%s) on circ %u valid on half-closed " "stream id %d", stream_end_reason_to_string(reason), - ocirc->global_identifier, rh.stream_id); + ocirc->global_identifier, rh->stream_id); return 0; } } @@ -1825,7 +1769,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, /* Total all valid application bytes delivered */ if (CIRCUIT_IS_ORIGIN(circ)) { - circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length); + circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length); } } return 0; @@ -1833,7 +1777,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, case RELAY_COMMAND_EXTEND2: { static uint64_t total_n_extend=0, total_nonearly=0; total_n_extend++; - if (rh.stream_id) { + if (rh->stream_id) { log_fn(LOG_PROTOCOL_WARN, domain, "'extend' cell received for non-zero stream. Dropping."); return 0; @@ -1874,9 +1818,9 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, log_debug(domain,"Got an extended cell! Yay."); { extended_cell_t extended_cell; - if (extended_cell_parse(&extended_cell, rh.command, + if (extended_cell_parse(&extended_cell, rh->command, (const uint8_t*)cell->payload+RELAY_HEADER_SIZE, - rh.length)<0) { + rh->length)<0) { log_warn(LD_PROTOCOL, "Can't parse EXTENDED cell; killing circuit."); return -END_CIRC_REASON_TORPROTOCOL; @@ -1894,7 +1838,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, } /* Total all valid bytes delivered. */ if (CIRCUIT_IS_ORIGIN(circ)) { - circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length); + circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length); } return 0; case RELAY_COMMAND_TRUNCATE: @@ -1938,7 +1882,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, * circuit is being torn down anyway, though. */ if (CIRCUIT_IS_ORIGIN(circ)) { circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), - rh.length); + rh->length); } circuit_truncated(TO_ORIGIN_CIRCUIT(circ), get_uint8(cell->payload + RELAY_HEADER_SIZE)); @@ -1953,11 +1897,11 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); if (connection_half_edge_is_valid_connected(ocirc->half_streams, - rh.stream_id)) { - circuit_read_valid_data(ocirc, rh.length); + rh->stream_id)) { + circuit_read_valid_data(ocirc, rh->length); log_info(domain, "connected cell on circ %u valid on half-closed " - "stream id %d", ocirc->global_identifier, rh.stream_id); + "stream id %d", ocirc->global_identifier, rh->stream_id); return 0; } } @@ -1965,10 +1909,10 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, log_info(domain, "'connected' received on circid %u for streamid %d, " "no conn attached anymore. Ignoring.", - (unsigned)circ->n_circ_id, rh.stream_id); + (unsigned)circ->n_circ_id, rh->stream_id); return 0; case RELAY_COMMAND_SENDME: - return process_sendme_cell(&rh, cell, circ, conn, layer_hint, domain); + return process_sendme_cell(rh, cell, circ, conn, layer_hint, domain); case RELAY_COMMAND_RESOLVE: if (layer_hint) { log_fn(LOG_PROTOCOL_WARN, LD_APP, @@ -1996,11 +1940,11 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); if (connection_half_edge_is_valid_resolved(ocirc->half_streams, - rh.stream_id)) { - circuit_read_valid_data(ocirc, rh.length); + rh->stream_id)) { + circuit_read_valid_data(ocirc, rh->length); log_info(domain, "resolved cell on circ %u valid on half-closed " - "stream id %d", ocirc->global_identifier, rh.stream_id); + "stream id %d", ocirc->global_identifier, rh->stream_id); return 0; } } @@ -2018,17 +1962,96 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, case RELAY_COMMAND_INTRO_ESTABLISHED: case RELAY_COMMAND_RENDEZVOUS_ESTABLISHED: rend_process_relay_cell(circ, layer_hint, - rh.command, rh.length, + rh->command, rh->length, cell->payload+RELAY_HEADER_SIZE); return 0; } log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Received unknown relay command %d. Perhaps the other side is using " "a newer version of Tor? Dropping.", - rh.command); + rh->command); return 0; /* for forward compatibility, don't kill the circuit */ } +/** An incoming relay cell has arrived on circuit <b>circ</b>. If + * <b>conn</b> is NULL this is a control cell, else <b>cell</b> is + * destined for <b>conn</b>. + * + * If <b>layer_hint</b> is defined, then we're the origin of the + * circuit, and it specifies the hop that packaged <b>cell</b>. + * + * Return -reason if you want to warn and tear down the circuit, else 0. + */ +STATIC int +connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, + edge_connection_t *conn, + crypt_path_t *layer_hint) +{ + static int num_seen=0; + relay_header_t rh; + unsigned domain = layer_hint?LD_APP:LD_EXIT; + int optimistic_data = 0; /* Set to 1 if we receive data on a stream + * that's in the EXIT_CONN_STATE_RESOLVING + * or EXIT_CONN_STATE_CONNECTING states. */ + + tor_assert(cell); + tor_assert(circ); + + relay_header_unpack(&rh, cell->payload); +// log_fn(LOG_DEBUG,"command %d stream %d", rh.command, rh.stream_id); + num_seen++; + log_debug(domain, "Now seen %d relay cells here (command %d, stream %d).", + num_seen, rh.command, rh.stream_id); + + if (rh.length > RELAY_PAYLOAD_SIZE) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Relay cell length field too long. Closing circuit."); + return - END_CIRC_REASON_TORPROTOCOL; + } + + if (rh.stream_id == 0) { + switch (rh.command) { + case RELAY_COMMAND_BEGIN: + case RELAY_COMMAND_CONNECTED: + case RELAY_COMMAND_END: + case RELAY_COMMAND_RESOLVE: + case RELAY_COMMAND_RESOLVED: + case RELAY_COMMAND_BEGIN_DIR: + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay command %d with zero " + "stream_id. Dropping.", (int)rh.command); + return 0; + default: + ; + } + } + + /* Tell circpad that we've received a recognized cell */ + circpad_deliver_recognized_relay_cell_events(circ, rh.command, layer_hint); + + /* either conn is NULL, in which case we've got a control cell, or else + * conn points to the recognized stream. */ + if (conn && !connection_state_is_open(TO_CONN(conn))) { + if (conn->base_.type == CONN_TYPE_EXIT && + (conn->base_.state == EXIT_CONN_STATE_CONNECTING || + conn->base_.state == EXIT_CONN_STATE_RESOLVING) && + rh.command == RELAY_COMMAND_DATA) { + /* Allow DATA cells to be delivered to an exit node in state + * EXIT_CONN_STATE_CONNECTING or EXIT_CONN_STATE_RESOLVING. + * This speeds up HTTP, for example. */ + optimistic_data = 1; + } else if (rh.stream_id == 0 && rh.command == RELAY_COMMAND_DATA) { + log_warn(LD_BUG, "Somehow I had a connection that matched a " + "data cell with stream ID 0."); + } else { + return connection_edge_process_relay_cell_not_open( + &rh, cell, circ, conn, layer_hint); + } + } + + return handle_relay_command(cell, circ, conn, layer_hint, + &rh, optimistic_data); +} + /** How many relay_data cells have we built, ever? */ uint64_t stats_n_data_cells_packaged = 0; /** How many bytes of data have we put in relay_data cells have we built,