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@torproject.org>
This commit is contained in:
David Goulet 2018-03-20 15:27:58 -04:00
parent 3d5bf12ac2
commit d064122e70
4 changed files with 70 additions and 0 deletions

4
changes/bug25226 Normal file
View File

@ -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.

View File

@ -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

View File

@ -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 <b>cell</b> to the queue of <b>circ</b> writing to <b>chan</b>
* transmitting in <b>direction</b>.
*
@ -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,

View File

@ -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);