From d0ba508d92bc4a7c5d673b302ae5258338ed41fa Mon Sep 17 00:00:00 2001 From: Nguyen Van Long Date: Tue, 6 Oct 2026 12:04:07 +0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(ratelimit):=20canonicalise=20client=20k?= =?UTF-8?q?eys=20=E2=80=94=20IPv6=20to=20/64,=20mapped=20v4=20unified=20(F?= =?UTF-8?q?IX-RLKEY)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-client budget multiplied with the client's address pool: an IPv6 client rotating temporary/privacy addresses inside its /64 got a fresh local bucket and a fresh Postgres window row per /128, and the ::ffff:a.b.c.d spellings keyed separately from the native v4 address. canon() now Unmap()s before rendering and folds IPv6 keys to the masked /64 base; the socket-peer paths canonicalise too so direct v6 clients fold identically. The one canonical string still keys the local buckets and the shared window rows end to end. The folded key remains a parseable address, so the gateway's forwarded-header rebuild (S18) now folds the runtime-bound client address at the same granularity. IPv4 keys are unchanged; IPv6 window rows under the old per-address format expire inside their minute window — no migration. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- deploy/helm/tinycdi/README.md | 21 ++++++++- deploy/helm/tinycdi/values.yaml | 7 ++- docs/runbooks/capacity.md | 23 ++++++++-- docs/security/threat-model.md | 2 +- internal/gateway/proxy.go | 4 +- internal/ratelimit/fuzz_test.go | 2 +- internal/ratelimit/ratelimit.go | 41 +++++++++++++---- internal/ratelimit/ratelimit_test.go | 69 ++++++++++++++++++++++++++++ internal/ratelimit/shared_test.go | 39 ++++++++++++++++ 9 files changed, 190 insertions(+), 18 deletions(-) diff --git a/deploy/helm/tinycdi/README.md b/deploy/helm/tinycdi/README.md index e61cf873..b86d6ace 100644 --- a/deploy/helm/tinycdi/README.md +++ b/deploy/helm/tinycdi/README.md @@ -512,7 +512,14 @@ 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. + +**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 @@ -522,6 +529,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 diff --git a/deploy/helm/tinycdi/values.yaml b/deploy/helm/tinycdi/values.yaml index d7e71a43..0f0c90de 100644 --- a/deploy/helm/tinycdi/values.yaml +++ b/deploy/helm/tinycdi/values.yaml @@ -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 diff --git a/docs/runbooks/capacity.md b/docs/runbooks/capacity.md index c4d27a44..b4d4b3ac 100644 --- a/docs/runbooks/capacity.md +++ b/docs/runbooks/capacity.md @@ -213,9 +213,26 @@ 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 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"). diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 0763c69b..f6151575 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -512,7 +512,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. Tests `internal/broker/lease_session_test.go`, `signout_test.go` (api, broker, gateway) | diff --git a/internal/gateway/proxy.go b/internal/gateway/proxy.go index 4b11d110..3ce9cd45 100644 --- a/internal/gateway/proxy.go +++ b/internal/gateway/proxy.go @@ -831,7 +831,9 @@ func (g *Gateway) direct(r *http.Request) { // 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 +// TrustedProxies — an IPv6 client appears as its /64 base address, so the +// runtime blacklist folds at the same granularity the limiter bills. +// 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 ", ". diff --git a/internal/ratelimit/fuzz_test.go b/internal/ratelimit/fuzz_test.go index 53b3b8f9..7ac2d5d9 100644 --- a/internal/ratelimit/fuzz_test.go +++ b/internal/ratelimit/fuzz_test.go @@ -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-- { diff --git a/internal/ratelimit/ratelimit.go b/internal/ratelimit/ratelimit.go index a1c55d83..81e0f4dc 100644 --- a/internal/ratelimit/ratelimit.go +++ b/internal/ratelimit/ratelimit.go @@ -7,7 +7,10 @@ // 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. Recognized +// client addresses are canonicalised (IPv4-mapped forms unmapped, IPv6 +// folded to its /64) so address-spelling and in-prefix rotation cannot +// multiply one client's budget. package ratelimit import ( @@ -173,11 +176,15 @@ 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. Recognized addresses are 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 { peer := PeerIP(r.RemoteAddr) if !inTrusted(peer, trusted) { - return peer + return canon(peer) } var leftmost string entries := xffEntries(r) @@ -191,16 +198,32 @@ func ClientKey(r *http.Request, trusted []netip.Prefix) string { if leftmost != "" { return canon(leftmost) } - return peer + return canon(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. A parseable address is +// first Unmap'd, so the IPv4-mapped spellings "::ffff:a.b.c.d" (any +// notation) and the native "a.b.c.d" land on one key — a dual-stack +// client cannot double its budget by switching family spelling. An IPv6 +// address folds to its /64 — rendered as 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. The result stays a valid +// address (or the original token) — consumers that re-parse it, like the +// gateway's forwarded-header rebuild, keep working. func canon(s string) string { - if a, err := netip.ParseAddr(s); err == nil { - return a.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 s + return a.String() } // inTrusted reports whether s parses as an address inside the trusted diff --git a/internal/ratelimit/ratelimit_test.go b/internal/ratelimit/ratelimit_test.go index 745c84a9..a5511433 100644 --- a/internal/ratelimit/ratelimit_test.go +++ b/internal/ratelimit/ratelimit_test.go @@ -90,6 +90,75 @@ func TestClientKey_RightmostUntrusted(t *testing.T) { } } +// TestClientKey_IPv6FoldedTo64 pins the per-client budget at the /64 — +// the prefix one subscriber controls: every address inside it (temporary +// privacy addresses, deliberate rotation) shares one bucket, while a +// different /64 is a different client. Covers both the socket-peer path +// and the right-most-untrusted XFF path. +func TestClientKey_IPv6FoldedTo64(t *testing.T) { + trusted := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")} + xf := func(xff string) *http.Request { + return &http.Request{ + RemoteAddr: "10.1.2.3:443", + Header: http.Header{"X-Forwarded-For": {xff}}, + } + } + k1 := ClientKey(xf("2001:db8::1"), trusted) + k2 := ClientKey(xf("2001:db8::ffff"), trusted) + if k1 != k2 { + t.Fatalf("addresses in one /64 keyed separately: %q vs %q", k1, k2) + } + if k3 := ClientKey(xf("2001:db8:0:1::1"), trusted); k3 == k1 { + t.Fatalf("different /64 shares a key: %q", k3) + } + // A direct (untrusted peer) IPv6 client folds the same way. + peer := &http.Request{RemoteAddr: "[2001:db8::5]:5150", Header: http.Header{}} + if k := ClientKey(peer, nil); k != k1 { + t.Fatalf("socket peer key = %q, want the same /64 bucket %q", k, k1) + } + + // The folded key is what the bucket sees: 101 addresses inside one + // /64 draw from ONE budget, not 101. + l := New(1, 1, 100000, nil) // 1/min, burst 1 + mk := func(xff string) string { return ClientKey(xf(xff), trusted) } + if ok, _ := l.Allow(mk("2001:db8::1")); !ok { + t.Fatal("first request refused") + } + for i := 2; i <= 101; i++ { + if ok, _ := l.Allow(mk(fmt.Sprintf("2001:db8::%x", i))); ok { + t.Fatalf("rotation to 2001:db8::%x escaped the /64 budget", i) + } + } + // A different /64 is a different client and gets its own budget. + if ok, _ := l.Allow(mk("2001:db8:0:1::1")); !ok { + t.Fatal("first request from a different /64 refused") + } +} + +// TestClientKey_MappedFormsUnified: the IPv4-mapped spellings of one +// address — "::ffff:1.2.3.4", "0:0:0:0:0:ffff:1.2.3.4" and the native +// "1.2.3.4" — must land on the single IPv4 key, so a dual-stack client +// cannot double its budget by switching spelling. +func TestClientKey_MappedFormsUnified(t *testing.T) { + trusted := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")} + mk := func(xff string) string { + return ClientKey(&http.Request{ + RemoteAddr: "10.1.2.3:443", + Header: http.Header{"X-Forwarded-For": {xff}}, + }, trusted) + } + k1 := mk("1.2.3.4") + for _, form := range []string{"::ffff:1.2.3.4", "0:0:0:0:0:ffff:1.2.3.4"} { + if k := mk(form); k != k1 { + t.Fatalf("mapped form %q keyed %q, want %q", form, k, k1) + } + } + // The mapped spelling of a direct socket peer unifies too. + if k := ClientKey(&http.Request{RemoteAddr: "[::ffff:1.2.3.4]:443", Header: http.Header{}}, nil); k != k1 { + t.Fatalf("mapped socket peer keyed %q, want %q", k, k1) + } +} + func TestClientKey_MultipleHeadersAndNonIP(t *testing.T) { trusted := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")} r := &http.Request{ diff --git a/internal/ratelimit/shared_test.go b/internal/ratelimit/shared_test.go index e240d34a..07080bdc 100644 --- a/internal/ratelimit/shared_test.go +++ b/internal/ratelimit/shared_test.go @@ -8,6 +8,8 @@ import ( "errors" "fmt" "log/slog" + "net/http" + "net/netip" "strings" "sync" "sync/atomic" @@ -773,3 +775,40 @@ func TestShared_PanicProbeReprobes(t *testing.T) { t.Fatal("6th shared hit allowed — the window bound was lost after recovery") } } + +// TestShared_IPv6WindowAndLocalShareKey pins the single-key invariant +// under the /64 fold: requests ClientKey folds into one IPv6 /64 hit ONE +// Postgres window row AND one local bucket — the two enforcement layers +// can never disagree about which client a hit belongs to, because both +// are keyed by the same canonical string end to end. +func TestShared_IPv6WindowAndLocalShareKey(t *testing.T) { + ws := newFakeWindowStore() + trusted := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")} + l := NewShared(New(6000, 100, 100, nil), New(1, 1, 100, nil), ws, "login", 2, nil, nil, nil) + + mk := func(xff string) *http.Request { + return &http.Request{ + RemoteAddr: "10.1.2.3:443", + Header: http.Header{"X-Forwarded-For": {xff}}, + } + } + k1 := ClientKey(mk("2001:db8::1"), trusted) + k2 := ClientKey(mk("2001:db8::2"), trusted) + if k1 != k2 { + t.Fatalf("/64 fold diverged: %q vs %q", k1, k2) + } + // Both hits land on the single folded window row; the third — + // another address in the same /64 — is refused by the window bound. + if ok, _ := l.Allow(k1); !ok { + t.Fatal("first hit refused") + } + if ok, _ := l.Allow(k2); !ok { + t.Fatal("second hit refused — same /64, same window row") + } + if ok, _ := l.Allow(ClientKey(mk("2001:db8::3"), trusted)); ok { + t.Fatal("third hit inside the /64 allowed past window limit 2") + } + if got := ws.counts["login/"+k1]; got != 3 { + t.Fatalf("window row login/%q counted %d, want 3 — every hit keyed identically", k1, got) + } +} From ac834004966d4be5cc83e10618df4daeb3d244c3 Mon Sep 17 00:00:00 2001 From: Nguyen Van Long Date: Tue, 6 Oct 2026 12:35:57 +0700 Subject: [PATCH 2/2] fix(ratelimit): keep the real client address on the forwarded path (FIX-RLKEY) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Advisor gate: the /64 fold applies to rate-limit keys only. The shared selection splits into two renders — ClientKey (folded key) and the new ClientAddr (unmapped but unfolded real address) — and the gateway's rewriteForwarded now derives headers from ClientAddr, so the runtime and attribution keep the exact client address while billing stays per-/64. Docs state the consequence plainly: hosts sharing one /64 share one rate-limit budget, nothing else. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- deploy/helm/tinycdi/README.md | 6 +++ docs/runbooks/capacity.md | 7 +++- internal/gateway/forwarded_test.go | 44 ++++++++++++++++++++ internal/gateway/proxy.go | 10 ++--- internal/ratelimit/ratelimit.go | 64 +++++++++++++++++++++--------- 5 files changed, 106 insertions(+), 25 deletions(-) diff --git a/deploy/helm/tinycdi/README.md b/deploy/helm/tinycdi/README.md index b86d6ace..63155d6a 100644 --- a/deploy/helm/tinycdi/README.md +++ b/deploy/helm/tinycdi/README.md @@ -518,6 +518,12 @@ 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 diff --git a/docs/runbooks/capacity.md b/docs/runbooks/capacity.md index b4d4b3ac..cee160a7 100644 --- a/docs/runbooks/capacity.md +++ b/docs/runbooks/capacity.md @@ -218,7 +218,12 @@ What that means for sizing: 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 one canonical + 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 diff --git a/internal/gateway/forwarded_test.go b/internal/gateway/forwarded_test.go index a5d6cdd6..da2eb3a6 100644 --- a/internal/gateway/forwarded_test.go +++ b/internal/gateway/forwarded_test.go @@ -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 @@ -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) { diff --git a/internal/gateway/proxy.go b/internal/gateway/proxy.go index 3ce9cd45..4f1c6c00 100644 --- a/internal/gateway/proxy.go +++ b/internal/gateway/proxy.go @@ -829,10 +829,10 @@ 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 — an IPv6 client appears as its /64 base address, so the -// runtime blacklist folds at the same granularity the limiter bills. +// 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 @@ -841,7 +841,7 @@ func (g *Gateway) direct(r *http.Request) { // 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 diff --git a/internal/ratelimit/ratelimit.go b/internal/ratelimit/ratelimit.go index 81e0f4dc..4dc6825f 100644 --- a/internal/ratelimit/ratelimit.go +++ b/internal/ratelimit/ratelimit.go @@ -7,10 +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. Recognized -// client addresses are canonicalised (IPv4-mapped forms unmapped, IPv6 -// folded to its /64) so address-spelling and in-prefix rotation cannot -// multiply one client's budget. +// 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 ( @@ -176,15 +178,33 @@ 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 canonicalised by canon: mapped forms +// 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 canon(peer) + return peer } var leftmost string entries := xffEntries(r) @@ -192,28 +212,23 @@ 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 canon(peer) + return peer } -// canon renders a rate-limit key canonically. A parseable address is -// first Unmap'd, so the IPv4-mapped spellings "::ffff:a.b.c.d" (any -// notation) and the native "a.b.c.d" land on one key — a dual-stack -// client cannot double its budget by switching family spelling. An IPv6 -// address folds to its /64 — rendered as 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 +// 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. The result stays a valid -// address (or the original token) — consumers that re-parse it, like the -// gateway's forwarded-header rebuild, keep working. +// still discriminates one key from another. func canon(s string) string { a, err := netip.ParseAddr(s) if err != nil { @@ -226,6 +241,17 @@ func canon(s string) 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.Unmap().String() + } + return s +} + // inTrusted reports whether s parses as an address inside the trusted // prefixes. A non-IP token is never trusted. func inTrusted(s string, trusted []netip.Prefix) bool {