[tor-commits] [tor/release-0.3.3] relay: Implement a circuit cell queue maximum size

nickm at torproject.org nickm at torproject.org
Mon Apr 16 14:06:26 UTC 2018


commit d064122e706575c99e8d2162de93f7f08cc8d41e
Author: David Goulet <dgoulet at 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 at 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 <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,
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);





More information about the tor-commits mailing list