commit a0eeadfba2c1d7d33214286ef7697971120cbe16 Author: Mike Perry <mikeperry-git@torproject.org> Date: Fri Nov 5 20:50:39 2021 +0000 Handle other places that use onion handshake type values We want ntor and ntorv3 to use the same queues and stats. --- src/core/crypto/onion_crypto.c | 1 - src/core/or/onion.c | 14 +++++++++ src/core/or/or.h | 2 +- src/feature/relay/onion_queue.c | 64 ++++++++++++++++++++++++++++------------- src/feature/stats/rephist.c | 59 ++++++++++++++++++------------------- src/feature/stats/rephist.h | 8 ++++-- 6 files changed, 95 insertions(+), 53 deletions(-) diff --git a/src/core/crypto/onion_crypto.c b/src/core/crypto/onion_crypto.c index 13f8f54b35..4a83a73dab 100644 --- a/src/core/crypto/onion_crypto.c +++ b/src/core/crypto/onion_crypto.c @@ -49,7 +49,6 @@ #include "core/or/extend_info_st.h" #include "trunnel/circ_params.h" -/* TODO-324: Add this to the specification! */ static const uint8_t NTOR3_CIRC_VERIFICATION[] = "circuit extend"; static const size_t NTOR3_CIRC_VERIFICATION_LEN = 14; diff --git a/src/core/or/onion.c b/src/core/or/onion.c index 62ad7af3fe..0bdd2a6d35 100644 --- a/src/core/or/onion.c +++ b/src/core/or/onion.c @@ -88,6 +88,10 @@ check_create_cell(const create_cell_t *cell, int unknown_ok) if (cell->handshake_len != NTOR_ONIONSKIN_LEN) return -1; break; + case ONION_HANDSHAKE_TYPE_NTOR_V3: + /* ntor v3 has variable length fields that are checked + * elsewhere. Fall through to always valid here. */ + break; default: if (! unknown_ok) return -1; @@ -521,6 +525,11 @@ create_cell_format_impl(cell_t *cell_out, const create_cell_t *cell_in, switch (cell_in->cell_type) { case CELL_CREATE: + if (BUG(cell_in->handshake_type == ONION_HANDSHAKE_TYPE_NTOR_V3)) { + log_warn(LD_BUG, "Create cells cannot contain ntorv3."); + return -1; + } + if (cell_in->handshake_type == ONION_HANDSHAKE_TYPE_NTOR) { memcpy(p, NTOR_CREATE_MAGIC, 16); p += 16; @@ -619,6 +628,11 @@ extend_cell_format(uint8_t *command_out, uint16_t *len_out, switch (cell_in->cell_type) { case RELAY_COMMAND_EXTEND: { + if (BUG(cell_in->create_cell.handshake_type == + ONION_HANDSHAKE_TYPE_NTOR_V3)) { + log_warn(LD_BUG, "Extend cells cannot contain ntorv3!"); + return -1; + } *command_out = RELAY_COMMAND_EXTEND; *len_out = 6 + TAP_ONIONSKIN_CHALLENGE_LEN + DIGEST_LEN; set_uint32(p, tor_addr_to_ipv4n(&cell_in->orport_ipv4.addr)); diff --git a/src/core/or/or.h b/src/core/or/or.h index 885c0e8b11..dc8f516f0a 100644 --- a/src/core/or/or.h +++ b/src/core/or/or.h @@ -793,7 +793,7 @@ typedef enum { #define ONION_HANDSHAKE_TYPE_TAP 0x0000 #define ONION_HANDSHAKE_TYPE_FAST 0x0001 #define ONION_HANDSHAKE_TYPE_NTOR 0x0002 -#define ONION_HANDSHAKE_TYPE_NTOR_V3 0x0003 /* TODO-324: Add to spec */ +#define ONION_HANDSHAKE_TYPE_NTOR_V3 0x0003 #define MAX_ONION_HANDSHAKE_TYPE 0x0003 typedef struct onion_handshake_state_t onion_handshake_state_t; diff --git a/src/feature/relay/onion_queue.c b/src/feature/relay/onion_queue.c index c09f4d5b9b..b0bb71a084 100644 --- a/src/feature/relay/onion_queue.c +++ b/src/feature/relay/onion_queue.c @@ -42,7 +42,7 @@ typedef struct onion_queue_t { TOR_TAILQ_ENTRY(onion_queue_t) next; or_circuit_t *circ; - uint16_t handshake_type; + uint16_t queue_idx; create_cell_t *onionskin; time_t when_added; } onion_queue_t; @@ -53,20 +53,41 @@ typedef struct onion_queue_t { TOR_TAILQ_HEAD(onion_queue_head_t, onion_queue_t); typedef struct onion_queue_head_t onion_queue_head_t; +/** We have 3 queues: tap, fast, and ntor. (ntorv3 goes into ntor queue). */ +#define MAX_QUEUE_IDX ONION_HANDSHAKE_TYPE_NTOR + /** Array of queues of circuits waiting for CPU workers. An element is NULL * if that queue is empty.*/ -static onion_queue_head_t ol_list[MAX_ONION_HANDSHAKE_TYPE+1] = +static onion_queue_head_t ol_list[MAX_QUEUE_IDX+1] = { TOR_TAILQ_HEAD_INITIALIZER(ol_list[0]), /* tap */ TOR_TAILQ_HEAD_INITIALIZER(ol_list[1]), /* fast */ TOR_TAILQ_HEAD_INITIALIZER(ol_list[2]), /* ntor */ }; /** Number of entries of each type currently in each element of ol_list[]. */ -static int ol_entries[MAX_ONION_HANDSHAKE_TYPE+1]; +static int ol_entries[MAX_QUEUE_IDX+1]; static int num_ntors_per_tap(void); static void onion_queue_entry_remove(onion_queue_t *victim); +/** + * We combine ntorv3 and ntor into the same queue, so we must + * use this function to covert the cell type to a queue index. + */ +static inline uint16_t +onionskin_type_to_queue(uint16_t type) +{ + if (type == ONION_HANDSHAKE_TYPE_NTOR_V3) { + return ONION_HANDSHAKE_TYPE_NTOR; + } + + if (BUG(type > MAX_QUEUE_IDX)) { + return MAX_QUEUE_IDX; // use ntor if out of range + } + + return type; +} + /* XXXX Check lengths vs MAX_ONIONSKIN_{CHALLENGE,REPLY}_LEN. * * (By which I think I meant, "make sure that no @@ -144,6 +165,7 @@ onion_pending_add(or_circuit_t *circ, create_cell_t *onionskin) { onion_queue_t *tmp; time_t now = time(NULL); + uint16_t queue_idx = 0; if (onionskin->handshake_type > MAX_ONION_HANDSHAKE_TYPE) { /* LCOV_EXCL_START @@ -154,18 +176,20 @@ onion_pending_add(or_circuit_t *circ, create_cell_t *onionskin) /* LCOV_EXCL_STOP */ } + queue_idx = onionskin_type_to_queue(onionskin->handshake_type); + tmp = tor_malloc_zero(sizeof(onion_queue_t)); tmp->circ = circ; - tmp->handshake_type = onionskin->handshake_type; + tmp->queue_idx = queue_idx; tmp->onionskin = onionskin; tmp->when_added = now; - if (!have_room_for_onionskin(onionskin->handshake_type)) { + if (!have_room_for_onionskin(queue_idx)) { #define WARN_TOO_MANY_CIRC_CREATIONS_INTERVAL (60) static ratelim_t last_warned = RATELIM_INIT(WARN_TOO_MANY_CIRC_CREATIONS_INTERVAL); - rep_hist_note_circuit_handshake_dropped(onionskin->handshake_type); - if (onionskin->handshake_type == ONION_HANDSHAKE_TYPE_NTOR) { + rep_hist_note_circuit_handshake_dropped(queue_idx); + if (queue_idx == ONION_HANDSHAKE_TYPE_NTOR) { char *m; /* Note this ntor onionskin drop as an overload */ rep_hist_note_overload(OVERLOAD_GENERAL); @@ -183,18 +207,18 @@ onion_pending_add(or_circuit_t *circ, create_cell_t *onionskin) return -1; } - ++ol_entries[onionskin->handshake_type]; + ++ol_entries[queue_idx]; log_info(LD_OR, "New create (%s). Queues now ntor=%d and tap=%d.", - onionskin->handshake_type == ONION_HANDSHAKE_TYPE_NTOR ? "ntor" : "tap", + queue_idx == ONION_HANDSHAKE_TYPE_NTOR ? "ntor" : "tap", ol_entries[ONION_HANDSHAKE_TYPE_NTOR], ol_entries[ONION_HANDSHAKE_TYPE_TAP]); circ->onionqueue_entry = tmp; - TOR_TAILQ_INSERT_TAIL(&ol_list[onionskin->handshake_type], tmp, next); + TOR_TAILQ_INSERT_TAIL(&ol_list[queue_idx], tmp, next); /* cull elderly requests. */ while (1) { - onion_queue_t *head = TOR_TAILQ_FIRST(&ol_list[onionskin->handshake_type]); + onion_queue_t *head = TOR_TAILQ_FIRST(&ol_list[queue_idx]); if (now - head->when_added < (time_t)ONIONQUEUE_WAIT_CUTOFF) break; @@ -282,15 +306,15 @@ onion_next_task(create_cell_t **onionskin_out) return NULL; /* no onions pending, we're done */ tor_assert(head->circ); - tor_assert(head->handshake_type <= MAX_ONION_HANDSHAKE_TYPE); + tor_assert(head->queue_idx <= MAX_QUEUE_IDX); // tor_assert(head->circ->p_chan); /* make sure it's still valid */ /* XXX I only commented out the above line to make the unit tests * more manageable. That's probably not good long-term. -RD */ circ = head->circ; if (head->onionskin) - --ol_entries[head->handshake_type]; + --ol_entries[head->queue_idx]; log_info(LD_OR, "Processing create (%s). Queues now ntor=%d and tap=%d.", - head->handshake_type == ONION_HANDSHAKE_TYPE_NTOR ? "ntor" : "tap", + head->queue_idx == ONION_HANDSHAKE_TYPE_NTOR ? "ntor" : "tap", ol_entries[ONION_HANDSHAKE_TYPE_NTOR], ol_entries[ONION_HANDSHAKE_TYPE_TAP]); @@ -306,7 +330,7 @@ onion_next_task(create_cell_t **onionskin_out) int onion_num_pending(uint16_t handshake_type) { - return ol_entries[handshake_type]; + return ol_entries[onionskin_type_to_queue(handshake_type)]; } /** Go through ol_list, find the onion_queue_t element which points to @@ -332,23 +356,23 @@ onion_pending_remove(or_circuit_t *circ) static void onion_queue_entry_remove(onion_queue_t *victim) { - if (victim->handshake_type > MAX_ONION_HANDSHAKE_TYPE) { + if (victim->queue_idx > MAX_QUEUE_IDX) { /* LCOV_EXCL_START * We should have rejected this far before this point */ log_warn(LD_BUG, "Handshake %d out of range! Dropping.", - victim->handshake_type); + victim->queue_idx); /* XXX leaks */ return; /* LCOV_EXCL_STOP */ } - TOR_TAILQ_REMOVE(&ol_list[victim->handshake_type], victim, next); + TOR_TAILQ_REMOVE(&ol_list[victim->queue_idx], victim, next); if (victim->circ) victim->circ->onionqueue_entry = NULL; if (victim->onionskin) - --ol_entries[victim->handshake_type]; + --ol_entries[victim->queue_idx]; tor_free(victim->onionskin); tor_free(victim); @@ -360,7 +384,7 @@ clear_pending_onions(void) { onion_queue_t *victim, *next; int i; - for (i=0; i<=MAX_ONION_HANDSHAKE_TYPE; i++) { + for (i=0; i<=MAX_QUEUE_IDX; i++) { for (victim = TOR_TAILQ_FIRST(&ol_list[i]); victim; victim = next) { next = TOR_TAILQ_NEXT(victim,next); onion_queue_entry_remove(victim); diff --git a/src/feature/stats/rephist.c b/src/feature/stats/rephist.c index 5ff4ef1d2e..11a75bbb14 100644 --- a/src/feature/stats/rephist.c +++ b/src/feature/stats/rephist.c @@ -2053,21 +2053,38 @@ rep_hist_note_desc_served(const char * desc) * * They are reset at every heartbeat. * @{ */ -STATIC int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1] = {0}; -STATIC int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1] = {0}; +STATIC int onion_handshakes_requested[MAX_ONION_STAT_TYPE+1] = {0}; +STATIC int onion_handshakes_assigned[MAX_ONION_STAT_TYPE+1] = {0}; /**@}*/ /** Counters keeping the same stats as above but for the entire duration of the * process (not reset). */ -static uint64_t stats_n_onionskin_assigned[MAX_ONION_HANDSHAKE_TYPE+1] = {0}; -static uint64_t stats_n_onionskin_dropped[MAX_ONION_HANDSHAKE_TYPE+1] = {0}; +static uint64_t stats_n_onionskin_assigned[MAX_ONION_STAT_TYPE+1] = {0}; +static uint64_t stats_n_onionskin_dropped[MAX_ONION_STAT_TYPE+1] = {0}; + +/** + * We combine ntorv3 and ntor into the same stat, so we must + * use this function to covert the cell type to a stat index. + */ +static inline uint16_t +onionskin_type_to_stat(uint16_t type) +{ + if (type == ONION_HANDSHAKE_TYPE_NTOR_V3) { + return ONION_HANDSHAKE_TYPE_NTOR; + } + + if (BUG(type > MAX_ONION_STAT_TYPE)) { + return MAX_ONION_STAT_TYPE; // use ntor if out of range + } + + return type; +} /** A new onionskin (using the <b>type</b> handshake) has arrived. */ void rep_hist_note_circuit_handshake_requested(uint16_t type) { - if (type <= MAX_ONION_HANDSHAKE_TYPE) - onion_handshakes_requested[type]++; + onion_handshakes_requested[onionskin_type_to_stat(type)]++; } /** We've sent an onionskin (using the <b>type</b> handshake) to a @@ -2075,10 +2092,8 @@ rep_hist_note_circuit_handshake_requested(uint16_t type) void rep_hist_note_circuit_handshake_assigned(uint16_t type) { - if (type <= MAX_ONION_HANDSHAKE_TYPE) { - onion_handshakes_assigned[type]++; - stats_n_onionskin_assigned[type]++; - } + onion_handshakes_assigned[onionskin_type_to_stat(type)]++; + stats_n_onionskin_assigned[onionskin_type_to_stat(type)]++; } /** We've just drop an onionskin (using the <b>type</b> handshake) due to being @@ -2086,49 +2101,35 @@ rep_hist_note_circuit_handshake_assigned(uint16_t type) void rep_hist_note_circuit_handshake_dropped(uint16_t type) { - if (type <= MAX_ONION_HANDSHAKE_TYPE) { - stats_n_onionskin_dropped[type]++; - } + stats_n_onionskin_dropped[onionskin_type_to_stat(type)]++; } /** Get the circuit handshake value that is requested. */ MOCK_IMPL(int, rep_hist_get_circuit_handshake_requested, (uint16_t type)) { - if (BUG(type > MAX_ONION_HANDSHAKE_TYPE)) { - return 0; - } - return onion_handshakes_requested[type]; + return onion_handshakes_requested[onionskin_type_to_stat(type)]; } /** Get the circuit handshake value that is assigned. */ MOCK_IMPL(int, rep_hist_get_circuit_handshake_assigned, (uint16_t type)) { - if (BUG(type > MAX_ONION_HANDSHAKE_TYPE)) { - return 0; - } - return onion_handshakes_assigned[type]; + return onion_handshakes_assigned[onionskin_type_to_stat(type)]; } /** Get the total number of circuit handshake value that is assigned. */ MOCK_IMPL(uint64_t, rep_hist_get_circuit_n_handshake_assigned, (uint16_t type)) { - if (BUG(type > MAX_ONION_HANDSHAKE_TYPE)) { - return 0; - } - return stats_n_onionskin_assigned[type]; + return stats_n_onionskin_assigned[onionskin_type_to_stat(type)]; } /** Get the total number of circuit handshake value that is dropped. */ MOCK_IMPL(uint64_t, rep_hist_get_circuit_n_handshake_dropped, (uint16_t type)) { - if (BUG(type > MAX_ONION_HANDSHAKE_TYPE)) { - return 0; - } - return stats_n_onionskin_dropped[type]; + return stats_n_onionskin_dropped[onionskin_type_to_stat(type)]; } /** Log our onionskin statistics since the last time we were called. */ diff --git a/src/feature/stats/rephist.h b/src/feature/stats/rephist.h index 7f414de4c8..2fb24a10a7 100644 --- a/src/feature/stats/rephist.h +++ b/src/feature/stats/rephist.h @@ -89,11 +89,15 @@ uint64_t rep_hist_get_n_dns_request(int type); void rep_hist_note_dns_request(int type); void rep_hist_note_dns_error(int type, uint8_t error); +/** We combine ntor and ntorv3 stats, so we have 3 stat types: + * tap, fast, and ntor. The max type is ntor (2) */ +#define MAX_ONION_STAT_TYPE ONION_HANDSHAKE_TYPE_NTOR + extern uint64_t rephist_total_alloc; extern uint32_t rephist_total_num; #ifdef TOR_UNIT_TESTS -extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1]; -extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1]; +extern int onion_handshakes_requested[MAX_ONION_STAT_TYPE+1]; +extern int onion_handshakes_assigned[MAX_ONION_STAT_TYPE+1]; #endif #ifdef REPHIST_PRIVATE