[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor] 01/04: dos: Apply circuit creation defenses if circ max queue cell reached



This is an automated email from the git hooks/post-receive script.

dgoulet pushed a commit to branch main
in repository tor.

commit a2c034d8f514cea2ce2f700a76ec22d29e584067
Author: David Goulet <dgoulet@xxxxxxxxxxxxxx>
AuthorDate: Mon Oct 17 11:34:57 2022 -0400

    dos: Apply circuit creation defenses if circ max queue cell reached
    
    This adds two consensus parameters to control the outbound max circuit
    queue cell size limit and how many times it is allowed to reach that
    limit for a single client IP.
    
    Closes #40680
    
    Signed-off-by: David Goulet <dgoulet@xxxxxxxxxxxxxx>
---
 changes/ticket40680 |  6 ++++
 src/core/or/dos.c   | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/core/or/dos.h   |  4 +++
 src/core/or/relay.c | 45 +++++++++++++++++++++++---
 src/core/or/relay.h |  2 ++
 5 files changed, 143 insertions(+), 7 deletions(-)

diff --git a/changes/ticket40680 b/changes/ticket40680
new file mode 100644
index 0000000000..1383844969
--- /dev/null
+++ b/changes/ticket40680
@@ -0,0 +1,6 @@
+  o Minor feature (relay, DoS):
+    - Apply circuit creation anti-DoS defenses if the outbound circuit max cell
+      queue size is reached too many times. This introduces two new consensus
+      parameters to control the queue size limit and number of times allowed to
+      go over that limit. Close ticket 40680.
+
diff --git a/src/core/or/dos.c b/src/core/or/dos.c
index 2eb5782481..560abd7691 100644
--- a/src/core/or/dos.c
+++ b/src/core/or/dos.c
@@ -49,6 +49,7 @@ static int32_t dos_cc_defense_time_period;
 /* Keep some stats for the heartbeat so we can report out. */
 static uint64_t cc_num_rejected_cells;
 static uint32_t cc_num_marked_addrs;
+static uint32_t cc_num_marked_addrs_max_queue;
 
 /*
  * Concurrent connection denial of service mitigation.
@@ -72,6 +73,10 @@ static int32_t dos_conn_connect_defense_time_period =
 static uint64_t conn_num_addr_rejected;
 static uint64_t conn_num_addr_connect_rejected;
 
+/** Consensus parameter: How many times a client IP is allowed to hit the
+ * circ_max_cell_queue_size_out limit before being marked. */
+static uint32_t dos_num_circ_max_outq;
+
 /*
  * General interface of the denial of service mitigation subsystem.
  */
@@ -79,6 +84,22 @@ static uint64_t conn_num_addr_connect_rejected;
 /* Keep stats for the heartbeat. */
 static uint64_t num_single_hop_client_refused;
 
+/** Return the consensus parameter for the outbound circ_max_cell_queue_size
+ * limit. */
+static uint32_t
+get_param_dos_num_circ_max_outq(const networkstatus_t *ns)
+{
+#define DOS_NUM_CIRC_MAX_OUTQ_DEFAULT 3
+#define DOS_NUM_CIRC_MAX_OUTQ_MIN 0
+#define DOS_NUM_CIRC_MAX_OUTQ_MAX INT32_MAX
+
+  /* Update the circuit max cell queue size from the consensus. */
+  return networkstatus_get_param(ns, "dos_num_circ_max_outq",
+                                 DOS_NUM_CIRC_MAX_OUTQ_DEFAULT,
+                                 DOS_NUM_CIRC_MAX_OUTQ_MIN,
+                                 DOS_NUM_CIRC_MAX_OUTQ_MAX);
+}
+
 /* Return true iff the circuit creation mitigation is enabled. We look at the
  * consensus for this else a default value is returned. */
 MOCK_IMPL(STATIC unsigned int,
@@ -258,6 +279,9 @@ set_dos_parameters(const networkstatus_t *ns)
   dos_conn_connect_burst = get_param_conn_connect_burst(ns);
   dos_conn_connect_defense_time_period =
     get_param_conn_connect_defense_time_period(ns);
+
+  /* Circuit. */
+  dos_num_circ_max_outq = get_param_dos_num_circ_max_outq(ns);
 }
 
 /* Free everything for the circuit creation DoS mitigation subsystem. */
