[security] Slowloris timeouts and HSTS max-age are asserted against themselves — 0 passes CI #101

New issue

Closed

opened 2026-08-23 09:52:27 +00:00 by a73x · 2 comments

a73x commented

2026-08-23 09:52:27 +00:00

(Migrated from git.a73x.sh)

[claude 2026-08-23] Split out of d30269d2, because these are not a test-hygiene nit — they are security constants that CI would let anyone set to zero.

A repo-wide sweep for assertions that name a constant on both sides found the pattern in internal/server/boot. Verified: the suite passes with

the Slowloris timeouts (ReadHeaderTimeout, IdleTimeout) set to 0s, and
the HSTS header set to max-age=0.

Both tests assert the header/timeout matches the constant, so the constant is free to move to the exact value an attacker would choose and nothing rings.

These guards were added deliberately (see the v0.0.6 hardening batch: ReadHeaderTimeout+IdleTimeout only, ReadTimeout=0 preserved so SSE/WS survive). The reasoning is sound and recorded — it is only the assertion that is circular.

Also unpinned by the same pattern

MinAllocatedHostPort — the reserved-range floor.
enrollBurst and enrollRefillEvery / maxLimiterEntries (ratelimit_test.go) — 30s→30ms and 10k→10 both pass.
vfkit's maxSocketPath = 103 — stands in for the macOS sun_path limit, so the number IS the fact; asserting it against itself pins nothing.

Partially retired

maxKeyCommentLen's truncation behaviour is genuinely pinned — removing the truncation fails. Only the value floats.

Retired outright (legitimately shape-only)

OnlineWindow >= 3×tick (model construction, not a magic number), remoteToolCount, tenantExt.

The fix

Assert the behaviour at the boundary, not the constant: a request that stalls past the header deadline is dropped; a response carries an HSTS max-age of at least N seconds where N is a literal in the test. Six more borderline sites are listed in Fable's design comment on d30269d2.

Migrated from git-collab issue 7c8b320f44ac3f054f7aa30c3e685e73bfbc7b90.

Original author: a73x dev@a73x.sh.

Close reason: [claude 2026-08-23] Merged to main in d0c032f (pushed to both remotes). Every test proven by the mutant it kills; Fable's checkpoint review passed it merge-as-is. make ci green, boot gate PASS (full smoke incl. UDP, cold_start=20s).

[claude 2026-08-23] Split out of d30269d2, because these are not a test-hygiene nit — they are security constants that CI would let anyone set to zero. A repo-wide sweep for assertions that name a constant on both sides found the pattern in `internal/server/boot`. **Verified**: the suite passes with - the Slowloris timeouts (`ReadHeaderTimeout`, `IdleTimeout`) set to **`0s`**, and - the HSTS header set to **`max-age=0`**. Both tests assert the header/timeout *matches the constant*, so the constant is free to move to the exact value an attacker would choose and nothing rings. These guards were added deliberately (see the v0.0.6 hardening batch: `ReadHeaderTimeout`+`IdleTimeout` only, `ReadTimeout=0` preserved so SSE/WS survive). The reasoning is sound and recorded — it is only the assertion that is circular. ### Also unpinned by the same pattern - `MinAllocatedHostPort` — the reserved-range floor. - `enrollBurst` and `enrollRefillEvery` / `maxLimiterEntries` (`ratelimit_test.go`) — 30s→30ms and 10k→10 both pass. - `vfkit`'s `maxSocketPath = 103` — stands in for the macOS `sun_path` limit, so the number IS the fact; asserting it against itself pins nothing. ### Partially retired `maxKeyCommentLen`'s **truncation behaviour** is genuinely pinned — removing the truncation fails. Only the value floats. ### Retired outright (legitimately shape-only) `OnlineWindow >= 3×tick` (model construction, not a magic number), `remoteToolCount`, `tenantExt`. ### The fix Assert the *behaviour at the boundary*, not the constant: a request that stalls past the header deadline is dropped; a response carries an HSTS max-age of at least N seconds where N is a literal in the test. Six more borderline sites are listed in Fable's design comment on d30269d2. --- Migrated from git-collab issue `7c8b320f44ac3f054f7aa30c3e685e73bfbc7b90`. Original author: a73x <dev@a73x.sh>. Close reason: [claude 2026-08-23] Merged to main in d0c032f (pushed to both remotes). Every test proven by the mutant it kills; Fable's checkpoint review passed it merge-as-is. make ci green, boot gate PASS (full smoke incl. UDP, cold_start=20s).

a73x commented

2026-08-23 10:27:34 +00:00

(Migrated from git.a73x.sh)

[claude 2026-08-23] Claiming. Working branch: self-referential-constants. Scope: the Tier-1 five (boot httpserver timeouts, boot HSTS, store MinAllocatedHostPort, vfkit maxSocketPath, api limiter trio) plus a triage pass over the six Tier-2 borderline sites in Fable's design comment. Discipline: every pin is proven by setting the constant to the attacker's value, observing the FAIL with the new message, and restoring the tree. Staying off the eight lying tests (d30269d2, other agent).

