tor-commits April 2018

tor-commits@lists.torproject.org

17 participants
1638 discussions

[torspec/master] prop291: Change MOAR to MORE
by dgoulet＠torproject.org 16 Apr '18

16 Apr '18

commit f7fa4b925b40779731c401ddb5b827fe6635e3bd Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 16 11:12:52 2018 -0400 prop291: Change MOAR to MORE Some of us understands that "MOAR" is a funny word that means MORE++ but unfortunately, not everyone can get that reference especially when you don't speak English as your first language. Closes #25758 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- proposals/291-two-guard-nodes.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/291-two-guard-nodes.txt b/proposals/291-two-guard-nodes.txt index dc0aa74..2a884ef 100644 --- a/proposals/291-two-guard-nodes.txt +++ b/proposals/291-two-guard-nodes.txt @@ -95,7 +95,7 @@ Status: Open multiplexing also means that observers along the path to this secondary guard gain more information per observation. -2.2. Why not MOAR guards? +2.2. Why not MORE guards? We do not want to increase the number of observation points for client activity into the Tor network[1]. We merely want better multiplexing for

1 0

[torspec/master] dir-spec: Add consensus param circ_max_cell_queue_size
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 7a25d149332a737a0116ddf0d562bdb24894ad91 Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 16 10:56:27 2018 -0400 dir-spec: Add consensus param circ_max_cell_queue_size Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- dir-spec.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dir-spec.txt b/dir-spec.txt index 7657e39..1adfe69 100644 --- a/dir-spec.txt +++ b/dir-spec.txt @@ -2042,6 +2042,11 @@ "DoSRefuseSingleHopClientRendezvous" -- Refuse establishment of rendezvous points for single hop clients. + "circ_max_cell_queue_size" -- This parameter determines the maximum + number of cells allowed per circuit queue. + Min 1000. Max 4294967295. Default 50000. + First-appeared: 0.3.3.6-rc. + "shared-rand-previous-value" SP NumReveals SP Value NL [At most once]

1 0

[tor/master] relay: Remove max middle cells dead code
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 3d5bf12ac24bfbabad5f14c6cd45373b227d2816 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Mar 20 14:58:12 2018 -0400 relay: Remove max middle cells dead code Next commit is addressing the circuit queue cell limit so cleanup before doing anything else. Part of #25226 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/or.h | 6 ------ src/or/relay.c | 3 --- 2 files changed, 9 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 2397f6651..8fcba42ad 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3524,12 +3524,6 @@ typedef struct or_circuit_t { * exit-ward queues of this circuit; reset every time when writing * buffer stats to disk. */ uint64_t total_cell_waiting_time; - - /** Maximum cell queue size for a middle relay; this is stored per circuit - * so append_cell_to_circuit_queue() can adjust it if it changes. If set - * to zero, it is initialized to the default value. - */ - uint32_t max_middle_cells; } or_circuit_t; #if REND_COOKIE_LEN != DIGEST_LEN diff --git a/src/or/relay.c b/src/or/relay.c index 506b7eccb..c8e0a2e61 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -99,9 +99,6 @@ static void adjust_exit_policy_from_exitpolicy_failure(origin_circuit_t *circ, entry_connection_t *conn, node_t *node, const tor_addr_t *addr); -#if 0 -static int get_max_middle_cells(void); -#endif /** Stop reading on edge connections when we have this many cells * waiting on the appropriate queue. */

1 0

[tor/maint-0.3.3] relay: Remove max middle cells dead code
by nickm＠torproject.org 16 Apr '18

16 Apr '18

1 0

[tor/maint-0.3.3] Merge remote-tracking branch 'dgoulet/bug25226_033_02' into maint-0.3.3
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 22845df2a7503ed73ed325c3a98916f289918caa Merge: 3ee4c9b1f d064122e7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 10:04:36 2018 -0400 Merge remote-tracking branch 'dgoulet/bug25226_033_02' into maint-0.3.3 changes/bug25226 | 4 +++ src/or/networkstatus.c | 1 + src/or/or.h | 6 ----- src/or/relay.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++--- src/or/relay.h | 1 + 5 files changed, 70 insertions(+), 9 deletions(-)

1 0

