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.

5 files changed, 40 insertions, 14 deletions

diff --git a/src/core/or/channel.c b/src/core/or/channel.c
index 9d15d35ac9..b5b3f4c4f0 100644
--- 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 9db8e2392d..1f5a466777 100644
--- 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 8e0fe9e641..b9f8eb22f2 100644
--- 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 b54304337a..3a455b2705 100644
--- 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 8c9ddfcbe5..110441892c 100644
--- 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));