From 09f4fe868aba86cef63d259132875f0a3ec169a2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 17 Apr 2025 20:21:06 -0400 Subject: [PATCH] Rename and hand-audit all users of RELAY_PAYLOAD_SIZE. Since the maximum number of bytes you can put in a relay message is no longer constant, it doesn't make sense to have a "size" for this. Instead, we can only have a "max" or "min" size. --- src/app/main/main.c | 5 +++-- src/core/mainloop/connection.c | 4 ++-- src/core/or/circuitbuild.c | 8 +++++++- src/core/or/circuituse.c | 12 +++++++----- src/core/or/command.c | 7 ++++++- src/core/or/conflux_cell.c | 21 +++++++++++--------- src/core/or/congestion_control_flow.c | 12 +++++++----- src/core/or/connection_edge.c | 15 ++++++++------ src/core/or/onion.c | 28 +++++++++++++++------------ src/core/or/onion.h | 7 +++++-- src/core/or/or.h | 22 +++++++++++++++------ src/core/or/relay.c | 2 +- src/core/or/sendme.c | 6 +++--- src/core/or/status.c | 3 ++- src/feature/client/circpathbias.c | 4 ++-- src/feature/hs/hs_cell.c | 21 +++++++++++--------- src/feature/hs/hs_circuit.c | 8 ++++---- src/feature/relay/dns.c | 10 +++++++--- src/test/test_cell_formats.c | 4 ++-- 19 files changed, 123 insertions(+), 76 deletions(-) diff --git a/src/app/main/main.c b/src/app/main/main.c index 962e6e9d67..b4957ce0d9 100644 --- a/src/app/main/main.c +++ b/src/app/main/main.c @@ -377,6 +377,7 @@ dumpstats(int severity) channel_dumpstats(severity); channel_listener_dumpstats(severity); + // TODO CGO: Use of RELAY_PAYLOAD_SIZE_MAX may make this a bit wrong. tor_log(severity, LD_NET, "Cells processed: %"PRIu64" padding\n" " %"PRIu64" create\n" @@ -395,11 +396,11 @@ dumpstats(int severity) if (stats_n_data_cells_packaged) tor_log(severity,LD_NET,"Average packaged cell fullness: %2.3f%%", 100*(((double)stats_n_data_bytes_packaged) / - ((double)stats_n_data_cells_packaged*RELAY_PAYLOAD_SIZE)) ); + ((double)stats_n_data_cells_packaged*RELAY_PAYLOAD_SIZE_MAX)) ); if (stats_n_data_cells_received) tor_log(severity,LD_NET,"Average delivered cell fullness: %2.3f%%", 100*(((double)stats_n_data_bytes_received) / - ((double)stats_n_data_cells_received*RELAY_PAYLOAD_SIZE)) ); + ((double)stats_n_data_cells_received*RELAY_PAYLOAD_SIZE_MAX)) ); cpuworker_log_onionskin_overhead(severity, ONION_HANDSHAKE_TYPE_TAP, "TAP"); cpuworker_log_onionskin_overhead(severity, ONION_HANDSHAKE_TYPE_NTOR,"ntor"); diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 559c640f42..d8fd4839f9 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -3481,7 +3481,7 @@ connection_bucket_get_share(int base, int priority, static ssize_t connection_bucket_read_limit(connection_t *conn, time_t now) { - int base = RELAY_PAYLOAD_SIZE; + int base = RELAY_PAYLOAD_SIZE_MAX; int priority = conn->type != CONN_TYPE_DIR; ssize_t conn_bucket = -1; size_t global_bucket_val = token_bucket_rw_get_read(&global_bucket); @@ -3535,7 +3535,7 @@ connection_bucket_read_limit(connection_t *conn, time_t now) ssize_t connection_bucket_write_limit(connection_t *conn, time_t now) { - int base = RELAY_PAYLOAD_SIZE; + int base = RELAY_PAYLOAD_SIZE_MAX; int priority = conn->type != CONN_TYPE_DIR; size_t conn_bucket = buf_datalen(conn->outbuf); size_t global_bucket_val = token_bucket_rw_get_write(&global_bucket); diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 5047724a72..da8b51c374 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -1185,12 +1185,18 @@ circuit_send_intermediate_onion_skin(origin_circuit_t *circ, { uint8_t command = 0; uint16_t payload_len=0; - uint8_t payload[RELAY_PAYLOAD_SIZE]; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX]; if (extend_cell_format(&command, &payload_len, payload, &ec)<0) { log_warn(LD_CIRC,"Couldn't format extend cell"); return -END_CIRC_REASON_INTERNAL; } + if (payload_len > circuit_max_relay_payload( + TO_CIRCUIT(circ), hop->prev, command)) { + log_warn(LD_BUG, "Generated a too-long extend cell"); + return -END_CIRC_REASON_INTERNAL; + } + /* send it to hop->prev, because that relay will transfer * it to a create cell and then send to hop */ if (relay_send_command_from_edge(0, TO_CIRCUIT(circ), diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index e114a52541..1967593cfa 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -3181,15 +3181,16 @@ circuit_sent_valid_data(origin_circuit_t *circ, uint16_t relay_body_len) { if (!circ) return; - tor_assertf_nonfatal(relay_body_len <= RELAY_PAYLOAD_SIZE, + tor_assertf_nonfatal(relay_body_len <= RELAY_PAYLOAD_SIZE_MAX, "Wrong relay_body_len: %d (should be at most %d)", - relay_body_len, RELAY_PAYLOAD_SIZE); + relay_body_len, RELAY_PAYLOAD_SIZE_MAX); circ->n_delivered_written_circ_bw = tor_add_u32_nowrap(circ->n_delivered_written_circ_bw, relay_body_len); + // TODO CGO: I think this may now be somewhat incorrect. circ->n_overhead_written_circ_bw = tor_add_u32_nowrap(circ->n_overhead_written_circ_bw, - RELAY_PAYLOAD_SIZE-relay_body_len); + RELAY_PAYLOAD_SIZE_MAX-relay_body_len); } /** @@ -3202,11 +3203,12 @@ circuit_read_valid_data(origin_circuit_t *circ, uint16_t relay_body_len) { if (!circ) return; - tor_assert_nonfatal(relay_body_len <= RELAY_PAYLOAD_SIZE); + tor_assert_nonfatal(relay_body_len <= RELAY_PAYLOAD_SIZE_MAX); circ->n_delivered_read_circ_bw = tor_add_u32_nowrap(circ->n_delivered_read_circ_bw, relay_body_len); + // TODO CGO: I think this may now be somewhat incorrect. circ->n_overhead_read_circ_bw = tor_add_u32_nowrap(circ->n_overhead_read_circ_bw, - RELAY_PAYLOAD_SIZE-relay_body_len); + RELAY_PAYLOAD_SIZE_MAX-relay_body_len); } diff --git a/src/core/or/command.c b/src/core/or/command.c index c35400d7a1..414f5217a8 100644 --- a/src/core/or/command.c +++ b/src/core/or/command.c @@ -456,7 +456,7 @@ command_process_created_cell(cell_t *cell, channel_t *chan) } else { /* pack it into an extended relay cell, and send it. */ uint8_t command=0; uint16_t len=0; - uint8_t payload[RELAY_PAYLOAD_SIZE]; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX]; log_debug(LD_OR, "Converting created cell to extended relay cell, sending."); memset(payload, 0, sizeof(payload)); @@ -469,6 +469,11 @@ command_process_created_cell(cell_t *cell, channel_t *chan) circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL); return; } + if (len > circuit_max_relay_payload(circ, NULL, command)) { + log_fn(LOG_PROTOCOL_WARN, LD_OR, "Created cell too big to package."); + circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL); + return; + } relay_send_command_from_edge(0, circ, command, (const char*)payload, len, NULL); diff --git a/src/core/or/conflux_cell.c b/src/core/or/conflux_cell.c index 24b85e3005..ae4a6c4a6f 100644 --- a/src/core/or/conflux_cell.c +++ b/src/core/or/conflux_cell.c @@ -63,7 +63,8 @@ build_link_cell(const conflux_cell_link_t *link, uint8_t *cell_out) trn_cell_conflux_link_getlen_payload(cell), payload); /* Encode cell. */ - cell_len = trn_cell_conflux_link_encode(cell_out, RELAY_PAYLOAD_SIZE, cell); + cell_len = trn_cell_conflux_link_encode(cell_out, + RELAY_PAYLOAD_SIZE_MAX, cell); trn_cell_conflux_link_payload_v1_free(payload); trn_cell_conflux_link_free(cell); @@ -87,7 +88,8 @@ build_linked_ack_cell(uint8_t *cell_out) tor_assert(cell_out); cell = trn_cell_conflux_linked_ack_new(); - cell_len = trn_cell_conflux_linked_ack_encode(cell_out, RELAY_PAYLOAD_SIZE, + cell_len = trn_cell_conflux_linked_ack_encode(cell_out, + RELAY_PAYLOAD_SIZE_MAX, cell); trn_cell_conflux_linked_ack_free(cell); @@ -97,7 +99,7 @@ build_linked_ack_cell(uint8_t *cell_out) bool conflux_cell_send_link(const conflux_cell_link_t *link, origin_circuit_t *circ) { - uint8_t payload[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX] = {0}; ssize_t cell_len; tor_assert(link); @@ -131,7 +133,7 @@ conflux_cell_send_link(const conflux_cell_link_t *link, origin_circuit_t *circ) bool conflux_cell_send_linked(const conflux_cell_link_t *link, or_circuit_t *circ) { - uint8_t payload[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX] = {0}; ssize_t cell_len; tor_assert(link); @@ -164,7 +166,7 @@ conflux_cell_send_linked(const conflux_cell_link_t *link, or_circuit_t *circ) bool conflux_cell_send_linked_ack(origin_circuit_t *circ) { - uint8_t payload[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX] = {0}; ssize_t cell_len; tor_assert(circ); @@ -319,7 +321,7 @@ conflux_send_switch_command(circuit_t *send_circ, uint64_t relative_seq) trn_cell_conflux_switch_set_seqnum(switch_cell, (uint32_t)relative_seq); - if (trn_cell_conflux_switch_encode(cell.payload, RELAY_PAYLOAD_SIZE, + if (trn_cell_conflux_switch_encode(cell.payload, RELAY_PAYLOAD_SIZE_MAX, switch_cell) < 0) { log_warn(LD_BUG, "Failed to encode conflux switch cell"); ret = false; @@ -327,21 +329,22 @@ conflux_send_switch_command(circuit_t *send_circ, uint64_t relative_seq) } /* Send the switch command to the new hop */ + // TODO CGO XXXXX Fix bug #41056. if (CIRCUIT_IS_ORIGIN(send_circ)) { relay_send_command_from_edge(0, send_circ, RELAY_COMMAND_CONFLUX_SWITCH, (const char*)cell.payload, - RELAY_PAYLOAD_SIZE, + RELAY_PAYLOAD_SIZE_MAX, TO_ORIGIN_CIRCUIT(send_circ)->cpath->prev); } else { relay_send_command_from_edge(0, send_circ, RELAY_COMMAND_CONFLUX_SWITCH, (const char*)cell.payload, - RELAY_PAYLOAD_SIZE, NULL); + RELAY_PAYLOAD_SIZE_MAX, + NULL); } end: trn_cell_conflux_switch_free(switch_cell); return ret; } - diff --git a/src/core/or/congestion_control_flow.c b/src/core/or/congestion_control_flow.c index 4e1c545870..9f3461a2d8 100644 --- a/src/core/or/congestion_control_flow.c +++ b/src/core/or/congestion_control_flow.c @@ -71,13 +71,15 @@ double cc_stats_flow_xon_outbuf_ma = 0; void flow_control_new_consensus_params(const networkstatus_t *ns) { + // TODO CGO: These numbers might be wrong for the v1 cell format! + #define CC_XOFF_CLIENT_DFLT 500 #define CC_XOFF_CLIENT_MIN 1 #define CC_XOFF_CLIENT_MAX 10000 xoff_client = networkstatus_get_param(ns, "cc_xoff_client", CC_XOFF_CLIENT_DFLT, CC_XOFF_CLIENT_MIN, - CC_XOFF_CLIENT_MAX)*RELAY_PAYLOAD_SIZE; + CC_XOFF_CLIENT_MAX)*RELAY_PAYLOAD_SIZE_MAX; #define CC_XOFF_EXIT_DFLT 500 #define CC_XOFF_EXIT_MIN 1 @@ -85,7 +87,7 @@ flow_control_new_consensus_params(const networkstatus_t *ns) xoff_exit = networkstatus_get_param(ns, "cc_xoff_exit", CC_XOFF_EXIT_DFLT, CC_XOFF_EXIT_MIN, - CC_XOFF_EXIT_MAX)*RELAY_PAYLOAD_SIZE; + CC_XOFF_EXIT_MAX)*RELAY_PAYLOAD_SIZE_MAX; #define CC_XON_CHANGE_PCT_DFLT 25 #define CC_XON_CHANGE_PCT_MIN 1 @@ -101,7 +103,7 @@ flow_control_new_consensus_params(const networkstatus_t *ns) xon_rate_bytes = networkstatus_get_param(ns, "cc_xon_rate", CC_XON_RATE_BYTES_DFLT, CC_XON_RATE_BYTES_MIN, - CC_XON_RATE_BYTES_MAX)*RELAY_PAYLOAD_SIZE; + CC_XON_RATE_BYTES_MAX)*RELAY_PAYLOAD_SIZE_MAX; #define CC_XON_EWMA_CNT_DFLT (2) #define CC_XON_EWMA_CNT_MIN (2) @@ -477,7 +479,8 @@ flow_control_decide_xoff(edge_connection_t *stream) * do this because writes only happen when the socket unblocks, so * may not otherwise notice accumulation of data in the outbuf for * advisory XONs. */ - if (total_buffered > MAX_EXPECTED_CELL_BURST*RELAY_PAYLOAD_SIZE) { + // TODO CGO: This might be wrong for the v1 cell format! + if (total_buffered > MAX_EXPECTED_CELL_BURST*RELAY_PAYLOAD_SIZE_MAX) { flow_control_decide_xon(stream, 0); } @@ -710,4 +713,3 @@ conn_uses_flow_control(connection_t *conn) return ret; } - diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index 14a45a7b53..640ba157c1 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -513,7 +513,7 @@ clip_dns_fuzzy_ttl(uint32_t ttl) int connection_edge_end(edge_connection_t *conn, uint8_t reason) { - char payload[RELAY_PAYLOAD_SIZE]; + char payload[RELAY_PAYLOAD_SIZE_MAX]; size_t payload_len=1; circuit_t *circ; uint8_t control_reason = reason; @@ -3238,8 +3238,8 @@ connection_ap_get_begincell_flags(entry_connection_t *ap_conn) MOCK_IMPL(int, connection_ap_handshake_send_begin,(entry_connection_t *ap_conn)) { - char payload[CELL_PAYLOAD_SIZE]; - int payload_len; + char payload[RELAY_PAYLOAD_SIZE_MAX]; + size_t payload_len; int begin_type; const or_options_t *options = get_options(); origin_circuit_t *circ; @@ -3264,17 +3264,20 @@ connection_ap_handshake_send_begin,(entry_connection_t *ap_conn)) return -1; } + size_t payload_max = circuit_max_relay_payload( + edge_conn->on_circuit, edge_conn->cpath_layer, + RELAY_COMMAND_BEGIN); /* Set up begin cell flags. */ edge_conn->begincell_flags = connection_ap_get_begincell_flags(ap_conn); - tor_snprintf(payload,RELAY_PAYLOAD_SIZE, "%s:%d", + tor_snprintf(payload,payload_max, "%s:%d", (circ->base_.purpose == CIRCUIT_PURPOSE_C_GENERAL || circ->base_.purpose == CIRCUIT_PURPOSE_CONFLUX_LINKED || circ->base_.purpose == CIRCUIT_PURPOSE_CONTROLLER) ? ap_conn->socks_request->address : "", ap_conn->socks_request->port); - payload_len = (int)strlen(payload)+1; - if (payload_len <= RELAY_PAYLOAD_SIZE - 4 && edge_conn->begincell_flags) { + payload_len = strlen(payload)+1; + if (payload_len <= payload_max - 4 && edge_conn->begincell_flags) { set_uint32(payload + payload_len, htonl(edge_conn->begincell_flags)); payload_len += 4; } diff --git a/src/core/or/onion.c b/src/core/or/onion.c index 07c26a80e8..1474cb8126 100644 --- a/src/core/or/onion.c +++ b/src/core/or/onion.c @@ -128,7 +128,7 @@ parse_create2_payload(create_cell_t *cell_out, const uint8_t *p, size_t p_len) handshake_type = ntohs(get_uint16(p)); handshake_len = ntohs(get_uint16(p+2)); - if (handshake_len > CELL_PAYLOAD_SIZE - 4 || handshake_len > p_len - 4) + if (handshake_len > MAX_CREATE_LEN || handshake_len > p_len - 4) return -1; if (handshake_type == ONION_HANDSHAKE_TYPE_FAST) return -1; @@ -183,7 +183,7 @@ check_created_cell(const created_cell_t *cell) return -1; break; case CELL_CREATED2: - if (cell->handshake_len > RELAY_PAYLOAD_SIZE-2) + if (cell->handshake_len > MAX_CREATED_LEN) return -1; break; } @@ -211,7 +211,7 @@ created_cell_parse(created_cell_t *cell_out, const cell_t *cell_in) const uint8_t *p = cell_in->payload; cell_out->cell_type = CELL_CREATED2; cell_out->handshake_len = ntohs(get_uint16(p)); - if (cell_out->handshake_len > CELL_PAYLOAD_SIZE - 2) + if (cell_out->handshake_len > MAX_CREATED_LEN) return -1; memcpy(cell_out->reply, p+2, cell_out->handshake_len); break; @@ -353,7 +353,7 @@ extend_cell_parse,(extend_cell_t *cell_out, tor_assert(cell_out); tor_assert(payload); - if (payload_length > RELAY_PAYLOAD_SIZE) + if (payload_length > RELAY_PAYLOAD_SIZE_MAX) return -1; switch (command) { @@ -412,7 +412,7 @@ extended_cell_parse(extended_cell_t *cell_out, tor_assert(payload); memset(cell_out, 0, sizeof(*cell_out)); - if (payload_len > RELAY_PAYLOAD_SIZE) + if (payload_len > RELAY_PAYLOAD_SIZE_MAX) return -1; switch (command) { @@ -423,7 +423,7 @@ extended_cell_parse(extended_cell_t *cell_out, cell_out->cell_type = RELAY_COMMAND_EXTENDED2; cell_out->created_cell.cell_type = CELL_CREATED2; cell_out->created_cell.handshake_len = ntohs(get_uint16(payload)); - if (cell_out->created_cell.handshake_len > RELAY_PAYLOAD_SIZE - 2 || + if (cell_out->created_cell.handshake_len > RELAY_PAYLOAD_SIZE_MAX - 2 || cell_out->created_cell.handshake_len > payload_len - 2) return -1; memcpy(cell_out->created_cell.reply, payload+2, @@ -544,7 +544,9 @@ should_include_ed25519_id_extend_cells(const networkstatus_t *ns, /** Format the EXTEND{,2} cell in cell_in, storing its relay payload in * payload_out, the number of bytes used in *len_out, and the * relay command in *command_out. The payload_out must have - * RELAY_PAYLOAD_SIZE bytes available. Return 0 on success, -1 on failure. */ + * RELAY_PAYLOAD_SIZE_MAX bytes available. + * + * Return 0 on success, -1 on failure. */ int extend_cell_format(uint8_t *command_out, uint16_t *len_out, uint8_t *payload_out, const extend_cell_t *cell_in) @@ -555,7 +557,7 @@ extend_cell_format(uint8_t *command_out, uint16_t *len_out, p = payload_out; - memset(p, 0, RELAY_PAYLOAD_SIZE); + memset(p, 0, RELAY_PAYLOAD_SIZE_MAX); switch (cell_in->cell_type) { case RELAY_COMMAND_EXTEND: @@ -618,7 +620,7 @@ extend_cell_format(uint8_t *command_out, uint16_t *len_out, cell_in->create_cell.handshake_len); ssize_t len_encoded = extend2_cell_body_encode( - payload_out, RELAY_PAYLOAD_SIZE, + payload_out, RELAY_PAYLOAD_SIZE_MAX, cell); extend2_cell_body_free(cell); if (len_encoded < 0 || len_encoded > UINT16_MAX) @@ -636,7 +638,9 @@ extend_cell_format(uint8_t *command_out, uint16_t *len_out, /** Format the EXTENDED{,2} cell in cell_in, storing its relay payload * in payload_out, the number of bytes used in *len_out, and the * relay command in *command_out. The payload_out must have - * RELAY_PAYLOAD_SIZE bytes available. Return 0 on success, -1 on failure. */ + * RELAY_PAYLOAD_SIZE_MAX bytes available. + * + * Return 0 on success, -1 on failure. */ int extended_cell_format(uint8_t *command_out, uint16_t *len_out, uint8_t *payload_out, const extended_cell_t *cell_in) @@ -646,7 +650,7 @@ extended_cell_format(uint8_t *command_out, uint16_t *len_out, return -1; p = payload_out; - memset(p, 0, RELAY_PAYLOAD_SIZE); + memset(p, 0, RELAY_PAYLOAD_SIZE_MAX); switch (cell_in->cell_type) { case RELAY_COMMAND_EXTENDED: @@ -656,7 +660,7 @@ extended_cell_format(uint8_t *command_out, uint16_t *len_out, *command_out = RELAY_COMMAND_EXTENDED2; *len_out = 2 + cell_in->created_cell.handshake_len; set_uint16(payload_out, htons(cell_in->created_cell.handshake_len)); - if (2+cell_in->created_cell.handshake_len > RELAY_PAYLOAD_SIZE) + if (cell_in->created_cell.handshake_len > MAX_CREATED_LEN) return -1; memcpy(payload_out+2, cell_in->created_cell.reply, cell_in->created_cell.handshake_len); diff --git a/src/core/or/onion.h b/src/core/or/onion.h index bba2e38411..173b60fa5c 100644 --- a/src/core/or/onion.h +++ b/src/core/or/onion.h @@ -20,6 +20,9 @@ struct curve25519_public_key_t; #define MAX_ONIONSKIN_CHALLENGE_LEN 255 #define MAX_ONIONSKIN_REPLY_LEN 255 +#define MAX_CREATE_LEN (CELL_PAYLOAD_SIZE - 4) +#define MAX_CREATED_LEN (CELL_PAYLOAD_SIZE - 2) + /** A parsed CREATE, CREATE_FAST, or CREATE2 cell. */ typedef struct create_cell_t { /** The cell command. One of CREATE{,_FAST,2} */ @@ -29,7 +32,7 @@ typedef struct create_cell_t { /** The number of bytes used in onionskin. */ uint16_t handshake_len; /** The client-side message for the circuit creation handshake. */ - uint8_t onionskin[CELL_PAYLOAD_SIZE - 4]; + uint8_t onionskin[MAX_CREATE_LEN]; } create_cell_t; /** A parsed CREATED, CREATED_FAST, or CREATED2 cell. */ @@ -39,7 +42,7 @@ typedef struct created_cell_t { /** The number of bytes used in reply. */ uint16_t handshake_len; /** The server-side message for the circuit creation handshake. */ - uint8_t reply[CELL_PAYLOAD_SIZE - 2]; + uint8_t reply[MAX_CREATED_LEN]; } created_cell_t; /** A parsed RELAY_EXTEND or RELAY_EXTEND2 cell */ diff --git a/src/core/or/or.h b/src/core/or/or.h index 57d5d3e694..44ec332838 100644 --- a/src/core/or/or.h +++ b/src/core/or/or.h @@ -550,12 +550,6 @@ static inline int get_circ_id_size(int wide_circ_ids) return wide_circ_ids ? 4 : 2; } -/* TODO #41051: It would be better if these went away. */ -/** Number of bytes in a relay cell's header (not including general cell - * header). */ -/** Largest number of bytes that can fit in a relay cell payload. */ -#define RELAY_PAYLOAD_SIZE (CELL_PAYLOAD_SIZE-(1+2+2+4+2)) - /** Number of bytes used for a relay cell's header, in the v0 format. */ #define RELAY_HEADER_SIZE_V0 (1+2+2+4+2) /** Number of bytes used for a relay cell's header, in the v1 format, @@ -565,6 +559,22 @@ static inline int get_circ_id_size(int wide_circ_ids) * if a StreamID is used. */ #define RELAY_HEADER_SIZE_V1_WITH_STREAM_ID (16+1+2+2) +/** Largest number of bytes that can fit in any relay cell payload. + * + * Note that the actual maximum may be smaller if the V1 cell format + * is in use; see relay_cell_max_payload_size() for the real maximum. + */ +#define RELAY_PAYLOAD_SIZE_MAX (CELL_PAYLOAD_SIZE - RELAY_HEADER_SIZE_V0) + +/** Smallest capacity of any relay cell payload. */ +#define RELAY_PAYLOAD_SIZE_MIN \ + (CELL_PAYLOAD_SIZE - RELAY_HEADER_SIZE_V1_WITH_STREAM_ID) + +#ifdef TOR_UNIT_TESTS +// This name is for testing only. +#define RELAY_PAYLOAD_SIZE RELAY_PAYLOAD_SIZE_MAX +#endif + /** Identifies a circuit on an or_connection */ typedef uint32_t circid_t; /** Identifies a stream on a circuit */ diff --git a/src/core/or/relay.c b/src/core/or/relay.c index aa8c64f1e2..9aed85f31f 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1178,7 +1178,7 @@ resolved_cell_parse(const relay_msg_t *msg, smartlist_t *addresses_out, *errcode_out = 0; - if (msg->length > RELAY_PAYLOAD_SIZE) + if (msg->length > RELAY_PAYLOAD_SIZE_MAX) return -1; addrs = smartlist_new(); diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 03af990484..933170d9f0 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -229,7 +229,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, } /* Build and encode a version 1 SENDME cell into payload, which must be at - * least of RELAY_PAYLOAD_SIZE bytes, using the digest for the cell data. + * least of RELAY_PAYLOAD_SIZE_MAX bytes, using the digest for the cell data. * * Return the size in bytes of the encoded cell in payload. A negative value * is returned on encoding failure. */ @@ -254,7 +254,7 @@ build_cell_payload_v1(const uint8_t *cell_digest, uint8_t *payload) sendme_cell_get_data_len(cell)); /* Finally, encode the cell into the payload. */ - len = sendme_cell_encode(payload, RELAY_PAYLOAD_SIZE, cell); + len = sendme_cell_encode(payload, RELAY_PAYLOAD_SIZE_MAX, cell); sendme_cell_free(cell); return len; @@ -270,7 +270,7 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint, const uint8_t *cell_digest) { uint8_t emit_version; - uint8_t payload[RELAY_PAYLOAD_SIZE]; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX]; ssize_t payload_len; tor_assert(circ); diff --git a/src/core/or/status.c b/src/core/or/status.c index 11912ffc25..fd50a76b88 100644 --- a/src/core/or/status.c +++ b/src/core/or/status.c @@ -221,10 +221,11 @@ log_heartbeat(time_t now) } double fullness_pct = 100; + // TODO CGO: This is slightly wrong? if (stats_n_data_cells_packaged && !hibernating) { fullness_pct = 100*(((double)stats_n_data_bytes_packaged) / - ((double)stats_n_data_cells_packaged*RELAY_PAYLOAD_SIZE)); + ((double)stats_n_data_cells_packaged*RELAY_PAYLOAD_SIZE_MAX)); } const double overhead_pct = ( r - 1.0 ) * 100.0; diff --git a/src/feature/client/circpathbias.c b/src/feature/client/circpathbias.c index e388c55125..47aa0efe99 100644 --- a/src/feature/client/circpathbias.c +++ b/src/feature/client/circpathbias.c @@ -798,7 +798,7 @@ static int pathbias_send_usable_probe(circuit_t *circ) { /* Based on connection_ap_handshake_send_begin() */ - char payload[CELL_PAYLOAD_SIZE]; + char payload[RELAY_PAYLOAD_SIZE_MAX]; int payload_len; origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); crypt_path_t *cpath_layer = NULL; @@ -853,7 +853,7 @@ pathbias_send_usable_probe(circuit_t *circ) return -1; } - tor_snprintf(payload,RELAY_PAYLOAD_SIZE, "%s:25", probe_nonce); + tor_snprintf(payload,RELAY_PAYLOAD_SIZE_MAX, "%s:25", probe_nonce); payload_len = (int)strlen(payload)+1; // XXX: need this? Can we assume ipv4 will always be supported? diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 0039825f3c..584bc35c4d 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -41,7 +41,7 @@ compute_introduce_mac(const uint8_t *encoded_cell, size_t encoded_cell_len, { size_t offset = 0; size_t mac_msg_len; - uint8_t mac_msg[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t mac_msg[RELAY_PAYLOAD_SIZE_MAX] = {0}; tor_assert(encoded_cell); tor_assert(encrypted); @@ -299,8 +299,8 @@ introduce1_encrypt_and_encode(trn_cell_introduce1_t *cell, size_t offset = 0; ssize_t encrypted_len; ssize_t encoded_cell_len, encoded_enc_cell_len; - uint8_t encoded_cell[RELAY_PAYLOAD_SIZE] = {0}; - uint8_t encoded_enc_cell[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t encoded_cell[RELAY_PAYLOAD_SIZE_MAX] = {0}; + uint8_t encoded_enc_cell[RELAY_PAYLOAD_SIZE_MAX] = {0}; uint8_t *encrypted = NULL; uint8_t mac[DIGEST256_LEN]; crypto_cipher_t *cipher = NULL; @@ -339,7 +339,7 @@ introduce1_encrypt_and_encode(trn_cell_introduce1_t *cell, * ENCRYPTED_DATA and MAC length. */ encrypted_len = sizeof(data->client_kp->pubkey) + encoded_enc_cell_len + sizeof(mac); - tor_assert(encrypted_len < RELAY_PAYLOAD_SIZE); + tor_assert(encrypted_len < RELAY_PAYLOAD_SIZE_MAX); encrypted = tor_malloc_zero(encrypted_len); /* Put the CLIENT_PK first. */ @@ -709,7 +709,7 @@ hs_cell_build_establish_intro(const char *circ_nonce, ssize_t tmp_cell_mac_offset = sig_len + sizeof(cell->sig_len) + trn_cell_establish_intro_getlen_handshake_mac(cell); - uint8_t tmp_cell_enc[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t tmp_cell_enc[RELAY_PAYLOAD_SIZE_MAX] = {0}; uint8_t mac[TRUNNEL_SHA3_256_LEN], *handshake_ptr; /* We first encode the current fields we have in the cell so we can @@ -738,7 +738,7 @@ hs_cell_build_establish_intro(const char *circ_nonce, { ssize_t tmp_cell_enc_len = 0; ssize_t tmp_cell_sig_offset = (sig_len + sizeof(cell->sig_len)); - uint8_t tmp_cell_enc[RELAY_PAYLOAD_SIZE] = {0}, *sig_ptr; + uint8_t tmp_cell_enc[RELAY_PAYLOAD_SIZE_MAX] = {0}, *sig_ptr; ed25519_signature_t sig; /* We first encode the current fields we have in the cell so we can @@ -764,7 +764,8 @@ hs_cell_build_establish_intro(const char *circ_nonce, } /* Encode the cell. Can't be bigger than a standard cell. */ - cell_len = trn_cell_establish_intro_encode(cell_out, RELAY_PAYLOAD_SIZE, + cell_len = trn_cell_establish_intro_encode(cell_out, + RELAY_PAYLOAD_SIZE_MAX, cell); done: @@ -1161,7 +1162,8 @@ hs_cell_build_rendezvous1(const uint8_t *rendezvous_cookie, memcpy(trn_cell_rendezvous1_getarray_handshake_info(cell), rendezvous_handshake_info, rendezvous_handshake_info_len); /* Encoding. */ - cell_len = trn_cell_rendezvous1_encode(cell_out, RELAY_PAYLOAD_SIZE, cell); + cell_len = trn_cell_rendezvous1_encode(cell_out, + RELAY_PAYLOAD_SIZE_MAX, cell); tor_assert(cell_len > 0); trn_cell_rendezvous1_free(cell); @@ -1200,7 +1202,8 @@ hs_cell_build_introduce1(const hs_cell_introduce1_data_t *data, introduce1_set_encrypted(cell, data); /* Final encoding. */ - cell_len = trn_cell_introduce1_encode(cell_out, RELAY_PAYLOAD_SIZE, cell); + cell_len = trn_cell_introduce1_encode(cell_out, + RELAY_PAYLOAD_SIZE_MAX, cell); trn_cell_introduce1_free(cell); return cell_len; diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 306550f99c..e11fc8915a 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -271,7 +271,7 @@ send_establish_intro(const hs_service_t *service, hs_service_intro_point_t *ip, origin_circuit_t *circ) { ssize_t cell_len; - uint8_t payload[RELAY_PAYLOAD_SIZE]; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX]; tor_assert(service); tor_assert(ip); @@ -1156,7 +1156,7 @@ hs_circ_service_rp_has_opened(const hs_service_t *service, origin_circuit_t *circ) { size_t payload_len; - uint8_t payload[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX] = {0}; tor_assert(service); tor_assert(circ); @@ -1445,7 +1445,7 @@ hs_circ_send_introduce1(origin_circuit_t *intro_circ, { int ret = -1; ssize_t payload_len; - uint8_t payload[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t payload[RELAY_PAYLOAD_SIZE_MAX] = {0}; hs_cell_introduce1_data_t intro1_data; tor_assert(intro_circ); @@ -1529,7 +1529,7 @@ int hs_circ_send_establish_rendezvous(origin_circuit_t *circ) { ssize_t cell_len = 0; - uint8_t cell[RELAY_PAYLOAD_SIZE] = {0}; + uint8_t cell[RELAY_PAYLOAD_SIZE_MAX] = {0}; tor_assert(circ); tor_assert(TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_C_ESTABLISH_REND); diff --git a/src/feature/relay/dns.c b/src/feature/relay/dns.c index eb31678189..bd27b91c79 100644 --- a/src/feature/relay/dns.c +++ b/src/feature/relay/dns.c @@ -508,7 +508,9 @@ MOCK_IMPL(STATIC void, send_resolved_cell,(edge_connection_t *conn, uint8_t answer_type, const cached_resolve_t *resolved)) { - char buf[RELAY_PAYLOAD_SIZE], *cp = buf; + // (We use the minimum here to ensure that we never + // generate a too-big message.) + char buf[RELAY_PAYLOAD_SIZE_MIN], *cp = buf; size_t buflen = 0; uint32_t ttl; @@ -581,7 +583,7 @@ MOCK_IMPL(STATIC void, send_resolved_hostname_cell,(edge_connection_t *conn, const char *hostname)) { - char buf[RELAY_PAYLOAD_SIZE]; + char buf[RELAY_PAYLOAD_SIZE_MAX]; size_t buflen; uint32_t ttl; @@ -590,7 +592,9 @@ send_resolved_hostname_cell,(edge_connection_t *conn, size_t namelen = strlen(hostname); - tor_assert(namelen < 256); + if (BUG(namelen >= 256)) { + return; + } ttl = conn->address_ttl; buf[0] = RESOLVED_TYPE_HOSTNAME; diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c index b46726aa4a..0c02325fac 100644 --- a/src/test/test_cell_formats.c +++ b/src/test/test_cell_formats.c @@ -495,11 +495,11 @@ test_cfmt_created_cells(void *arg) memset(b, 0, sizeof(b)); crypto_rand((char*)b, 496); cell.command = CELL_CREATED2; - memcpy(cell.payload, "\x01\xF1", 2); + memcpy(cell.payload, "\x02\xFF", 2); tt_int_op(-1, OP_EQ, created_cell_parse(&cc, &cell)); /* Unformattable CREATED2 cell: too long! */ - cc.handshake_len = 497; + cc.handshake_len = 508; tt_int_op(-1, OP_EQ, created_cell_format(&cell2, &cc)); done: -- 2.47.2