[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Limiting Unused Connections



Hello,

I've been looking at bug 469, where servers are screwed after being
deprived of file descriptors in simple DoS attacks:

https://bugs.torproject.org/flyspray/index.php?id=469&do=details

I've also had a chance to work on a small patch to limit unused
connections, which is copied below. It tries to work as Roger
suggested in his flyspray posting (which is a few years old now,
but still valid in concept, I think), and it also extends to cover
direct connections to the DirPort.

Basically, new connections to ORPort and DirPort are marked "unused"
until either something happens on the connection or the connection
is closed/freed. If there are too many unused conns from a single IP,
future connections are dropped quickly after accept().
The patch is currently unfinished. It's missing the configuration
directive, and the log levels/messages need to be reconsidered.

Is this work moving in the right direction, or should I consider
writing a proposal instead?

-- 
Christopher Davis
Mangrin Remailer Admin
PGP: 0x0F8DA163

Index: src/or/connection_or.c
===================================================================
--- src/or/connection_or.c	(revision 19229)
+++ src/or/connection_or.c	(working copy)
@@ -1160,6 +1160,8 @@
     or_handshake_state_free(conn->handshake_state);
     conn->handshake_state = NULL;
   }
+  if (tor_tls_is_server(conn->tls) && TO_CONN(conn)->should_mark_used)
+    connection_mark_used(TO_CONN(conn));
   connection_start_reading(TO_CONN(conn));
   circuit_n_conn_done(conn, 1); /* send the pending creates, if any. */
 
Index: src/or/or.h
===================================================================
--- src/or/or.h	(revision 19229)
+++ src/or/or.h	(working copy)
@@ -927,6 +927,11 @@
   /** True iff we've called connection_close_immediate() on this linked
    * connection. */
   unsigned int linked_conn_is_closed:1;
+  /** True if this connection is currently considered unused and should be
+   * marked as used at some point. */
+  unsigned int should_mark_used:1;
+  /** True if this connection was marked used by connection_mark_used() */
+  unsigned int marked_used:1;
 
   int s; /**< Our socket; -1 if this connection is closed, or has no
           * socket. */
@@ -3006,6 +3011,8 @@
 void connection_dump_buffer_mem_stats(int severity);
 void remove_file_if_very_old(const char *fname, time_t now);
 
+void connection_mark_used(connection_t *conn);
+
 /********************************* connection_edge.c *************************/
 
 #define connection_mark_unattached_ap(conn, endreason) \
Index: src/or/connection.c
===================================================================
--- src/or/connection.c	(revision 19229)
+++ src/or/connection.c	(working copy)
@@ -11,6 +11,7 @@
  **/
 
 #include "or.h"
+#include "ht.h"
 
 static connection_t *connection_create_listener(
                                struct sockaddr *listensockaddr,
@@ -39,9 +40,124 @@
  * Used to detect IP address changes. */
 static smartlist_t *outgoing_addrs = NULL;
 
+/** Map remote addreses to count of unused connections. */
+typedef struct unused_conns {
+  HT_ENTRY(unused_conns) node;
+  tor_addr_t addr;
+  uint16_t count;
+} unused_conns_t;
+
+static INLINE int
+unused_conns_eq(const unused_conns_t *uca, const unused_conns_t *ucb)
+{
+  return tor_addr_eq(&uca->addr, &ucb->addr);
+}
+
+static INLINE unsigned int
+unused_conns_hash(const unused_conns_t *uc)
+{
+  return tor_addr_hash(&uc->addr);
+}
+
+static HT_HEAD(unused_conns_map, unused_conns)
+              unused_conns_map_root = HT_INITIALIZER();
+
+HT_PROTOTYPE(unused_conns_map, unused_conns, node, unused_conns_hash,
+              unused_conns_eq)
+HT_GENERATE(unused_conns_map, unused_conns, node, unused_conns_hash,
+              unused_conns_eq, 0.6, malloc, realloc, free)
+
 /**************************************************************/
 
 /**
+ * Find <b>addr</b> in the unused conns hash table.
+ */
+static unused_conns_t *
+unused_conns_find(const tor_addr_t *addr)
+{
+  unused_conns_t search, *uc;
+
+  memset(&search, 0, sizeof search);
+  tor_addr_copy(&search.addr, addr);
+  uc = HT_FIND(unused_conns_map, &unused_conns_map_root, &search);
+
+  return uc;
+}
+
+/**
+ * Check that the count of unused conns from <b>addr</b> is within limits,
+ * and then increment the count. If no entry for addr exists within the
+ * hash table, add it now.
+ */
+static int
+unused_conns_check_and_increment(const tor_addr_t *addr)
+{
+//XXX make this a configuration directive?
+#define MAX_UNUSED_CONNECTIONS 10
+
+  unused_conns_t *uc;
+
+  uc = unused_conns_find(addr);
+
+  if (uc) {
+    if (uc->count >= MAX_UNUSED_CONNECTIONS)
+      return 0;
+    uc->count++;
+  } else {
+    uc = tor_malloc_zero(sizeof *uc);
+    tor_addr_copy(&uc->addr, addr);
+    uc->count = 1;
+    HT_INSERT(unused_conns_map, &unused_conns_map_root, uc);
+  }
+
+  log_warn(LD_NET, "increment unused conns %s, %u", fmt_addr(&uc->addr),
+            (unsigned)uc->count);
+
+  return 1;
+}
+
+/**
+ * Decrement the count of unused conns for <b>addr</b>. If there're no
+ * more conns left, remove it from the hash table.
+ */
+static void
+unused_conns_decrement(const tor_addr_t *addr)
+{
+  unused_conns_t *uc;
+
+  uc = unused_conns_find(addr);
+
+  if (uc) {
+    tor_assert(uc->count > 0);
+    log_warn(LD_NET, "decrement unused conns %s, %u", fmt_addr(&uc->addr),
+              (unsigned)uc->count-1);
+    if (--uc->count == 0) {
+      HT_REMOVE(unused_conns_map, &unused_conns_map_root, uc);
+      tor_free(uc);
+    }
+  } else {
+    log_err(LD_BUG, "Can't find address in map to decrement unused conns.");
+    tor_fragile_assert();
+  }
+}
+
+/**
+ * Mark <b>conn</b> used, updating the entry in the hash table.
+ */
+void
+connection_mark_used(connection_t *conn)
+{
+  if (conn->should_mark_used && !conn->marked_used) {
+    unused_conns_decrement(&conn->addr);
+    conn->marked_used = 1;
+    conn->should_mark_used = 0;
+  } else {
+    log_err(LD_BUG, "Connection marked used already!");
+    tor_fragile_assert();
+  }
+}
+
+/**
  * Return the human-readable name for the connection type <b>type</b>
  */
 const char *
@@ -433,6 +549,9 @@
     conn->s = -1;
   }
 
+  if (conn->should_mark_used && !conn->marked_used)
+    connection_mark_used(conn);
+
   if (conn->type == CONN_TYPE_OR &&
       !tor_digest_is_zero(TO_OR_CONN(conn)->identity_digest)) {
     log_warn(LD_BUG, "called on OR conn with non-zeroed identity_digest");
@@ -1094,6 +1213,8 @@
   if (conn->socket_family == AF_INET || conn->socket_family == AF_INET6) {
     tor_addr_t addr;
     uint16_t port;
+    int should_mark_used = 0;
+
     if (check_sockaddr(remote, remotelen, LOG_INFO)<0) {
       log_info(LD_NET,
                "accept() returned a strange address; trying getsockname().");
@@ -1130,8 +1251,7 @@
         tor_close_socket(news);
         return 0;
       }
-    }
-    if (new_type == CONN_TYPE_DIR) {
+    } else if (new_type == CONN_TYPE_DIR) {
       /* check dirpolicy to see if we should accept it */
       if (dir_policy_permits_address(&addr) == 0) {
         log_notice(LD_DIRSERV,"Denying dir connection from address %s.",
@@ -1141,8 +1261,21 @@
       }
     }
 
+    /* check that there aren't too many unused connections */
+    if (new_type == CONN_TYPE_OR || new_type == CONN_TYPE_DIR) {
+      if (unused_conns_check_and_increment(&addr) == 0) {
+        //XXX this message would spam the log file during DoS attack
+        log_warn(LD_NET, "too many unused connections from %s.",
+                  fmt_addr(&addr));
+        tor_close_socket(news);
+        return 0;
+      }
+      should_mark_used = 1;
+    }
+
     newconn = connection_new(new_type, conn->socket_family);
     newconn->s = news;
+    newconn->should_mark_used = should_mark_used;
 
     /* remember the remote address */
     tor_addr_copy(&newconn->addr, &addr);
Index: src/or/directory.c
===================================================================
--- src/or/directory.c	(revision 19229)
+++ src/or/directory.c	(working copy)
@@ -3205,6 +3205,11 @@
     r = -1;
   }
 
+  /* make sure to only mark direct dir conns used; tunneled conns are marked
+   * in connection_or.c after handshaking */
+  if (TO_CONN(conn)->should_mark_used)
+    connection_mark_used(TO_CONN(conn));
+
   tor_free(headers); tor_free(body);
   return r;
 }