Extend DoS protection to partially-open channels

tor only marks a channel as 'open' once the TLS and OR handshakes have both
completed, and normal "client" (ORPort) DoS protection is not enabled until
the channel becomes open. This patch adds an additional earlier initialization
path for DoS protection on incoming TLS connections.

This leaves the existing dos_new_client_conn() call sites intact, but adds a
guard against multiple-initialization using the existing
tracked_for_dos_mitigation flag. Other types of channels shouldn't be affected
by this patch.

diff --git a/src/core/or/channel.c b/src/core/or/channel.c
index 9d15d35ac971d7ff46918e8f4ac7ab854eae36ca..b5b3f4c4f0ddd12f634dc492a1af47f5f59b7777 100644 (file)
--- a/src/core/or/channel.c
+++ b/src/core/or/channel.c
@@ -83,6 +83,7 @@
 #include "lib/time/compat_time.h"
 
 #include "core/or/cell_queue_st.h"
+#include "core/or/or_connection_st.h"
 
 /* Global lists of channels */
 
@@ -1864,7 +1865,6 @@ channel_do_open_actions(channel_t *chan)
 {
   tor_addr_t remote_addr;
   int started_here;
-  time_t now = time(NULL);
   int close_origin_circuits = 0;
 
   tor_assert(chan);
@@ -1875,22 +1875,25 @@ channel_do_open_actions(channel_t *chan)
     circuit_build_times_network_is_live(get_circuit_build_times_mutable());
     router_set_status(chan->identity_digest, 1);
   } else {
-    /* only report it to the geoip module if it's a client */
+    /* only report it to the geoip module if it's a client and it hasn't
+     * already been set up for tracking earlier. (Incoming TLS connections
+     * are tracked before the handshake.) */
     if (channel_is_client(chan)) {
       if (channel_get_addr_if_possible(chan, &remote_addr)) {
-        char *transport_name = NULL;
         channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
-        if (chan->get_transport_name(chan, &transport_name) < 0)
-          transport_name = NULL;
-
-        geoip_note_client_seen(GEOIP_CLIENT_CONNECT,
-                               &remote_addr, transport_name,
-                               now);
-        /* Notify the DoS subsystem of a new client. */
-        if (tlschan && tlschan->conn) {
-          dos_new_client_conn(tlschan->conn, transport_name);
+        if (!tlschan->conn->tracked_for_dos_mitigation) {
+          char *transport_name = NULL;
+          if (chan->get_transport_name(chan, &transport_name) < 0) {
+            transport_name = NULL;
+          }
+          geoip_note_client_seen(GEOIP_CLIENT_CONNECT,
+                                 &remote_addr, transport_name,
+                                 time(NULL));
+          if (tlschan && tlschan->conn) {
+            dos_new_client_conn(tlschan->conn, transport_name);
+          }
+          tor_free(transport_name);
         }
-        tor_free(transport_name);
       }
       /* Otherwise the underlying transport can't tell us this, so skip it */
     }

diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c
index 9db8e2392dab70a7c235798e0f9bbb5f6d4a604e..1f5a46677702f1c18668da732eb5d2c35e5c6fe8 100644 (file)
--- a/src/core/or/channeltls.c
+++ b/src/core/or/channeltls.c
@@ -44,6 +44,7 @@
 #include "core/or/circuitmux.h"
 #include "core/or/circuitmux_ewma.h"
 #include "core/or/command.h"
+#include "core/or/dos.h"
 #include "app/config/config.h"
 #include "app/config/resolve_addr.h"
 #include "core/mainloop/connection.h"
@@ -54,6 +55,7 @@
 #include "trunnel/link_handshake.h"
 #include "core/or/relay.h"
 #include "feature/stats/rephist.h"
+#include "feature/stats/geoip_stats.h"
 #include "feature/relay/router.h"
 #include "feature/relay/routermode.h"
 #include "feature/nodelist/dirlist.h"
@@ -358,6 +360,14 @@ channel_tls_handle_incoming(or_connection_t *orconn)
   /* Register it */
   channel_register(chan);
 
+  /* Start tracking TLS connections in the DoS subsystem as soon as possible,
+   * so we can protect against attacks that use partially open connections.
+   */
+  geoip_note_client_seen(GEOIP_CLIENT_CONNECT,
+                         &TO_CONN(orconn)->addr, NULL,
+                         time(NULL));
+  dos_new_client_conn(orconn, NULL);
+
   return chan;
 }
 

diff --git a/src/core/or/dos.c b/src/core/or/dos.c
index 8e0fe9e641eba889edb217a355500211c7fea870..b9f8eb22f2efa80cac323cd82f138178452213d3 100644 (file)
--- a/src/core/or/dos.c
+++ b/src/core/or/dos.c
@@ -968,6 +968,7 @@ dos_new_client_conn(or_connection_t *or_conn, const char *transport_name)
   clientmap_entry_t *entry;
 
   tor_assert(or_conn);
+  tor_assert_nonfatal(!or_conn->tracked_for_dos_mitigation);
 
   /* Past that point, we know we have at least one DoS detection subsystem
    * enabled so we'll start allocating stuff. */

diff --git a/src/feature/stats/geoip_stats.h b/src/feature/stats/geoip_stats.h
index b54304337a89b7929d88ec3b05d5360130fb0402..3a455b2705a4620ecbee54a534205f7a467314df 100644 (file)
--- a/src/feature/stats/geoip_stats.h
+++ b/src/feature/stats/geoip_stats.h
@@ -20,7 +20,7 @@
  * the others, we're not.
  */
 typedef enum {
-  /** We've noticed a connection as a bridge relay or entry guard. */
+  /** An incoming ORPort connection */
   GEOIP_CLIENT_CONNECT = 0,
   /** We've served a networkstatus consensus as a directory server. */
   GEOIP_CLIENT_NETWORKSTATUS = 1,

diff --git a/src/test/test_dos.c b/src/test/test_dos.c
index 8c9ddfcbe561464f782b93c4b729d1f6d3495272..110441892cebd5b7502daecc9f676f266ac50d08 100644 (file)
--- a/src/test/test_dos.c
+++ b/src/test/test_dos.c
@@ -90,6 +90,7 @@ test_dos_conn_creation(void *arg)
        * second for each connection. */
       monotime_coarse_set_mock_time_nsec(monotime_now += BILLION);
       update_approx_time(++wallclock_now);
+      or_conn.tracked_for_dos_mitigation = 0;
       dos_new_client_conn(&or_conn, NULL);
     }
   }
@@ -99,12 +100,14 @@ test_dos_conn_creation(void *arg)
             dos_conn_addr_get_defense_type(addr));
 
   /* Register another conn and check that new conns are not allowed anymore */
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
   tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ,
             dos_conn_addr_get_defense_type(addr));
 
   /* Close a client conn and see that a new conn will be permitted again */
   dos_close_client_conn(&or_conn);
+  or_conn.tracked_for_dos_mitigation = 0;
   tt_int_op(DOS_CONN_DEFENSE_NONE, OP_EQ,
             dos_conn_addr_get_defense_type(addr));
 
@@ -165,6 +168,7 @@ test_dos_circuit_creation(void *arg)
    * circuit counting subsystem */
   geoip_note_client_seen(GEOIP_CLIENT_CONNECT, addr, NULL, now);
   for (i = 0; i < min_conc_conns_for_cc ; i++) {
+    or_conn.tracked_for_dos_mitigation = 0;
     dos_new_client_conn(&or_conn, NULL);
   }
 
@@ -474,9 +478,13 @@ test_known_relay(void *arg)
   /* Suppose we have 5 connections in rapid succession, the counter should
    * always be 0 because we should ignore this. */
   dos_new_client_conn(&or_conn, NULL);
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
   entry = geoip_lookup_client(&TO_CONN(&or_conn)->addr, NULL,
                               GEOIP_CLIENT_CONNECT);
@@ -489,7 +497,9 @@ test_known_relay(void *arg)
   tor_addr_parse(&TO_CONN(&or_conn)->addr, "42.42.42.43");
   geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &TO_CONN(&or_conn)->addr,
                          NULL, 0);
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
   entry = geoip_lookup_client(&TO_CONN(&or_conn)->addr, NULL,
                               GEOIP_CLIENT_CONNECT);
@@ -535,6 +545,7 @@ test_dos_conn_rate(void *arg)
   { /* Register many conns from this client but not enough to get it blocked */
     unsigned int i;
     for (i = 0; i < burst_conn - 1; i++) {
+      or_conn.tracked_for_dos_mitigation = 0;
       dos_new_client_conn(&or_conn, NULL);
     }
   }
@@ -545,6 +556,7 @@ test_dos_conn_rate(void *arg)
 
   /* Register another conn and check that new conns are not allowed anymore.
    * We should have reached our burst. */
+  or_conn.tracked_for_dos_mitigation = 0;
   dos_new_client_conn(&or_conn, NULL);
   tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ,
             dos_conn_addr_get_defense_type(addr));