Merge pull request 'fix(http): don't let a half-open TLS connection wedge the accept loop' (#16) from fix/iris-tls-handshake-wedge into master

Reviewed-on: #16

gateway-plugin/http_server.py

@@ -45,6 +45,7 @@ import contextlib
 import json
 import logging
 import queue
+import socket
 import ssl
 import threading
 import time
@@ -220,7 +221,14 @@ class HttpServer:
             if self._adapter.http_cert and self._adapter.http_key:
                 ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
                 ctx.load_cert_chain(self._adapter.http_cert, self._adapter.http_key)
-                httpd.socket = ctx.wrap_socket(httpd.socket, server_side=True)
+                # The handshake runs in the per-connection thread with a
+                # hard timeout (see _ThreadingHTTPD.process_request).
+                # Wrapping the *listening* socket here instead would make
+                # serve_forever's accept() block inside do_handshake() on
+                # a half-open connection (TCP established, client gone
+                # mid-handshake), wedging ALL new device connections until
+                # the gateway is restarted.
+                httpd.set_tls(ctx)
         except Exception as e:
             logger.warning("iris: HTTP server disabled (bind %s:%s failed: %s)", host, port, e)
             self._release_lock()
@@ -245,17 +253,35 @@ class HttpServer:
                 s.q.put_nowait(_STOP)
         httpd = self._httpd
         self._httpd = None
-        if httpd is not None:
-            # shutdown() must be called from a thread other than the one
-            # running serve_forever(); we are on the asyncio loop thread.
-            with contextlib.suppress(Exception):
-                httpd.shutdown()
-            with contextlib.suppress(Exception):
-                httpd.server_close()
         t = self._thread
         self._thread = None
-        if t is not None and t is not threading.current_thread():
+        if httpd is not None or t is not None:
-            t.join(timeout=5.0)
+            # shutdown() blocks until the serve_forever loop exits and
+            # server_close() may join handler threads — both must run on a
+            # worker thread (never the asyncio loop thread) with a hard
+            # timeout, or a wedged server would freeze the whole gateway.
+            # The threads are daemons: if the bounded wait expires they die
+            # with the process and there is nothing left to do.
+            loop = asyncio.get_running_loop()
+
+            def _stop_httpd() -> None:
+                if httpd is not None:
+                    with contextlib.suppress(Exception):
+                        httpd.shutdown()
+                    with contextlib.suppress(Exception):
+                        httpd.server_close()
+                if t is not None and t is not threading.current_thread():
+                    t.join(timeout=5.0)
+
+            try:
+                await asyncio.wait_for(
+                    loop.run_in_executor(None, _stop_httpd),
+                    timeout=10.0,
+                )
+            except Exception:
+                logger.warning(
+                    "iris: HTTP server teardown did not finish in time; abandoning daemon threads"
+                )
         self._release_lock()
 
     def _release_lock(self) -> None:
@@ -783,14 +809,60 @@ class HttpServer:
 
 class _ThreadingHTTPD(ThreadingHTTPServer):
     """One thread per connection (fine at single-user scale); daemon
-    threads so a stuck handler can't block process exit."""
+    threads so a stuck handler can't block process exit.
+
+    With TLS enabled (``set_tls``) the handshake runs in the
+    per-connection thread under a hard timeout — never in the
+    ``serve_forever`` accept loop. ``ssl.SSLSocket.accept()`` would
+    otherwise block that loop inside ``do_handshake()`` on a half-open
+    connection (TCP established but the client vanished mid-handshake,
+    e.g. a phone losing its network/VPN), and the gateway would stop
+    accepting any new device connections until it is restarted.
+    """
 
     daemon_threads = True
     allow_reuse_address = True
 
+    # A client that completes TCP but never finishes the TLS handshake
+    # must not hold the connection open indefinitely.
+    HANDSHAKE_TIMEOUT_S = 10.0
+
     def __init__(self, addr: tuple[str, int], http_server: HttpServer):
         super().__init__(addr, _Handler)
         self.http_server = http_server
+        self._tls_ctx: ssl.SSLContext | None = None
+
+    def set_tls(self, ctx: ssl.SSLContext) -> None:
+        self._tls_ctx = ctx
+
+    def process_request(  # noqa: A003  # type: ignore[override]
+        self, request: socket.socket, client_address: Any
+    ) -> None:
+        """Spawn the handler thread; with TLS, the handshake happens in
+        that thread first, under ``HANDSHAKE_TIMEOUT_S`` (see class
+        docstring). A failed/timed-out handshake just closes the socket —
+        the accept loop is never blocked by it."""
+        if self._tls_ctx is None:
+            super().process_request(request, client_address)
+            return
+        tls_ctx = self._tls_ctx
+
+        def _handshake_then_handle() -> None:
+            try:
+                request.settimeout(self.HANDSHAKE_TIMEOUT_S)
+                # wrap_socket() performs the handshake (default
+                # do_handshake_on_connect=True); restore blocking mode for
+                # the request handler afterwards.
+                tls_sock = tls_ctx.wrap_socket(request, server_side=True)
+                tls_sock.settimeout(None)
+            except OSError as e:  # ssl.SSLError, timeout, reset, ...
+                with contextlib.suppress(OSError):
+                    request.close()
+                logger.debug("iris http: TLS handshake failed (%s): %s", client_address, e)
+                return
+            super(_ThreadingHTTPD, self).process_request(tls_sock, client_address)
+
+        threading.Thread(target=_handshake_then_handle, name="iris-tls", daemon=True).start()
 
 
 class _Handler(BaseHTTPRequestHandler):

tests/test_android_http.py

@@ -29,11 +29,14 @@ import asyncio
 import base64
 import contextlib
 import importlib.util
+import ipaddress
 import json
 import os
+import socket
+import ssl
 import sys
 import time
-from http.client import HTTPConnection
+from http.client import HTTPConnection, HTTPSConnection
 from pathlib import Path
 from types import SimpleNamespace
 from unittest.mock import AsyncMock
@@ -835,6 +838,135 @@ async def test_media_pull_denied_path_404(gw):
     assert body["payload"]["code"] == "not_found"
 
 
+# ── TLS: a half-open connection must not wedge the accept loop ─────────────
+#
+# Regression (ARIA journal 2026-09-11 / 2026-09-23): the listening socket
+# used to be wrapped in a server-side ssl.SSLSocket, so serve_forever's
+# accept() ran the TLS handshake inline. A client that completed TCP but
+# vanished mid-handshake (a phone losing its network/VPN while traveling)
+# blocked do_handshake() forever: the gateway stopped accepting ANY new
+# device connections (the app could not reconnect), and on the next restart
+# httpd.shutdown() froze the whole event loop until the shutdown watchdog
+# killed the process.
+
+
+def _make_self_signed_cert(tmp_path: Path) -> tuple[Path, Path] | None:
+    """Self-signed cert + key for the TLS tests; None when
+    ``cryptography`` is unavailable (the tests then skip)."""
+    try:
+        from cryptography import x509
+        from cryptography.hazmat.primitives import hashes, serialization
+        from cryptography.hazmat.primitives.asymmetric import rsa
+        from cryptography.x509.oid import NameOID
+    except ImportError:
+        return None
+    import datetime
+
+    key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
+    name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "iris-test")])
+    now = datetime.datetime.now(datetime.timezone.utc)
+    cert = (
+        x509.CertificateBuilder()
+        .subject_name(name)
+        .issuer_name(name)
+        .public_key(key.public_key())
+        .serial_number(x509.random_serial_number())
+        .not_valid_before(now - datetime.timedelta(days=1))
+        .not_valid_after(now + datetime.timedelta(days=1))
+        .add_extension(
+            x509.SubjectAlternativeName(
+                [
+                    x509.DNSName("localhost"),
+                    x509.IPAddress(ipaddress.ip_address("127.0.0.1")),
+                ]
+            ),
+            critical=False,
+        )
+        .sign(key, hashes.SHA256())
+    )
+    cert_path = tmp_path / "iris-test.crt"
+    key_path = tmp_path / "iris-test.key"
+    cert_path.write_bytes(cert.public_bytes(serialization.Encoding.PEM))
+    key_path.write_bytes(
+        key.private_bytes(
+            serialization.Encoding.PEM,
+            serialization.PrivateFormat.TraditionalOpenSSL,
+            serialization.NoEncryption(),
+        )
+    )
+    return cert_path, key_path
+
+
+@pytest_asyncio.fixture
+async def gw_tls(adapter, tmp_path, monkeypatch):
+    """Connected adapter with the HTTP leg TLS-enabled; the handshake
+    timeout is shortened so the half-open connection cleans itself up
+    quickly."""
+    paths = _make_self_signed_cert(tmp_path)
+    if paths is None:
+        pytest.skip("cryptography not available; TLS wedge test skipped")
+    cert_path, key_path = paths
+    plugin = _load_plugin()
+    monkeypatch.setattr(
+        plugin.http_server._ThreadingHTTPD, "HANDSHAKE_TIMEOUT_S", 0.5, raising=False
+    )
+    adapter.http_cert = str(cert_path)
+    adapter.http_key = str(key_path)
+    await adapter.connect()
+    try:
+        yield adapter
+    finally:
+        await adapter.disconnect()
+
+
+def _tls_health(port: int) -> int:
+    """GET /v1/health over a fresh TLS connection; returns the status."""
+    ctx = ssl.create_default_context()
+    ctx.check_hostname = False
+    ctx.verify_mode = ssl.CERT_NONE
+    conn = HTTPSConnection("127.0.0.1", port, timeout=5.0, context=ctx)
+    conn.request("GET", "/v1/health")
+    resp = conn.getresponse()
+    status = resp.status
+    resp.read()
+    conn.close()
+    return status
+
+
+@pytest.mark.asyncio
+async def test_half_open_tls_connection_does_not_wedge_accept_loop(gw_tls):
+    """A client that completes TCP but never finishes the TLS handshake
+    must not stop the server from accepting new connections (see section
+    comment for the incident)."""
+    port = http_port(gw_tls)
+
+    # 1) Half-open connection: TCP established, then silence — the
+    #    phone-loses-its-VPN scenario (the ClientHello never arrives).
+    wedge = socket.create_connection(("127.0.0.1", port), timeout=5.0)
+    try:
+        # 2) While the half-open connection sits un-handshaked, a fresh,
+        #    well-formed TLS connection must still be accepted promptly.
+        deadline = time.monotonic() + 10.0
+        status = None
+        while time.monotonic() < deadline:
+            try:
+                status = await asyncio.to_thread(_tls_health, port)
+                break
+            except OSError:
+                await asyncio.sleep(0.2)
+        assert status == 200, f"health over TLS failed (status={status})"
+
+        # 3) Teardown must stay bounded with the half-open connection still
+        #    open: stop() used to block the event loop on httpd.shutdown()
+        #    until the shutdown watchdog killed the process.
+        t0 = time.monotonic()
+        await gw_tls._http_server.stop()
+        assert time.monotonic() - t0 < 15.0
+    finally:
+        with contextlib.suppress(OSError):
+            wedge.close()
+
+
 # ── Helpers ─────────────────────────────────────────────────────────────────