@@ -745,6 +769,69 @@ dos_geoip_entry_init(clientmap_entry_t *geoip_ent)
                         (uint32_t) approx_time());
 }
 
+/** Note that the given channel has sent outbound the maximum amount of cell
+ * allowed on the next channel. */
+void
+dos_note_circ_max_outq(const channel_t *chan)
+{
+  tor_addr_t addr;
+  clientmap_entry_t *entry;
+
+  tor_assert(chan);
+
+  /* Skip everything if circuit creation defense is disabled. */
+  if (!dos_cc_enabled) {
+    goto end;
+  }
+
+  /* Must be a client connection else we ignore. */
+  if (!channel_is_client(chan)) {
+    goto end;
+  }
+  /* Without an IP address, nothing can work. */
+  if (!channel_get_addr_if_possible(chan, &addr)) {
+    goto end;
+  }
+
+  /* We are only interested in client connection from the geoip cache. */
+  entry = geoip_lookup_client(&addr, NULL, GEOIP_CLIENT_CONNECT);
+  if (entry == NULL) {
+    goto end;
+  }
+
+  /* Is the client marked? If yes, just ignore. */
+  if (entry->dos_stats.cc_stats.marked_until_ts >= approx_time()) {
+    goto end;
+  }
+
+  /* If max outq parameter is 0, it means disabled, just ignore. */
+  if (dos_num_circ_max_outq == 0) {
+    goto end;
+  }
+
+  entry->dos_stats.num_circ_max_cell_queue_size++;
+
+  /* This is the detection. If we have reached the maximum amount of times a
+   * client IP is allowed to reach this limit, mark client. */
+  if (entry->dos_stats.num_circ_max_cell_queue_size >=
+      dos_num_circ_max_outq) {
+    /* Only account for this marked address if this is the first time we block
+     * it else our counter is inflated with non unique entries. */
+    if (entry->dos_stats.cc_stats.marked_until_ts == 0) {
+      cc_num_marked_addrs_max_queue++;
+    }
+    log_info(LD_DOS, "Detected outbound max circuit queue from addr: %s",
+             fmt_addr(&addr));
+    cc_mark_client(&entry->dos_stats.cc_stats);
+
+    /* Reset after being marked so once unmarked, we start back clean. */
+    entry->dos_stats.num_circ_max_cell_queue_size = 0;
+  }
+
+ end:
+  return;
+}
+
 /* Note down that we've just refused a single hop client. This increments a
  * counter later used for the heartbeat. */
 void
@@ -786,8 +873,10 @@ dos_log_heartbeat(void)
   if (dos_cc_enabled) {
     smartlist_add_asprintf(elems,
                            "%" PRIu64 " circuits rejected, "
-                           "%" PRIu32 " marked addresses",
-                           cc_num_rejected_cells, cc_num_marked_addrs);
+                           "%" PRIu32 " marked addresses, "
+                           "%" PRIu32 " marked addresses for max queue",
+                           cc_num_rejected_cells, cc_num_marked_addrs,
+                           cc_num_marked_addrs_max_queue);
   } else {
     smartlist_add_asprintf(elems, "[DoSCircuitCreationEnabled disabled]");
   }
