Skip to content

security(auth): aggregate invalid-session throttling beyond exact token identity #1348

Description

@seonghobae

Security boundary

The protected develop authentication path originally keyed failed-session throttling by sha256(full_bearer_token). Repeated attempts with the same invalid token were bounded, but an unauthenticated caller could vary the token bytes and obtain a fresh failure bucket for each request. That made the exact-token throttle insufficient as an aggregate abuse-control boundary for unique invalid bearer tokens.

Verified RCA

The defect was verified before remediation:

  • backend/api/auth.py::_session_auth_failure_key() keyed failures by the complete bearer token;
  • _reject_if_session_auth_rate_limited() and _record_session_auth_failure() therefore counted failures per exact token;
  • test_signed_bearer_session_rate_limits_repeated_invalid_token proved only repeated identical-token throttling;
  • test_signed_bearer_session_failure_buckets_are_bounded proved storage boundedness, not aggregate request throttling.

A Strix run against PR #1297 surfaced this behavior while scanning an unrelated diff. The finding's separate claim that AUTH_SESSION_HMAC_SECRET was protected only by minimum length was not current-source accurate: backend/core/runtime_secrets.py::validate_auth_session_hmac_secret_value() already rejects repeated/public/placeholder secrets and enforces at least 12 distinct characters, three character classes, and 128 bits of estimated Shannon entropy. Do not weaken or replace that existing secret-strength gate.

Required remediation contract

Implement a non-spoofable aggregate failure scope for the HTTP authentication boundary so unique invalid bearer tokens from the same abuse source cannot receive unlimited independent attempts. Preserve the existing exact-token bucket as a secondary control if useful. Do not trust arbitrary client-supplied forwarding headers as the sole rate-limit identity. Account/subject claims extracted before signature verification are untrusted and must not become authorization or bypass authority.

The design must explicitly address shared reverse-proxy/NAT behavior, lockout/DoS risk, bounded memory, expiry, successful-auth reset semantics, and the direct non-HTTP build_auth_context() contract. Add a realistic RED regression proving that varying invalid tokens from one trusted request scope hit one aggregate budget, while independent scopes remain isolated and a valid session is not turned into an attacker-controlled reset primitive.

Active implementation status

PR #1321 is the canonical auth lane and was observed carrying the bounded remediation on head 2790a7edff5f5ed29a6a7aedcd398bc0d5ef7c06 against protected develop@bc98789521d21271e84789888413c182aa111b4d.

The branch keeps the exact-token failure budget and adds a coarser server-observed HTTP peer budget derived from ASGI request.client.host; application code does not use caller-controlled Forwarded or X-Forwarded-For values for that scope. The peer budget is deliberately looser than the exact-token budget for NAT/reverse-proxy tolerance, uses the existing bounded expiry/capacity storage, is isolated between observed peers, and is not reset by a successful bearer token. Direct non-HTTP build_auth_context() calls retain exact-token-only semantics. Focused realistic tests cover varying invalid tokens, spoofed forwarding headers, independent peer scopes, successful-auth reset behavior, and the direct-call boundary.

This issue remains open until the remediation is actually shipped to protected develop. Merge-governance correction: repository ruleset 15586698 by itself requires zero approving reviews, but repository ruleset 17214772 and inherited organization ruleset 18156473 are also active on the default branch and each requires one approving review, stale-review dismissal, approval after the last push, and review-thread resolution. The effective live contract therefore still requires a qualifying independent post-last-push approval, plus exact-head status checks and required workflows; no active ruleset currently requires CODEOWNER review. Issue #1371 remains open because the current collaborator inventory exposes only seonghobae, so the independent human approval route is not presently verifiable from this repository writer's authority. Refetch #1321's exact head/base, formal reviews/threads, required checks, workflows/jobs, and every active ruleset immediately before any merge decision.

Standards / references

  • NIST SP 800-63B-4 §3.2.2 requires controls against online guessing and discusses IP address and other risk signals as possible adaptive techniques; its failed-attempt counters are scoped to the authenticator/subscriber rather than individual guessed values.
  • RFC 7519 defines JWT/JWS bearer-token structure and HS256 use; unverified claims are not trusted identity.

Delivery

Keep #1321 as the canonical implementation lane unless a fresh exact-head comparison proves otherwise. Close this issue only after the unchanged implementation is merged to the protected branch with all live checks, review threads, required workflows, and other applicable protected-branch controls satisfied together.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: apiAPI, protocol, event, or external contractarea: authAuthentication, authorization, identity, or tenant isolationarea: ci-cdCI, GitHub Actions, checks, release, or supply chainarea: securitySecurity boundary, hardening, or vulnerability preventionbugSomething isn't workingpriority: highHigh-priority or P1 workstatus: triagedOpen issue has an organization taxonomy assignmenttype: securitySecurity vulnerability or security-specific remediation

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions