Timeout idle sessions (#6332)

CHANGELOG.md

@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [5.0.0-rc1]
+
+[5.0.0-rc1]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-rc1
+
+### Added
+
+- The `cchost` configuration file now includes an `idle_connection_timeout` option. This controls how long the node will keep idle connections (for user TLS sessions) before automatically closing them. This may be set to `null` to restore the previous behaviour, where idle connections are never closed. By default connections will be closed after 60s of idle time.
+
 ## [5.0.0-rc0]
 
 [5.0.0-rc0]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-rc0

doc/host_config_schema/cchost_config.json

@@ -662,6 +662,11 @@
       "default": "2000ms",
       "description": "Maximum duration after which unestablished client connections will be marked as timed out and either re-established or discarded"
     },
+    "idle_connection_timeout": {
+      "type": ["string", "null"],
+      "default": "60s",
+      "description": "Timeout for idle connections. Null is a valid option, and means idle connections are retained indefinitely"
+    },
     "worker_threads": {
       "type": "integer",
       "default": 0,

src/host/configuration.h

@@ -74,6 +74,8 @@ namespace host
     ccf::ds::TimeString slow_io_logging_threshold = {"10ms"};
     std::optional<std::string> node_client_interface = std::nullopt;
     ccf::ds::TimeString client_connection_timeout = {"2000ms"};
+    std::optional<ccf::ds::TimeString> idle_connection_timeout =
+      ccf::ds::TimeString("60s");
     std::optional<std::string> node_data_json_file = std::nullopt;
     std::optional<std::string> service_data_json_file = std::nullopt;
     bool ignore_first_sigterm = false;
@@ -234,6 +236,7 @@ namespace host
     slow_io_logging_threshold,
     node_client_interface,
     client_connection_timeout,
+    idle_connection_timeout,
     node_data_json_file,
     service_data_json_file,
     ignore_first_sigterm,

src/host/main.cpp

@@ -416,16 +416,24 @@ int main(int argc, char** argv)
     asynchost::ConnIDGenerator idGen;
 
     asynchost::RPCConnections<asynchost::TCP> rpc(
-      writer_factory, idGen, config.client_connection_timeout);
-    rpc.register_message_handlers(bp.get_dispatcher());
+      1s, // Tick once-per-second to track idle connections,
+      writer_factory,
+      idGen,
+      config.client_connection_timeout,
+      config.idle_connection_timeout);
+    rpc->behaviour.register_message_handlers(bp.get_dispatcher());
 
     // This is a temporary solution to keep UDP RPC handlers in the same
     // way as the TCP ones without having to parametrize per connection,
     // which is not yet possible, due to UDP and TCP not being derived
     // from the same abstract class.
     asynchost::RPCConnections<asynchost::UDP> rpc_udp(
-      writer_factory, idGen, config.client_connection_timeout);
-    rpc_udp.register_udp_message_handlers(bp.get_dispatcher());
+      1s,
+      writer_factory,
+      idGen,
+      config.client_connection_timeout,
+      config.idle_connection_timeout);
+    rpc_udp->behaviour.register_udp_message_handlers(bp.get_dispatcher());
 
     ResolvedAddresses resolved_rpc_addresses;
     for (auto& [name, interface] : config.network.rpc_interfaces)