[tor/maint-0.3.3] relay: Implement a circuit cell queue maximum size
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit d064122e706575c99e8d2162de93f7f08cc8d41e Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Mar 20 15:27:58 2018 -0400 relay: Implement a circuit cell queue maximum size This commit introduces the consensus parameter "circ_max_cell_queue_size" which controls the maximum number of cells a circuit queue should have. The default value is currently 50000 cells which is above what should be expected but keeps us a margin of error for padding cells. Related to this is #9072. Back in 0.2.4.14-alpha, we've removed that limit due to a Guard discovery attack. Ticket #25226 details why we are putting back the limit due to the memory pressure issue on relays. Fixes #25226 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug25226 | 4 ++++ src/or/networkstatus.c | 1 + src/or/relay.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/relay.h | 1 + 4 files changed, 70 insertions(+) diff --git a/changes/bug25226 b/changes/bug25226 new file mode 100644 index 000000000..b594a7a42 --- /dev/null +++ b/changes/bug25226 @@ -0,0 +1,4 @@ + o Major bugfixes (relay, denial of service): + - Impose a limit on circuit cell queue size. The limit can be controlled by + a consensus parameter. Fixes bug 25226; bugfix on 0.2.4.14-alpha. + diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 7777473f1..1f21e8adb 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1596,6 +1596,7 @@ notify_before_networkstatus_changes(const networkstatus_t *old_c, { notify_control_networkstatus_changed(old_c, new_c); dos_consensus_has_changed(new_c); + relay_consensus_has_changed(new_c); } /* Called after a new consensus has been put in the global state. It is safe diff --git a/src/or/relay.c b/src/or/relay.c index c8e0a2e61..5a3c43e05 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2951,6 +2951,60 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max)) return n_flushed; } +/* Minimum value is the maximum circuit window size. + * + * SENDME cells makes it that we can control how many cells can be inflight on + * a circuit from end to end. This logic makes it that on any circuit cell + * queue, we have a maximum of cells possible. + * + * Because the Tor protocol allows for a client to exit at any hop in a + * circuit and a circuit can be of a maximum of 8 hops, so in theory the + * normal worst case will be the circuit window start value times the maximum + * number of hops (8). Having more cells then that means something is wrong. + * + * However, because padding cells aren't counted in the package window, we set + * the maximum size to a reasonably large size for which we expect that we'll + * never reach in theory. And if we ever do because of future changes, we'll + * be able to control it with a consensus parameter. + * + * XXX: Unfortunately, END cells aren't accounted for in the circuit window + * which means that for instance if a client opens 8001 streams, the 8001 + * following END cells will queue up in the circuit which will get closed if + * the max limit is 8000. Which is sad because it is allowed by the Tor + * protocol. But, we need an upper bound on circuit queue in order to avoid + * DoS memory pressure so the default size is a middle ground between not + * having any limit and having a very restricted one. This is why we can also + * control it through a consensus parameter. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_MIN CIRCWINDOW_START_MAX +/* We can't have a consensus parameter above this value. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_MAX INT32_MAX +/* Default value is set to a large value so we can handle padding cells + * properly which aren't accounted for in the SENDME window. Default is 50000 + * allowed cells in the queue resulting in ~25MB. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT \ + (50 * RELAY_CIRC_CELL_QUEUE_SIZE_MIN) + +/* The maximum number of cell a circuit queue can contain. This is updated at + * every new consensus and controlled by a parameter. */ +static int32_t max_circuit_cell_queue_size = + RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT; + +/* Called when the consensus has changed. At this stage, the global consensus + * object has NOT been updated. It is called from + * notify_before_networkstatus_changes(). */ +void +relay_consensus_has_changed(const networkstatus_t *ns) +{ + tor_assert(ns); + + /* Update the circuit max cell queue size from the consensus. */ + max_circuit_cell_queue_size = + networkstatus_get_param(ns, "circ_max_cell_queue_size", + RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT, + RELAY_CIRC_CELL_QUEUE_SIZE_MIN, + RELAY_CIRC_CELL_QUEUE_SIZE_MAX); +} + /** Add cell to the queue of circ writing to chan * transmitting in direction. * @@ -2980,6 +3034,16 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, streams_blocked = circ->streams_blocked_on_p_chan; } + if (PREDICT_UNLIKELY(queue->n >= max_circuit_cell_queue_size)) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "%s circuit has %d cells in its queue, maximum allowed is %d. " + "Closing circuit for safety reasons.", + (exitward) ? "Outbound" : "Inbound", queue->n, + max_circuit_cell_queue_size); + circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); + return; + } + /* Very important that we copy to the circuit queue because all calls to * this function use the stack for the cell memory. */ cell_queue_append_packed_copy(circ, queue, exitward, cell, diff --git a/src/or/relay.h b/src/or/relay.h index f0fa7e987..c9281c5ea 100644 --- a/src/or/relay.h +++ b/src/or/relay.h @@ -15,6 +15,7 @@ extern uint64_t stats_n_relay_cells_relayed; extern uint64_t stats_n_relay_cells_delivered; +void relay_consensus_has_changed(const networkstatus_t *ns); int circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, cell_direction_t cell_direction); size_t cell_queues_get_total_allocation(void);

1 0

[tor/release-0.3.3] relay: Remove max middle cells dead code
by nickm＠torproject.org 16 Apr '18

16 Apr '18

1 0

[tor/master] Merge branch 'maint-0.3.3'
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 3463b4e0652bacca51fecd2c256e3e9d61ce920e Merge: 197d1992d 22845df2a Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 10:06:21 2018 -0400 Merge branch 'maint-0.3.3' changes/bug25226 | 4 +++ src/or/networkstatus.c | 1 + src/or/or.h | 6 ----- src/or/relay.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++--- src/or/relay.h | 1 + 5 files changed, 70 insertions(+), 9 deletions(-)

1 0

[tor/master] relay: Implement a circuit cell queue maximum size
by nickm＠torproject.org 16 Apr '18

16 Apr '18

1 0

[tor/release-0.3.3] relay: Implement a circuit cell queue maximum size
by nickm＠torproject.org 16 Apr '18

16 Apr '18

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits April 2018