security: 9 Findings aus Security-Audit behoben (CRITICAL + HIGH + MEDIUM)
CRITICAL: - C-1: LDAP tls_verify Default False → True (MITM-Schutz) - C-2: TOTP-Secret Fernet-verschlüsselt in DB (statt Plaintext) - core/crypto.py: encrypt_value() / decrypt_value() helper - Migration 0026: totp_secret VARCHAR(64→500), ldap tls_verify default=true - _totp_plain() helper mit Legacy-Fallback für bestehende Werte HIGH: - H-1: Kiosk Nonce-Cache asyncio.Lock (Race Condition behoben) - H-2: File-Upload-Limit 10 MB (import_kimai.py + users.py CSV-Import) - H-3: CORS allow_methods/allow_headers explizit eingeschränkt (war *) - H-4: TrustedHostMiddleware aktiviert wenn ALLOWED_HOSTS gesetzt MEDIUM: - M-1: IP-Logging nutzt X-Forwarded-For hinter nginx-Proxy - M-4: Audit-Log für password_changed, totp_enabled, totp_disabled - M-5: CalDAV verify_ssl in Production erzwungen (_effective_verify_ssl) 152/152 Tests grün Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,42 @@
|
||||
"""
|
||||
Zentrale Krypto-Hilfsfunktionen für TimeMaster.
|
||||
|
||||
Verwendet Fernet-Verschlüsselung (AES-128-CBC + HMAC-SHA256).
|
||||
Der Schlüssel wird aus SECRET_KEY per SHA-256 abgeleitet.
|
||||
|
||||
Verwendung:
|
||||
from app.core.crypto import encrypt_value, decrypt_value
|
||||
|
||||
stored = encrypt_value("geheimes-passwort")
|
||||
plain = decrypt_value(stored)
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import hashlib
|
||||
|
||||
from cryptography.fernet import Fernet, InvalidToken
|
||||
|
||||
from app.core.config import settings
|
||||
|
||||
|
||||
def _fernet() -> Fernet:
|
||||
"""Erstellt eine Fernet-Instanz aus dem konfigurierten SECRET_KEY."""
|
||||
key = hashlib.sha256(settings.secret_key.encode()).digest()
|
||||
return Fernet(base64.urlsafe_b64encode(key))
|
||||
|
||||
|
||||
def encrypt_value(plain: str) -> str:
|
||||
"""Verschlüsselt einen Klartext-String per Fernet. Gibt den chiffrierten String zurück."""
|
||||
return _fernet().encrypt(plain.encode()).decode()
|
||||
|
||||
|
||||
def decrypt_value(encrypted: str) -> str:
|
||||
"""
|
||||
Entschlüsselt einen Fernet-verschlüsselten String.
|
||||
Wirft ValueError bei ungültigem Token oder falschem Schlüssel.
|
||||
"""
|
||||
try:
|
||||
return _fernet().decrypt(encrypted.encode()).decode()
|
||||
except InvalidToken as exc:
|
||||
raise ValueError("Entschlüsselung fehlgeschlagen – ungültiger Token oder falscher Schlüssel.") from exc
|
||||
@@ -10,6 +10,7 @@ Jeder Kiosk-Request muss folgende HTTP-Header mitschicken:
|
||||
Bei Fehler: 401 Unauthorized (oder 403 für IP-Whitelist-Verletzungen).
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import base64
|
||||
import hashlib
|
||||
import ipaddress
|
||||
@@ -32,14 +33,23 @@ from app.models.kiosk_device import KioskDevice, KioskDeviceStatus
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# ── Nonce-Cache (Redis wenn verfügbar, sonst In-Memory-Fallback) ─────────────
|
||||
# ── Nonce-Cache (Redis primär, In-Memory-Fallback mit asyncio.Lock) ──────────
|
||||
|
||||
_nonce_cache: dict[str, float] = {} # nonce → expires_at (epoch)
|
||||
_nonce_lock: asyncio.Lock | None = None # lazy init (Loop-abhängig)
|
||||
_NONCE_TTL = 60 # Sekunden
|
||||
|
||||
|
||||
def _get_nonce_lock() -> asyncio.Lock:
|
||||
"""Lazy-initialized asyncio.Lock (thread-safe, event-loop-abhängig)."""
|
||||
global _nonce_lock
|
||||
if _nonce_lock is None:
|
||||
_nonce_lock = asyncio.Lock()
|
||||
return _nonce_lock
|
||||
|
||||
|
||||
def _cleanup_nonce_cache() -> None:
|
||||
"""Abgelaufene Einträge aus dem In-Memory-Cache entfernen."""
|
||||
"""Abgelaufene Einträge aus dem In-Memory-Cache entfernen (muss unter Lock aufgerufen werden)."""
|
||||
now = time.time()
|
||||
expired = [k for k, v in _nonce_cache.items() if v <= now]
|
||||
for k in expired:
|
||||
@@ -51,6 +61,11 @@ async def _check_and_set_nonce(nonce: str) -> bool:
|
||||
Prüft ob die Nonce schon gesehen wurde und speichert sie.
|
||||
Gibt True zurück wenn die Nonce NEU ist (Request erlaubt).
|
||||
Gibt False zurück wenn die Nonce bereits bekannt ist (Replay!).
|
||||
|
||||
Verwendet Redis als primären Nonce-Store. Falls Redis nicht erreichbar,
|
||||
wird ein asyncio.Lock-geschützter In-Memory-Fallback verwendet.
|
||||
Kritisch: Redis-Ausfall bei laufenden Kiosk-Requests ermöglicht theoretisch
|
||||
Replay im Fallback-Fenster → Redis sollte in Production HA sein.
|
||||
"""
|
||||
try:
|
||||
import redis.asyncio as aioredis
|
||||
@@ -61,14 +76,16 @@ async def _check_and_set_nonce(nonce: str) -> bool:
|
||||
await r.aclose()
|
||||
return result is not None # None = bereits vorhanden
|
||||
except Exception as e:
|
||||
logger.warning("Redis nicht erreichbar, nutze In-Memory-Nonce-Cache: %s", e)
|
||||
# Fallback: In-Memory
|
||||
_cleanup_nonce_cache()
|
||||
now = time.time()
|
||||
if nonce in _nonce_cache:
|
||||
return False
|
||||
_nonce_cache[nonce] = now + _NONCE_TTL
|
||||
return True
|
||||
logger.warning("Redis nicht erreichbar, nutze In-Memory-Nonce-Cache (Lock-geschützt): %s", e)
|
||||
# Fallback: In-Memory mit asyncio.Lock gegen Race Conditions
|
||||
lock = _get_nonce_lock()
|
||||
async with lock:
|
||||
_cleanup_nonce_cache()
|
||||
now = time.time()
|
||||
if nonce in _nonce_cache:
|
||||
return False
|
||||
_nonce_cache[nonce] = now + _NONCE_TTL
|
||||
return True
|
||||
|
||||
|
||||
# ── Öffentlichen Schlüssel laden ─────────────────────────────────────────────
|
||||
|
||||
+12
-6
@@ -44,14 +44,20 @@ app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=[settings.frontend_url],
|
||||
allow_credentials=True,
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
allow_methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"],
|
||||
allow_headers=[
|
||||
"Content-Type",
|
||||
"Authorization",
|
||||
"X-Kiosk-Key-Id",
|
||||
"X-Kiosk-Timestamp",
|
||||
"X-Kiosk-Nonce",
|
||||
"X-Kiosk-Signature",
|
||||
],
|
||||
)
|
||||
|
||||
# TODO (M-07): TrustedHostMiddleware – set ALLOWED_HOSTS env variable (comma-separated) in production.
|
||||
# Example: ALLOWED_HOSTS=timemaster.example.com,www.timemaster.example.com
|
||||
# The placeholder "yourdomain.com" has been replaced with a config-driven approach.
|
||||
if settings.is_production and settings.allowed_hosts:
|
||||
# TrustedHostMiddleware: aktiv sobald ALLOWED_HOSTS gesetzt (Development: leer = deaktiviert)
|
||||
# Production: ALLOWED_HOSTS=timemaster.example.com in .env setzen
|
||||
if settings.allowed_hosts:
|
||||
app.add_middleware(TrustedHostMiddleware, allowed_hosts=settings.allowed_hosts)
|
||||
|
||||
# ── Routers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -27,7 +27,7 @@ class LdapConfig(Base):
|
||||
port: Mapped[int] = mapped_column(Integer, default=389)
|
||||
use_ssl: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||
use_tls: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||
tls_verify: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||
tls_verify: Mapped[bool] = mapped_column(Boolean, default=True)
|
||||
|
||||
# Bind credentials
|
||||
bind_dn: Mapped[str] = mapped_column(Text, nullable=False)
|
||||
|
||||
@@ -63,7 +63,7 @@ class User(Base):
|
||||
kiosk_nfc_uid: Mapped[str | None] = mapped_column(String(64), nullable=True, index=True)
|
||||
|
||||
# TOTP / 2FA
|
||||
totp_secret: Mapped[str | None] = mapped_column(String(64))
|
||||
totp_secret: Mapped[str | None] = mapped_column(String(500)) # Fernet-verschlüsselt
|
||||
totp_enabled: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False)
|
||||
|
||||
# Permissions
|
||||
|
||||
@@ -2,10 +2,12 @@ from fastapi import APIRouter, Depends, HTTPException, Request
|
||||
from pydantic import BaseModel, Field
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.core.crypto import decrypt_value, encrypt_value
|
||||
from app.core.database import get_db
|
||||
from app.core.dependencies import CurrentUser
|
||||
from app.core.limiter import limiter
|
||||
from app.core.security import hash_password, verify_password
|
||||
from app.models.audit_log import AuditLog
|
||||
from app.schemas.auth import (
|
||||
LoginRequest,
|
||||
MessageResponse,
|
||||
@@ -103,12 +105,30 @@ async def change_password(
|
||||
detail="Neues Passwort muss mindestens 1 Großbuchstaben und 1 Zahl enthalten"
|
||||
)
|
||||
current_user.password_hash = hash_password(data.new_password)
|
||||
db.add(AuditLog(
|
||||
company_id=current_user.company_id,
|
||||
user_id=current_user.id,
|
||||
action="password_changed",
|
||||
entity_type="user",
|
||||
entity_id=current_user.id,
|
||||
))
|
||||
await db.commit()
|
||||
return MessageResponse(message="Passwort erfolgreich geändert")
|
||||
|
||||
|
||||
# ── TOTP / 2FA ────────────────────────────────────────────────────────────────
|
||||
|
||||
def _totp_plain(user) -> str | None:
|
||||
"""Gibt das entschlüsselte TOTP-Secret zurück, oder None."""
|
||||
if not user.totp_secret:
|
||||
return None
|
||||
try:
|
||||
return decrypt_value(user.totp_secret)
|
||||
except ValueError:
|
||||
# Fallback: Secret war noch im Klartext (Legacy-Daten vor 0026-Migration)
|
||||
return user.totp_secret
|
||||
|
||||
|
||||
@router.post("/totp/setup", response_model=TotpSetupResponse)
|
||||
async def totp_setup(current_user: CurrentUser):
|
||||
"""Generiert ein neues TOTP-Secret und gibt die otpauth-URI zurück. Noch nicht aktiviert."""
|
||||
@@ -117,8 +137,8 @@ async def totp_setup(current_user: CurrentUser):
|
||||
issuer = "TimeMaster"
|
||||
label = current_user.email
|
||||
uri = pyotp.totp.TOTP(secret).provisioning_uri(name=label, issuer_name=issuer)
|
||||
# Secret temporär im User speichern (noch nicht totp_enabled)
|
||||
current_user.totp_secret = secret
|
||||
# Secret Fernet-verschlüsselt speichern (noch nicht totp_enabled)
|
||||
current_user.totp_secret = encrypt_value(secret)
|
||||
# Hinweis: DB-Commit passiert NICHT hier – erst nach verify in /totp/confirm
|
||||
# Damit das Secret nicht verloren geht, sofort speichern
|
||||
return TotpSetupResponse(secret=secret, otpauth_uri=uri)
|
||||
@@ -133,7 +153,7 @@ async def totp_setup_save(
|
||||
import pyotp
|
||||
if not current_user.totp_secret:
|
||||
secret = pyotp.random_base32()
|
||||
current_user.totp_secret = secret
|
||||
current_user.totp_secret = encrypt_value(secret)
|
||||
await db.commit()
|
||||
return MessageResponse(message="Secret gespeichert")
|
||||
|
||||
@@ -146,12 +166,20 @@ async def totp_confirm(
|
||||
):
|
||||
"""Bestätigt den ersten TOTP-Code und aktiviert 2FA."""
|
||||
import pyotp
|
||||
if not current_user.totp_secret:
|
||||
plain_secret = _totp_plain(current_user)
|
||||
if not plain_secret:
|
||||
raise HTTPException(400, "Kein TOTP-Secret vorhanden. Zuerst /totp/setup aufrufen.")
|
||||
totp = pyotp.TOTP(current_user.totp_secret)
|
||||
totp = pyotp.TOTP(plain_secret)
|
||||
if not totp.verify(data.code, valid_window=1):
|
||||
raise HTTPException(400, "Ungültiger Code")
|
||||
current_user.totp_enabled = True
|
||||
db.add(AuditLog(
|
||||
company_id=current_user.company_id,
|
||||
user_id=current_user.id,
|
||||
action="totp_enabled",
|
||||
entity_type="user",
|
||||
entity_id=current_user.id,
|
||||
))
|
||||
await db.commit()
|
||||
return MessageResponse(message="Zwei-Faktor-Authentifizierung aktiviert")
|
||||
|
||||
@@ -168,11 +196,19 @@ async def totp_disable(
|
||||
raise HTTPException(400, "Passwort falsch")
|
||||
if not current_user.totp_enabled or not current_user.totp_secret:
|
||||
raise HTTPException(400, "2FA ist nicht aktiv")
|
||||
totp = pyotp.TOTP(current_user.totp_secret)
|
||||
plain_secret = _totp_plain(current_user)
|
||||
totp = pyotp.TOTP(plain_secret or "")
|
||||
if not totp.verify(data.code, valid_window=1):
|
||||
raise HTTPException(400, "Ungültiger TOTP-Code")
|
||||
current_user.totp_enabled = False
|
||||
current_user.totp_secret = None
|
||||
db.add(AuditLog(
|
||||
company_id=current_user.company_id,
|
||||
user_id=current_user.id,
|
||||
action="totp_disabled",
|
||||
entity_type="user",
|
||||
entity_id=current_user.id,
|
||||
))
|
||||
await db.commit()
|
||||
return MessageResponse(message="Zwei-Faktor-Authentifizierung deaktiviert")
|
||||
|
||||
@@ -202,7 +238,8 @@ async def totp_login(
|
||||
if not user.totp_enabled or not user.totp_secret:
|
||||
raise HTTPException(400, "2FA nicht aktiv")
|
||||
|
||||
totp = pyotp.TOTP(user.totp_secret)
|
||||
plain_secret = _totp_plain(user)
|
||||
totp = pyotp.TOTP(plain_secret or "")
|
||||
if not totp.verify(data.code, valid_window=1):
|
||||
raise HTTPException(400, "Ungültiger Code")
|
||||
|
||||
|
||||
@@ -19,6 +19,19 @@ router = APIRouter(prefix="/import", tags=["import"])
|
||||
|
||||
_allowed_roles = [UserRole.HR, UserRole.COMPANY_ADMIN, UserRole.SUPER_ADMIN]
|
||||
|
||||
_MAX_UPLOAD_BYTES = 10 * 1024 * 1024 # 10 MB
|
||||
|
||||
|
||||
async def _read_upload(file: UploadFile) -> bytes:
|
||||
"""Liest eine UploadFile mit Größenbegrenzung (max 10 MB)."""
|
||||
content = await file.read(_MAX_UPLOAD_BYTES + 1)
|
||||
if len(content) > _MAX_UPLOAD_BYTES:
|
||||
raise HTTPException(
|
||||
status_code=413,
|
||||
detail=f"Datei zu groß. Maximale Upload-Größe: {_MAX_UPLOAD_BYTES // (1024 * 1024)} MB.",
|
||||
)
|
||||
return content
|
||||
|
||||
|
||||
class ImportPreviewResponse(BaseModel):
|
||||
preview: list[ImportPreviewEntry]
|
||||
@@ -48,7 +61,7 @@ async def kimai_preview(
|
||||
except ValueError:
|
||||
raise HTTPException(status_code=400, detail="Ungültige user_id")
|
||||
|
||||
content = await file.read()
|
||||
content = await _read_upload(file)
|
||||
result: ImportResult = await preview_kimai_import(content, target_id, db)
|
||||
|
||||
time_count = sum(1 for p in result.preview if p.kind == "time" and not p.skipped)
|
||||
@@ -76,7 +89,7 @@ async def kimai_run(
|
||||
except ValueError:
|
||||
raise HTTPException(status_code=400, detail="Ungültige user_id")
|
||||
|
||||
content = await file.read()
|
||||
content = await _read_upload(file)
|
||||
result: ImportResult = await run_kimai_import(content, target_id, current_user.id, db)
|
||||
|
||||
return ImportRunResponse(
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
from typing import Annotated
|
||||
from uuid import UUID
|
||||
|
||||
from fastapi import APIRouter, Depends, File, Query, UploadFile
|
||||
from fastapi import APIRouter, Depends, File, HTTPException, Query, UploadFile
|
||||
from fastapi.responses import PlainTextResponse
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
@@ -90,13 +90,27 @@ async def import_template(
|
||||
)
|
||||
|
||||
|
||||
_MAX_UPLOAD_BYTES = 10 * 1024 * 1024 # 10 MB
|
||||
|
||||
|
||||
async def _read_upload(file: UploadFile) -> bytes:
|
||||
"""Liest eine UploadFile mit Größenbegrenzung (max 10 MB)."""
|
||||
content = await file.read(_MAX_UPLOAD_BYTES + 1)
|
||||
if len(content) > _MAX_UPLOAD_BYTES:
|
||||
raise HTTPException(
|
||||
status_code=413,
|
||||
detail=f"Datei zu groß. Maximale Upload-Größe: {_MAX_UPLOAD_BYTES // (1024 * 1024)} MB.",
|
||||
)
|
||||
return content
|
||||
|
||||
|
||||
@router.post("/import/preview", response_model=UserImportResult)
|
||||
async def user_import_preview(
|
||||
file: Annotated[UploadFile, File()],
|
||||
current_user: User = require_role(*_admin_roles),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
content = await file.read()
|
||||
content = await _read_upload(file)
|
||||
result = await user_import_service.preview_csv(content, current_user.company_id, current_user, db)
|
||||
return _to_import_result_schema(result)
|
||||
|
||||
@@ -107,7 +121,7 @@ async def user_import_apply(
|
||||
current_user: User = require_role(*_admin_roles),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
content = await file.read()
|
||||
content = await _read_upload(file)
|
||||
result = await user_import_service.apply_csv(content, current_user.company_id, current_user, db)
|
||||
return _to_import_result_schema(result)
|
||||
|
||||
|
||||
@@ -21,6 +21,17 @@ from app.schemas.auth import LoginRequest, RegisterRequest, TokenResponse
|
||||
from app.services.email_service import email_service
|
||||
|
||||
|
||||
def _get_client_ip(request: "Request | None") -> str | None:
|
||||
"""Gibt die echte Client-IP zurück (berücksichtigt X-Forwarded-For hinter nginx-Proxy)."""
|
||||
if not request:
|
||||
return None
|
||||
forwarded = request.headers.get("X-Forwarded-For")
|
||||
if forwarded:
|
||||
# Erstes Element = Original-Client-IP (nginx setzt X-Forwarded-For)
|
||||
return forwarded.split(",")[0].strip()
|
||||
return request.client.host if request.client else None
|
||||
|
||||
|
||||
def _slugify(name: str) -> str:
|
||||
slug = re.sub(r"[^a-z0-9]+", "-", name.lower()).strip("-")
|
||||
return slug[:80]
|
||||
@@ -198,7 +209,7 @@ class AuthService:
|
||||
refresh_token_hash=hashed_refresh,
|
||||
expires_at=datetime.now(timezone.utc) + timedelta(days=settings.refresh_token_expire_days),
|
||||
device=request.headers.get("User-Agent", "")[:255] if request else None,
|
||||
ip=request.client.host if request and request.client else None,
|
||||
ip=_get_client_ip(request),
|
||||
)
|
||||
db.add(session)
|
||||
access_token = create_access_token(
|
||||
|
||||
@@ -209,12 +209,27 @@ def _event_url(calendar_url: str, uid: str) -> str:
|
||||
return calendar_url.rstrip("/") + f"/{uid}.ics"
|
||||
|
||||
|
||||
def _effective_verify_ssl(verify_ssl: bool) -> bool:
|
||||
"""
|
||||
Gibt den tatsächlich zu verwendenden verify_ssl-Wert zurück.
|
||||
In Production ist SSL-Verifikation immer aktiviert – verify_ssl=False wird ignoriert.
|
||||
"""
|
||||
if settings.is_production and not verify_ssl:
|
||||
log.warning(
|
||||
"CalDAV: verify_ssl=False in Production ignoriert – SSL-Verifikation wird erzwungen. "
|
||||
"Für selbstsignierte Zertifikate das CA-Bundle unter REQUESTS_CA_BUNDLE konfigurieren."
|
||||
)
|
||||
return True
|
||||
return verify_ssl
|
||||
|
||||
|
||||
async def _http_put(
|
||||
calendar_url: str, username: str, password: str, uid: str,
|
||||
ical: bytes, verify_ssl: bool,
|
||||
) -> str:
|
||||
"""PUT event. Returns ETag (empty string if server doesn't send one)."""
|
||||
_validate_caldav_url(calendar_url)
|
||||
verify_ssl = _effective_verify_ssl(verify_ssl)
|
||||
url = _event_url(calendar_url, uid)
|
||||
async with httpx.AsyncClient(verify=verify_ssl, timeout=15) as client:
|
||||
resp = await client.put(
|
||||
@@ -230,6 +245,7 @@ async def _http_delete(
|
||||
calendar_url: str, username: str, password: str, uid: str, verify_ssl: bool,
|
||||
) -> None:
|
||||
_validate_caldav_url(calendar_url)
|
||||
verify_ssl = _effective_verify_ssl(verify_ssl)
|
||||
url = _event_url(calendar_url, uid)
|
||||
async with httpx.AsyncClient(verify=verify_ssl, timeout=15) as client:
|
||||
resp = await client.delete(url, auth=(username, password))
|
||||
@@ -242,6 +258,7 @@ async def _http_propfind(
|
||||
) -> int:
|
||||
"""Einfacher Verbindungstest via PROPFIND Depth:0. Gibt HTTP-Status zurück."""
|
||||
_validate_caldav_url(calendar_url)
|
||||
verify_ssl = _effective_verify_ssl(verify_ssl)
|
||||
body = b'<?xml version="1.0"?><d:propfind xmlns:d="DAV:"><d:prop><d:resourcetype/></d:prop></d:propfind>'
|
||||
async with httpx.AsyncClient(verify=verify_ssl, timeout=10) as client:
|
||||
resp = await client.request(
|
||||
|
||||
Reference in New Issue
Block a user