fix: corrige 6 bugs mineurs (BUG-035 a BUG-040)
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
|
||||
from fastapi import Depends, HTTPException, Request
|
||||
from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer
|
||||
@@ -17,6 +18,9 @@ logger = logging.getLogger("obsigate.auth.middleware")
|
||||
|
||||
security = HTTPBearer(auto_error=False)
|
||||
|
||||
#: Hosts considered safe to bind without authentication (loopback only).
|
||||
_LOOPBACK_HOSTS = {"127.0.0.1", "::1", "localhost", "0:0:0:0:0:0:0:1"}
|
||||
|
||||
|
||||
def is_auth_enabled() -> bool:
|
||||
"""Check if authentication is enabled via environment variable.
|
||||
@@ -26,6 +30,34 @@ def is_auth_enabled() -> bool:
|
||||
return os.environ.get("OBSIGATE_AUTH_ENABLED", "true").lower() != "false"
|
||||
|
||||
|
||||
def is_insecure_mode_allowed() -> bool:
|
||||
"""True when the operator explicitly accepts running without auth (BUG-037)."""
|
||||
return os.environ.get("OBSIGATE_ALLOW_INSECURE", "false").lower() in ("1", "true", "yes", "on")
|
||||
|
||||
|
||||
def bind_host_from_argv(argv: list[str] | None = None) -> str | None:
|
||||
"""Extract the ``--host`` value from the process arguments (uvicorn), if any.
|
||||
|
||||
Returns ``None`` when no explicit host is passed (uvicorn then defaults to
|
||||
loopback ``127.0.0.1``).
|
||||
"""
|
||||
args = sys.argv if argv is None else argv
|
||||
for i, arg in enumerate(args):
|
||||
if arg == "--host" and i + 1 < len(args):
|
||||
return args[i + 1]
|
||||
if arg.startswith("--host="):
|
||||
return arg.split("=", 1)[1]
|
||||
return None
|
||||
|
||||
|
||||
def is_loopback_host(host: str | None) -> bool:
|
||||
"""True when *host* is a loopback address (or unset → uvicorn default)."""
|
||||
if not host:
|
||||
return True
|
||||
normalized = host.strip().strip("[]").lower()
|
||||
return normalized in _LOOPBACK_HOSTS
|
||||
|
||||
|
||||
def get_current_user(
|
||||
request: Request,
|
||||
credentials: HTTPAuthorizationCredentials | None = Depends(security),
|
||||
|
||||
@@ -1,14 +1,21 @@
|
||||
# backend/auth/password.py
|
||||
# Argon2id password hashing — OWASP 2024 recommended algorithm.
|
||||
# Parameters: time_cost=2, memory_cost=64MB, parallelism=2
|
||||
# Parameters (BUG-038): time_cost=2, memory_cost=19 MiB, parallelism=1
|
||||
# (OWASP current recommendation for Argon2id). The previous 64 MiB setting
|
||||
# allowed memory exhaustion under concurrent login attempts.
|
||||
|
||||
from argon2 import PasswordHasher
|
||||
from argon2.exceptions import VerificationError, VerifyMismatchError
|
||||
|
||||
#: Argon2id cost parameters (OWASP 2024: m=19456 KiB, t=2, p=1).
|
||||
ARGON2_TIME_COST = 2
|
||||
ARGON2_MEMORY_COST_KIB = 19456 # 19 MiB
|
||||
ARGON2_PARALLELISM = 1
|
||||
|
||||
ph = PasswordHasher(
|
||||
time_cost=2,
|
||||
memory_cost=65536, # 64 MB
|
||||
parallelism=2,
|
||||
time_cost=ARGON2_TIME_COST,
|
||||
memory_cost=ARGON2_MEMORY_COST_KIB,
|
||||
parallelism=ARGON2_PARALLELISM,
|
||||
hash_len=32,
|
||||
salt_len=16,
|
||||
)
|
||||
|
||||
+18
-17
@@ -124,31 +124,32 @@ async def auth_status():
|
||||
async def login(body: LoginRequest, response: Response, request: Request):
|
||||
"""Authenticate a user. Returns access token and sets refresh cookie.
|
||||
|
||||
Implements timing-safe responses to prevent user enumeration:
|
||||
a failed login with an unknown user takes the same time as one
|
||||
with a known user (dummy hash is computed).
|
||||
Implements timing-safe responses to prevent user enumeration: a failed
|
||||
login with an unknown user takes the same time as one with a known user
|
||||
(dummy hash is computed). BUG-039: unknown, inactive, locked and
|
||||
per-account rate-limited accounts all answer the same ``401`` so the HTTP
|
||||
status can never reveal whether an account exists.
|
||||
"""
|
||||
client_ip = get_client_ip(request)
|
||||
|
||||
# IP-based rate limiting (10 failures / 15 min per IP). It is not
|
||||
# account-specific, so a 429 here cannot be used to enumerate accounts.
|
||||
if is_rate_limited(client_ip):
|
||||
raise HTTPException(429, "Trop de tentatives depuis cette adresse IP (15min)")
|
||||
|
||||
user = get_user(body.username)
|
||||
|
||||
if not user:
|
||||
# BUG-039: uniform 401 + equivalent timing for every account-state outcome.
|
||||
if not user or not user.get("active"):
|
||||
# Timing-safe: simulate hash computation to prevent user enumeration
|
||||
hash_password("dummy_timing_protection")
|
||||
raise HTTPException(401, "Identifiants invalides")
|
||||
|
||||
if not user.get("active"):
|
||||
raise HTTPException(403, "Compte désactivé")
|
||||
|
||||
# IP-based rate limiting (10 failures / 15 min per IP)
|
||||
client_ip = get_client_ip(request)
|
||||
if is_rate_limited(client_ip):
|
||||
raise HTTPException(429, "Trop de tentatives depuis cette adresse IP (15min)")
|
||||
|
||||
# BUG-031: per-account budget still applies when the attacker rotates IPs.
|
||||
if is_account_rate_limited(body.username):
|
||||
raise HTTPException(429, "Trop de tentatives sur ce compte (15min)")
|
||||
|
||||
if is_locked(body.username):
|
||||
raise HTTPException(429, "Compte temporairement verrouillé (15min)")
|
||||
# Kept indistinguishable from a wrong password (BUG-039).
|
||||
if is_account_rate_limited(body.username) or is_locked(body.username):
|
||||
hash_password("dummy_timing_protection")
|
||||
raise HTTPException(401, "Identifiants invalides")
|
||||
|
||||
if not verify_password(body.password, user["password_hash"]):
|
||||
attempts = record_login_failure(body.username)
|
||||
|
||||
Reference in New Issue
Block a user