From adb248b6d6e0779719e6b873ee12a1e22fa390f4 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 3 Jun 2021 09:33:21 -0400 Subject: TROVE-2021-003: Check layer_hint before half-closed end and resolve cells This issue was reported by Jann Horn part of Google's Project Zero. Jann's one-sentence summary: entry/middle relays can spoof RELAY_END cells on half-closed streams, which can lead to stream confusion between OP and exit. Fixes #40389 --- changes/ticket40389 | 3 +++ src/core/or/relay.c | 25 +++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 changes/ticket40389 diff --git a/changes/ticket40389 b/changes/ticket40389 new file mode 100644 index 0000000000..7dcf65b32e --- /dev/null +++ b/changes/ticket40389 @@ -0,0 +1,3 @@ + o Major bugfixes (relay, TROVE): + - Don't allow entry or middle relays to spoof RELAY_END or RELAY_RESOLVED + cell on half-closed streams. Fixes bug 40389; bugfix on 0.3.5.1-alpha. diff --git a/src/core/or/relay.c b/src/core/or/relay.c index f5fc1cfbb3..00353f47a9 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1428,6 +1428,25 @@ connection_edge_process_relay_cell_not_open( // return -1; } +/** + * Return true iff our decryption layer_hint is from the last hop + * in a circuit. + */ +static bool +relay_crypt_from_last_hop(origin_circuit_t *circ, crypt_path_t *layer_hint) +{ + tor_assert(circ); + tor_assert(layer_hint); + tor_assert(circ->cpath); + + if (layer_hint != circ->cpath->prev) { + log_fn(LOG_PROTOCOL_WARN, LD_CIRC, + "Got unexpected relay data from intermediate hop"); + return false; + } + return true; +} + /** An incoming relay cell has arrived on circuit circ. If * conn is NULL this is a control cell, else cell is * destined for conn. @@ -1616,7 +1635,8 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, 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, + if (relay_crypt_from_last_hop(ocirc, layer_hint) && + connection_half_edge_is_valid_end(ocirc->half_streams, rh.stream_id)) { circuit_read_valid_data(ocirc, rh.length); @@ -1918,7 +1938,8 @@ 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, + if (relay_crypt_from_last_hop(ocirc, layer_hint) && + connection_half_edge_is_valid_resolved(ocirc->half_streams, rh.stream_id)) { circuit_read_valid_data(ocirc, rh.length); log_info(domain, -- cgit v1.2.3-54-g00ecf