http: Configurable inactivity timeout

We used 60 seconds timeout for disconnecting inactive clients. This is
too long to protect from bad clients leaving open connections, and too
short for applications that need long timeout[1].

Change the default timeout to 15 seconds, configurable via
daemon:auth_timeout. This timeout is used for new unauthorized
connections. If a connection does not authorize within this timeout, it
is disconnected.

When a connection is authorized during the first request, the connection
timeout is increased to ticket.inactivity_timeout. The default value is
60 seconds, configurable via daemon:inactivity_timeout.

Application with special needs can request a larger timeout when
creating an image transfer. Engine need to include the transfer
inactivity timeout in the ticket.

[1] https://bugzilla.redhat.com/2032324

Fixes oVirt#14.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>

data/README

@@ -47,6 +47,17 @@
 # The default value:
 #   max_connections = 8
 
+# Number of seconds before unauthorized connections are
+# disconnected.
+# The default value:
+#   auth_timeout = 15
+
+# Number of seconds before inactive connections are disconnected.
+# Clients with special needs can request a larger timeout when
+# creating an image transfer.
+# The default value:
+#   inactivity_timeout = 60
+
 [tls]
 # Enable TLS. Note that without TLS transfer tickets and image data are
 # transferred in clear text. If TLS is enabled, paths to related files

examples/nbd.json

@@ -4,6 +4,7 @@
     "size": 6442450944,
     "url": "nbd:unix:/tmp/nbd.sock",
     "timeout": 3000,
+    "inactivity_timeout": 300,
     "sparse": true,
     "ops": ["read", "write"]
 }

ovirt_imageio/_internal/auth.py

@@ -22,7 +22,7 @@
 
 class Ticket:
 
-    def __init__(self, ticket_dict):
+    def __init__(self, ticket_dict, cfg):
         if not isinstance(ticket_dict, dict):
             raise errors.InvalidTicket(
                 "Invalid ticket: %r, expecting a dict" % ticket_dict)
@@ -32,6 +32,10 @@ def __init__(self, ticket_dict):
         self._ops = _required(ticket_dict, "ops", list)
 
         self._timeout = _required(ticket_dict, "timeout", int)
+        self._inactivity_timeout = _optional(
+            ticket_dict, "inactivity_timeout", int,
+            default=cfg.daemon.inactivity_timeout)
+
         now = int(util.monotonic_time())
         self._expires = now + self._timeout
         self._access_time = now
@@ -124,6 +128,14 @@ def idle_time(self):
             return 0
         return int(util.monotonic_time()) - self._access_time
 
+    @property
+    def inactivity_timeout(self):
+        """
+        Return the number of seconds to wait before disconnecting inactive
+        client.
+        """
+        return self._inactivity_timeout
+
     @property
     def canceled(self):
         with self._lock:
@@ -249,6 +261,7 @@ def info(self):
             "connections": len(self._connections),
             "expires": self._expires,
             "idle_time": self.idle_time,
+            "inactivity_timeout": self._inactivity_timeout,
             "ops": list(self._ops),
             "size": self._size,
             "sparse": self._sparse,
@@ -336,6 +349,7 @@ def __repr__(self):
                 "canceled={self._canceled} "
                 "connections={connections} "
                 "expires={self.expires!r} "
+                "inactivity_timeout={self.inactivity_timeout} "
                 "filename={self.filename!r} "
                 "idle_time={self.idle_time} "
                 "ops={self.ops!r} "
@@ -388,7 +402,7 @@ def add(self, ticket_dict):
 
         Raises errors.InvalidTicket if ticket dict is invalid.
         """
-        ticket = Ticket(ticket_dict)
+        ticket = Ticket(ticket_dict, self._config)
         self._tickets[ticket.uuid] = ticket
 
     def remove(self, ticket_id):

ovirt_imageio/_internal/backends/__init__.py

@@ -98,6 +98,9 @@ def get(req, ticket, config):
             ctx.close()
             raise
 
+        # Authorized connections get a longer timeout from the ticket.
+        req.set_connection_timeout(ticket.inactivity_timeout)
+
         # Register a closer removing the context when the connection is closed.
         req.context[ticket.uuid] = Closer(
             partial(ticket.remove_context, req.connection_id))

ovirt_imageio/_internal/config.py

@@ -20,6 +20,15 @@ class daemon:
     # decrease throughput.
     max_connections = 8
 
+    # Number of seconds before unauthorized connections are
+    # disconnected.
+    auth_timeout = 15
+
+    # Number of seconds before inactive connections are disconnected.
+    # Clients with special needs can request a larger timeout when
+    # creating an image transfer.
+    inactivity_timeout = 60
+
     # Daemon run directory. Runtime stuff like socket or profile information
     # will be stored in this directory.
     # This is configurable only for development purposes and is not expected to

ovirt_imageio/_internal/http.py

@@ -169,10 +169,11 @@ class Connection(http.server.BaseHTTPRequestHandler):
     # to support long URIs, so we use small value.
     max_request_line = 4096
 
-    # Number of second to wait for recv() or send(). When the timeout expires
-    # we close the connection. This is important when working with clients that
-    # keep the connection open after upload or download (e.g browsers).
-    timeout = 60
+    # Number of second to wait for recv() or send() on unauthorized
+    # connections.  When the timeout expires we close the connection.
+    # Authorized connections get a larger timeout using the ticket
+    # inactivity timeout.
+    timeout = 15
 
     # For generating connection ids. Start from 1 to match the connection
     # thread name.
@@ -271,6 +272,10 @@ def version_string(self):
         """
         return "imageio/" + version.string
 
+    def set_timeout(self, timeout):
+        log.debug("Setting connection timeout to %s seconds", timeout)
+        self.connection.settimeout(timeout)
+
 
 class Request:
 
@@ -453,6 +458,9 @@ def connection_lost(self):
         """
         return self._con.connection_error() in _DISCONNECTED
 
+    def set_connection_timeout(self, timeout):
+        self._con.set_timeout(timeout)
+
 
 class Response:
 

test/admin_test.py

@@ -89,6 +89,7 @@ def test_get_ticket(srv, fake_time):
             "size": ticket["size"],
             "sparse": ticket["sparse"],
             "dirty": ticket["dirty"],
+            "inactivity_timeout": ticket["inactivity_timeout"],
             "timeout": ticket["timeout"],
             "url": ticket["url"],
             "uuid": ticket["uuid"],