## 28) Login failure delay can enable app-layer DoS
This commit is contained in:
@@ -1907,7 +1907,7 @@ The login endpoint (`POST /api/auth/login`) is protected against brute-force att
|
|||||||
**Rate Limit Rules:**
|
**Rate Limit Rules:**
|
||||||
- **5 attempts per 60 seconds** per IP address.
|
- **5 attempts per 60 seconds** per IP address.
|
||||||
- Requests exceeding the limit return **HTTP 429 Too Many Requests** with a `Retry-After` header.
|
- 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):**
|
**IP Extraction (Proxy Safety):**
|
||||||
- When behind nginx, the rate limiter reads the real client IP from `X-Forwarded-For` or `X-Real-IP` headers.
|
- When behind nginx, the rate limiter reads the real client IP from `X-Forwarded-For` or `X-Real-IP` headers.
|
||||||
|
|||||||
@@ -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.
|
- 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.
|
- **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.
|
- 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.
|
- The rate limiter tracks attempts in memory per IP, ensuring that rapid-fire attacks from a single source are quickly throttled.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
@@ -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
|
## 29) Blocklist URL validation has DNS-rebinding window
|
||||||
- Where found:
|
- Where found:
|
||||||
- [backend/app/utils/ip_utils.py](backend/app/utils/ip_utils.py#L145)
|
- [backend/app/utils/ip_utils.py](backend/app/utils/ip_utils.py#L145)
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ from __future__ import annotations
|
|||||||
import asyncio
|
import asyncio
|
||||||
|
|
||||||
import structlog
|
import structlog
|
||||||
from fastapi import APIRouter, Request, Response, status
|
from fastapi import APIRouter, Request, Response
|
||||||
|
|
||||||
from app.dependencies import (
|
from app.dependencies import (
|
||||||
AuthDep,
|
AuthDep,
|
||||||
@@ -98,11 +98,16 @@ async def login(
|
|||||||
session_repo=session_ctx.session_repo,
|
session_repo=session_ctx.session_repo,
|
||||||
)
|
)
|
||||||
except ValueError as exc:
|
except ValueError as exc:
|
||||||
# Add delay on wrong password to slow down brute-force attacks.
|
# Progressive penalty delay on wrong password to slow down brute-force
|
||||||
# The bcrypt checkpw already takes ~100ms at cost factor 12,
|
# attacks without exhausting request capacity (app-layer DoS resistance).
|
||||||
# but an extra 10 seconds makes automation much less feasible.
|
penalty = rate_limiter.record_failure(client_ip)
|
||||||
await asyncio.sleep(10.0)
|
acquired = rate_limiter.acquire(client_ip)
|
||||||
log.warning("login_failed", client_ip=client_ip, error=str(exc))
|
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
|
raise AuthenticationError(str(exc)) from exc
|
||||||
|
|
||||||
response.set_cookie(
|
response.set_cookie(
|
||||||
|
|||||||
@@ -45,6 +45,19 @@ SESSION_TOKEN_SIGNATURE_SEPARATOR: Final[str] = "."
|
|||||||
SESSION_COOKIE_NAME: Final[str] = "bangui_session"
|
SESSION_COOKIE_NAME: Final[str] = "bangui_session"
|
||||||
"""Name of the session cookie used by the browser SPA."""
|
"""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)
|
# Time-range presets (used by dashboard and history endpoints)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -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
|
Process-local implementation — in multi-worker setups, each worker has
|
||||||
independent counters. This constraint limits the blast radius of brute-force
|
independent counters. This constraint limits the blast radius of brute-force
|
||||||
attacks to a single worker.
|
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
|
from __future__ import annotations
|
||||||
@@ -17,6 +21,12 @@ from typing import TYPE_CHECKING
|
|||||||
|
|
||||||
import structlog
|
import structlog
|
||||||
|
|
||||||
|
from app.utils.constants import (
|
||||||
|
LOGIN_PENALTY_BASE_SECONDS,
|
||||||
|
LOGIN_PENALTY_MAX_SECONDS,
|
||||||
|
LOGIN_PENALTY_MULTIPLIER,
|
||||||
|
)
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from collections.abc import Mapping
|
from collections.abc import Mapping
|
||||||
|
|
||||||
@@ -48,6 +58,8 @@ class RateLimiter:
|
|||||||
self.max_attempts: int = max_attempts
|
self.max_attempts: int = max_attempts
|
||||||
self.window_seconds: int = window_seconds
|
self.window_seconds: int = window_seconds
|
||||||
self._attempts: dict[str, deque[float]] = {}
|
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:
|
def is_allowed(self, ip_address: str) -> bool:
|
||||||
"""Check if a request from *ip_address* is allowed.
|
"""Check if a request from *ip_address* is allowed.
|
||||||
@@ -126,3 +138,84 @@ class RateLimiter:
|
|||||||
def reset(self) -> None:
|
def reset(self) -> None:
|
||||||
"""Clear all tracked attempts (for testing)."""
|
"""Clear all tracked attempts (for testing)."""
|
||||||
self._attempts.clear()
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user