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)