@@ -439,11 +447,11 @@ int main(int argc, char** argv)
         rpc_port);
       if (interface.protocol == "udp")
       {
-        rpc_udp.listen(0, rpc_host, rpc_port, name);
+        rpc_udp->behaviour.listen(0, rpc_host, rpc_port, name);
       }
       else
       {
-        rpc.listen(0, rpc_host, rpc_port, name);
+        rpc->behaviour.listen(0, rpc_host, rpc_port, name);
       }
       LOG_INFO_FMT(
         "Registered RPC interface {}, on {} {}:{}",

src/host/rpc_connections.h

@@ -44,8 +44,8 @@ namespace
 namespace asynchost
 {
   /**
-   * Generates next ID, passed as an argument to RPCConnections so that we can
-   * have multiple and avoid reusing the same ConnID across each.
+   * Generates next ID, passed as an argument to RPCConnectionsImpl so that we
+   * can have multiple and avoid reusing the same ConnID across each.
    */
   class ConnIDGenerator
   {
@@ -88,21 +88,23 @@ namespace asynchost
   };
 
   template <class ConnType>
-  class RPCConnections
+  class RPCConnectionsImpl
   {
     using ConnID = ConnIDGenerator::ConnID;
 
     class RPCClientBehaviour : public SocketBehaviour<ConnType>
     {
     public:
-      RPCConnections& parent;
+      RPCConnectionsImpl& parent;
       ConnID id;
 
-      RPCClientBehaviour(RPCConnections& parent, ConnID id) :
+      RPCClientBehaviour(RPCConnectionsImpl& parent, ConnID id) :
         SocketBehaviour<ConnType>("RPC Client", getConnTypeName<ConnType>()),
         parent(parent),
         id(id)
-      {}
+      {
+        parent.mark_active(id);
+      }
 
       void on_resolve_failed() override
       {
@@ -120,6 +122,8 @@ namespace asynchost
       {
         LOG_DEBUG_FMT("rpc read {}: {}", id, len);
 
+        parent.mark_active(id);
+
         RINGBUFFER_WRITE_MESSAGE(
           ::tls::tls_inbound,
           parent.to_enclave,
@@ -147,10 +151,10 @@ namespace asynchost
     class RPCServerBehaviour : public SocketBehaviour<ConnType>
     {
     public:
-      RPCConnections& parent;
+      RPCConnectionsImpl& parent;
       ConnID id;
 
-      RPCServerBehaviour(RPCConnections& parent, ConnID id) :
+      RPCServerBehaviour(RPCConnectionsImpl& parent, ConnID id) :
         SocketBehaviour<ConnType>("RPC Client", getConnTypeName<ConnType>()),
         parent(parent),
         id(id)
@@ -226,18 +230,27 @@ namespace asynchost
     std::unordered_map<ConnID, ConnType> sockets;
     ConnIDGenerator& idGen;
 
+    // Measured in seconds
+    std::unordered_map<ConnID, size_t> idle_times;
+
     std::optional<std::chrono::milliseconds> client_connection_timeout =
       std::nullopt;
+
+    std::optional<std::chrono::seconds> idle_connection_timeout = std::nullopt;
+
     ringbuffer::WriterPtr to_enclave;
 
   public:
-    RPCConnections(
+    RPCConnectionsImpl(
       ringbuffer::AbstractWriterFactory& writer_factory,
       ConnIDGenerator& idGen,
       std::optional<std::chrono::milliseconds> client_connection_timeout_ =
+        std::nullopt,
+      std::optional<std::chrono::seconds> idle_connection_timeout_ =
         std::nullopt) :
       idGen(idGen),
       client_connection_timeout(client_connection_timeout_),
+      idle_connection_timeout(idle_connection_timeout_),
       to_enclave(writer_factory.create_writer_to_inside())
     {}
 
@@ -318,7 +331,11 @@ namespace asynchost
       }
 
       if (s->second.is_null())
+      {
         return false;
+      }
+
+      mark_active(id);
 
       return s->second->write(len, data, addr);
     }
@@ -341,6 +358,8 @@ namespace asynchost
         return false;
       }
 
+      idle_times.erase(id);
+
       return true;
     }
 
@@ -410,6 +429,38 @@ namespace asynchost
         });
     }
 
+    void mark_active(ConnID id)
+    {
+      idle_times[id] = 0;
+    }
+
+    void on_timer()
+    {
+      if (!idle_connection_timeout.has_value())
+      {
+        return;
+      }
+
+      const size_t max_idle_time = idle_connection_timeout->count();
+
+      for (auto& [id, idle_time] : idle_times)
+      {
+        if (idle_time > max_idle_time)
+        {
+          LOG_INFO_FMT(
+            "Closing socket {} after {}s idle (max = {}s)",
+            id,
+            idle_time,
+            max_idle_time);
+          stop(id);
+        }
+        else
+        {
+          idle_time += 1;
+        }
+      }
+    }
+
   private:
     ConnID get_next_id()
     {
@@ -441,4 +492,7 @@ namespace asynchost
       return listen_name.value();
     }
   };
+
+  template <class ConnType>
+  using RPCConnections = proxy_ptr<Timer<RPCConnectionsImpl<ConnType>>>;
 }

src/host/tcp.h

@@ -400,8 +400,16 @@ namespace asynchost
 
         if (connection_timeout.has_value())
         {
-          auto const t = connection_timeout.value();
-          setsockopt(sock, IPPROTO_TCP, TCP_USER_TIMEOUT, &t, sizeof(t));
+          const unsigned int ms = connection_timeout->count();
+          const auto ret =
+            setsockopt(sock, IPPROTO_TCP, TCP_USER_TIMEOUT, &ms, sizeof(ms));
+          if (ret != 0)
+          {
+            LOG_FAIL_FMT(
+              "Failed to set socket option (TCP_USER_TIMEOUT): {}",
+              strerror(errno));
+            return false;
+          }
         }
 
         if ((rc = uv_tcp_open(&uv_handle, sock)) < 0)

tests/config.jinja

@@ -92,7 +92,8 @@
   },
   "tick_interval": "{{ tick_ms }}ms",
   "slow_io_logging_threshold": "10ms",
-  "client_connection_timeout": "2s",
+  "client_connection_timeout": "2s"{% if idle_connection_timeout_s %},
+  "idle_connection_timeout": "{{ idle_connection_timeout_s }}s"{% endif %},
   "node_client_interface": {{ node_client_interface|tojson }},
   "worker_threads": {{ worker_threads }},
   "memory": {