From e1abac12714a22acac082558b8d7c1a203a72231 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:18:40 +0100 Subject: [PATCH] feat(multiplayer): enforce account and IP rate limits --- multiplayer-next.md | 2 +- server/api/rate_limit.go | 71 ++++++++++++++++++++++++++--------- server/api/rate_limit_test.go | 20 ++++++++++ server/api/service.go | 2 +- 4 files changed, 76 insertions(+), 19 deletions(-) diff --git a/multiplayer-next.md b/multiplayer-next.md index a423922d..31faa6dc 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -1191,7 +1191,7 @@ the local/CI/community transport, not a silent production fallback. | 8.9 `[D:8.4,8.7]` | **IN PROGRESS.** Pure Go join policy binds SteamID/player/match/server/team/slot/protocol/expiry, rejects duplicate roster slots, permits same-identity reclaim through backend loss and fences prior server-owned generations | `server/domain/reconnect.go` covers SteamID/server/slot binding, expiry, repeated reclaim, grace boundary and old-generation fencing; signed token issuance/verification, persistent leases, Godot `hello` transport and production integration remain | | 8.10 `[D:8.5,8.31]` | **IN PROGRESS.** Dependency-free workload credential policy validates an adapter-verified signature, issuer/audience/time bounds, namespace/service account, pod UID, GameServer UID, allocation ID, match ID and server ID before result submission; `server/workload` now parses projected compact JWTs, rejects unsigned/malformed/ambiguous tokens, verifies signing input through an injected trust callback, and delegates exact binding to the domain policy; the API exposes the workload-authenticated server result route. **`WorkloadVerify` is now wired for real** -- the blocker this row previously named (`cmd/control-plane/main.go` never wiring it, so `/register` and `/result` 503'd on every real request) is closed by taking a different, equally-valid approach instead of the Kubernetes-JWT one: `serverMutation` only ever compares `WorkloadBinding.ServerID`/`.MatchID` (and `AdvanceServerRegistration` only additionally needs `.AllocationID`) -- nothing downstream requires Namespace/ServiceAcct/PodUID/GameServerUID, so a Kubernetes-issued token was never actually required, just the assumption that it was. `server/workload/signed_token.go` mints and verifies a short-lived HMAC-signed token with a secret only the control plane holds (the same trust model `domain.SessionStore` already uses for player sessions), needing no cluster JWKS/TokenReview to validate -- signature and expiry are fully self-contained. The token binds only `allocation_id` (deliberately, not `match_id`/`server_id` too: it must be requestable in the same Agones request that asks for a server, before Agones has picked one) -- `store.AllocationBindingByAllocationID` resolves `match_id`/`server_id` durably from the `allocations` table at verify time, so a token can never claim a pairing that wasn't actually, durably allocated. `api.WorkloadVerifierFromSignedToken` combines both and is wired into `cmd/control-plane` (`--workload-secret`/`COSMIC_CLASH_WORKLOAD_SECRET`; a startup warning fires and the route stays 503 if it's left unset) and `cmd/testkit-api` (fixed test secret). **The delivery channel is now wired too**: `agones.Client` gains `WorkloadSecret`/`WorkloadTokenTTL` -- when set, `Allocate` mints a token for the allocation and requests it as a third `cosmic-clash.io/workload-token` annotation alongside match-id/allocation-id, wired from `cmd/allocator`'s own `--workload-secret`/`COSMIC_CLASH_WORKLOAD_SECRET` (must match the control plane's); `supervisor.Supervisor.workloadToken()` resolves the bearer credential with an explicit `--workload-token-path` (kept for a possible future Kubernetes-JWT path) always winning, otherwise falling back to that same annotation -- the same fallback pattern `matchID()` already used for `cosmic-clash.io/match-id`. `WorkloadTokenPath` is accordingly no longer required at construction time | `server/domain/workload.go`, `server/workload/jwt.go`, `server/api/service.go` and adversarial tests reject every binding mutation, missing/unverified signature, `none`/malformed JWT, ambiguous audience, server/match mismatch and time boundary; `server/testkit/pipeline_test.go` carries allocation identity through the offline result path; `cmd/control-plane` now wires `ResultSubmitter: store.PostgresResults{DB: db}` (a ready-made adapter that had been referenced from nowhere at all, not even a test); `server/workload/signed_token_test.go` covers round-trip, tampered payload, wrong secret, malformed input and the exact expiry boundary; `server/api/workload_verifier_integration_test.go` (opt-in, real PostgreSQL) covers acceptance against a real allocation row, rejection of an unrecorded allocation, that two distinct real allocations each resolve to their own and only their own match/server pairing, and the previously-503 `Service.WorkloadVerify` field itself now succeeding -- all verified clean with `-race` across multiple runs; `cmd/control-plane/main_test.go`'s `TestServerRoutesRequireWorkloadVerifyToBeWired` now documents and pins the misconfigured-secret case specifically, not the "always unwired" case; `server/agones/allocation_test.go`'s `TestAllocateRequestsAWorkloadTokenAnnotationWhenConfigured` and `server/supervisor/supervisor_test.go`'s new annotation-fallback/fails-closed pair cover the delivery channel end to end (mint -> annotation -> supervisor read -> Authorization header) short of a live cluster. **What's still missing**: this has never run against a real Agones cluster (only HTTP-level fakes), so the exact `object_meta` JSON casing Agones actually returns remains unverified from this sandbox (see `GameServer.ObjectMeta`'s existing caveat); live duplicate/conflict alerting also remains | | 8.11 `[D:8.1]` | **DONE.** Write the threat model: forged clients/replay/queues/results, floods/bots, pod/insider compromise, gameplay and API DDoS, SDR signing-key theft, dependencies and denial-of-wallet | [`docs/THREAT-MODEL.md`](docs/THREAT-MODEL.md) records prevention, detection/response, owner and residual risk for every threat; it separates offline CA/online signer, client/game-server/PostgreSQL/Redis trust boundaries and recovery behavior | -| 8.12 `[D:8.11]` | **IN PROGRESS.** Provider-neutral Kubernetes baseline enforces restricted namespace admission, non-root/read-only/no-capability workloads, separate service accounts, allocator-only RBAC, default-deny networking and explicit edge/data/DNS/Agones/metrics flows; application manifests consume externally populated Secret objects; the Go API and allocator now have bounded per-replica rate/quota boundaries and metrics endpoints, plus an atomic operator-controlled degraded-mode gate that sheds new login/queue/proposal work while preserving live-match paths | `deploy/k8s/base/` includes hardened control-plane, allocator, and game-server resources; `server/security/test_kubernetes_policies.py`, `server/api/rate_limit.go`, `server/api/admission.go`, allocator metrics tests and adversarial checks cover static hardening, secret-reference invariants, fixed-window limits, bounded labels/key memory, route classification, concurrent toggling and degraded responses; production `cmd/control-plane` now wires the bounded limiter by default with `--rate-limit`, `--rate-limit-window`, and `--rate-limit-max-keys`, and the base Deployment declares those defaults explicitly; start with control-plane `--degraded`, enable with SIGUSR1, disable with SIGUSR2; private-store provisioning, distributed/global quotas, edge DDoS/WAF/origin shielding, WebSocket limits, encrypted backups and live policy/load tests remain | +| 8.12 `[D:8.11]` | **IN PROGRESS.** Provider-neutral Kubernetes baseline enforces restricted namespace admission, non-root/read-only/no-capability workloads, separate service accounts, allocator-only RBAC, default-deny networking and explicit edge/data/DNS/Agones/metrics flows; application manifests consume externally populated Secret objects; the Go API and allocator now have bounded per-replica account+IP rate/quota boundaries and metrics endpoints, plus an atomic operator-controlled degraded-mode gate that sheds new login/queue/proposal work while preserving live-match paths | `deploy/k8s/base/` includes hardened control-plane, allocator, and game-server resources; `server/security/test_kubernetes_policies.py`, `server/api/rate_limit.go`, `server/api/admission.go`, allocator metrics tests and adversarial checks cover static hardening, secret-reference invariants, fixed-window limits, bounded labels/key memory, atomic account+IP charging, route classification, concurrent toggling and degraded responses; production `cmd/control-plane` now wires the bounded limiter by default with `--rate-limit`, `--rate-limit-window`, and `--rate-limit-max-keys`, and the base Deployment declares those defaults explicitly; start with control-plane `--degraded`, enable with SIGUSR1, disable with SIGUSR2; private-store provisioning, distributed/global quotas, edge DDoS/WAF/origin shielding, WebSocket limits, encrypted backups and live policy/load tests remain | | 8.13 `[D:8.12]` | **IN PROGRESS.** Docker/Kubernetes references are digest-pinned, a dependency-free checker rejects mutable tags/plaintext credentials and concrete release overlays can reject template digests; release documentation defines SBOM, dependency/image scanning, signing, admission verification and a 24-hour critical-fix SLA | `scripts/verify_supply_chain.py`, `server/security/test_supply_chain.py`, `docs/SUPPLY-CHAIN.md` and `.github/workflows/supply-chain.yml` cover repository policy and provenance requirements; registry SBOM/scan/sign/admission execution and a concrete production overlay remain | #### 8C — Queueing, matchmaking, playlists and rating diff --git a/server/api/rate_limit.go b/server/api/rate_limit.go index 9a83a594..33f737d2 100644 --- a/server/api/rate_limit.go +++ b/server/api/rate_limit.go @@ -35,7 +35,14 @@ func NewRateLimiter(limit int, window time.Duration, maxKeys int) (*RateLimiter, } func (l *RateLimiter) Allow(key string, now time.Time) bool { - if l == nil || key == "" || now.IsZero() { + return l.AllowKeys([]string{key}, now) +} + +// AllowKeys atomically charges every non-empty key for a request. This lets +// the HTTP boundary enforce both the authenticated credential and source IP +// limits without charging one dimension when the other dimension rejects. +func (l *RateLimiter) AllowKeys(keys []string, now time.Time) bool { + if l == nil || now.IsZero() { return false } l.mu.Lock() @@ -45,37 +52,67 @@ func (l *RateLimiter) Allow(key string, now time.Time) bool { delete(l.entries, storedKey) } } - entry, exists := l.entries[key] - if !exists { - if len(l.entries) >= l.maxKeys { - return false + unique := make([]string, 0, len(keys)) + seen := make(map[string]struct{}, len(keys)) + for _, key := range keys { + if key == "" { + continue } - l.entries[key] = rateWindow{started: now, count: 1} - return true + if _, exists := seen[key]; exists { + continue + } + seen[key] = struct{}{} + unique = append(unique, key) } - if !now.Before(entry.started.Add(l.window)) { - l.entries[key] = rateWindow{started: now, count: 1} - return true - } - if entry.count >= l.limit { + if len(unique) == 0 { return false } - entry.count++ - l.entries[key] = entry + newKeys := 0 + for _, key := range unique { + entry, exists := l.entries[key] + if !exists { + newKeys++ + continue + } + if now.Before(entry.started.Add(l.window)) && entry.count >= l.limit { + return false + } + } + if len(l.entries)+newKeys > l.maxKeys { + return false + } + for _, key := range unique { + entry, exists := l.entries[key] + if !exists || !now.Before(entry.started.Add(l.window)) { + l.entries[key] = rateWindow{started: now, count: 1} + continue + } + entry.count++ + l.entries[key] = entry + } return true } func requestRateKey(r *http.Request) string { + keys := requestRateKeys(r) + if len(keys) == 0 { + return "" + } + return keys[0] +} + +func requestRateKeys(r *http.Request) []string { + keys := make([]string, 0, 2) if authorization := strings.TrimSpace(r.Header.Get("Authorization")); authorization != "" { digest := sha256.Sum256([]byte(authorization)) - return "auth:" + hex.EncodeToString(digest[:]) + keys = append(keys, "auth:"+hex.EncodeToString(digest[:])) } host := r.RemoteAddr if parsedHost, _, err := net.SplitHostPort(host); err == nil { host = parsedHost } if host == "" { - return "" + return keys } - return "ip:" + host + return append(keys, "ip:"+host) } diff --git a/server/api/rate_limit_test.go b/server/api/rate_limit_test.go index 2d15a3c1..187785b6 100644 --- a/server/api/rate_limit_test.go +++ b/server/api/rate_limit_test.go @@ -28,6 +28,26 @@ func TestRateLimiterEnforcesWindowAndBoundsKeyMemory(t *testing.T) { } } +func TestRateLimiterChargesCredentialAndIPDimensionsAtomically(t *testing.T) { + limiter, err := NewRateLimiter(1, time.Minute, 8) + if err != nil { + t.Fatal(err) + } + start := time.Unix(1000, 0) + if !limiter.AllowKeys([]string{"auth:player", "ip:one"}, start) { + t.Fatal("first request was rejected") + } + if limiter.AllowKeys([]string{"auth:player", "ip:two"}, start) { + t.Fatal("same credential bypassed the account dimension by changing IP") + } + if limiter.AllowKeys([]string{"auth:other", "ip:one"}, start) { + t.Fatal("same IP bypassed the IP dimension by changing credential") + } + if !limiter.AllowKeys([]string{"auth:other", "ip:two"}, start) { + t.Fatal("unrelated credential/IP pair was charged by a rejected request") + } +} + func TestRateLimitedHTTPBoundaryReturnsGeneric429(t *testing.T) { limiter, err := NewRateLimiter(1, time.Minute, 8) if err != nil { diff --git a/server/api/service.go b/server/api/service.go index 39a70a73..22079758 100644 --- a/server/api/service.go +++ b/server/api/service.go @@ -207,7 +207,7 @@ func (s *Service) Handler() http.Handler { var handler http.Handler = mux if s.RateLimiter != nil { handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !s.RateLimiter.Allow(requestRateKey(r), s.now()) { + if !s.RateLimiter.AllowKeys(requestRateKeys(r), s.now()) { writeError(w, http.StatusTooManyRequests, "rate_limited") return }