fix(auth): thread-safe login rate limiter, PBKDF2 key separation, and migration path
Concurrent failed logins raced on _login_attempts because no lock guarded the dict. Add _LOGIN_ATTEMPTS_LOCK and wrap both _check_login_rate() and _record_login_attempt() with it. Extract _load_key() to de-duplicate key file I/O. Add _pbkdf2_key() that loads .pbkdf2_key (separate from .signing_key) so PBKDF2 and HMAC signing no longer share a key — key reuse across cryptographic primitives is unsafe. Update _hash_password() to use _pbkdf2_key() as its default salt, with an optional *salt* kwarg so verify_password() can try the legacy .signing_key salt during transparent migration. When the old hash matches, save_settings() re-hashes with _pbkdf2_key() and _invalidate_password_hash_cache() ensures the next request sees the upgraded hash without a restart. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
109
api/auth.py
109
api/auth.py
@@ -155,58 +155,80 @@ def _save_login_attempts(attempts: dict[str, list[float]]) -> None:
|
||||
|
||||
|
||||
_login_attempts = _load_login_attempts() # ip -> [timestamp, ...]
|
||||
_LOGIN_ATTEMPTS_LOCK = threading.Lock()
|
||||
|
||||
|
||||
def _check_login_rate(ip: str) -> bool:
|
||||
"""Return True if the IP is allowed to attempt login."""
|
||||
now = time.time()
|
||||
attempts = _login_attempts.get(ip, [])
|
||||
# Prune old attempts
|
||||
attempts = [t for t in attempts if now - t < _LOGIN_WINDOW]
|
||||
if attempts:
|
||||
_login_attempts[ip] = attempts
|
||||
else:
|
||||
_login_attempts.pop(ip, None)
|
||||
_save_login_attempts(_login_attempts)
|
||||
return len(attempts) < _LOGIN_MAX_ATTEMPTS
|
||||
"""Return True if the IP is allowed to attempt login (thread-safe)."""
|
||||
with _LOGIN_ATTEMPTS_LOCK:
|
||||
now = time.time()
|
||||
attempts = _login_attempts.get(ip, [])
|
||||
# Prune old attempts
|
||||
attempts = [t for t in attempts if now - t < _LOGIN_WINDOW]
|
||||
if attempts:
|
||||
_login_attempts[ip] = attempts
|
||||
else:
|
||||
_login_attempts.pop(ip, None)
|
||||
_save_login_attempts(_login_attempts)
|
||||
return len(attempts) < _LOGIN_MAX_ATTEMPTS
|
||||
|
||||
|
||||
def _record_login_attempt(ip: str) -> None:
|
||||
now = time.time()
|
||||
attempts = _login_attempts.get(ip, [])
|
||||
attempts.append(now)
|
||||
_login_attempts[ip] = attempts
|
||||
_save_login_attempts(_login_attempts)
|
||||
"""Record a login attempt for rate limiting (thread-safe)."""
|
||||
with _LOGIN_ATTEMPTS_LOCK:
|
||||
now = time.time()
|
||||
attempts = _login_attempts.get(ip, [])
|
||||
attempts.append(now)
|
||||
_login_attempts[ip] = attempts
|
||||
_save_login_attempts(_login_attempts)
|
||||
|
||||
|
||||
def _signing_key():
|
||||
"""Return a random signing key, generating and persisting one on first call."""
|
||||
key_file = STATE_DIR / '.signing_key'
|
||||
def _load_key(filename: str) -> bytes:
|
||||
"""Load a 32-byte key from STATE_DIR, generating and persisting one if missing."""
|
||||
key_file = STATE_DIR / filename
|
||||
try:
|
||||
if key_file.exists():
|
||||
raw = key_file.read_bytes()
|
||||
if len(raw) >= 32:
|
||||
return raw[:32]
|
||||
except Exception:
|
||||
logger.debug("Failed to read or access signing key file, using in-memory key")
|
||||
# Generate a new random key
|
||||
logger.debug("Failed to read key %s", filename)
|
||||
key = secrets.token_bytes(32)
|
||||
try:
|
||||
STATE_DIR.mkdir(parents=True, exist_ok=True)
|
||||
key_file.write_bytes(key)
|
||||
key_file.chmod(0o600)
|
||||
except Exception:
|
||||
logger.debug("Failed to persist signing key, using in-memory key only")
|
||||
logger.debug("Failed to persist key %s", filename)
|
||||
return key
|
||||
|
||||
|
||||
def _hash_password(password):
|
||||
def _pbkdf2_key() -> bytes:
|
||||
"""Salt for password hashing (PBKDF2). Persisted so password hashes remain
|
||||
valid across restarts. Separate from _signing_key to avoid key reuse across
|
||||
different cryptographic primitives."""
|
||||
return _load_key('.pbkdf2_key')
|
||||
|
||||
|
||||
def _signing_key() -> bytes:
|
||||
"""HMAC key for session signing. Persisted so signed cookies remain
|
||||
valid across restarts."""
|
||||
return _load_key('.signing_key')
|
||||
|
||||
|
||||
def _hash_password(password, *, salt: bytes | None = None) -> str:
|
||||
"""PBKDF2-SHA256 with 600k iterations (OWASP recommendation).
|
||||
Salt is the persisted random signing key, which is secret and unique per
|
||||
Salt is the persisted PBKDF2 key, which is secret and unique per
|
||||
installation. This keeps the stored hash format a plain hex string
|
||||
(no format change to settings.json) while replacing the predictable
|
||||
STATE_DIR-derived salt from the original implementation."""
|
||||
salt = _signing_key()
|
||||
STATE_DIR-derived salt from the original implementation.
|
||||
|
||||
The *salt* parameter exists solely to support transparent migration
|
||||
of password hashes that were computed with a different key (e.g. the
|
||||
old `.signing_key`). Normal callers should never pass it.
|
||||
"""
|
||||
if salt is None:
|
||||
salt = _pbkdf2_key()
|
||||
dk = hashlib.pbkdf2_hmac('sha256', password.encode(), salt, 600_000)
|
||||
return dk.hex()
|
||||
|
||||
@@ -216,6 +238,15 @@ _AUTH_HASH_COMPUTED: bool = False
|
||||
_AUTH_HASH_CACHE: str | None = None
|
||||
|
||||
|
||||
def _invalidate_password_hash_cache() -> None:
|
||||
"""Invalidate the in-process password hash cache so the next call to
|
||||
get_password_hash() re-reads from settings.json or the env var."""
|
||||
global _AUTH_HASH_COMPUTED, _AUTH_HASH_CACHE
|
||||
with _AUTH_HASH_LOCK:
|
||||
_AUTH_HASH_COMPUTED = False
|
||||
_AUTH_HASH_CACHE = None
|
||||
|
||||
|
||||
def get_password_hash() -> str | None:
|
||||
"""Return the active password hash, or None if auth is disabled.
|
||||
Priority: env var > settings.json.
|
||||
@@ -258,11 +289,33 @@ def is_auth_enabled() -> bool:
|
||||
|
||||
|
||||
def verify_password(plain) -> bool:
|
||||
"""Verify a plaintext password against the stored hash."""
|
||||
"""Verify a plaintext password against the stored hash.
|
||||
|
||||
Supports transparent migration of password hashes that were computed
|
||||
with the old `.signing_key` salt. When the two keys differ and the
|
||||
legacy-salted hash matches, the password is transparently re-hashed
|
||||
with the current `.pbkdf2_key` and persisted to settings.json.
|
||||
"""
|
||||
expected = get_password_hash()
|
||||
if not expected:
|
||||
return False
|
||||
return hmac.compare_digest(_hash_password(plain), expected)
|
||||
# Fast path: current PBKDF2 key
|
||||
if hmac.compare_digest(_hash_password(plain), expected):
|
||||
return True
|
||||
# Migration: some hashes were computed with `.signing_key` before the
|
||||
# PBKDF2 key was separated. Try the legacy salt; if it matches,
|
||||
# transparently upgrade so the next login uses the fast path.
|
||||
legacy_salt = _signing_key()
|
||||
current_salt = _pbkdf2_key()
|
||||
if legacy_salt != current_salt:
|
||||
if hmac.compare_digest(_hash_password(plain, salt=legacy_salt), expected):
|
||||
from api.config import save_settings
|
||||
|
||||
save_settings({'_set_password': plain})
|
||||
_invalidate_password_hash_cache()
|
||||
get_password_hash()
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def create_session() -> str:
|
||||
|
||||
Reference in New Issue
Block a user