Merge remote-tracking branch 'tor-gitlab/mr/276'

changes/ticket40253

@@ -0,0 +1,3 @@
+  o Major feature (relay, denial of service):
+    - Add a new DoS subsystem feature to control the rate of client connections
+      for relays. Closes ticket 40253.

doc/man/tor.1.txt

@@ -2936,6 +2936,30 @@ Denial of Service mitigation subsystem described above.
     consensus, the value is 100.
     (Default: 0)
 
+[[DoSConnectionConnectRate]] **DoSConnectionConnectRate** __NUM__::
+
+    The allowed rate of client connection from a single address per second.
+    Coupled with the burst (see below), if the limit is reached, the address
+    is marked and a defense is applied (DoSConnectionDefenseType) for a period
+    of time defined by DoSConnectionConnectDefenseTimePeriod. If not defined
+    or set to 0, it is controlled by a consensus parameter.
+    (Default: 0)
+
+[[DoSConnectionConnectBurst]] **DoSConnectionConnectBurst** __NUM__::
+
+    The allowed burst of client connection from a single address per second.
+    See the DoSConnectionConnectRate for more details on this detection. If
+    not defined or set to 0, it is controlled by a consensus parameter.
+    (Default: 0)
+
+[[DoSConnectionConnectDefenseTimePeriod]] **DoSConnectionConnectDefenseTimePeriod** __N__ **seconds**|**minutes**|**hours**::
+
+    The base time period in seconds that the client connection defense is
+    activated for. The actual value is selected randomly for each activation
+    from N+1 to 3/2 * N. If not defined or set to 0, it is controlled by a
+    consensus parameter.
+    (Default: 24 hours)
+
 [[DoSRefuseSingleHopClientRendezvous]] **DoSRefuseSingleHopClientRendezvous** **0**|**1**|**auto**::
 
     Refuse establishment of rendezvous points for single hop clients. In other

src/core/or/dos.c

@@ -63,9 +63,14 @@ static unsigned int dos_conn_enabled = 0;
  * They are initialized with the hardcoded default values. */
 static uint32_t dos_conn_max_concurrent_count;
 static dos_conn_defense_type_t dos_conn_defense_type;
+static uint32_t dos_conn_connect_rate = DOS_CONN_CONNECT_RATE_DEFAULT;
+static uint32_t dos_conn_connect_burst = DOS_CONN_CONNECT_BURST_DEFAULT;
+static int32_t dos_conn_connect_defense_time_period =
+  DOS_CONN_CONNECT_DEFENSE_TIME_PERIOD_DEFAULT;
 
 /* Keep some stats for the heartbeat so we can report out. */
 static uint64_t conn_num_addr_rejected;
+static uint64_t conn_num_addr_connect_rejected;
 
 /*
  * General interface of the denial of service mitigation subsystem.
@@ -190,6 +195,47 @@ get_param_conn_defense_type(const networkstatus_t *ns)
                                  DOS_CONN_DEFENSE_NONE, DOS_CONN_DEFENSE_MAX);
 }
 
+/* Return the connection connect rate parameters either from the configuration
+ * file or, if not found, consensus parameter. */
+static uint32_t
+get_param_conn_connect_rate(const networkstatus_t *ns)
+{
+  if (dos_get_options()->DoSConnectionConnectRate) {
+    return dos_get_options()->DoSConnectionConnectRate;
+  }
+  return networkstatus_get_param(ns, "DoSConnectionConnectRate",
+                                 DOS_CONN_CONNECT_RATE_DEFAULT,
+                                 1, INT32_MAX);
+}
+
+/* Return the connection connect burst parameters either from the
+ * configuration file or, if not found, consensus parameter. */
+STATIC uint32_t
+get_param_conn_connect_burst(const networkstatus_t *ns)
+{
+  if (dos_get_options()->DoSConnectionConnectBurst) {
+    return dos_get_options()->DoSConnectionConnectBurst;
+  }
+  return networkstatus_get_param(ns, "DoSConnectionConnectBurst",
+                                 DOS_CONN_CONNECT_BURST_DEFAULT,
+                                 1, INT32_MAX);
+}
+
+/* Return the connection connect defense time period from the configuration
+ * file or, if not found, the consensus parameter. */
+static int32_t
+get_param_conn_connect_defense_time_period(const networkstatus_t *ns)
+{
+  /* Time in seconds. */
+  if (dos_get_options()->DoSConnectionConnectDefenseTimePeriod) {
+    return dos_get_options()->DoSConnectionConnectDefenseTimePeriod;
+  }
+  return networkstatus_get_param(ns, "DoSConnectionConnectDefenseTimePeriod",
+                                 DOS_CONN_CONNECT_DEFENSE_TIME_PERIOD_DEFAULT,
+                                 DOS_CONN_CONNECT_DEFENSE_TIME_PERIOD_MIN,
+                                 INT32_MAX);
+}
+
 /* Set circuit creation parameters located in the consensus or their default
  * if none are present. Called at initialization or when the consensus
  * changes. */
