June 2021 - tor-commits - lists.torproject.org

[tor/release-0.4.4] Assert on _all_ failures from RAND_bytes().
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit c0aa9e0a1b1633d3b221199d516294e14a5db9c6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon May 17 08:50:01 2021 -0400 Assert on _all_ failures from RAND_bytes(). Previously, we would detect errors from a missing RNG implementation, but not failures from the RNG code itself. Fortunately, it appears those failures do not happen in practice when Tor is using OpenSSL's default RNG implementation. Fixes bug 40390; bugfix on 0.2.8.1-alpha. This issue is also tracked as TROVE-2021-004. Reported by Jann Horn at Google's Project Zero. --- changes/ticket40390 | 8 ++++++++ src/lib/crypt_ops/crypto_rand.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/changes/ticket40390 b/changes/ticket40390 new file mode 100644 index 0000000000..b56fa4d9da --- /dev/null +++ b/changes/ticket40390 @@ -0,0 +1,8 @@ + o Major bugfixes (security, defense-in-depth): + - Detect a wider variety of failure conditions from the OpenSSL RNG + code. Previously, we would detect errors from a missing RNG + implementation, but not failures from the RNG code itself. + Fortunately, it appears those failures do not happen in practice + when Tor is using OpenSSL's default RNG implementation. + Fixes bug 40390; bugfix on 0.2.8.1-alpha. This issue is also tracked as + TROVE-2021-004. Reported by Jann Horn at Google's Project Zero. diff --git a/src/lib/crypt_ops/crypto_rand.c b/src/lib/crypt_ops/crypto_rand.c index 915fe0870d..206929d6b3 100644 --- a/src/lib/crypt_ops/crypto_rand.c +++ b/src/lib/crypt_ops/crypto_rand.c @@ -525,7 +525,7 @@ crypto_rand_unmocked(char *to, size_t n) /* We consider a PRNG failure non-survivable. Let's assert so that we get a * stack trace about where it happened. */ - tor_assert(r >= 0); + tor_assert(r == 1); #endif }

1 0

[tor/release-0.4.4] Merge branch 'maint-0.3.5' into maint-0.4.4
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 7c19a4d924120f42c224ea357b388b73d05c2d89 Merge: 57a41348ee 7fdfc2ea54 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:37:26 2021 -0400 Merge branch 'maint-0.3.5' into maint-0.4.4 changes/ticket40390 | 8 ++++++++ src/lib/crypt_ops/crypto_rand.c | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-)

1 0

[tor/release-0.4.4] Merge branch 'bug40390_035_squashed' into maint-0.3.5
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 7fdfc2ea5470c572a113321e19cdb5144b10cdd8 Merge: 520d5c108f c0aa9e0a1b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:34:25 2021 -0400 Merge branch 'bug40390_035_squashed' into maint-0.3.5 changes/ticket40390 | 8 ++++++++ src/lib/crypt_ops/crypto_rand.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-)

1 0

[tor/release-0.4.4] Resolve remaining merge conflicts in relay.c
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 014345ed6abca3305fcbdea3b75a23adc3976926 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 09:55:35 2021 -0400 Resolve remaining merge conflicts in relay.c (My bad!) --- src/core/or/relay.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 78fda99a45..892dbe2d0a 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1735,17 +1735,9 @@ handle_relay_cell_command(cell_t *cell, circuit_t *circ, if (!conn) { if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); -<<<<<<< HEAD - if (connection_half_edge_is_valid_end(ocirc->half_streams, - rh->stream_id)) { -||||||| d71bf986b4faf7 - if (connection_half_edge_is_valid_end(ocirc->half_streams, - rh.stream_id)) { -======= if (relay_crypt_from_last_hop(ocirc, layer_hint) && connection_half_edge_is_valid_end(ocirc->half_streams, - rh.stream_id)) { ->>>>>>> maint-0.3.5 + rh->stream_id)) { circuit_read_valid_data(ocirc, rh->length); log_info(domain, @@ -1954,20 +1946,10 @@ handle_relay_cell_command(cell_t *cell, circuit_t *circ, if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); -<<<<<<< HEAD - if (connection_half_edge_is_valid_resolved(ocirc->half_streams, - rh->stream_id)) { - circuit_read_valid_data(ocirc, rh->length); -||||||| d71bf986b4faf7 - if (connection_half_edge_is_valid_resolved(ocirc->half_streams, - rh.stream_id)) { - circuit_read_valid_data(ocirc, rh.length); -======= 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); ->>>>>>> maint-0.3.5 + rh->stream_id)) { + circuit_read_valid_data(ocirc, rh->length); log_info(domain, "resolved cell on circ %u valid on half-closed " "stream id %d", ocirc->global_identifier, rh->stream_id);

1 0

[tor/release-0.4.4] Merge branch 'maint-0.4.4' into release-0.4.4
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 15617f29cb8a067ac399557cc6e72e0a415946da Merge: aa4da024ad 31eaa81f59 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:42:15 2021 -0400 Merge branch 'maint-0.4.4' into release-0.4.4 changes/bug40391 | 9 +++++++++ src/core/or/circuitmux.c | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-)

1 0

[tor/release-0.4.4] Merge branch 'maint-0.4.4' into release-0.4.4
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit aa4da024ad5aac5630ca957bcc9b521185d12f3f Merge: 0e2d413d2e 7c19a4d924 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:37:34 2021 -0400 Merge branch 'maint-0.4.4' into release-0.4.4 changes/ticket40390 | 8 ++++++++ src/lib/crypt_ops/crypto_rand.c | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-)

1 0

[tor/release-0.4.4] Merge branch 'maint-0.4.4' into release-0.4.4
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 096644038cef709b04bf5af49d1f1c3e8a05527a Merge: 15617f29cb 0f2d45328d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:53:07 2021 -0400 Merge branch 'maint-0.4.4' into release-0.4.4 changes/ticket40389 | 3 +++ src/core/or/relay.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)

1 0

[tor/release-0.4.4] TROVE-2021-003: Check layer_hint before half-closed end and resolve cells
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit adb248b6d6e0779719e6b873ee12a1e22fa390f4 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jun 3 09:33:21 2021 -0400 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(-) 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,

1 0

[tor/release-0.4.4] Merge branch 'maint-0.3.5' into maint-0.4.4
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 0f2d45328dfcb16acb4a517711413a6f0386487a Merge: 31eaa81f59 adb248b6d6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:52:39 2021 -0400 Merge branch 'maint-0.3.5' into maint-0.4.4 Conflicts resolved: src/core/or/relay.c changes/ticket40389 | 3 +++ src/core/or/relay.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --cc src/core/or/relay.c index 75d2d479e7,00353f47a9..78fda99a45 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@@ -1503,108 -1428,104 +1503,127 @@@ connection_edge_process_relay_cell_not_ // 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. +/** Process a SENDME cell that arrived on circ. If it is a stream level + * cell, it is destined for the given conn. If it is a circuit level + * cell, it is destined for the layer_hint. The domain is the + * logging domain that should be used. * - * If layer_hint is defined, then we're the origin of the - * circuit, and it specifies the hop that packaged cell. - * - * Return -reason if you want to warn and tear down the circuit, else 0. - */ -STATIC int -connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, - edge_connection_t *conn, - crypt_path_t *layer_hint) + * Return 0 if everything went well or a negative value representing a circuit + * end reason on error for which the caller is responsible for closing it. */ +static int +process_sendme_cell(const relay_header_t *rh, const cell_t *cell, + circuit_t *circ, edge_connection_t *conn, + crypt_path_t *layer_hint, int domain) { - static int num_seen=0; - relay_header_t rh; - unsigned domain = layer_hint?LD_APP:LD_EXIT; - int reason; - int optimistic_data = 0; /* Set to 1 if we receive data on a stream - * that's in the EXIT_CONN_STATE_RESOLVING - * or EXIT_CONN_STATE_CONNECTING states. */ + int ret; - tor_assert(cell); - tor_assert(circ); - - relay_header_unpack(&rh, cell->payload); -// log_fn(LOG_DEBUG,"command %d stream %d", rh.command, rh.stream_id); - num_seen++; - log_debug(domain, "Now seen %d relay cells here (command %d, stream %d).", - num_seen, rh.command, rh.stream_id); + tor_assert(rh); - if (rh.length > RELAY_PAYLOAD_SIZE) { - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Relay cell length field too long. Closing circuit."); - return - END_CIRC_REASON_TORPROTOCOL; + if (!rh->stream_id) { + /* Circuit level SENDME cell. */ + ret = sendme_process_circuit_level(layer_hint, circ, + cell->payload + RELAY_HEADER_SIZE, + rh->length); + if (ret < 0) { + return ret; + } + /* Resume reading on any streams now that we've processed a valid + * SENDME cell that updated our package window. */ + circuit_resume_edge_reading(circ, layer_hint); + /* We are done, the rest of the code is for the stream level. */ + return 0; } - if (rh.stream_id == 0) { - switch (rh.command) { - case RELAY_COMMAND_BEGIN: - case RELAY_COMMAND_CONNECTED: - case RELAY_COMMAND_END: - case RELAY_COMMAND_RESOLVE: - case RELAY_COMMAND_RESOLVED: - case RELAY_COMMAND_BEGIN_DIR: - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay command %d with zero " - "stream_id. Dropping.", (int)rh.command); - return 0; - default: - ; + /* No connection, might be half edge state. We are done if so. */ + if (!conn) { + if (CIRCUIT_IS_ORIGIN(circ)) { + origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); + if (connection_half_edge_is_valid_sendme(ocirc->half_streams, + rh->stream_id)) { + circuit_read_valid_data(ocirc, rh->length); + log_info(domain, "Sendme cell on circ %u valid on half-closed " + "stream id %d", + ocirc->global_identifier, rh->stream_id); + } } + + log_info(domain, "SENDME cell dropped, unknown stream (streamid %d).", + rh->stream_id); + return 0; } - /* either conn is NULL, in which case we've got a control cell, or else - * conn points to the recognized stream. */ + /* Stream level SENDME cell. */ + ret = sendme_process_stream_level(conn, circ, rh->length); + if (ret < 0) { + /* Means we need to close the circuit with reason ret. */ + return ret; + } - if (conn && !connection_state_is_open(TO_CONN(conn))) { - if (conn->base_.type == CONN_TYPE_EXIT && - (conn->base_.state == EXIT_CONN_STATE_CONNECTING || - conn->base_.state == EXIT_CONN_STATE_RESOLVING) && - rh.command == RELAY_COMMAND_DATA) { - /* Allow DATA cells to be delivered to an exit node in state - * EXIT_CONN_STATE_CONNECTING or EXIT_CONN_STATE_RESOLVING. - * This speeds up HTTP, for example. */ - optimistic_data = 1; - } else if (rh.stream_id == 0 && rh.command == RELAY_COMMAND_DATA) { - log_warn(LD_BUG, "Somehow I had a connection that matched a " - "data cell with stream ID 0."); - } else { - return connection_edge_process_relay_cell_not_open( - &rh, cell, circ, conn, layer_hint); - } + /* We've now processed properly a SENDME cell, all windows have been + * properly updated, we'll read on the edge connection to see if we can + * get data out towards the end point (Exit or client) since we are now + * allowed to deliver more cells. */ + + if (circuit_queue_streams_are_blocked(circ)) { + /* Still waiting for queue to flush; don't touch conn */ + return 0; } + connection_start_reading(TO_CONN(conn)); + /* handle whatever might still be on the inbuf */ + if (connection_edge_package_raw_inbuf(conn, 1, NULL) < 0) { + /* (We already sent an end cell if possible) */ + connection_mark_for_close(TO_CONN(conn)); + return 0; + } + return 0; +} - switch (rh.command) { - case RELAY_COMMAND_DROP: - rep_hist_padding_count_read(PADDING_TYPE_DROP); -// log_info(domain,"Got a relay-level padding cell. Dropping."); - return 0; +/** A helper for connection_edge_process_relay_cell(): Actually handles the + * cell that we received on the connection. + * + * The arguments are the same as in the parent function + * connection_edge_process_relay_cell(), plus the relay header rh as + * unpacked by the parent function, and optimistic_data as set by the + * parent function. + */ +STATIC int +handle_relay_cell_command(cell_t *cell, circuit_t *circ, + edge_connection_t *conn, crypt_path_t *layer_hint, + relay_header_t *rh, int optimistic_data) +{ + unsigned domain = layer_hint?LD_APP:LD_EXIT; + int reason; + + tor_assert(rh); + + /* First pass the cell to the circuit padding subsystem, in case it's a + * padding cell or circuit that should be handled there. */ + if (circpad_check_received_cell(cell, circ, layer_hint, rh) == 0) { + log_debug(domain, "Cell handled as circuit padding"); + return 0; + } + + /* Now handle all the other commands */ + switch (rh->command) { case RELAY_COMMAND_BEGIN: case RELAY_COMMAND_BEGIN_DIR: if (layer_hint && @@@ -1716,10 -1635,11 +1735,19 @@@ if (!conn) { if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); ++<<<<<<< HEAD + if (connection_half_edge_is_valid_end(ocirc->half_streams, + rh->stream_id)) { ++||||||| d71bf986b4faf7 ++ if (connection_half_edge_is_valid_end(ocirc->half_streams, ++ rh.stream_id)) { ++======= + if (relay_crypt_from_last_hop(ocirc, layer_hint) && + connection_half_edge_is_valid_end(ocirc->half_streams, + rh.stream_id)) { ++>>>>>>> maint-0.3.5 - circuit_read_valid_data(ocirc, rh.length); + circuit_read_valid_data(ocirc, rh->length); log_info(domain, "end cell (%s) on circ %u valid on half-closed " "stream id %d", @@@ -1926,12 -1938,13 +1954,23 @@@ if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); ++<<<<<<< HEAD + if (connection_half_edge_is_valid_resolved(ocirc->half_streams, + rh->stream_id)) { + circuit_read_valid_data(ocirc, rh->length); ++||||||| d71bf986b4faf7 ++ if (connection_half_edge_is_valid_resolved(ocirc->half_streams, ++ rh.stream_id)) { ++ circuit_read_valid_data(ocirc, rh.length); ++======= + 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); ++>>>>>>> maint-0.3.5 log_info(domain, "resolved cell on circ %u valid on half-closed " - "stream id %d", ocirc->global_identifier, rh.stream_id); + "stream id %d", ocirc->global_identifier, rh->stream_id); return 0; } }

1 0

[tor/release-0.4.4] Merge branch 'maint-0.3.5' into maint-0.4.4
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 31eaa81f59749ed7e9d4f1bad24225d23eb6f654 Merge: 7c19a4d924 d71bf986b4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:42:15 2021 -0400 Merge branch 'maint-0.3.5' into maint-0.4.4 changes/bug40391 | 9 +++++++++ src/core/or/circuitmux.c | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --cc src/core/or/circuitmux.c index b770e40bf2,e7309553c4..be54ae6ec6 --- a/src/core/or/circuitmux.c +++ b/src/core/or/circuitmux.c @@@ -169,17 -216,21 +169,18 @@@ chanid_circid_entries_eq(chanid_circid_ static inline unsigned int chanid_circid_entry_hash(chanid_circid_muxinfo_t *a) { - return (((unsigned int)(a->circ_id) << 8) ^ - ((unsigned int)((a->chan_id >> 32) & 0xffffffff)) ^ - ((unsigned int)(a->chan_id & 0xffffffff))); + uint8_t data[8 + 4]; + set_uint64(data, a->chan_id); + set_uint32(data + 8, a->circ_id); + return (unsigned) siphash24g(data, sizeof(data)); } -/* Declare the struct chanid_circid_muxinfo_map type */ -HT_HEAD(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t); - /* Emit a bunch of hash table stuff */ HT_PROTOTYPE(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t, node, - chanid_circid_entry_hash, chanid_circid_entries_eq) + chanid_circid_entry_hash, chanid_circid_entries_eq); HT_GENERATE2(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t, node, chanid_circid_entry_hash, chanid_circid_entries_eq, 0.6, - tor_reallocarray_, tor_free_) + tor_reallocarray_, tor_free_); /* * Circuitmux alloc/free functions

1 0