Commit Graph
2 Commits
Author SHA1 Message Date
lukaszraczylo 3536c32667 fix(auth): let a valid key always win over IP throttle
Fixes critical finding 1 from security review round 2, 2026-08-22:
identify checked the per-source-IP throttle BEFORE verifying the
presented key. This plugin runs behind a Kubernetes Service, and with
externalTrafficPolicy: Cluster (this operator's own deployment),
r.RemoteAddr is the node's own SNAT address shared by every external
caller reaching that node — not a per-tenant address. 20 failed requests
from anywhere on the internet sharing that node locked out all 10 live
users, including the admin dashboard's own poll.

- identify now verifies the key FIRST, unconditionally: a valid key
  always succeeds before any throttle state is read. Every failure path
  still records against the caller's IP, but throttling is now pure
  observability (bounding log volume and tracker memory), never
  enforcement — an attacker with no valid key already got 'false'
  either way, so nothing is lost against brute force.
- Replaces the per-source-IP tracker's backing store: the general-
  purpose memoryStore (limits.go) has no size cap and a full mutex-held
  sweep that scales with tracked-IP count (~83ms at 1M IPs). authFailureTracker
  is a bounded, two-generation, O(1)-rotation structure capped at
  2x50,000 entries (important finding 1).
- logAuthEvent now emits a distinct, separately rate-limited
  'auth throttle:' line the first time a source crosses the limit
  (important finding 5), and both the routine and throttle-engaged
  lines report how many events they suppressed since the last line
  actually written, so several distinct attacker IPs in one window no
  longer collapse into a single indistinguishable line (important
  finding 6).
- The success path no longer touches the throttle tracker's mutex at
  all (important finding 2).
2026-08-22 15:26:35 +01:00
lukaszraczylo 87ebee9003 fix(auth): throttle repeated auth failures, rate-limit failure log
Fixes finding 3 from the 2026-08-22 security audit: authStore.identify
had no counter, no backoff, and no lockout on failed attempts, and
logger.go wrote one unbuffered stderr line per attempt including the
caller's RemoteAddr. An unauthenticated attacker got unlimited digest
guesses at line rate and could drive synchronous stderr writes on the
shared Traefik ingress.

- Add per-source-IP failure throttling to identify, reusing the existing
  in-process memoryStore (limits.go) as authStore's own dedicated
  instance rather than a new dependency: authFailureLimit (20) failed
  attempts per authFailureWindow (1 minute) throttles further attempts
  from that IP outright, including a subsequently valid key, and resets
  once the window elapses.
- clientIP reads r.RemoteAddr only, never a client-supplied header,
  matching the same trust boundary the rest of this package already
  assumes (routes_passthrough.go strips X-Forwarded-*/X-Auth-* before any
  outbound request).
- Rate-limit logAuthEvent's failure line via authStore.shouldLogAuthFailure,
  the identical technique limiter.logStoreError already applies to store
  errors. Successful-auth logging stays unbounded (bounded by legitimate,
  already-authenticated traffic elsewhere).
2026-08-22 15:26:25 +01:00