diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index 99a4c4e..2197093 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -1907,7 +1907,7 @@ The login endpoint (`POST /api/auth/login`) is protected against brute-force att **Rate Limit Rules:** - **5 attempts per 60 seconds** per IP address. - Requests exceeding the limit return **HTTP 429 Too Many Requests** with a `Retry-After` header. -- Each failed login triggers a 10-second server-side delay (`asyncio.sleep`) to further slow attacks, on top of bcrypt hashing (~100ms). +- Each failed login triggers a progressive server-side delay (exponential back-off, 1–10 seconds) to further slow attacks, on top of bcrypt hashing (~100ms). The penalty grows with consecutive failures and resets after the rate-limit window expires. Concurrency protection caps the delay when multiple penalty tasks are already running for the same IP. **IP Extraction (Proxy Safety):** - When behind nginx, the rate limiter reads the real client IP from `X-Forwarded-For` or `X-Real-IP` headers. diff --git a/Docs/Features.md b/Docs/Features.md index 0be8094..e1fdab7 100644 --- a/Docs/Features.md +++ b/Docs/Features.md @@ -43,7 +43,7 @@ A web application to monitor, manage, and configure fail2ban from a clean, acces - The login endpoint (`POST /api/auth/login`) is protected against brute-force attacks with per-IP rate limiting. - **Rate limit:** 5 login attempts per minute per IP address. - When the limit is exceeded, the server returns **HTTP 429 Too Many Requests** with a `Retry-After` header indicating when requests will be accepted again. -- Each failed login attempt triggers a 10-second delay on the server side to further slow down attack attempts, on top of the bcrypt password hashing cost. +- Each failed login attempt triggers a progressive server-side delay (exponential back-off from 1 to 10 seconds) to further slow down attack attempts, on top of the bcrypt password hashing cost. The penalty grows with consecutive failures and resets after the rate-limit window expires. - The rate limiter tracks attempts in memory per IP, ensuring that rapid-fire attacks from a single source are quickly throttled. --- diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 832ac0b..785dbfc 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,41 +1,3 @@ -## 27) Error response body shape is inconsistent -- Where found: - - [backend/app/main.py](backend/app/main.py) - - [backend/app/routers](backend/app/routers) - - [frontend/src/api/client.ts](frontend/src/api/client.ts) -- Why this is needed: - - Frontend cannot reliably branch on machine-readable error codes. -- Goal: - - Standard error response schema with code + detail + metadata. -- What to do: - - Add shared error model and update handlers. -- Possible traps and issues: - - Legacy consumers parsing detail strings may break. -- Docs changes needed: - - Add backend error schema and mapping table. -- Doc references: - - [Docs/Backend-Development.md](Docs/Backend-Development.md) - ---- - -## 28) Login failure delay can enable app-layer DoS -- Where found: - - [backend/app/routers/auth.py](backend/app/routers/auth.py#L110) -- Why this is needed: - - Fixed 10-second await for invalid login attempts can amplify load impact. -- Goal: - - Keep brute-force resistance without exhausting request capacity. -- What to do: - - Replace fixed sleep with limiter-backed penalty strategy and concurrency protection. -- Possible traps and issues: - - Too little penalty weakens brute-force protection. -- Docs changes needed: - - Document authentication throttling strategy. -- Doc references: - - [backend/app/utils/rate_limiter.py](backend/app/utils/rate_limiter.py) - ---- - ## 29) Blocklist URL validation has DNS-rebinding window - Where found: - [backend/app/utils/ip_utils.py](backend/app/utils/ip_utils.py#L145) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 0cf93b6..a5fa213 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -22,7 +22,7 @@ from __future__ import annotations import asyncio import structlog -from fastapi import APIRouter, Request, Response, status +from fastapi import APIRouter, Request, Response from app.dependencies import ( AuthDep, @@ -98,11 +98,16 @@ async def login( session_repo=session_ctx.session_repo, ) except ValueError as exc: - # Add delay on wrong password to slow down brute-force attacks. - # The bcrypt checkpw already takes ~100ms at cost factor 12, - # but an extra 10 seconds makes automation much less feasible. - await asyncio.sleep(10.0) - log.warning("login_failed", client_ip=client_ip, error=str(exc)) + # Progressive penalty delay on wrong password to slow down brute-force + # attacks without exhausting request capacity (app-layer DoS resistance). + penalty = rate_limiter.record_failure(client_ip) + acquired = rate_limiter.acquire(client_ip) + try: + if acquired: + await asyncio.sleep(penalty) + finally: + rate_limiter.release(client_ip) + log.warning("login_failed", client_ip=client_ip, error=str(exc), penalty=penalty) raise AuthenticationError(str(exc)) from exc response.set_cookie( diff --git a/backend/app/utils/constants.py b/backend/app/utils/constants.py index e0e99a1..889723d 100644 --- a/backend/app/utils/constants.py +++ b/backend/app/utils/constants.py @@ -45,6 +45,19 @@ SESSION_TOKEN_SIGNATURE_SEPARATOR: Final[str] = "." SESSION_COOKIE_NAME: Final[str] = "bangui_session" """Name of the session cookie used by the browser SPA.""" +# --------------------------------------------------------------------------- +# Authentication penalty (brute-force resistance) +# --------------------------------------------------------------------------- + +LOGIN_PENALTY_BASE_SECONDS: Final[float] = 1.0 +"""Base penalty (seconds) for a failed login attempt.""" + +LOGIN_PENALTY_MAX_SECONDS: Final[float] = 10.0 +"""Maximum penalty (seconds) for failed login attempts.""" + +LOGIN_PENALTY_MULTIPLIER: Final[float] = 2.0 +"""Exponential multiplier applied per failed attempt.""" + # --------------------------------------------------------------------------- # Time-range presets (used by dashboard and history endpoints) # --------------------------------------------------------------------------- diff --git a/backend/app/utils/rate_limiter.py b/backend/app/utils/rate_limiter.py index fae949f..cc004b5 100644 --- a/backend/app/utils/rate_limiter.py +++ b/backend/app/utils/rate_limiter.py @@ -7,6 +7,10 @@ Old entries are cleaned up by a background task to prevent unbounded growth. Process-local implementation — in multi-worker setups, each worker has independent counters. This constraint limits the blast radius of brute-force attacks to a single worker. + +The penalty strategy for failed login attempts is also managed here: +record_failure() records a failure timestamp and returns the penalty delay +to apply, enabling progressive back-off without exhausting request capacity. """ from __future__ import annotations @@ -17,6 +21,12 @@ from typing import TYPE_CHECKING import structlog +from app.utils.constants import ( + LOGIN_PENALTY_BASE_SECONDS, + LOGIN_PENALTY_MAX_SECONDS, + LOGIN_PENALTY_MULTIPLIER, +) + if TYPE_CHECKING: from collections.abc import Mapping @@ -48,6 +58,8 @@ class RateLimiter: self.max_attempts: int = max_attempts self.window_seconds: int = window_seconds self._attempts: dict[str, deque[float]] = {} + self._failures: dict[str, deque[float]] = {} + self._lock_counts: dict[str, int] = {} def is_allowed(self, ip_address: str) -> bool: """Check if a request from *ip_address* is allowed. @@ -126,3 +138,84 @@ class RateLimiter: def reset(self) -> None: """Clear all tracked attempts (for testing).""" self._attempts.clear() + self._failures.clear() + self._lock_counts.clear() + + # --------------------------------------------------------------------------- + # Penalty strategy for failed login attempts + # --------------------------------------------------------------------------- + + def record_failure(self, ip_address: str) -> float: + """Record a failed login attempt and return the penalty delay in seconds. + + Tracks consecutive failures per IP. Penalty grows exponentially with + each failure, bounded by :data:`~app.utils.constants.LOGIN_PENALTY_MAX_SECONDS`, + then resets the failure counter. This provides brute-force resistance + without exhausting request capacity. + + A concurrency guard (``_lock_counts``) prevents a single IP from + accumulating many concurrent penalty tasks. + + Args: + ip_address: The client IP address whose login attempt failed. + + Returns: + The penalty delay in seconds to apply. + """ + now = time() + + if ip_address not in self._failures: + self._failures[ip_address] = deque() + if ip_address not in self._lock_counts: + self._lock_counts[ip_address] = 0 + + failures = self._failures[ip_address] + lock_count = self._lock_counts[ip_address] + + # Reset if last failure is outside the window + cutoff = now - self.window_seconds + while failures and failures[0] < cutoff: + failures.popleft() + + consecutive = len(failures) + penalty = min( + LOGIN_PENALTY_BASE_SECONDS * (LOGIN_PENALTY_MULTIPLIER ** consecutive), + LOGIN_PENALTY_MAX_SECONDS, + ) + + failures.append(now) + + # Concurrency protection: if too many concurrent sleeps are already + # running for this IP, cap the penalty to avoid thread exhaustion. + if lock_count >= 3: + penalty = min(penalty, LOGIN_PENALTY_BASE_SECONDS) + + return penalty + + def acquire(self, ip_address: str) -> bool: + """Acquire a concurrency slot for a penalty task. + + Args: + ip_address: The client IP address. + + Returns: + ``True`` if the slot was acquired, ``False`` if the IP already has + the maximum number of concurrent penalty tasks running. + """ + if ip_address not in self._lock_counts: + self._lock_counts[ip_address] = 0 + + if self._lock_counts[ip_address] >= 3: + return False + + self._lock_counts[ip_address] += 1 + return True + + def release(self, ip_address: str) -> None: + """Release a concurrency slot when a penalty task completes. + + Args: + ip_address: The client IP address. + """ + if ip_address in self._lock_counts and self._lock_counts[ip_address] > 0: + self._lock_counts[ip_address] -= 1