diff --git a/src/core/or/dos.h b/src/core/or/dos.h
index 6dcfa3cb94..b6412f4280 100644
--- a/src/core/or/dos.h
+++ b/src/core/or/dos.h
@@ -58,6 +58,9 @@ typedef struct dos_client_stats_t {
   /* Circuit creation statistics. This is only used if the circuit creation
    * subsystem has been enabled (dos_cc_enabled). */
   cc_client_stats_t cc_stats;
+
+  /** Number of times the circ_max_cell_queue_size limit has been reached. */
+  uint32_t num_circ_max_cell_queue_size;
 } dos_client_stats_t;
 
 /* General API. */
@@ -79,6 +82,7 @@ void dos_close_client_conn(const or_connection_t *or_conn);
 
 int dos_should_refuse_single_hop_client(void);
 void dos_note_refuse_single_hop_client(void);
+void dos_note_circ_max_outq(const channel_t *chan);
 
 /*
  * Circuit creation DoS mitigation subsystemn interface.
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 68fddd1ae7..5046c9cc55 100644
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -3170,6 +3170,33 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
  * every new consensus and controlled by a parameter. */
 static int32_t max_circuit_cell_queue_size =
   RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT;
+/** Maximum number of cell on an outbound circuit queue. This is updated at
+ * every new consensus and controlled by a parameter. This default is incorrect
+ * and won't be used at all except in unit tests. */
+static int32_t max_circuit_cell_queue_size_out =
+  RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT;
+
+/** Return consensus parameter "circ_max_cell_queue_size". The given ns can be
+ * NULL. */
+static uint32_t
+get_param_max_circuit_cell_queue_size(const networkstatus_t *ns)
+{
+  return 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);
+}
+
+/** Return consensus parameter "circ_max_cell_queue_size_out". The given ns can
+ * be NULL. */
+static uint32_t
+get_param_max_circuit_cell_queue_size_out(const networkstatus_t *ns)
+{
+  return networkstatus_get_param(ns, "circ_max_cell_queue_size_out",
+                                 get_param_max_circuit_cell_queue_size(ns),
+                                 RELAY_CIRC_CELL_QUEUE_SIZE_MIN,
+                                 RELAY_CIRC_CELL_QUEUE_SIZE_MAX);
+}
 
 /* Called when the consensus has changed. At this stage, the global consensus
  * object has NOT been updated. It is called from
@@ -3181,10 +3208,9 @@ relay_consensus_has_changed(const networkstatus_t *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);
+    get_param_max_circuit_cell_queue_size(ns);
+  max_circuit_cell_queue_size_out =
+    get_param_max_circuit_cell_queue_size_out(ns);
 }
 
 /** Add <b>cell</b> to the queue of <b>circ</b> writing to <b>chan</b>
@@ -3201,6 +3227,7 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
 {
   or_circuit_t *orcirc = NULL;
   cell_queue_t *queue;
+  int32_t max_queue_size;
   int streams_blocked;
   int exitward;
   if (circ->marked_for_close)
@@ -3210,13 +3237,21 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan,
   if (exitward) {
     queue = &circ->n_chan_cells;
     streams_blocked = circ->streams_blocked_on_n_chan;
+    max_queue_size = max_circuit_cell_queue_size_out;
   } else {
     orcirc = TO_OR_CIRCUIT(circ);
     queue = &orcirc->p_chan_cells;
     streams_blocked = circ->streams_blocked_on_p_chan;
+    max_queue_size = max_circuit_cell_queue_size;
   }
 
-  if (PREDICT_UNLIKELY(queue->n >= max_circuit_cell_queue_size)) {
+  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)) {
+      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.",
diff --git a/src/core/or/relay.h b/src/core/or/relay.h
index eac920f491..71e07562cd 100644
--- a/src/core/or/relay.h
+++ b/src/core/or/relay.h
@@ -17,6 +17,8 @@ extern uint64_t stats_n_relay_cells_delivered;
 extern uint64_t stats_n_circ_max_cell_reached;
 
 void relay_consensus_has_changed(const networkstatus_t *ns);
+uint32_t relay_get_param_max_circuit_cell_queue_size(
+                                     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);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits