• Web server: no limit on HTTP/1.1 keep-alive connection lifetime or req

    From Rob Swindell@1:103/705 to GitLab issue in main/sbbs on Tue Jun 2 22:00:41 2026
    open https://gitlab.synchro.net/main/sbbs/-/issues/1154

    ## Summary

    The web server places **no upper bound on an HTTP/1.1 keep-alive connection** — neither a maximum number of requests per connection nor a maximum total connection lifetime. The only time bound is `MaxInactivity`, which is a *gap-between-socket-reads* timeout, not a connection-age or request-count limit. A client that keeps issuing requests (or even dribbling bytes) inside the inactivity window can hold a single connection open indefinitely.

    This was observed on a production system (`vert`) under a web-scrape flood: crawler IPs (e.g. Baidu's `116.179.37.*`) showed **16+ hour** connection durations in sbbsctrl. These are not stale/zombie entries — they are live keep-alive sockets being continuously reused by a steadily-crawling client.

    ## Current behavior (where the bound is missing)

    `src/sbbs3/websrvr.cpp`:

    - HTTP/1.1 defaults to keep-alive — `:3544` (`session->req.keep_alive = true`)
    - The session loop runs `while (!session.finished)` servicing unlimited successive requests on one connection — `:7059`
    - `close_request()` only closes the socket when `!keep_alive || terminate_server` — `:1186`
    - `MaxInactivity` is enforced only while *waiting for socket I/O* in `sockreadline()` — `:2292` (`socket_readable(sock, max_inactivity*1000)`); any byte resets it, and it does not run during header parse, request processing, or response transmission.
    - Connect timestamp is stamped once at accept and never updated — `:6999` (`session.client.time = time32(NULL)`), so the duration sbbsctrl shows is the real socket age.

    There is no `MaxKeepAliveRequests` equivalent and no absolute connection-lifetime cap.

    ## Why this matters

    1. **Resource pinning.** Each long-lived crawler connection permanently occupies one of `MaxClients` web slots and one session thread, and (for authenticated or file-area requests) keeps driving `user.tab` / `name.dat` lookups for hours. A handful of well-behaved-but-relentless crawlers can hold a large fraction of the pool.
    2. **It defeats the rate-limit / accept-time filters.** The connect rate limiter and the `ip.can` / `ip-silent.can` accept-time block only fire on a *new* `accept()`. A client that never reconnects is never re-evaluated. Capping requests-per-connection forces periodic reconnects, at which point the existing filtering machinery (connect limiter, `.can` checks) actually applies to a persistent abuser. Note also that a per-request `429` does **not** currently tear down the keep-alive connection.
    3. **Slowloris exposure.** Because `MaxInactivity` only limits the gap between reads (not the time to complete a request), a client dribbling one byte every <`MaxInactivity` seconds can hold a connection open without ever completing a request. A per-request/header read deadline and an absolute connection cap both mitigate this.

    ## Prior art (this is a standard feature)

    Essentially every production web server bounds keep-alive connections, typically with two knobs:

    | Server | Max requests / connection | Max connection age | |--------|---------------------------|--------------------|
    | nginx | `keepalive_requests` (default 1000) | `keepalive_time` (default 1h) | | Apache httpd | `MaxKeepAliveRequests` (default 100) | — (covered by `KeepAliveTimeout` + `Timeout`) |
    | Node.js `http` | `server.maxRequestsPerSocket` | — (via `requestTimeout`) | | Go `net/http` | (bounded via `ReadTimeout`) | `IdleTimeout` |
    | HAProxy | — | `timeout http-keep-alive` |

    The closest direct analogs are Apache's `MaxKeepAliveRequests` / Node's `maxRequestsPerSocket` (per-connection request cap) and nginx's `keepalive_time` (absolute connection-age cap).

    ## Proposed change

    Add to the web server two configurable limits, plumbed through `web_startup_t` (`startup.h`), `sbbs_ini.c`, and SCFG (`scfg/scfgsrvr.c`), defaulting to sane non-zero values; `0` = unlimited (preserve current behavior for anyone who wants it):

    - **`MaxKeepAliveRequests`** — after N requests on one keep-alive connection, finish the in-flight response with `Connection: close` and close the socket (forcing a reconnect that re-runs accept-time filtering). Suggested default ~100–1000 (cf. Apache 100 / nginx 1000).
    - **`MaxConnectionDuration`** (a.k.a. max keep-alive time / connection age) — close a connection once its total age since accept exceeds the limit, regardless of activity. Suggested default ~1h (cf. nginx `keepalive_time`). Enforced by checking `now - session->client.time` at the top of the request loop and in the read-wait path.

    Both are cheap: the request loop already has the connect timestamp (`session->client.time`) and a natural per-iteration check point. Closing should be graceful — emit `Connection: close` on the final response so compliant clients don't see a truncated reply.

    Optionally (separate, smaller follow-ups): tear down the connection on repeated `429`s from the same connection, and add an absolute per-request/header read deadline (distinct from `MaxInactivity`) for slowloris hardening.

    ## Notes

    - Adding fields to `web_startup_t` is an ABI bump for the struct; ship a rebuilt `sbbsctrl` alongside (its startup-struct size check fails on mismatch).
    - Per project convention, expose the new settings in SCFG (TUI), not just `sbbs.ini`.

    ---
    *Found while investigating an SMB `user.tab` lock-contention DoS under a live scrape attack; the unlimited keep-alive connections are what let crawlers sustain that load for hours on a single socket. Related lock-layer root cause filed separately as #1153.*

    — *Authored by Claude (Claude Code), on behalf of @rswindell*
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)