Merge pull request #810 from seapagan/fix/timing-attacks

Security: fix timing attacks and token validation vulnerabilities

Author: seapagan

.pre-commit-config.yaml

@@ -11,7 +11,7 @@ repos:
       - id: check-added-large-files
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.14.10
+    rev: v0.14.11
     hooks:
       - id: ruff
         args: ["--output-format=concise"]
@@ -31,7 +31,7 @@ repos:
 
   - repo: https://github.com/astral-sh/uv-pre-commit
     # uv version.
-    rev: 0.9.18
+    rev: 0.9.22
     hooks:
       # Update the uv lockfile
       - id: uv-lock

README.md

@@ -134,6 +134,9 @@ following advantages to starting your own from scratch :
   applied to login (5/15min), registration (3/hour), password recovery (3/hour),
   and other auth routes. Returns HTTP 429 with `Retry-After` header when limits
   exceeded. Violations tracked via Prometheus metrics when enabled.
+- **Timing attack protection** in authentication flows. Login operations use
+  constant-time password verification to prevent user enumeration, and token
+  validation uses constant-time comparisons for defense-in-depth.
 - **A command-line admin tool**. This allows to configure the project metadata
   very easily, add users (and make admin), and run a development server. This
   can easily be modified to add your own functionality (for example bulk add

SECURITY-REVIEW.md

@@ -116,6 +116,11 @@
 
 ### 6. Timing Attack in Login - User Enumeration
 
+> [!NOTE]
+> ✅ **Done**: Login now uses a precomputed dummy hash to ensure password
+> verification always occurs, maintaining consistent response times regardless
+> of user existence.
+
 **Location**: `app/managers/user.py:172-199`
 
 - **Issue**: Different execution paths leak information through timing:
@@ -136,6 +141,12 @@
 
 ### 7. Timing Attack in Token Type Checking
 
+> [!NOTE]
+> ✅ **Done**: Token type comparisons now use `secrets.compare_digest()` for
+> constant-time comparison. Note: The timing difference (nanoseconds) is
+> negligible compared to network jitter (milliseconds), making this purely
+> defense-in-depth rather than addressing a realistic threat vector.
+
 **Location**: `app/managers/auth.py:191, 265, 380`
 
 - **Issue**: String comparison `if payload["typ"] != "refresh"` may be vulnerable
@@ -208,6 +219,12 @@
 
 ### 12. KeyError Throws 500 Instead of 401
 
+> [!NOTE]
+> ✅ **Done**: All JWT claim accesses now use `payload.get()` with explicit
+> None checks in all token validation flows (refresh, verify, reset,
+> get_jwt_user). Malformed tokens missing 'sub' or 'typ' claims now properly
+> return 401 Unauthorized instead of 500 Internal Server Error.
+
 **Location**: `app/managers/auth.py:191, 265, 380`
 
 - **Issue**: `payload["typ"]` / `payload["sub"]` are accessed directly in
@@ -446,6 +463,13 @@
 
 ### 30. Timing Attack in API Key Validation
 
+> [!NOTE]
+> **Not fixing**: The prefix (`"fta_"`) is public, documented information that
+> provides no security value. Timing differences (nanoseconds) are completely
+> buried by network jitter. The actual secret (32-byte random key) is compared
+> via HMAC hash lookup in the database. No security benefit from constant-time
+> prefix comparison.
+
 **Location**: `app/managers/api_key.py:136-137`
 
 - **Issue**: String prefix comparison `if not raw_key.startswith(cls.KEY_PREFIX)`
@@ -568,7 +592,7 @@
 | Priority     | Count         | Must Fix Before Production?         |
 |--------------|---------------|-------------------------------------|
 | **CRITICAL** | 5 (4 closed)  | ✅ YES - Security vulnerabilities   |
-| **High**     | 9 (0 closed)  | ✅ YES - Important security/quality |
+| **High**     | 9 (3 closed)  | ✅ YES - Important security/quality |
 | **Medium**   | 14 (0 closed) | ⚠️ Recommended - Hardening needed   |
 | **Low**      | 5 (0 closed)  | 💡 Optional - Nice to have          |
 
@@ -600,12 +624,12 @@ rate limiting, token validation, and API key scope enforcement.
 
 ### Sprint 2 - High Priority (Next Week)
 
-1. **Fix timing attacks** (#6, #7) - Login and token validation
+1. ✅ **Fix timing attacks** (#6, #7) - Login and token validation
 2. **Implement token revocation** (#8) - Add jti claims + blacklist
 3. **Add database index** (#16) - `api_key.user_id`
 4. **Refactor token encoding** (#15) - Remove code duplication
 5. **Fix password reset reuse** (#9) - One-time tokens
-6. **Add KeyError protection** (#12) - Use `payload.get()`
+6. ✅ **Add KeyError protection** (#12) - Use `payload.get()`
 7. **Add JWT format guards** (#13) - to `get_jwt_user()`
 
 ### Sprint 3 - Hardening (Next 2 Weeks)