Skip to content

tor-proto: Set upper bound on number of cells queued on circuit

C-tor has an upper bound on how many relay cells can be queued on a circuit. This is implemented in append_cell_to_circuit_queue() and is controlled by the consensus parameters circ_max_cell_queue_size and circ_max_cell_queue_size_out. This is an important DoS defense.

if (PREDICT_UNLIKELY(queue->n >= max_queue_size)) {
  /* This DoS defense only applies at the Guard as in the p_chan is likely
   * a client IP attacking the network. */
  if (exitward && CIRCUIT_IS_ORCIRC(circ)) {
    stats_n_circ_max_cell_outq_reached++;
    dos_note_circ_max_outq(CONST_TO_OR_CIRCUIT(circ)->p_chan);
  }

  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_queue_size);
  stats_n_circ_max_cell_reached++;
  return -1;
}

When the circuit is closed, the reason RESOURCELIMIT or INTERNAL is given, depending on if the data came from an edge or not.

Arti doesn't have clear queues like C-tor does. For example C-tor doesn't have outgoing stream queues like arti does, and the circuit queue is hidden in a SometimesUnboundedSink.

I think that the behaviour we want is:

  • Edge: The size of the outgoing stream queue StreamTarget::tx + the size of the circuit queue Circuit::chan_sender should not exceed some limit. This applies to arti clients and relays. We don't want a fast-writing client application to cause us to exceed this limit and close the circuit at an OP though (we should apply backpressure to the edge socket), but I'm not sure yet how C-tor prevents this.

  • Non-edge: The size of the circuit queue Circuit::chan_sender should not exceed some limit. This applies to arti relays.

If new data arrives that would cause us to exceed the limit, then we close the circuit. This should prevent the current issue where arti's circuit queues in the SometimesUnboundedSink are unbounded (in practice circuit sendmes prevent it from growing too large, but this might change with congestion control). (Edit: This part is wrong, see @gabi-250's comment below.)

/cc @dgoulet @gabi-250

Edited by opara