aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Goulet <dgoulet@torproject.org>2021-06-03 09:33:21 -0400
committerNick Mathewson <nickm@torproject.org>2021-06-10 08:50:05 -0400
commitadb248b6d6e0779719e6b873ee12a1e22fa390f4 (patch)
treee8f633706ef4013390d413f9f37708113b6e9a9d
parentd71bf986b4faf7cb3b654192bc67d5b674cfcf02 (diff)
downloadtor-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/ticket403893
-rw-r--r--src/core/or/relay.c25
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,