dos: Clear connection tracked flag if geoip entry is removed

Imagine this scenario. We had 10 connections over the 24h lifetime of a geoip
cache entry. The lifetime of the entry has been reached so it is about to get
freed but 2 connections remain for it. After the free, a third connection
comes in thus making us create a new geoip entry for that address matching the
2 previous ones that are still alive. If they end up being closed, we'll have
a concurrent count desynch from what the reality is.

To mitigate this probably very rare scenario in practice, when we free a geoip
entry and it has a concurrent count above 0, we'll go over all connections
matching the address and clear out the tracked flag. So once they are closed,
we don't try to decrement the count.

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

1 files changed, 35 insertions, 0 deletions

diff --git a/src/or/dos.c b/src/or/dos.c
index 40e88aead0..5af75ca57d 100644
--- a/src/or/dos.c
+++ b/src/or/dos.c
@@ -528,6 +528,41 @@ dos_conn_addr_get_defense_type(const tor_addr_t *addr)
 
 /* General API */
 
+/* Take any appropriate actions for the given geoip entry that is about to get
+ * freed. This is called for every entry that is being freed.
+ *
+ * This function will clear out the connection tracked flag if the concurrent
+ * count of the entry is above 0 so if those connections end up being seen by
+ * this subsystem, we won't try to decrement the counter for a new geoip entry
+ * that might have been added after this call for the same address. */
+void
+dos_geoip_entry_about_to_free(const clientmap_entry_t *geoip_ent)
+{
+  tor_assert(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) {
+    goto end;
+  }
+
+  /* For each connection matching the geoip entry address, we'll clear the
+   * tracked flag because the entry is about to get removed from the geoip
+   * cache. We do not try to decrement if the flag is not set. */
+  SMARTLIST_FOREACH_BEGIN(get_connection_array(), connection_t *, conn) {
+    if (conn->type == CONN_TYPE_OR) {
+      or_connection_t *or_conn = TO_OR_CONN(conn);
+      if (!tor_addr_compare(&geoip_ent->addr, &or_conn->real_addr,
+                            CMP_EXACT)) {
+        or_conn->tracked_for_dos_mitigation = 0;
+      }
+    }
+  } SMARTLIST_FOREACH_END(conn);
+
+ end:
+  return;
+}
+
 /* Note down that we've just refused a single hop client. This increments a
  * counter later used for the heartbeat. */
 void