diff options
author | David Goulet <dgoulet@torproject.org> | 2021-06-03 09:33:21 -0400 |
---|---|---|
committer | Nick Mathewson <nickm@torproject.org> | 2021-06-10 08:50:05 -0400 |
commit | adb248b6d6e0779719e6b873ee12a1e22fa390f4 (patch) | |
tree | e8f633706ef4013390d413f9f37708113b6e9a9d | |
parent | d71bf986b4faf7cb3b654192bc67d5b674cfcf02 (diff) | |
download | tor-adb248b6d6e0779719e6b873ee12a1e22fa390f4.tar.gz tor-adb248b6d6e0779719e6b873ee12a1e22fa390f4.zip |
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
-rw-r--r-- | changes/ticket40389 | 3 | ||||
-rw-r--r-- | src/core/or/relay.c | 25 |
2 files changed, 26 insertions, 2 deletions
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 <b>circ</b>. If * <b>conn</b> is NULL this is a control cell, else <b>cell</b> is * destined for <b>conn</b>. @@ -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, |