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
This commit was merged in pull request #16.
This commit is contained in:
commit
2c1d444348
2 files changed
+211
-7
No files matched your search
@@ -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
|
||||
t = self._thread
|
||||
self._thread = None
|
||||
if httpd is not None or t is not None:
|
||||
# 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:
|
||||
# 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():
|
||||
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):
|
||||
|
||||
+133
-1
@@ -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 ─────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user