Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion deploy/helm/tinycdi/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,20 @@ while Postgres is healthy the series measures the shared bound.

The client address is the socket peer — unless the peer is inside
`backend.trustedProxies`, in which case the right-most untrusted
`X-Forwarded-For` entry stands in. **Behind any ingress or Gateway the value
`X-Forwarded-For` entry stands in. Recognized client addresses are
canonicalised before keying: IPv4-mapped spellings (`::ffff:a.b.c.d` in any
notation) unify with the native IPv4 key, and an IPv6 address keys by its
**/64 prefix** — the smallest block a single subscriber is delegated — so
temporary/privacy-address rotation inside one prefix draws from one
budget instead of minting a fresh one per address. IPv4 stays per-/32.
The corollary: every host inside ONE shared /64 — a LAN segment, or
subscribers an ISP delegates a single prefix to — shares one rate-limit
budget. This affects only billing granularity: the `X-Forwarded-For` /
`Forwarded` / `X-Real-IP` headers the workspace pod sees still carry the
client's real (unmapped, unfolded) address, and audit attribution stays
per-address.

**Behind any ingress or Gateway the value
is required, not optional**: with it empty every user arriving through the
same edge keys on the edge's own address — one shared bucket (~60 logins and
~120 launches per minute for the whole organisation) and a self-inflicted
Expand All @@ -522,6 +535,18 @@ is empty. The same list feeds the `X-Forwarded-For` / `Forwarded` /
stripped and rebuilt from the trusted chain only (S18), so a spoofed address
can never poison the runtime's brute-force blacklist.

The right-most-untrusted derivation is only as good as the list's coverage
of the proxy chain. Every hop in front that terminates the client
connection must appear — a trusted hop that *passes* a client-supplied
`X-Forwarded-For` through instead of appending the address it observed
(an L4 load balancer that forwards the header, for example) hands the
rate-limit key to unverified client bytes. Conversely a real hop missing
from the list becomes "the client": every user behind it shares its one
bucket. **Dual-stack edges must list their IPv6 ranges alongside the IPv4
ones** — a proxy that reaches the backend over IPv6 but is only listed by
its v4 CIDR is untrusted on the v6 path and collapses every v6 client
into that proxy's /64 bucket.

