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

[tor/master] Don't queue more cells as a middle relay than the spec allows to be in flight
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit 4cce58d3c22c65350fffb3c4bc4dccd73e0193d7 Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 13 20:41:00 2013 -0700 Don't queue more cells as a middle relay than the spec allows to be in flight --- changes/bug9063 | 3 +++ src/or/or.h | 8 ++++++++ src/or/relay.c | 23 ++++++++++++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/changes/bug9063 b/changes/bug9063 new file mode 100644 index 0000000..af3b1a8 --- /dev/null +++ b/changes/bug9063 @@ -0,0 +1,3 @@ + o Normal bugfixes: + - Close any circuit that has more cells queued than the spec permits. + Fixes bug #9063; bugfix on 0.2.3.25. diff --git a/src/or/or.h b/src/or/or.h index 6728b86..2565777 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -539,6 +539,8 @@ typedef enum { #define CIRCUIT_PURPOSE_IS_ESTABLISHED_REND(p) \ ((p) == CIRCUIT_PURPOSE_C_REND_JOINED || \ (p) == CIRCUIT_PURPOSE_S_REND_JOINED) +/** True iff the circuit_t c is actually an or_circuit_t */ +#define CIRCUIT_IS_ORCIRC(c) (((circuit_t *)(c))->magic == OR_CIRCUIT_MAGIC) /** How many circuits do we want simultaneously in-progress to handle * a given stream? */ @@ -814,6 +816,12 @@ typedef enum { /** Amount to increment a stream window when we get a stream SENDME. */ #define STREAMWINDOW_INCREMENT 50 +/** Maximum number of queued cells on a circuit for which we are the + * midpoint before we give up and kill it. This must be >= circwindow + * to avoid killing innocent circuits. + */ +#define ORCIRC_MAX_MIDDLE_CELLS (11*(CIRCWINDOW_START_MAX)/10) + /* Cell commands. These values are defined in tor-spec.txt. */ #define CELL_PADDING 0 #define CELL_CREATE 1 diff --git a/src/or/relay.c b/src/or/relay.c index a17c333..087459c 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2532,8 +2532,10 @@ append_cell_to_circuit_queue(circuit_t *circ, or_connection_t *orconn, cell_t *cell, cell_direction_t direction, streamid_t fromstream) { + or_circuit_t *orcirc = NULL; cell_queue_t *queue; int streams_blocked; + if (circ->marked_for_close) return; @@ -2541,11 +2543,30 @@ append_cell_to_circuit_queue(circuit_t *circ, or_connection_t *orconn, queue = &circ->n_conn_cells; streams_blocked = circ->streams_blocked_on_n_conn; } else { - or_circuit_t *orcirc = TO_OR_CIRCUIT(circ); + orcirc = TO_OR_CIRCUIT(circ); queue = &orcirc->p_conn_cells; streams_blocked = circ->streams_blocked_on_p_conn; } + /* Are we a middle circuit about to exceed ORCIRC_MAX_MIDDLE_CELLS? */ + if ((circ->n_conn != NULL) && CIRCUIT_IS_ORCIRC(circ)) { + orcirc = TO_OR_CIRCUIT(circ); + if (orcirc->p_conn) { + if (queue->n + 1 >= ORCIRC_MAX_MIDDLE_CELLS) { + /* Queueing this cell would put queue over the cap */ + log_warn(LD_CIRC, + "Got a cell exceeding the cap of %u in the %s direction " + "on middle circ ID %u; killing the circuit.", + ORCIRC_MAX_MIDDLE_CELLS, + (direction == CELL_DIRECTION_OUT) ? "n" : "p", + (direction == CELL_DIRECTION_OUT) ? + circ->n_circ_id : orcirc->p_circ_id); + circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); + return; + } + } + } + cell_queue_append_packed_copy(queue, cell); /* If we have too many cells on the circuit, we should stop reading from

1 0

[tor/master] Don't queue more cells as a middle relay than the spec allows to be in flight
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit 418c2845d03bf425ac2a0a9758b89e151dc3b9aa Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 13 20:49:01 2013 -0700 Don't queue more cells as a middle relay than the spec allows to be in flight --- changes/bug9063 | 3 +++ src/or/or.h | 8 ++++++++ src/or/relay.c | 28 +++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/changes/bug9063 b/changes/bug9063 new file mode 100644 index 0000000..dcbecf6 --- /dev/null +++ b/changes/bug9063 @@ -0,0 +1,3 @@ + o Normal bugfixes: + - Close any circuit that has more cells queued than the spec permits. + Fixes bug #9063; bugfix on 0.2.4.12. diff --git a/src/or/or.h b/src/or/or.h index ab5e3aa..36b6778 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -541,6 +541,8 @@ typedef enum { #define CIRCUIT_PURPOSE_IS_ESTABLISHED_REND(p) \ ((p) == CIRCUIT_PURPOSE_C_REND_JOINED || \ (p) == CIRCUIT_PURPOSE_S_REND_JOINED) +/** True iff the circuit_t c is actually an or_circuit_t */ +#define CIRCUIT_IS_ORCIRC(c) (((circuit_t *)(c))->magic == OR_CIRCUIT_MAGIC) /** How many circuits do we want simultaneously in-progress to handle * a given stream? */ @@ -818,6 +820,12 @@ typedef enum { /** Amount to increment a stream window when we get a stream SENDME. */ #define STREAMWINDOW_INCREMENT 50 +/** Maximum number of queued cells on a circuit for which we are the + * midpoint before we give up and kill it. This must be >= circwindow + * to avoid killing innocent circuits. + */ +#define ORCIRC_MAX_MIDDLE_CELLS (11*(CIRCWINDOW_START_MAX)/10) + /* Cell commands. These values are defined in tor-spec.txt. */ #define CELL_PADDING 0 #define CELL_CREATE 1 diff --git a/src/or/relay.c b/src/or/relay.c index d57ceaa..a26d418 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2466,8 +2466,10 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, cell_t *cell, cell_direction_t direction, streamid_t fromstream) { + or_circuit_t *orcirc = NULL; cell_queue_t *queue; int streams_blocked; + if (circ->marked_for_close) return; @@ -2475,11 +2477,35 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, queue = &circ->n_chan_cells; streams_blocked = circ->streams_blocked_on_n_chan; } else { - or_circuit_t *orcirc = TO_OR_CIRCUIT(circ); + orcirc = TO_OR_CIRCUIT(circ); queue = &orcirc->p_chan_cells; streams_blocked = circ->streams_blocked_on_p_chan; } + /* Are we a middle circuit about to exceed ORCIRC_MAX_MIDDLE_CELLS? */ + if ((circ->n_chan != NULL) && CIRCUIT_IS_ORCIRC(circ)) { + orcirc = TO_OR_CIRCUIT(circ); + if (orcirc->p_chan) { + if (queue->n + 1 >= ORCIRC_MAX_MIDDLE_CELLS) { + /* Queueing this cell would put queue over the cap */ + log_warn(LD_CIRC, + "Got a cell exceeding the cap of %u in the %s direction " + "on middle circ ID %u on chan ID " U64_FORMAT + "; killing the circuit.", + ORCIRC_MAX_MIDDLE_CELLS, + (direction == CELL_DIRECTION_OUT) ? "n" : "p", + (direction == CELL_DIRECTION_OUT) ? + circ->n_circ_id : orcirc->p_circ_id, + U64_PRINTF_ARG( + (direction == CELL_DIRECTION_OUT) ? + circ->n_chan->global_identifier : + orcirc->p_chan->global_identifier)); + circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); + return; + } + } + } + cell_queue_append_packed_copy(queue, cell, chan->wide_circ_ids); /* If we have too many cells on the circuit, we should stop reading from

1 0

[tor/master] Increase the limit so leaky pipe might work
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit 9e8c104ab879f979d313d1f1cef4c1480f431ceb Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 14 01:35:21 2013 -0400 Increase the limit so leaky pipe might work --- src/or/or.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 2565777..83bf8c8 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -818,9 +818,10 @@ typedef enum { /** Maximum number of queued cells on a circuit for which we are the * midpoint before we give up and kill it. This must be >= circwindow - * to avoid killing innocent circuits. + * to avoid killing innocent circuits, and >= circwindow*2 to give + * leaky-pipe a chance for being useful someday. */ -#define ORCIRC_MAX_MIDDLE_CELLS (11*(CIRCWINDOW_START_MAX)/10) +#define ORCIRC_MAX_MIDDLE_CELLS (21*(CIRCWINDOW_START_MAX)/10) /* Cell commands. These values are defined in tor-spec.txt. */ #define CELL_PADDING 0

1 0

[tor/master] Don't queue more cells as a middle relay than the spec allows to be in flight
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit 459aada4d0e2ca1c4538fb6f4ef4e275ae162600 Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 13 20:32:31 2013 -0700 Don't queue more cells as a middle relay than the spec allows to be in flight --- changes/bug9063 | 4 +++ src/or/or.h | 20 ++++++++++++ src/or/relay.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/changes/bug9063 b/changes/bug9063 new file mode 100644 index 0000000..e1d0a5e --- /dev/null +++ b/changes/bug9063 @@ -0,0 +1,4 @@ + o Normal bugfixes: + - Close any circuit that has 10% more cells queued than the spec permits + and warn when the queue length exceeds that threshold. Fixes bug + #9063; bugfix on 0.2.4.12. diff --git a/src/or/or.h b/src/or/or.h index daff6de..fbf6f52 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -543,6 +543,8 @@ typedef enum { #define CIRCUIT_PURPOSE_IS_ESTABLISHED_REND(p) \ ((p) == CIRCUIT_PURPOSE_C_REND_JOINED || \ (p) == CIRCUIT_PURPOSE_S_REND_JOINED) +/** True iff the circuit_t c is actually an or_circuit_t */ +#define CIRCUIT_IS_ORCIRC(c) (((circuit_t *)(c))->magic == OR_CIRCUIT_MAGIC) /** How many circuits do we want simultaneously in-progress to handle * a given stream? */ @@ -820,6 +822,18 @@ typedef enum { /** Amount to increment a stream window when we get a stream SENDME. */ #define STREAMWINDOW_INCREMENT 50 +/** Maximum number of queued cells on a circuit for which we are the + * midpoint before we give up and kill it. This must be >= circwindow + * to avoid killing innocent circuits. The ORCIRC_MAX_MIDDLE_KILL_THRESH + * ratio controls the margin of error between emitting a warning and + * killing the circuit. + */ +#define ORCIRC_MAX_MIDDLE_CELLS CIRCWINDOW_START_MAX +/** Ratio of hard (circuit kill) to soft (warning) thresholds for the + * ORCIRC_MAX_MIDDLE_CELLS tests. + */ +#define ORCIRC_MAX_MIDDLE_KILL_THRESH (1.1f) + /* Cell commands. These values are defined in tor-spec.txt. */ #define CELL_PADDING 0 #define CELL_CREATE 1 @@ -3157,6 +3171,12 @@ 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; /** Convert a circuit subtype to a circuit_t. */ diff --git a/src/or/relay.c b/src/or/relay.c index cef138e..f682d6a 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -58,6 +58,7 @@ static void adjust_exit_policy_from_exitpolicy_failure(origin_circuit_t *circ, entry_connection_t *conn, node_t *node, const tor_addr_t *addr); +static int get_max_middle_cells(void); /** Stop reading on edge connections when we have this many cells * waiting on the appropriate queue. */ @@ -2461,6 +2462,18 @@ channel_flush_from_first_active_circuit(channel_t *chan, int max) return n_flushed; } +/** Indicate the current preferred cap for middle circuits; zero disables + * the cap. Right now it's just a constant, ORCIRC_MAX_MIDDLE_CELLS, but + * the logic in append_cell_to_circuit_queue() is written to be correct + * if we want to base it on a consensus param or something that might change + * in the future. + */ +static int +get_max_middle_cells(void) +{ + return ORCIRC_MAX_MIDDLE_CELLS; +} + /** Add cell to the queue of circ writing to chan * transmitting in direction. */ void @@ -2468,8 +2481,11 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, cell_t *cell, cell_direction_t direction, streamid_t fromstream) { + or_circuit_t *orcirc = NULL; cell_queue_t *queue; int streams_blocked; + uint32_t tgt_max_middle_cells, p_len, n_len, tmp, hard_max_middle_cells; + if (circ->marked_for_close) return; @@ -2477,11 +2493,88 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, queue = &circ->n_chan_cells; streams_blocked = circ->streams_blocked_on_n_chan; } else { - or_circuit_t *orcirc = TO_OR_CIRCUIT(circ); + orcirc = TO_OR_CIRCUIT(circ); queue = &orcirc->p_chan_cells; streams_blocked = circ->streams_blocked_on_p_chan; } + /* Are we a middle circuit about to exceed ORCIRC_MAX_MIDDLE_CELLS? */ + if ((circ->n_chan != NULL) && CIRCUIT_IS_ORCIRC(circ)) { + orcirc = TO_OR_CIRCUIT(circ); + if (orcirc->p_chan) { + /* We are a middle circuit if we have both n_chan and p_chan */ + /* We'll need to know the current preferred maximum */ + tgt_max_middle_cells = get_max_middle_cells(); + if (tgt_max_middle_cells > 0) { + /* Do we need to initialize middle_max_cells? */ + if (orcirc->max_middle_cells == 0) { + orcirc->max_middle_cells = tgt_max_middle_cells; + } else { + if (tgt_max_middle_cells > orcirc->max_middle_cells) { + /* If we want to increase the cap, we can do so right away */ + orcirc->max_middle_cells = tgt_max_middle_cells; + } else if (tgt_max_middle_cells < orcirc->max_middle_cells) { + /* + * If we're shrinking the cap, we can't shrink past either queue; + * compare tgt_max_middle_cells rather than tgt_max_middle_cells * + * ORCIRC_MAX_MIDDLE_KILL_THRESH so the queues don't shrink enough + * to generate spurious warnings, either. + */ + n_len = circ->n_chan_cells.n; + p_len = orcirc->p_chan_cells.n; + tmp = tgt_max_middle_cells; + if (tmp < n_len) tmp = n_len; + if (tmp < p_len) tmp = p_len; + orcirc->max_middle_cells = tmp; + } + /* else no change */ + } + } else { + /* tgt_max_middle_cells == 0 indicates we should disable the cap */ + orcirc->max_middle_cells = 0; + } + + /* Now we know orcirc->max_middle_cells is set correctly */ + if (orcirc->max_middle_cells > 0) { + hard_max_middle_cells = + (uint32_t)(((double)orcirc->max_middle_cells) * + ORCIRC_MAX_MIDDLE_KILL_THRESH); + + if (queue->n + 1 >= hard_max_middle_cells) { + /* Queueing this cell would put queue over the kill theshold */ + log_warn(LD_CIRC, + "Got a cell exceeding the hard cap of %u in the " + "%s direction on middle circ ID %u on chan ID " + U64_FORMAT "; killing the circuit.", + hard_max_middle_cells, + (direction == CELL_DIRECTION_OUT) ? "n" : "p", + (direction == CELL_DIRECTION_OUT) ? + circ->n_circ_id : orcirc->p_circ_id, + U64_PRINTF_ARG( + (direction == CELL_DIRECTION_OUT) ? + circ->n_chan->global_identifier : + orcirc->p_chan->global_identifier)); + circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); + return; + } else if (queue->n + 1 == orcirc->max_middle_cells) { + /* Only use ==, not >= for this test so we don't spam the log */ + log_warn(LD_CIRC, + "While trying to queue a cell, reached the soft cap of %u " + "in the %s direction on middle circ ID %u " + "on chan ID " U64_FORMAT ".", + orcirc->max_middle_cells, + (direction == CELL_DIRECTION_OUT) ? "n" : "p", + (direction == CELL_DIRECTION_OUT) ? + circ->n_circ_id : orcirc->p_circ_id, + U64_PRINTF_ARG( + (direction == CELL_DIRECTION_OUT) ? + circ->n_chan->global_identifier : + orcirc->p_chan->global_identifier)); + } + } + } + } + cell_queue_append_packed_copy(queue, cell, chan->wide_circ_ids); /* If we have too many cells on the circuit, we should stop reading from

1 0

[tor/master] Increase the limit so leaky pipe might work
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit 79cdf81ec12c4f692db7c88e5f94a8419dfcea62 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 14 01:37:22 2013 -0400 Increase the limit so leaky pipe might work --- src/or/or.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 36b6778..935da53 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -822,9 +822,10 @@ typedef enum { /** Maximum number of queued cells on a circuit for which we are the * midpoint before we give up and kill it. This must be >= circwindow - * to avoid killing innocent circuits. + * to avoid killing innocent circuits, and >= circwindow*2 to give + * leaky-pipe a chance for being useful someday. */ -#define ORCIRC_MAX_MIDDLE_CELLS (11*(CIRCWINDOW_START_MAX)/10) +#define ORCIRC_MAX_MIDDLE_CELLS (21*(CIRCWINDOW_START_MAX)/10) /* Cell commands. These values are defined in tor-spec.txt. */ #define CELL_PADDING 0

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.3' into maint-0.2.4
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit 82fea1b0dffe2cb808e997b891106651d6a7bb97 Merge: 72f775e d780cd5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 14 01:45:35 2013 -0400 Merge remote-tracking branch 'origin/maint-0.2.3' into maint-0.2.4 Using "ours" strategy to avoid taking changes.

