Skip to content

[Security] Pre-auth PBKDF2 DoS: 100k-iteration hash for any garbage x-api-key stalls event loop / exhausts worker pool #333

Description

Audit findings: BUG-R2-S2-A1-H1 + BUG-R2-S2-A1-H3 | CWE-400 | CVSS 7.5 (AV:N/AC:L/PR:N, A:H) | PoC: pending (mechanism verified in code)

Summary

hash_api_key (auth/security.py:91-96) is PBKDF2-HMAC-SHA256 with 100,000 iterations (~67 ms measured), used purely as a deterministic lookup index over 258-bit random keys. The KDF cost buys no brute-force resistance there, but it sits on the hottest unauthenticated code path:

  1. Event loop surface: get_current_tenant is async def and calls hash_api_key synchronously (tenant_context.py:33) — FastAPI awaits it inline on the single event-loop thread. Every request with any non-empty x-api-key (valid or garbage) pays the full KDF before the 401. All rate limiting is post-auth inside handler bodies, and failed auth never increments any bucket. ~15 req/s of garbage keys saturates the single-process service (~30 req/s for the 2-replica k8s deployment) — all tenants, all endpoints, /health probes -> crash-loop. get_api_key (auth/middleware.py:29) exposes the identical surface for /admin/*.
  2. Worker-pool surface: get_optional_api_key_record (api/main.py:181) is a plain def, so FastAPI runs it in the shared anyio threadpool (40 default tokens) — the same pool that runs get_session for every business route. A garbage-key flood of GET /metrics exhausts the pool service-wide; the GIL-free hashes additionally burn up to 40 host cores.

Important fix note

Moving the KDF off the loop (run_in_threadpool) converts surface 1 into surface 2. The expensive KDF must leave the unauthenticated lookup path entirely.

Suggested fix

Replace the lookup digest with a fast keyed MAC — hmac.new(SECRET_KEY + b":qwed_api_key_lookup", key, sha256) — microsecond cost, ample collision resistance for equality lookup of 258-bit tokens. Existing key_hash rows must be re-issued once via the rotation path; do not add a PBKDF2 fallback for legacy rows (re-introduces the bug). Defense in depth: pre-auth per-IP rate limiting middleware (see #312), uvicorn --limit-concurrency.

Code anchors

File Line Symbol
src/qwed_new/auth/security.py 91-96 hash_api_key (PBKDF2, 100k iters)
src/qwed_new/core/tenant_context.py 24/33 async get_current_tenant -> inline KDF
src/qwed_new/auth/middleware.py 16/29 async get_api_key -> inline KDF
src/qwed_new/api/main.py 181/204 get_optional_api_key_record -> threadpool
src/qwed_new/core/rate_limiter.py 39 no per-IP keying

Related: #223 (inconsistent credential hashing), #312 (pre-auth middleware).


Source: OpenVuln external audit of QWED-AI/qwed-verification (snapshot v7.0.0, re-verified against current main v7.1.0). All code anchors below were spot-checked against current main before filing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P0Critical priority — blocks trust boundary completionarchitecturearchitecturebugSomething isn't workingexternal-auditFindings from external security auditsqwed securityqwed securitysecuritySecurity finding from adversarial audit

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions