Merge remote-tracking branch 'mbeth-private/ticket40821_mr'

5 files changed, 49 insertions, 42 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..ccdb30dbee 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. */
@@ -975,14 +976,6 @@ dos_new_client_conn(or_connection_t *or_conn, const char *transport_name)
     goto end;
   }
 
-  /* We ignore any known address meaning an address of a known relay. The
-   * reason to do so is because network reentry is possible where a client
-   * connection comes from an Exit node. Even when we'll fix reentry, this is
-   * a robust defense to keep in place. */
-  if (nodelist_probably_contains_address(&TO_CONN(or_conn)->addr)) {
-    goto end;
-  }
-
   /* We are only interested in client connection from the geoip cache. */
   entry = geoip_lookup_client(&TO_CONN(or_conn)->addr, transport_name,
                               GEOIP_CLIENT_CONNECT);
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..6c57e85cb8 100644
--- a/src/test/test_dos.c
+++ b/src/test/test_dos.c
@@ -72,6 +72,7 @@ test_dos_conn_creation(void *arg)
 
   /* Initialize test data */
   or_connection_t or_conn;
+  memset(&or_conn, 0, sizeof or_conn);
   time_t wallclock_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"));
@@ -90,6 +91,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 +101,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));
 
@@ -150,6 +154,7 @@ test_dos_circuit_creation(void *arg)
 
   /* Initialize test data */
   or_connection_t or_conn;
+  memset(&or_conn, 0, sizeof 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"));
@@ -165,6 +170,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);
   }
 
@@ -214,6 +220,7 @@ test_dos_bucket_refill(void *arg)
   channel_init(chan);
   chan->is_client = 1;
   or_connection_t or_conn;
+  memset(&or_conn, 0, sizeof or_conn);
   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;
@@ -430,12 +437,12 @@ test_dos_bucket_refill(void *arg)
   dos_free_all();
 }
 
-/* Test if we avoid counting a known relay. */
+/* Test if we avoid counting a known relay. (We no longer do) */
 static void
 test_known_relay(void *arg)
 {
   clientmap_entry_t *entry = NULL;
-  routerstatus_t *rs = NULL; microdesc_t *md = NULL; routerinfo_t *ri = NULL;
+  routerstatus_t *rs = NULL;
 
   (void) arg;
 
@@ -455,6 +462,7 @@ test_known_relay(void *arg)
 
   /* Setup an OR conn so we can pass it to the DoS subsystem. */
   or_connection_t or_conn;
+  memset(&or_conn, 0, sizeof or_conn);
   tor_addr_parse(&TO_CONN(&or_conn)->addr, "42.42.42.42");
 
   rs = tor_malloc_zero(sizeof(*rs));
@@ -471,34 +479,24 @@ test_known_relay(void *arg)
    * client connection. */
   geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &TO_CONN(&or_conn)->addr,
                          NULL, 0);
-  /* Suppose we have 5 connections in rapid succession, the counter should
-   * always be 0 because we should ignore this. */
+  /* Suppose we have 5 connections in rapid succession */
   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);
   tt_assert(entry);
-  /* We should have a count of 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. */
-  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);
-  dos_new_client_conn(&or_conn, NULL);
-  dos_new_client_conn(&or_conn, NULL);
-  entry = geoip_lookup_client(&TO_CONN(&or_conn)->addr, NULL,
-                              GEOIP_CLIENT_CONNECT);
-  tt_assert(entry);
-  /* We should have a count of 2. */
-  tt_uint_op(entry->dos_stats.conn_stats.concurrent_count, OP_EQ, 2);
+  /* We should have a count of 5. */
+  tt_uint_op(entry->dos_stats.conn_stats.concurrent_count, OP_EQ, 5);
 
  done:
-  routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md);
+  routerstatus_free(rs);
   smartlist_clear(dummy_ns->routerstatus_list);
   networkstatus_vote_free(dummy_ns);
   dos_free_all();
@@ -520,6 +518,7 @@ test_dos_conn_rate(void *arg)
 
   /* Initialize test data */
   or_connection_t or_conn;
+  memset(&or_conn, 0, sizeof 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"));
@@ -535,6 +534,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 +545,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));