```yaml
# Cilium Gateway API / Ingress — the edge envoy runs host-network, so the
# peer the backend sees is the NODE the request lands on. List the node
Expand Down
7 changes: 5 additions & 2 deletions deploy/helm/tinycdi/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,11 @@ backend:
# rate limits and the runtime-bound forwarded headers derive the client
# from the right-most untrusted chain entry. REQUIRED behind an ingress
# or Gateway — empty keys every limit on the edge's own address, so all
# users share one bucket (the backend logs a startup warning). See the
# README "Rate limits and trusted proxies" for Cilium/Traefik examples.
# users share one bucket (the backend logs a startup warning). List
# every hop in front; a dual-stack edge needs its IPv6 CIDRs alongside
# the IPv4 ones or v6 clients collapse into the proxy's /64 bucket.
# See the README "Rate limits and trusted proxies" for Cilium/Traefik
# examples.
# Authenticated traffic keys per session (session probe, live-session
# re-launches) or per validated OIDC state (callback, bounded by a 10x
# per-IP ceiling), not per client IP; the anonymous surface — login
Expand Down
28 changes: 25 additions & 3 deletions docs/runbooks/capacity.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,31 @@ What that means for sizing:
to rate+burst locally before the divided floor binds — the just-failed
request draws a healthy-ceiling token. Size `-login-rate`/
`-launch-rate` as the aggregate you want to allow.
- The per-key limits and the limiter's key-space bound are unchanged;
`backend.trustedProxies` must still name the edge's CIDRs or every user
collapses into the edge's own IP bucket regardless.
- **The client key is canonicalised, not the literal address.** An IPv6
client keys by its **/64 prefix** — the smallest block one subscriber
is delegated — so temporary/privacy-address rotation inside the
prefix draws one budget instead of minting a fresh bucket per /128;
IPv4-mapped spellings (`::ffff:a.b.c.d`, any notation) unify with the
native IPv4 key, and IPv4 itself stays per-/32. The corollary: all
hosts inside ONE shared /64 — a LAN segment, or subscribers an ISP
gives a single prefix — share one rate-limit budget (billing
granularity only: the forwarded `X-Forwarded-For`/`Forwarded`/
`X-Real-IP` headers and audit attribution still carry the client's
real, unmapped-but-unfolded address). The one canonical
string keys the local buckets AND the shared Postgres window rows, so
every enforcement layer agrees on the client. The right-most-untrusted
XFF derivation behind that key is only as good as
`backend.trustedProxies`' coverage: it must name EVERY proxy hop in
front — a trusted hop that passes a client-supplied XFF through
instead of appending the observed address hands the key to unverified
bytes, and a dual-stack edge missing its IPv6 ranges collapses every
v6 client into the proxy's own /64 bucket.
- **Key format changed once at v1.0 — no migration.** Window rows are
keyed by the canonical string, so IPv6 clients move from per-address
to per-/64 rows on upgrade (IPv4 keys are unchanged). Rows written
under the old format simply expire inside their minute window on the
normal sweep; the only effect is that an IPv6 client mid-window at
upgrade time gets at most one fresh window's budget.

Set the flags through `backend.extraArgs`
(`deploy/helm/tinycdi/README.md`, "Rate limits and trusted proxies").
Expand Down
2 changes: 1 addition & 1 deletion docs/security/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ items that landed since.
| S11 | KASM-2 risk acceptance | The risk acceptance lapsed with v0.2; the kasm adapter + catalog scan ship now. Reviewer should confirm the catalog gate (`check-kasm-catalog.sh`, `kasm-contract` job) actually covers the documented minimum engine floor |
| S12 | Runtime image release train | Live (`runtime-images.yml`); intentionally no human gate — review job permissions, keyless identity, train-vs-release image distinguishability |
| S13 | G0–G5 merged without independent review | Standing: the whole v0.1→v0.3 delta has had no external security review — this document exists to scope it |
| S14 | App-layer rate limit for `/v1/login`, `/v1/launch` | Implemented (v0.4: `internal/ratelimit` + Postgres fixed-minute windows, ADR 0006 — FX-R30 keying, exact shared-window bound with the undivided local bucket as a healthy-mode store-protection prefilter and the divided local bucket as the degraded-mode floor (v0.5 RL-CEILING amendment), fail-open on store error). Since then: a circuit breaker bounds degraded-mode cost (500 ms per-call deadline, 10 s cool-down skipping the store, single-flight probe — `internal/ratelimit/shared.go`, `shared_test.go`), the v0.4.0 defaults restore the v0.3.x effective budgets as window bounds (`TestRateLimitDefaults_V040Budgets`), and `/v1/me/sessions:revoke-all` joined the session-keyed limiter. Reviewer verifies coverage, ceilings, bypass resistance and the outage degradation path. Residual: IP-keyed login budgets multiply with a client's address pool — a dual-stack client, IPv6 temporary addresses or several egress addresses each get their own local and window budget (per-address limiting as designed; authenticated routes key by session) |
| S14 | App-layer rate limit for `/v1/login`, `/v1/launch` | Implemented (v0.4: `internal/ratelimit` + Postgres fixed-minute windows, ADR 0006 — FX-R30 keying, exact shared-window bound with the undivided local bucket as a healthy-mode store-protection prefilter and the divided local bucket as the degraded-mode floor (v0.5 RL-CEILING amendment), fail-open on store error). Since then: a circuit breaker bounds degraded-mode cost (500 ms per-call deadline, 10 s cool-down skipping the store, single-flight probe — `internal/ratelimit/shared.go`, `shared_test.go`), the v0.4.0 defaults restore the v0.3.x effective budgets as window bounds (`TestRateLimitDefaults_V040Budgets`), and `/v1/me/sessions:revoke-all` joined the session-keyed limiter. Reviewer verifies coverage, ceilings, bypass resistance and the outage degradation path. Residual: IP-keyed budgets still multiply across the prefixes a client holds — IPv6 keys by /64 and mapped spellings unify with the native v4 key (v1.0), so rotation inside one prefix or across family spellings no longer helps, but a client holding several /64s or several egress addresses still gets a budget per prefix (per-prefix limiting as designed; authenticated routes key by session). The right-most-untrusted XFF derivation additionally assumes every proxy hop in front is in `-trusted-proxies` — a pass-through trusted hop would hand the key to client bytes; the invariant and the dual-stack listing requirement are documented in the chart README |
| S15 | Operator mTLS client cert / listener client-CA hot reload | **Implemented since** (`opclient` reload loop + `hotReloadClientCAs`; FX-R33 test); reviewer confirms rotation edge cases |
| S16 | `runtime.appArmor.requireRuntimeDefault` opt-out | Implemented (`AppArmorNotRequired`); review docs/default/preflight detection on AppArmor-less nodes |
| S17 | Sign-out vs live desktop streams | Implemented (#107 + #123; `LogoutHandler` → `Broker.RevokePortalSession`, `internal/api/auth.go:685`, `internal/broker/sessions.go:109`): sign-out revokes the session's digest-bound leases and outstanding tickets in one store tx — every replica's renew loop closes the bound stream within one renew cycle, and a replayed workspace cookie resolves to a revoked lease (401) on any replica; ticket-redemption re-checks the session row (`internal/broker/tickets.go`). Since the SIGNOUT-FIX pass both revoke sweeps pre-lock their covered rows in a deterministic key order (`lockByKeysInOrderTx`, `sessions.go:418` — tickets by `ticket_hash`, sessions by `id`, leases by `id`), so overlapping sweeps serialize on the first contested row instead of crossing waits. Defence-in-depth (#123, DR): every lease read that feeds renew/attach/claim or the cookie→lease resolve (`liveLease`/`LeaseBySession`, `internal/broker/leases.go`) re-validates the bound portal session row — epoch + absolute expiry, same semantics as the redeem-time check — in the same indexed read, and revokes the lease in one statement when it fails (`tinycdi_lease_session_missing_total{reason=absent\|invalid}` + one log line). A lease resurrected as `active` by a Postgres restore therefore dies at its next renew or attach even when the sign-out revoke itself was lost in the dump window; leases with a NULL `portal_session_digest` (pre-018 rows) are exempt — nothing to verify against, and revoking them on sight would mass-kill sessions mid-rolling-upgrade — they keep the TTL lifecycle and are covered by the DR runbook's post-restore lease sweep. Since the FIX-LOGOUT pass the session-row delete is the ordering's point of no return: a store failure on it answers retryable `503 UNAVAILABLE` (+ `Retry-After`) and tears nothing down — no cookie expiry, no lease/ticket revoke — so a response never claims a sign-out that did not commit, and the still-valid session keeps its live leases until a retry completes the destroy (revoking first would leave a live session with dead streams, a half-revoked state the refusal would be lying about). Tests `internal/broker/lease_session_test.go`, `signout_test.go` (api, broker, gateway), `TestLogout_StoreDeleteFailureReturns503` |
Expand Down
44 changes: 44 additions & 0 deletions internal/gateway/forwarded_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"testing"

"github.com/tinyorbitvn/tinycdi/internal/gateway"
"github.com/tinyorbitvn/tinycdi/internal/ratelimit"
)

// fwdRecorder is the fake runtime for these tests: it captures the
Expand Down Expand Up @@ -118,6 +119,49 @@ func TestProxy_XFFFromTrustedChainOnly(t *testing.T) {
}
}

// TestProxy_IPv6ForwardsExactAddrKeyFolds: the /64 fold applies to the
// rate-limit KEY only — the runtime-bound headers carry the client's real
// address. One request is billed against the "2001:db8::" /64 bucket while
// the runtime still attributes it to the exact "2001:db8::5".
func TestProxy_IPv6ForwardsExactAddrKeyFolds(t *testing.T) {
fb := newFakeBroker(t)
rec := newFwdRecorder(t)
trusted := []netip.Prefix{netip.MustParsePrefix("127.0.0.0/8")}
srv, cookie := launchOn(t, fb, rec, func(c *gateway.Config) {
c.TrustedProxies = trusted
})

resp := proxied(t, srv, testHost, "/", cookie, map[string]string{
"X-Forwarded-For": "2001:db8::5",
})
defer drain(resp)
if resp.StatusCode != http.StatusOK {
t.Fatalf("proxied GET = %d, want 200", resp.StatusCode)
}
xff, fwd, real := rec.seen()
if xff != "2001:db8::5, 127.0.0.1" {
t.Fatalf("xff = %q, want the exact client address + peer, not the /64 base", xff)
}
if fwd != `for="[2001:db8::5]", for=127.0.0.1` {
t.Fatalf("forwarded = %q, want the exact v6 address", fwd)
}
if real != "2001:db8::5" {
t.Fatalf("x-real-ip = %q, want the real address", real)
}

// The same request's limiter key is the folded /64 base.
r := &http.Request{
RemoteAddr: "127.0.0.1:9999",
Header: http.Header{"X-Forwarded-For": {"2001:db8::5"}},
}
if k := ratelimit.ClientKey(r, trusted); k != "2001:db8::" {
t.Fatalf("limiter key = %q, want the /64 base 2001:db8::", k)
}
if a := ratelimit.ClientAddr(r, trusted); a != "2001:db8::5" {
t.Fatalf("forwarded client addr = %q, want the exact 2001:db8::5", a)
}
}

// TestProxy_NoXFFAtAll: a request with no forwarding headers still lets the
// runtime identify the client — the socket peer.
func TestProxy_NoXFFAtAll(t *testing.T) {
Expand Down
10 changes: 6 additions & 4 deletions internal/gateway/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -829,17 +829,19 @@ func (g *Gateway) direct(r *http.Request) {
// stripped and re-derived from the trusted chain only. The runtime keys
// its brute-force blacklist on the forwarded address, so a spoofed client
// value must never reach the pod. The derived client is the same
// ratelimit.ClientKey result the launch limiter uses: the socket peer, or
// the right-most untrusted chain entry when the peer is inside
// TrustedProxies. When the peer itself is the client the header is left
// selection the launch limiter keys on — ratelimit.ClientAddr shares
// ClientKey's chain walk — but rendered as the real client address
// (unmapped, never folded to its /64): the limiter's per-prefix billing
// granularity must not blur the address attribution the runtime sees.
// When the peer itself is the client the header is left
// unset — the ReverseProxy appends the socket address itself; when the
// peer is a trusted proxy the derived client is prepended and the proxy
// still appends the peer, so the runtime sees "<client>, <peer>".
// Forwarded mirrors that chain in RFC 7239 form and X-Real-IP carries the
// derived client for runtimes that key on it.
func (g *Gateway) rewriteForwarded(r *http.Request) {
peer := ratelimit.PeerIP(r.RemoteAddr)
client := ratelimit.ClientKey(r, g.cfg.TrustedProxies)
client := ratelimit.ClientAddr(r, g.cfg.TrustedProxies)
if _, err := netip.ParseAddr(client); err != nil {
// A non-IP right-most entry (spoofed or "unknown") is not a
// usable client address: fall back to the verified peer rather
Expand Down
2 changes: 1 addition & 1 deletion internal/ratelimit/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func FuzzClientKey(f *testing.F) {
entries = append(entries, e)
}
}
want := peer
want := canon(peer)
if trustedPeer := inTrusted(peer, trusted); trustedPeer {
leftmost := ""
for i := len(entries) - 1; i >= 0; i-- {
Expand Down
63 changes: 56 additions & 7 deletions internal/ratelimit/ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@
// throttles /v1/login, /v1/auth/callback and GET /v1/session, the session
// gateway throttles /v1/launch, and both derive the client key the same
// way — the socket peer, or the right-most untrusted X-Forwarded-For entry
// when the peer sits inside the configured trusted CIDRs.
// when the peer sits inside the configured trusted CIDRs. One selection
// feeds two renders: rate-limit keys (ClientKey) fold harder — mapped
// spellings unmapped, IPv6 collapsed to its /64 — so address-spelling and
// in-prefix rotation cannot multiply one client's budget, while the
// address consumers (ClientAddr: the gateway's forwarded headers) carry
// the real, unmapped-but-unfolded client address.
package ratelimit

import (
Expand Down Expand Up @@ -173,8 +178,30 @@ func PeerIP(remoteAddr string) string {
// not claim). An entry or peer that does not parse as an IP is never
// trusted, so client-supplied bytes cannot claim a proxy's place; when
// every hop is trusted the left-most entry is the closest observable
// claim. Recognized addresses are returned in canonical netip form.
// claim. The selected claim is canonicalised by canon: mapped forms
// unify with the native IPv4 key and IPv6 folds to its /64. The one
// string keys every enforcement layer — the local buckets and the shared
// Postgres window rows — so the layers always agree on which client a
// hit belongs to.
func ClientKey(r *http.Request, trusted []netip.Prefix) string {
return canon(selectClient(r, trusted))
}

// ClientAddr returns the SAME selected client claim as ClientKey but
// rendered as the real address — IPv4-mapped forms unmapped, IPv6 NOT
// folded — for consumers that need the literal client address rather than
// a billing bucket: the gateway's forwarded-header rebuild toward the
// runtime (S18). A non-IP claim passes through verbatim so the caller's
// parse check still discriminates it.
func ClientAddr(r *http.Request, trusted []netip.Prefix) string {
return canonAddr(selectClient(r, trusted))
}

// selectClient picks the raw client claim both renders share: the socket
// peer, or under a trusted peer the right-most XFF entry outside the
// trusted set — else the left-most claim when every hop is trusted, else
// the peer.
func selectClient(r *http.Request, trusted []netip.Prefix) string {
peer := PeerIP(r.RemoteAddr)
if !inTrusted(peer, trusted) {
return peer
Expand All @@ -185,20 +212,42 @@ func ClientKey(r *http.Request, trusted []netip.Prefix) string {
e := entries[i]
leftmost = e
if !inTrusted(e, trusted) {
return canon(e)
return e
}
}
if leftmost != "" {
return canon(leftmost)
return leftmost
}
return peer
}

// canon renders a parseable address canonically; anything else passes
// through trimmed so it still discriminates one key from another.
// canon renders a rate-limit key canonically — canonAddr plus the IPv6
// /64 fold: the address collapses to the prefix's base address, so every
// address inside the prefix one subscriber controls (SLAAC, privacy /
// temporary addresses, deliberate rotation) shares one budget; /64 is
// the smallest prefix an end site is delegated, hence the granularity a
// single client can still rotate inside. IPv4 stays the address itself
// (/32). Anything that does not parse passes through verbatim so it
// still discriminates one key from another.
func canon(s string) string {
a, err := netip.ParseAddr(s)
if err != nil {
return s
}
a = a.Unmap()
if a.Is6() {
return netip.PrefixFrom(a, 64).Masked().Addr().String()
}
return a.String()
}

// canonAddr renders the real client address canonically: a parseable
// address is Unmap'd — the "::ffff:a.b.c.d" spellings become the native
// "a.b.c.d" — and printed in netip canonical form, never folded; anything
// else passes through verbatim.
func canonAddr(s string) string {
if a, err := netip.ParseAddr(s); err == nil {
return a.String()
return a.Unmap().String()
}
return s
}
Expand Down
Loading