dos: Pass transport name on new client connection

For a bridge configured with a pluggable transport, the transport name is
used, with the IP address, for the GeoIP client cache entry.

However, the DoS subsystem was not aware of it and always passing NULL when
doing a lookup into the GeoIP cache.

This resulted in bridges with a PT are never able to apply DoS defenses for
newly created connections.

Fixes #33491

Signed-off-by: David Goulet <dgoulet@torproject.org>

diff --git a/changes/ticket33491 b/changes/ticket33491
new file mode 100644 (file)
index 0000000..595ea86
--- /dev/null
+++ b/changes/ticket33491
@@ -0,0 +1,6 @@
+  o Major bugfixes (DoS defenses, bridges, pluggable transport):
+    - DoS subsystem was not given the transport name of the client connection
+      when tor is a bridge and thus failing to find the GeoIP cache entry for
+      that client address. This resulted in failing to apply DoS defenses on
+      bridges with a pluggable transport. Fixes bug 33491; bugfix on
+      0.3.3.2-alpha.

diff --git a/src/core/or/channel.c b/src/core/or/channel.c
index fd7bf62789f891661b7386448d6d83c09671ec21..3886906875211147e092ba78cabf3519c57e645d 100644 (file)
--- a/src/core/or/channel.c
+++ b/src/core/or/channel.c
@@ -1871,7 +1871,7 @@ channel_do_open_actions(channel_t *chan)
         tor_free(transport_name);
         /* Notify the DoS subsystem of a new client. */
         if (tlschan && tlschan->conn) {
-          dos_new_client_conn(tlschan->conn);
+          dos_new_client_conn(tlschan->conn, transport_name);
         }
       }
       /* Otherwise the underlying transport can't tell us this, so skip it */

diff --git a/src/core/or/dos.c b/src/core/or/dos.c
index 5f9bbf90ab27ad9108d51c6156b47fe079a3c24b..d06eaa6d05f3dd5ba84e1a4f47df01ead0eab113 100644 (file)
--- a/src/core/or/dos.c
+++ b/src/core/or/dos.c
@@ -671,7 +671,7 @@ dos_log_heartbeat(void)
 /* Called when a new client connection has been established on the given
  * address. */
 void
-dos_new_client_conn(or_connection_t *or_conn)
+dos_new_client_conn(or_connection_t *or_conn, const char *transport_name)
 {
   clientmap_entry_t *entry;
 
@@ -692,7 +692,7 @@ dos_new_client_conn(or_connection_t *or_conn)
   }
 
   /* We are only interested in client connection from the geoip cache. */
-  entry = geoip_lookup_client(&or_conn->real_addr, NULL,
+  entry = geoip_lookup_client(&or_conn->real_addr, transport_name,
                               GEOIP_CLIENT_CONNECT);
   if (BUG(entry == NULL)) {
     /* Should never happen because we note down the address in the geoip

diff --git a/src/core/or/dos.h b/src/core/or/dos.h
index 95448d0530dfe1419eb49b72b59e8340528c2305..058b7afce6bf792e58b7033a6b4b2d65a6565f4c 100644 (file)
--- a/src/core/or/dos.h
+++ b/src/core/or/dos.h
@@ -53,7 +53,8 @@ int dos_enabled(void);
 void dos_log_heartbeat(void);
 void dos_geoip_entry_about_to_free(const struct clientmap_entry_t *geoip_ent);
 
-void dos_new_client_conn(or_connection_t *or_conn);
+void dos_new_client_conn(or_connection_t *or_conn,
+                         const char *transport_name);
 void dos_close_client_conn(const or_connection_t *or_conn);
 
 int dos_should_refuse_single_hop_client(void);

diff --git a/src/test/test_dos.c b/src/test/test_dos.c
index 4756c5014e6e90ff2d63e9db5f578f277b49d90f..01d7cd006e802aca7cd348412a337485f4504c95 100644 (file)
--- a/src/test/test_dos.c
+++ b/src/test/test_dos.c
@@ -79,7 +79,7 @@ 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++) {
-      dos_new_client_conn(&or_conn);
+      dos_new_client_conn(&or_conn, NULL);
     }
   }
 
@@ -88,7 +88,7 @@ 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 */
-  dos_new_client_conn(&or_conn);
+  dos_new_client_conn(&or_conn, NULL);
   tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ,
             dos_conn_addr_get_defense_type(addr));
 
@@ -98,7 +98,7 @@ test_dos_conn_creation(void *arg)
             dos_conn_addr_get_defense_type(addr));
 
   /* Register another conn and see that defense measures get reactivated */
-  dos_new_client_conn(&or_conn);
+  dos_new_client_conn(&or_conn, NULL);
   tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ,
             dos_conn_addr_get_defense_type(addr));
 
@@ -153,7 +153,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++) {
-    dos_new_client_conn(&or_conn);
+    dos_new_client_conn(&or_conn, NULL);
   }
 
   /* Register new circuits for this client and conn, but not enough to get
@@ -217,7 +217,7 @@ test_dos_bucket_refill(void *arg)
 
   /* Register this client */
   geoip_note_client_seen(GEOIP_CLIENT_CONNECT, addr, NULL, now);
-  dos_new_client_conn(&or_conn);
+  dos_new_client_conn(&or_conn, NULL);
 
   /* Fetch this client from the geoip cache and get its DoS structs */
   clientmap_entry_t *entry = geoip_lookup_client(addr, NULL,
@@ -460,11 +460,11 @@ test_known_relay(void *arg)
   geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &or_conn.real_addr, NULL, 0);
   /* 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);
-  dos_new_client_conn(&or_conn);
-  dos_new_client_conn(&or_conn);
-  dos_new_client_conn(&or_conn);
-  dos_new_client_conn(&or_conn);
+  dos_new_client_conn(&or_conn, NULL);
+  dos_new_client_conn(&or_conn, NULL);
+  dos_new_client_conn(&or_conn, NULL);
+  dos_new_client_conn(&or_conn, NULL);
+  dos_new_client_conn(&or_conn, NULL);
   entry = geoip_lookup_client(&or_conn.real_addr, NULL, GEOIP_CLIENT_CONNECT);
   tt_assert(entry);
   /* We should have a count of 0. */
@@ -474,8 +474,8 @@ test_known_relay(void *arg)
    * connection and see if we do get it. */
   tor_addr_parse(&or_conn.real_addr, "42.42.42.43");
   geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &or_conn.real_addr, NULL, 0);
-  dos_new_client_conn(&or_conn);
-  dos_new_client_conn(&or_conn);
+  dos_new_client_conn(&or_conn, NULL);
+  dos_new_client_conn(&or_conn, NULL);
   entry = geoip_lookup_client(&or_conn.real_addr, NULL, GEOIP_CLIENT_CONNECT);
   tt_assert(entry);
   /* We should have a count of 2. */