@@ -208,6 +254,10 @@ set_dos_parameters(const networkstatus_t *ns)
   dos_conn_enabled = get_param_conn_enabled(ns);
   dos_conn_max_concurrent_count = get_param_conn_max_concurrent_count(ns);
   dos_conn_defense_type = get_param_conn_defense_type(ns);
+  dos_conn_connect_rate = get_param_conn_connect_rate(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);
 }
 
 /* Free everything for the circuit creation DoS mitigation subsystem. */
@@ -349,7 +399,7 @@ cc_has_exhausted_circuits(const dos_client_stats_t *stats)
 {
   tor_assert(stats);
   return stats->cc_stats.circuit_bucket == 0 &&
-         stats->concurrent_count >= dos_cc_min_concurrent_conn;
+         stats->conn_stats.concurrent_count >= dos_cc_min_concurrent_conn;
 }
 
 /* Mark client address by setting a timestamp in the stats object which tells
@@ -405,6 +455,20 @@ cc_channel_addr_is_marked(channel_t *chan)
 
 /* Concurrent connection private API. */
 
+/* Mark client connection stats by setting a timestamp which tells us until
+ * when it is marked as positively detected. */
+static void
+conn_mark_client(conn_client_stats_t *stats)
+{
+  tor_assert(stats);
+
+  /* We add a random offset of a maximum of half the defense time so it is
+   * less predictable and thus more difficult to game. */
+  stats->marked_until_ts =
+    approx_time() + dos_conn_connect_defense_time_period +
+    crypto_rand_int_range(1, dos_conn_connect_defense_time_period / 2);
+}
+
 /* Free everything for the connection DoS mitigation subsystem. */
 static void
 conn_free_all(void)
@@ -424,6 +488,63 @@ conn_consensus_has_changed(const networkstatus_t *ns)
   }
 }
 
+/** Called when a new client connection has arrived. The following will update
+ * the client connection statistics.
+ *
+ * The addr is used for logging purposes only.
+ *
+ * If the connect counter reaches its limit, it is marked. */
+static void
+conn_update_on_connect(conn_client_stats_t *stats, const tor_addr_t *addr)
+{
+  tor_assert(stats);
+  tor_assert(addr);
+
+  /* Update concurrent count for this new connect. */
+  stats->concurrent_count++;
+
+  /* Refill connect connection count. */
+  token_bucket_ctr_refill(&stats->connect_count, (uint32_t) approx_time());
+
+  /* Decrement counter for this new connection. */
+  if (token_bucket_ctr_get(&stats->connect_count) > 0) {
+    token_bucket_ctr_dec(&stats->connect_count, 1);
+  }
+
+  /* Assess connect counter. Mark it if counter is down to 0 and we haven't
+   * marked it before or it was reset. This is to avoid to re-mark it over and
+   * over again extending continously the blocked time. */
+  if (token_bucket_ctr_get(&stats->connect_count) == 0 &&
+      stats->marked_until_ts == 0) {
+    conn_mark_client(stats);
+  }
+
+  log_debug(LD_DOS, "Client address %s has now %u concurrent connections. "
+                    "Remaining %lu/sec connections are allowed.",
+            fmt_addr(addr), stats->concurrent_count,
+            token_bucket_ctr_get(&stats->connect_count));
+}
+
+/** Called when a client connection is closed. The following will update
+ * the client connection statistics.
+ *
+ * The addr is used for logging purposes only. */
+static void
+conn_update_on_close(conn_client_stats_t *stats, const tor_addr_t *addr)
+{
+  /* Extra super duper safety. Going below 0 means an underflow which could
+   * lead to most likely a false positive. In theory, this should never happen
+   * but lets be extra safe. */
+  if (BUG(stats->concurrent_count == 0)) {
+    return;
+  }
+
+  stats->concurrent_count--;
+  log_debug(LD_DOS, "Client address %s has lost a connection. Concurrent "
+                    "connections are now at %u",
+            fmt_addr(addr), stats->concurrent_count);
+}
+
 /* General private API */
 
 /* Return true iff we have at least one DoS detection enabled. This is used to
@@ -549,9 +670,20 @@ dos_conn_addr_get_defense_type(const tor_addr_t *addr)
     goto end;
   }
 
+  /* Is this address marked as making too many client connections? */
+  if (entry->dos_stats.conn_stats.marked_until_ts >= approx_time()) {
+    conn_num_addr_connect_rejected++;
+    return dos_conn_defense_type;
+  }
+  /* Reset it to 0 here so that if the marked timestamp has expired that is
+   * we've gone beyond it, we have to reset it so the detection can mark it
+   * again in the future. */
+  entry->dos_stats.conn_stats.marked_until_ts = 0;
+
   /* Need to be above the maximum concurrent connection count to trigger a
    * defense. */
-  if (entry->dos_stats.concurrent_count > dos_conn_max_concurrent_count) {
+  if (entry->dos_stats.conn_stats.concurrent_count >
+      dos_conn_max_concurrent_count) {
     conn_num_addr_rejected++;
     return dos_conn_defense_type;
   }
@@ -576,7 +708,7 @@ dos_geoip_entry_about_to_free(const clientmap_entry_t *geoip_ent)
 
   /* The count is down to 0 meaning no connections right now, we can safely
    * clear the geoip entry from the cache. */
-  if (geoip_ent->dos_stats.concurrent_count == 0) {
+  if (geoip_ent->dos_stats.conn_stats.concurrent_count == 0) {
     goto end;
   }
 
@@ -597,6 +729,22 @@ dos_geoip_entry_about_to_free(const clientmap_entry_t *geoip_ent)
   return;
 }
 
+/** A new geoip client entry has been allocated, initialize its DoS object. */
+void
+dos_geoip_entry_init(clientmap_entry_t *geoip_ent)
+{
+  tor_assert(geoip_ent);
+
+  /* Initialize the connection count counter with the rate and burst
+   * parameters taken either from configuration or consensus.
+   *
+   * We do this even if the DoS connection detection is not enabled because it
+   * can be enabled at runtime and these counters need to be valid. */
+  token_bucket_ctr_init(&geoip_ent->dos_stats.conn_stats.connect_count,
+                        dos_conn_connect_rate, dos_conn_connect_burst,
+                        (uint32_t) approx_time());
+}
+
 /* Note down that we've just refused a single hop client. This increments a
  * counter later used for the heartbeat. */
 void
@@ -650,6 +798,9 @@ dos_log_heartbeat(void)
     tor_asprintf(&conn_msg,
                  " %" PRIu64 " connections closed.",
                  conn_num_addr_rejected);
+    tor_asprintf(&conn_msg,
+                 " %" PRIu64 " connect() connections closed.",
+                 conn_num_addr_connect_rejected);
   }
 
   if (dos_should_refuse_single_hop_client()) {
@@ -711,11 +862,11 @@ dos_new_client_conn(or_connection_t *or_conn, const char *transport_name)
     goto end;
   }
 
