relay: Keep geoip cache entry if connection attempts are seen

Fixes #40637

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

changes/ticket40637

@@ -0,0 +1,4 @@
+  o Minor bugfixes (relay, DoS):
+    - Keep geoip entries from being deleted if the address is still trying to
+      connect but the DoS subsystem refuses them. Fixes bug 40637; bugfix on
+      0.4.6.1-alpha.

src/core/mainloop/connection.c

@@ -110,6 +110,7 @@
 #include "feature/stats/connstats.h"
 #include "feature/stats/rephist.h"
 #include "feature/stats/bwhist.h"
+#include "feature/stats/geoip_stats.h"
 #include "lib/crypt_ops/crypto_util.h"
 #include "lib/crypt_ops/crypto_format.h"
 #include "lib/geoip/geoip.h"
@@ -2031,6 +2032,7 @@ connection_handle_listener_read(connection_t *conn, int new_type)
     if (new_type == CONN_TYPE_OR) {
       /* Assess with the connection DoS mitigation subsystem if this address
        * can open a new connection. */
+      geoip_note_client_attempt(&addr, approx_time());
       if (dos_conn_addr_get_defense_type(&addr) == DOS_CONN_DEFENSE_CLOSE) {
         rep_hist_note_conn_rejected(new_type);
         tor_close_socket(news);

src/core/or/dos.h

@@ -46,6 +46,12 @@ typedef struct conn_client_stats_t {
    * rules, and thus is marked so defense(s) can be applied. It is
    * synchronized using the approx_time(). */
   time_t marked_until_ts;
+
+  /* Denotes when was the last connection attempt (successful or not). We use
+   * this to decide if we can keep or purge the stats entry in our cache. We
+   * can't rely on the "last_seen_in_minutes" because that is set when a
+   * successful channel is opened meaning TCP connect and Tor negotiation. */
+  time_t last_attempt;
 } conn_client_stats_t;
 
 /* This object is a top level object that contains everything related to the

src/feature/stats/geoip_stats.c

@@ -222,6 +222,32 @@ client_history_clear(void)
   }
 }
 
+/** Note that an OR connection attempt. This is called basically when a TCP
+ * connection comes in but has not completed the Tor negotiation. We used this
+ * information to keep entries in the geoip cache for the DoS subsystem.
+ *
+ * This won't find any bridge transport connections because at this stage, we
+ * don't know the transport name if any. And so, only applies to naked OR
+ * connections. */
+void
+geoip_note_client_attempt(const tor_addr_t *addr, time_t now)
+{
+  /* This is only useful to the DoS subsystem so ignore if not enabled. */
+  if (!dos_enabled()) {
+    return;
+  }
+
+  /* We lookup if this address/transport name has already connected
+   * successfully and if so, we will not this attempt time. Else, we do not add
+   * an entry on an attempt so to not clobber the cache with unsuccessful
+   * connections. */
+  clientmap_entry_t *ent = geoip_lookup_client(addr, NULL,
+                                               GEOIP_CLIENT_CONNECT);
+  if (ent) {
+    ent->dos_stats.conn_stats.last_attempt = now;
+  }
+}
+
 /** Note that we've seen a client connect from the IP <b>addr</b>
  * at time <b>now</b>. Ignored by all but bridges and directories if
  * configured accordingly. */
@@ -281,11 +307,15 @@ remove_old_client_helper_(struct clientmap_entry_t *ent, void *_cutoff)
 {
   time_t cutoff = *(time_t*)_cutoff / 60;
   if (ent->last_seen_in_minutes < cutoff) {
-    clientmap_entry_free(ent);
-    return 1;
-  } else {
-    return 0;
+    /* If the DoS subsystem is disabled, this is an automatic clean up. If it
+     * is enabled, check if the last attempt is below cutoff else keep it
+     * around. */
+    if (!dos_enabled() || ent->dos_stats.conn_stats.last_attempt < cutoff) {
+      clientmap_entry_free(ent);
+      return 1;
+    }
   }
+  return 0;
 }
 
 /** Forget about all clients that haven't connected since <b>cutoff</b>. */

src/feature/stats/geoip_stats.h

@@ -97,6 +97,7 @@ typedef struct clientmap_entry_t {
 
 int should_record_bridge_info(const or_options_t *options);
 
+void geoip_note_client_attempt(const tor_addr_t *addr, time_t now);
 void geoip_note_client_seen(geoip_client_action_t action,
                             const tor_addr_t *addr, const char *transport_name,
                             time_t now);