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

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

14 Jun '21

commit d71bf986b4faf7cb3b654192bc67d5b674cfcf02 Merge: 7fdfc2ea54 4c06c619fa Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:41:59 2021 -0400 Merge branch 'bug40391_035' into maint-0.3.5 changes/bug40391 | 9 +++++++++ src/core/or/circuitmux.c | 8 ++++---- 2 files changed, 13 insertions(+), 4 deletions(-)

1 0

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

14 Jun '21

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

1 0

[tor/main] 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

[tor/main] 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/main] Merge branch 'maint-0.4.5' into maint-0.4.6
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 69bd4a8a2db6645b69a902d5ef6eb8c832f122fc Merge: 1d11675adb 1da8621c0a Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:42:15 2021 -0400 Merge branch 'maint-0.4.5' into maint-0.4.6 changes/bug40391 | 9 +++++++++ src/core/or/circuitmux.c | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-)

1 0

[tor/main] Merge branch 'maint-0.4.6'
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit d9edf143abd05378246ebeadf0834d6ef57928fd Merge: 085bf61a35 69bd4a8a2d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:42:15 2021 -0400 Merge branch 'maint-0.4.6' changes/bug40391 | 9 +++++++++ src/core/or/circuitmux.c | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-)

1 0

[tor/main] 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/main] Merge branch 'maint-0.4.4' into maint-0.4.5
by nickm＠torproject.org 14 Jun '21

14 Jun '21

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

1 0

[tor/main] Use a more secure hash function for the circuitmux hashtable.
by nickm＠torproject.org 14 Jun '21

14 Jun '21

commit 4c06c619faceb5d158a725d97fda45cadb2cf9c9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue May 18 08:40:09 2021 -0400 Use a more secure hash function for the circuitmux hashtable. Fixes bug 40931; bugfix on 0.2.4.4-alpha. Also tracked as TROVE-2021-005. This issue was reported by Jann Horn from Google's Project Zero. --- changes/bug40391 | 9 +++++++++ src/core/or/circuitmux.c | 8 ++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/changes/bug40391 b/changes/bug40391 new file mode 100644 index 0000000000..e3c186275f --- /dev/null +++ b/changes/bug40391 @@ -0,0 +1,9 @@ + o Major bugfixes (security): + - Resist a hashtable-based CPU denial-of-service attack against + relays. Previously we used a naive unkeyed hash function to look up + circuits in a circuitmux object. An attacker could exploit this to + construct circuits with chosen circuit IDs in order to try to create + collisions and make the hash table inefficient. Now we use a SipHash + construction for this hash table instead. Fixes bug 40391; bugfix on + 0.2.4.4-alpha. This issue is also tracked as TROVE-2021-005. + Reported by Jann Horn from Google's Project Zero. diff --git a/src/core/or/circuitmux.c b/src/core/or/circuitmux.c index 88f9ac7923..e7309553c4 100644 --- a/src/core/or/circuitmux.c +++ b/src/core/or/circuitmux.c @@ -216,9 +216,10 @@ chanid_circid_entries_eq(chanid_circid_muxinfo_t *a, 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 */ @@ -1361,4 +1362,3 @@ circuitmux_compare_muxes, (circuitmux_t *cmux_1, circuitmux_t *cmux_2)) return 0; } } -

1 0

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

14 Jun '21

commit d60f8fe98a59ce13809e65de67e53b3dcf6199f0 Merge: c80b98520b 57a41348ee Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jun 10 08:21:48 2021 -0400 Merge branch 'maint-0.4.4' into maint-0.4.5 changes/geoip-2021-06-10 | 3 + src/config/geoip | 4764 +++++++++++++++++++++++++++------------------- src/config/geoip6 | 2826 ++++++++++++++++----------- 3 files changed, 4536 insertions(+), 3057 deletions(-)

1 0