-  entry->dos_stats.concurrent_count++;
+  /* Update stats from this new connect. */
+  conn_update_on_connect(&entry->dos_stats.conn_stats,
+                         &TO_CONN(or_conn)->addr);
+
   or_conn->tracked_for_dos_mitigation = 1;
-  log_debug(LD_DOS, "Client address %s has now %u concurrent connections.",
-            fmt_addr(&TO_CONN(or_conn)->addr),
-            entry->dos_stats.concurrent_count);
 
  end:
   return;
@@ -745,18 +896,8 @@ dos_close_client_conn(const or_connection_t *or_conn)
     goto end;
   }
 
-  /* Extra super duper safety. Going below 0 means an underflow which could
-   * lead to most likely a false positive. In theory, this should never happen
-   * but lets be extra safe. */
-  if (BUG(entry->dos_stats.concurrent_count == 0)) {
-    goto end;
-  }
-
-  entry->dos_stats.concurrent_count--;
-  log_debug(LD_DOS, "Client address %s has lost a connection. Concurrent "
-                    "connections are now at %u",
-            fmt_addr(&TO_CONN(or_conn)->addr),
-            entry->dos_stats.concurrent_count);
+  /* Update stats from this new close. */
+  conn_update_on_close(&entry->dos_stats.conn_stats, &TO_CONN(or_conn)->addr);
 
  end:
   return;

src/core/or/dos.h

@@ -11,7 +11,9 @@
 
 #include "core/or/or.h"
 
