HTTP/2 connections have no idle timeout (Slowloris / resource exhaustion)

[SSE4]

Add a description

The HTTP/2 connection loop waits for socket readiness with no deadline, so
an idle or slowly-dripping HTTP/2 connection is never closed by the server.
HTTP/1.1 closes such a connection after keepalive_timeout; HTTP/2 used to,
but the enforcement was dropped in the same refactor that disabled body
streaming. A client can therefore hold connections open indefinitely at
near-zero cost and exhaust the server's max_connections slots (default
32768), after which new connections are refused for every client — a
classic Slowloris.

Observed on 3.2-rc
(1838cdcec);
line links below are permalinks pinned to that commit. Still present on
current develop.

Scope (what this is and isn't)

• Not cross-connection stream starvation. Each connection runs in its own
  coroutine; a parked coroutine holds no OS thread and no CPU, so a stalled
  connection does not block other connections' processing. Per-stream HTTP/2
  flow control also prevents one request from starving other streams
  multiplexed on the same connection.
• It is unbounded accumulation of idle connections: file descriptors and
  coroutine stacks held until max_connections, then connection refusal
  service-wide. The attacker spends one SYN + the HTTP/2 preface (or an
  occasional PING) per held connection.

Reproduction

Open an HTTP/2 connection, send the client preface + SETTINGS, then send
nothing further (or one PING frame per minute). The connection stays open
forever. The equivalent HTTP/1.1 connection (open socket, send nothing) is
closed after keepalive_timeout with Closing idle connection on timeout.

A quick way to see the asymmetry:

# HTTP/1.1: server closes the idle socket after keepalive_timeout
$ printf 'GET / HTTP/1.1\r\nHost: x\r\n' | timeout 700 nc localhost 8080   # closed by server

# HTTP/2: hold the preface open and idle -> connection lives indefinitely
$ python3 - <<'PY'
import socket, time
import h2.connection
s = socket.create_connection(('localhost', 8080))
c = h2.connection.H2Connection(); c.initiate_connection()
s.sendall(c.data_to_send())
print('h2 connection open; idling...')
while True:            # never closed by the server
    time.sleep(60)
    c.ping(b'01234567'); s.sendall(c.data_to_send())
PY

Root cause

Http2Connection::ListenForRequests()
blocks in wait_any.Wait() — no deadline:

engine::WaitAnyContext wait_any{};
wait_any.Append(kSocketId, GetSocket().GetReadableBase());

while (!engine::current_task::ShouldCancel()) {
    StartAllRequestTasks(wait_any);
    const auto ready_id = wait_any.Wait();   // <-- no deadline; idle == forever
    ...

keepalive_timeout is applied inside
SocketBufferedReader::TryRead,
but that runs only after Wait() reports the socket readable. A connection
that never becomes readable never enters TryRead, so the deadline never
applies. The only remaining keepalive_timeout use in the HTTP/2 connection
is the h2c 101 upgrade write, not the request-wait.

By contrast, HTTP/1.1 goes through
ConnectionBase::TryParseRequests → TryRead
on every loop turn, so its blocking read carries the keepalive deadline.

It is a regression

Before the connection refactor
(f75379023,
separate HttpReader and ConnectionBase), the HTTP/2 loop waited on the
socket with the keepalive deadline explicitly:

// f75379023^ : core/src/server/net/http2_connection.cpp
auto deadline = engine::Deadline::FromDuration(config_.keepalive_timeout);
if (pending_data_size_ == 0) {
    if (!WaitOnSocket(deadline)) {   // returned false on timeout -> close
        return;
    }
}

WaitOnSocket did WaitReadable(deadline) / ReadSome(..., deadline) and
returned false on expiry, closing the connection. The refactor replaced this
with the deadline-less WaitAnyContext loop and did not carry the timeout
over. (This is the same refactor that dropped the HTTP/2 body-streaming wiring;
see userver-http2-streaming-issue.md.)

Suggested fix

Wait with the keepalive deadline and close the connection on expiry when it is
truly idle:

const auto ready_id = wait_any.WaitUntil(engine::Deadline::FromDuration(config_.keepalive_timeout));
if (!ready_id) {
    if (ready_id == utils::unexpected(engine::WaitAnyError::kTimeout)) {
        // Close only when nothing is in flight; do not kill a connection
        // that is mid-response.
        if (handler_tasks_.empty()) return;
        continue;  // re-arm and keep waiting while requests are being served
    }
    UASSERT(ready_id == utils::unexpected(engine::WaitAnyError::kCancelled));
    return;
}

Points to settle in review:

• In-flight requests / streaming responses. The deadline is an inbound
  idle timeout; a connection actively producing a (possibly slow, streamed)
  response must not be torn down for lack of incoming bytes. Gating the close
  on "no active handler tasks" (and, for the streaming branch, no pending
  streamed responses) covers this. A separate, longer whole-connection cap
  could be added later if desired.
• Config knob. Reusing keepalive_timeout keeps parity with HTTP/1.1;
  a distinct h2 idle setting could be introduced if different tuning is wanted.

Related, out of scope here

• Deadline-less WriteAll on the send path: a peer that stops reading parks
  the connection coroutine mid-send. This exists for HTTP/1.1 too and is not a
  regression, so it is tracked separately.
• Advertising SETTINGS_MAX_HEADER_LIST_SIZE (userver does not) would let
  clients avoid oversized-header rejections; unrelated to the idle timeout.

Environment

• userver 3.2-rc (1838cdcec), also present on develop (eed18325a)
• default keepalive_timeout 600 s, max_connections 32768
• found while reviewing the HTTP/2 body-streaming restore for DoS regressions

[ArkadyRudenko]

@SSE4
Yes, you are right. I skipped keepalive_timeout in that pr d1cdff2

You can make pr with that fix!) Thanks!

[SSE4]

You can make pr with that fix!) Thanks!

yes, I will take a look tomorrow