1 0

[tor/master] Increase the limit so leaky pipe might work
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit c9745822913e00a8664d68f62009fde231eff2aa Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 14 01:40:35 2013 -0400 Increase the limit so leaky pipe might work --- src/or/or.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index fbf6f52..a0776f1 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -824,11 +824,12 @@ typedef enum { /** Maximum number of queued cells on a circuit for which we are the * midpoint before we give up and kill it. This must be >= circwindow - * to avoid killing innocent circuits. The ORCIRC_MAX_MIDDLE_KILL_THRESH + * to avoid killing innocent circuits, and >= circwindow*2 to give + * leaky-pipe a chance of working someday. The ORCIRC_MAX_MIDDLE_KILL_THRESH * ratio controls the margin of error between emitting a warning and * killing the circuit. */ -#define ORCIRC_MAX_MIDDLE_CELLS CIRCWINDOW_START_MAX +#define ORCIRC_MAX_MIDDLE_CELLS (CIRCWINDOW_START_MAX*2) /** Ratio of hard (circuit kill) to soft (warning) thresholds for the * ORCIRC_MAX_MIDDLE_CELLS tests. */

1 0

[tor/master] Fix signed/unsigned comparison warning
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit bd6bd1c9be24dac2350403beebae2dae55c639b6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 14 01:41:53 2013 -0400 Fix signed/unsigned comparison warning --- src/or/relay.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/relay.c b/src/or/relay.c index f682d6a..2fe6448 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2540,7 +2540,7 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, (uint32_t)(((double)orcirc->max_middle_cells) * ORCIRC_MAX_MIDDLE_KILL_THRESH); - if (queue->n + 1 >= hard_max_middle_cells) { + if ((unsigned)queue->n + 1 >= hard_max_middle_cells) { /* Queueing this cell would put queue over the kill theshold */ log_warn(LD_CIRC, "Got a cell exceeding the hard cap of %u in the " @@ -2556,7 +2556,7 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, orcirc->p_chan->global_identifier)); circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); return; - } else if (queue->n + 1 == orcirc->max_middle_cells) { + } else if ((unsigned)queue->n + 1 == orcirc->max_middle_cells) { /* Only use ==, not >= for this test so we don't spam the log */ log_warn(LD_CIRC, "While trying to queue a cell, reached the soft cap of %u "

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.4'
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit b1f44fc20ba19690493a674f163cd55c117b6ce9 Merge: 483385d 8b6d0bd Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 14 01:48:16 2013 -0400 Merge remote-tracking branch 'origin/maint-0.2.4' Using "ours" strategy to avoid taking commits

1 0

[tor/master] Merge branch 'circuit_queue_cap-0.2.4-squashed' into maint-0.2.4
by nickm＠torproject.org 14 Jun '13

14 Jun '13

commit 8b6d0bd99cf233560f7e704a35e4f64a5271bfb1 Merge: 82fea1b 79cdf81 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 14 01:45:46 2013 -0400 Merge branch 'circuit_queue_cap-0.2.4-squashed' into maint-0.2.4 changes/bug9063 | 3 +++ src/or/or.h | 9 +++++++++ src/or/relay.c | 28 +++++++++++++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-)

1 0