-/* Structure that keeps stats of client connection per-IP. */
+#include "lib/evloop/token_bucket.h"
+
+/* Structure that keeps stats of circuit creation per client connection IP. */
 typedef struct cc_client_stats_t {
   /* Number of allocated circuits remaining for this address.  It is
    * decremented every time a new circuit is seen for this client address and
@@ -30,13 +32,28 @@ typedef struct cc_client_stats_t {
   time_t marked_until_ts;
 } cc_client_stats_t;
 
+/* Structure that keeps stats of client connection per-IP. */
+typedef struct conn_client_stats_t {
+  /* Concurrent connection count from the specific address. 2^32 - 1 is most
+   * likely way too big for the amount of allowed file descriptors. */
+  uint32_t concurrent_count;
+
+  /* Connect count from the specific address. We use a token bucket here to
+   * track the rate and burst of connections from the same IP address.*/
+  token_bucket_ctr_t connect_count;
+
+  /* The client address attempted too many connections, per the connect_count
+   * rules, and thus is marked so defense(s) can be applied. It is
+   * synchronized using the approx_time(). */
+  time_t marked_until_ts;
+} conn_client_stats_t;
+
 /* This object is a top level object that contains everything related to the
  * per-IP client DoS mitigation. Because it is per-IP, it is used in the geoip
  * clientmap_entry_t object. */
 typedef struct dos_client_stats_t {
-  /* Concurrent connection count from the specific address. 2^32 is most
-   * likely way too big for the amount of allowed file descriptors. */
-  uint32_t concurrent_count;
+  /* Client connection statistics. */
+  conn_client_stats_t conn_stats;
 
   /* Circuit creation statistics. This is only used if the circuit creation
    * subsystem has been enabled (dos_cc_enabled). */
@@ -53,6 +70,7 @@ void dos_free_all(void);
 void dos_consensus_has_changed(const networkstatus_t *ns);
 int dos_enabled(void);
 void dos_log_heartbeat(void);
+void dos_geoip_entry_init(struct clientmap_entry_t *geoip_ent);
 void dos_geoip_entry_about_to_free(const struct clientmap_entry_t *geoip_ent);
 
 void dos_new_client_conn(or_connection_t *or_conn,
@@ -104,6 +122,16 @@ dos_cc_defense_type_t dos_cc_get_defense_type(channel_t *chan);
 #define DOS_CONN_MAX_CONCURRENT_COUNT_DEFAULT 100
 /* DoSConnectionDefenseType maps to the dos_conn_defense_type_t enum. */
 #define DOS_CONN_DEFENSE_TYPE_DEFAULT DOS_CONN_DEFENSE_CLOSE
+/* DoSConnectionConnectRate default. Per second. */
+#define DOS_CONN_CONNECT_RATE_DEFAULT 20
+/* DoSConnectionConnectBurst default. Per second. */
+#define DOS_CONN_CONNECT_BURST_DEFAULT 40
+/* DoSConnectionConnectDefenseTimePeriod default. Set to 24 hours. */
+#define DOS_CONN_CONNECT_DEFENSE_TIME_PERIOD_DEFAULT (24 * 60 * 60)
+/* DoSCircuitCreationDefenseTimePeriod minimum value. Because we add a random
+ * offset to the marked timestamp, we need the minimum value to be non zero.
+ * We consider that 10 seconds is an acceptable lower bound. */
+#define DOS_CONN_CONNECT_DEFENSE_TIME_PERIOD_MIN (10)
 
 /* Type of defense that we can use for the concurrent connection DoS
  * mitigation. */
@@ -127,6 +155,7 @@ STATIC uint32_t get_param_conn_max_concurrent_count(
 STATIC uint32_t get_param_cc_circuit_burst(const networkstatus_t *ns);
 STATIC uint32_t get_param_cc_min_concurrent_connection(
                                             const networkstatus_t *ns);
+STATIC uint32_t get_param_conn_connect_burst(const networkstatus_t *ns);
 
 STATIC uint64_t get_circuit_rate_per_second(void);
 STATIC void cc_stats_refill_bucket(cc_client_stats_t *stats,

src/core/or/dos_options.inc

@@ -44,4 +44,16 @@ CONF_VAR(DoSConnectionDefenseType, INT, 0, "0")
 /** Autobool: Do we refuse single hop client rendezvous? */
 CONF_VAR(DoSRefuseSingleHopClientRendezvous, AUTOBOOL, 0, "auto")
 
+/** Allowed burst of client connection allowed per address. */
+CONF_VAR(DoSConnectionConnectBurst, POSINT, 0, "0")
+
+/** Allowed rate of client connection allowed per address. */
+CONF_VAR(DoSConnectionConnectRate, POSINT, 0, "0")
+
+/** For how much time (in seconds) the connection connect rate defense is
+* applicable for a malicious address. A random time delta is added to the
+* defense time of an address which will be between 1 second and half of this
+* value. */
+CONF_VAR(DoSConnectionConnectDefenseTimePeriod, INTERVAL, 0, "0")
+
 END_CONF_STRUCT(dos_options_t)

src/feature/stats/geoip_stats.c

@@ -196,6 +196,8 @@ clientmap_entry_new(geoip_client_action_t action, const tor_addr_t *addr,
   if (transport_name) {
     entry->transport_name = tor_strdup(transport_name);
   }
+  /* Initialize the DoS object. */
+  dos_geoip_entry_init(entry);
 
   /* Allocated and initialized, note down its size for the OOM handler. */
   geoip_increment_client_history_cache_size(clientmap_entry_size(entry));

src/test/test_dos.c

@@ -79,6 +79,9 @@ test_dos_conn_creation(void *arg)
   { /* Register many conns from this client but not enough to get it blocked */
     unsigned int i;
     for (i = 0; i < max_concurrent_conns; i++) {
+      /* Don't trigger the connect() rate limitation so advance the clock 1
+       * second for each connection. */
+      update_approx_time(++now);
       dos_new_client_conn(&or_conn, NULL);
     }
   }
@@ -470,7 +473,7 @@ test_known_relay(void *arg)
                               GEOIP_CLIENT_CONNECT);
   tt_assert(entry);
   /* We should have a count of 0. */
-  tt_uint_op(entry->dos_stats.concurrent_count, OP_EQ, 0);
+  tt_uint_op(entry->dos_stats.conn_stats.concurrent_count, OP_EQ, 0);
 
   /* To make sure that his is working properly, make a unknown client
    * connection and see if we do get it. */
@@ -483,7 +486,7 @@ test_known_relay(void *arg)
                               GEOIP_CLIENT_CONNECT);
   tt_assert(entry);
   /* We should have a count of 2. */
-  tt_uint_op(entry->dos_stats.concurrent_count, OP_EQ, 2);
+  tt_uint_op(entry->dos_stats.conn_stats.concurrent_count, OP_EQ, 2);
 
  done:
   routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md);
@@ -496,11 +499,69 @@ test_known_relay(void *arg)
   UNMOCK(get_param_cc_enabled);
 }
 
+/** Test that the connection tracker of the DoS subsystem will block clients
+ *  who try to establish too many connections */
+static void
+test_dos_conn_rate(void *arg)
+{
+  (void) arg;
+
+  MOCK(get_param_cc_enabled, mock_enable_dos_protection);
+  MOCK(get_param_conn_enabled, mock_enable_dos_protection);
+
+  /* Initialize test data */
+  or_connection_t or_conn;
+  time_t now = 1281533250; /* 2010-08-11 13:27:30 UTC */
+  tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&TO_CONN(&or_conn)->addr,
+                                          "18.0.0.1"));
+  tor_addr_t *addr = &TO_CONN(&or_conn)->addr;
+  update_approx_time(now);
+
+  /* Get DoS subsystem limits */
+  dos_init();
+  uint32_t burst_conn = get_param_conn_connect_burst(NULL);
+
+  /* Introduce new client */
+  geoip_note_client_seen(GEOIP_CLIENT_CONNECT, addr, NULL, now);
+  { /* Register many conns from this client but not enough to get it blocked */
+    unsigned int i;
+    for (i = 0; i < burst_conn - 1; i++) {
+      dos_new_client_conn(&or_conn, NULL);
+    }
+  }
+
+  /* Check that new conns are still permitted */
+  tt_int_op(DOS_CONN_DEFENSE_NONE, OP_EQ,
+            dos_conn_addr_get_defense_type(addr));
+
+  /* Register another conn and check that new conns are not allowed anymore.
+   * We should have reached our burst. */
+  dos_new_client_conn(&or_conn, NULL);
+  tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ,
+            dos_conn_addr_get_defense_type(addr));
+
+  /* Advance the time 12 hours. It should still be blocked. */
+  update_approx_time(now + (12 * 60 * 60));
+  tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ,
+            dos_conn_addr_get_defense_type(addr));
+
+  /* Advance the time 24 hours plus 13 hours. It should be unblocked.
+   * Remember, we had a random value between 24 hours and rand(24/2) thus
+   * adding 13 hours is safe. */
+  update_approx_time(now + (37 * 60 * 60));
+  tt_int_op(DOS_CONN_DEFENSE_NONE, OP_EQ,
+            dos_conn_addr_get_defense_type(addr));
+
+ done:
+  dos_free_all();
+}
+
 struct testcase_t dos_tests[] = {
   { "conn_creation", test_dos_conn_creation, TT_FORK, NULL, NULL },
   { "circuit_creation", test_dos_circuit_creation, TT_FORK, NULL, NULL },
   { "bucket_refill", test_dos_bucket_refill, TT_FORK, NULL, NULL },
   { "known_relay" , test_known_relay, TT_FORK,
     NULL, NULL },
+  { "conn_rate", test_dos_conn_rate, TT_FORK, NULL, NULL },
   END_OF_TESTCASES
 };