a73x commented

2026-08-23 10:41:50 +00:00

(Migrated from git.a73x.sh)

[claude 2026-08-23] Done on branch self-referential-constants (4 commits + a retro line, off c694896). Every pin below was proven by setting the constant to the attacker's value, running the test, watching it fail with the new message, and restoring the tree (git diff --stat empty each time). make ci green.

Tier 1 — the five filed sites

constant	how it is pinned now	mutant	failure
`httpServerReadHeaderTimeout`	`>= 1s` and `<= 30s`, literals	`0s`	`"0s" is not greater than or equal to "1s"` — "ReadHeaderTimeout must bound how long a client may dribble request headers: at 0 there is no deadline at all, a single host holds a goroutine and an fd open forever, and a few hundred of them exhaust the listener…"
`httpServerIdleTimeout`	`>= 10s` and `<= 10m`, literals	`0s`	`"0s" is not greater than or equal to "10s"` — "…at 0 net/http never closes an idle connection, so every probe that opens one and walks away costs an fd until the process restarts"
`hstsMaxAge`	max-age is now parsed out of the served response and held to a 15552000-second (180-day) floor	`max-age=0`	`"0" is not greater than or equal to "15552000"`, on both the proxied and the direct-TLS leg — "At 0 the header is an instruction to FORGET the pin, so the next visit can be downgraded to http…"
`MinAllocatedHostPort`	a real allocation must land in `[1024, 32767]`; `MaxAllocatedHostPort <= 32767`	`80`	`"80" is not greater than or equal to "1024"` — "below 1024 the agent's bind needs root and lands on ports the host's own sshd, DNS or web server already own". Also mutated the ceiling to `60999`: `"60999" is not less than or equal to "32767"` (Linux's `ip_local_port_range` floor).
`maxSocketPath`	Preflight is driven at the boundary: a state dir padded so the socket path is exactly 103 bytes must pass, 104 must refuse	`104`	`An error is expected but got nil` — "104 bytes does not fit sockaddr_un.sun_path, which is 104 INCLUDING the terminator… raising maxSocketPath does not raise sun_path". At `90` the accept side fails instead (and takes the default-state-dir test with it).
`enrollBurst`	count allowed requests on a fresh bucket, `== 5`	`500`	`expected: 5, actual: 100`
`enrollRefillEvery`	exhaust, advance 29s → still refused, advance to 30s → allowed	`30ms`	`Should be false` — "a token must take a full 30 seconds to drip back… at a refill of milliseconds the bucket is always full and the limiter is decoration"
`maxLimiterEntries`	feed 10 000 literal distinct addresses, assert the map holds all of them	`10`	`map[…] should have 10000 item(s), but has 10` — "set it low and the eleventh address evicts a real one, so a flood of fresh sources prunes the buckets holding the actual attacker and the requests that matter fail open"

ReadTimeout/WriteTimeout stay asserted Zero — deliberate, and the message now says so.

Tier 2 — the six borderline sites

Pinned (3):

maxKeyCommentLen — behaviour was already pinned, so only the value half moved: literal 128 and a real alex@workstation.example.com comment must survive intact. 4096 → "4096" is not less than or equal to "128"; 8 → expected "alex@workstation.example.com", actual "alex@wor".
abandonedVMReapGrace — the sweeps are driven at literal 14 min (must not reap) and 16 min (must reap) instead of at multiples of the grace. 5m → the 14-minute assertion fails; 24h → the 16-minute one fails. (Note: the old test survived both.)
outputCap — literal 1<<20. 1<<30 → Len fails: "the buffer is held in the plane's memory per concurrent exec, and the bytes go on to the model".
chronyDropInPath — the vendor-data doc must contain the literal /etc/chrony/conf.d/eitri-clock.conf; it is chronyd's path, not eitri's. Mutated to /etc/eitri-clock.conf → fails.
delegationTTL — added a literal one-hour ceiling on the minted cert's ValidBefore. 720h → "1802592000" is not less than or equal to "1800003600".

Left alone (1): netTapPrefix (named_test.go:490). The HasPrefix there is a cross-check that the nft rule pattern matches the names this host actually creates — the same construction as OnlineWindow >= 3×tick — and the budget that actually depends on the prefix length, IFNAMSIZ, is already pinned by a literal 15 in TestNetTapNameIsTruncatedBelowIFNAMSIZ. Nothing floats.

Retired sites (OnlineWindow, remoteToolCount, tenantExt) untouched, as filed.

Worth filing separately

make ci fails on a fresh worktree for a reason unrelated to any change: internal/server/web's coverage floor (95) was baselined against a built dist, but a fresh checkout embeds only .gitkeep, so TestInlineScriptHashesCoversBuiltSPA skips and the package reads 89.2%. make web once, and it reads 97.3%. The gate reports this as a coverage regression in a package the branch never touched; it should either say "UI not built" or cover should depend on web.

[claude 2026-08-23] Done on branch `self-referential-constants` (4 commits + a retro line, off c694896). Every pin below was proven by setting the constant to the attacker's value, running the test, watching it fail with the new message, and restoring the tree (`git diff --stat` empty each time). `make ci` green. ## Tier 1 — the five filed sites | constant | how it is pinned now | mutant | failure | |---|---|---|---| | `httpServerReadHeaderTimeout` | `>= 1s` and `<= 30s`, literals | `0s` | `"0s" is not greater than or equal to "1s"` — "ReadHeaderTimeout must bound how long a client may dribble request headers: at 0 there is no deadline at all, a single host holds a goroutine and an fd open forever, and a few hundred of them exhaust the listener…" | | `httpServerIdleTimeout` | `>= 10s` and `<= 10m`, literals | `0s` | `"0s" is not greater than or equal to "10s"` — "…at 0 net/http never closes an idle connection, so every probe that opens one and walks away costs an fd until the process restarts" | | `hstsMaxAge` | max-age is now **parsed out of the served response** and held to a 15552000-second (180-day) floor | `max-age=0` | `"0" is not greater than or equal to "15552000"`, on both the proxied and the direct-TLS leg — "At 0 the header is an instruction to FORGET the pin, so the next visit can be downgraded to http…" | | `MinAllocatedHostPort` | a real allocation must land in `[1024, 32767]`; `MaxAllocatedHostPort <= 32767` | `80` | `"80" is not greater than or equal to "1024"` — "below 1024 the agent's bind needs root and lands on ports the host's own sshd, DNS or web server already own". Also mutated the ceiling to `60999`: `"60999" is not less than or equal to "32767"` (Linux's `ip_local_port_range` floor). | | `maxSocketPath` | Preflight is **driven at the boundary**: a state dir padded so the socket path is exactly 103 bytes must pass, 104 must refuse | `104` | `An error is expected but got nil` — "104 bytes does not fit sockaddr_un.sun_path, which is 104 INCLUDING the terminator… raising maxSocketPath does not raise sun_path". At `90` the accept side fails instead (and takes the default-state-dir test with it). | | `enrollBurst` | count allowed requests on a fresh bucket, `== 5` | `500` | `expected: 5, actual: 100` | | `enrollRefillEvery` | exhaust, advance **29s** → still refused, advance to 30s → allowed | `30ms` | `Should be false` — "a token must take a full 30 seconds to drip back… at a refill of milliseconds the bucket is always full and the limiter is decoration" | | `maxLimiterEntries` | feed **10 000** literal distinct addresses, assert the map holds all of them | `10` | `map[…] should have 10000 item(s), but has 10` — "set it low and the eleventh address evicts a real one, so a flood of fresh sources prunes the buckets holding the actual attacker and the requests that matter fail open" | `ReadTimeout`/`WriteTimeout` stay asserted `Zero` — deliberate, and the message now says so. ## Tier 2 — the six borderline sites Pinned (3): - `maxKeyCommentLen` — behaviour was already pinned, so only the value half moved: literal 128 **and** a real `alex@workstation.example.com` comment must survive intact. `4096` → `"4096" is not less than or equal to "128"`; `8` → `expected "alex@workstation.example.com", actual "alex@wor"`. - `abandonedVMReapGrace` — the sweeps are driven at literal **14 min** (must not reap) and **16 min** (must reap) instead of at multiples of the grace. `5m` → the 14-minute assertion fails; `24h` → the 16-minute one fails. (Note: the old test survived both.) - `outputCap` — literal `1<<20`. `1<<30` → `Len` fails: "the buffer is held in the plane's memory per concurrent exec, and the bytes go on to the model". - `chronyDropInPath` — the vendor-data doc must contain the literal `/etc/chrony/conf.d/eitri-clock.conf`; it is chronyd's path, not eitri's. Mutated to `/etc/eitri-clock.conf` → fails. - `delegationTTL` — added a literal one-hour ceiling on the minted cert's `ValidBefore`. `720h` → `"1802592000" is not less than or equal to "1800003600"`. Left alone (1): **`netTapPrefix`** (`named_test.go:490`). The `HasPrefix` there is a cross-check that the nft rule pattern matches the names this host actually creates — the same construction as `OnlineWindow >= 3×tick` — and the budget that actually depends on the prefix length, IFNAMSIZ, is already pinned by a literal `15` in `TestNetTapNameIsTruncatedBelowIFNAMSIZ`. Nothing floats. Retired sites (`OnlineWindow`, `remoteToolCount`, `tenantExt`) untouched, as filed. ## Worth filing separately `make ci` fails on a fresh worktree for a reason unrelated to any change: `internal/server/web`'s coverage floor (95) was baselined against a **built** dist, but a fresh checkout embeds only `.gitkeep`, so `TestInlineScriptHashesCoversBuiltSPA` skips and the package reads 89.2%. `make web` once, and it reads 97.3%. The gate reports this as a coverage regression in a package the branch never touched; it should either say "UI not built" or `cover` should depend on `web`.