From 4dc69137ddea9a0999f67be424370ba0f9cfaaf7 Mon Sep 17 00:00:00 2001 From: patrick Date: Tue, 26 May 2026 11:35:18 +0200 Subject: [PATCH] security: H-1 settings-Whitelist + H-5 UUID-Guard + H-6 DNS-Pinning + H-7 Heartbeat-Timing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H-1: company.settings als typisiertes Sub-Schema - schemas/company.py: CompanySettingsUpdate mit extra=forbid - Nur bekannte Keys (carryover_expires_month/day) erlaubt - Unbekannte Keys → HTTP 422 H-5: SQL-Injection defensiv absichern - dependencies.py: UUID-Round-Trip str(_uuid.UUID(...)) + Sicherheitskommentar H-6: CalDAV DNS-Rebinding-Schutz - caldav_service.py: PinnedIPTransport — IP einmal auflösen, beim Request fixieren - _validate_caldav_url gibt aufgelöste IP zurück - Alle HTTP-Methoden nutzen PinnedIPTransport H-7: Heartbeat-Timestamp nach Route-Logik - kiosk_security.py: last_heartbeat_at-Update aus Dependency entfernt - kiosk_service.py: Update erst in process_heartbeat() nach erfolgreicher Auth Co-Authored-By: Claude Sonnet 4.6 --- DEVLOG.md | 42 ++++++++++++++ backend/app/core/dependencies.py | 12 ++-- backend/app/core/kiosk_security.py | 11 ++-- backend/app/routers/companies.py | 8 ++- backend/app/schemas/company.py | 17 +++++- backend/app/services/caldav_service.py | 78 ++++++++++++++++++++++---- backend/app/services/kiosk_service.py | 8 ++- 7 files changed, 152 insertions(+), 24 deletions(-) diff --git a/DEVLOG.md b/DEVLOG.md index 4e39fd5..0966ab7 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1707,3 +1707,45 @@ Keine Commits in dieser Session. - DEVLOG.md | 29 +++++++++++++++++++++++++++++ --- +## 2026-05-26 11:17 – 11:25 (7m) +**Beschreibung:** Claude Code Session +**Projekt:** timemaster + +### Commits +- 654258f security: M-2 HttpOnly-Cookie + M-4 TrustedHost-Warning + M-5 TOTP-Lockout + M-7 zentraler get_client_ip() + +### Geänderte Dateien +- DEVLOG.md | 22 +++++++ +- backend/app/core/dependencies.py | 18 +++++- +- backend/app/main.py | 8 +++ +- backend/app/routers/absences.py | 4 +- +- backend/app/routers/auth.py | 116 +++++++++++++++++++++++++++++++---- +- backend/app/routers/busylight.py | 6 +- +- backend/app/routers/hours_payouts.py | 6 +- +- backend/app/schemas/auth.py | 4 +- +- backend/app/services/auth_service.py | 7 ++- +- frontend/src/api/client.ts | 17 +++-- +- frontend/src/context/AuthContext.tsx | 14 +++-- + +--- +## 2026-05-26 11:26 – 11:27 (0m) +**Beschreibung:** Claude Code Session +**Projekt:** timemaster + +### Commits +Keine Commits in dieser Session. + +### Geänderte Dateien +- DEVLOG.md | 22 +++++++ +- backend/app/core/dependencies.py | 18 +++++- +- backend/app/main.py | 8 +++ +- backend/app/routers/absences.py | 4 +- +- backend/app/routers/auth.py | 116 +++++++++++++++++++++++++++++++---- +- backend/app/routers/busylight.py | 6 +- +- backend/app/routers/hours_payouts.py | 6 +- +- backend/app/schemas/auth.py | 4 +- +- backend/app/services/auth_service.py | 7 ++- +- frontend/src/api/client.ts | 17 +++-- +- frontend/src/context/AuthContext.tsx | 14 +++-- + +--- diff --git a/backend/app/core/dependencies.py b/backend/app/core/dependencies.py index d1c0aad..dec5f84 100644 --- a/backend/app/core/dependencies.py +++ b/backend/app/core/dependencies.py @@ -1,3 +1,4 @@ +import uuid as _uuid from typing import Annotated from uuid import UUID @@ -60,10 +61,13 @@ async def get_current_user( # bypass so subsequent queries in the same transaction are automatically # filtered to the user's company. if user.role != UserRole.SUPER_ADMIN and user.company_id is not None: - # SET LOCAL does not accept bind parameters in PostgreSQL; the value - # must be inlined. We sanitise by converting through uuid.UUID first - # so an attacker-supplied token payload cannot inject arbitrary SQL. - safe_company_id = str(user.company_id) # already a UUID object from db.get() + # Sicherheits-Invariante: safe_company_id muss eine valide UUID sein. + # Der _uuid.UUID()-Round-Trip verhindert SQL-Injection auch bei zukünftigen + # Refactorings (z.B. falls user.company_id einmal ein String aus einem + # anderen Pfad käme). SET LOCAL akzeptiert keine Bind-Parameter in + # PostgreSQL, daher ist String-Interpolation hier unvermeidlich — + # der UUID-Round-Trip ist die kryptographische Absicherung dagegen. + safe_company_id = str(_uuid.UUID(str(user.company_id))) await db.execute(text(f"SET LOCAL app.company_id = '{safe_company_id}'")) await db.execute(text("SET LOCAL app.bypass_rls = 'off'")) diff --git a/backend/app/core/kiosk_security.py b/backend/app/core/kiosk_security.py index d4c0b9c..d48a207 100644 --- a/backend/app/core/kiosk_security.py +++ b/backend/app/core/kiosk_security.py @@ -177,9 +177,10 @@ async def verify_kiosk_request( 3. Gerät laden + Status prüfen 4. IP-Whitelist (falls konfiguriert) 5. Ed25519-Signatur verifizieren - 6. last_heartbeat_at aktualisieren Gibt das verifizierte KioskDevice zurück. + Hinweis: last_heartbeat_at wird NICHT hier gesetzt (H-7) – nur der Heartbeat- + Endpoint setzt es, nach erfolgreicher Route-Logik. """ # 1. Timestamp-Check try: @@ -259,8 +260,10 @@ async def verify_kiosk_request( except InvalidSignature: raise HTTPException(status_code=401, detail="Ungültige Signatur.") - # 6. Heartbeat-Zeitstempel aktualisieren - device.last_heartbeat_at = datetime.now(timezone.utc) - await db.flush() + # Hinweis: last_heartbeat_at wird NICHT hier gesetzt, sondern erst im + # Heartbeat-Route-Handler (process_heartbeat). So wird der Timestamp nur + # committed wenn die gesamte Route-Logik erfolgreich war, nicht bereits + # bei jedem signierten Request. Andere Endpunkte (auth/pin etc.) aktualisieren + # last_heartbeat_at bewusst nicht – nur echter Heartbeat zählt als Liveness-Signal. return device diff --git a/backend/app/routers/companies.py b/backend/app/routers/companies.py index 8098918..053faf3 100644 --- a/backend/app/routers/companies.py +++ b/backend/app/routers/companies.py @@ -35,7 +35,13 @@ async def update_my_company( db: AsyncSession = Depends(get_db), ): company = await db.get(Company, current_user.company_id) - for field, value in data.model_dump(exclude_none=True).items(): + update_data = data.model_dump(exclude_none=True) + # settings ist ein CompanySettingsUpdate-Objekt → als dict ins JSONB mergen + if "settings" in update_data and isinstance(update_data["settings"], dict): + existing = dict(company.settings or {}) + existing.update(update_data.pop("settings")) + update_data["settings"] = existing + for field, value in update_data.items(): setattr(company, field, value) return CompanyOut.model_validate(company) diff --git a/backend/app/schemas/company.py b/backend/app/schemas/company.py index d2868b1..066bcfd 100644 --- a/backend/app/schemas/company.py +++ b/backend/app/schemas/company.py @@ -1,12 +1,25 @@ import uuid from typing import Literal -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field PersonnelNumberModeT = Literal["manual", "auto"] +class CompanySettingsUpdate(BaseModel): + """Validiertes Sub-Schema für das company.settings JSONB-Feld. + + Nur bekannte Top-Level-Keys sind erlaubt (extra="forbid"). + Derzeit genutzte Keys: carryover_expires_month, carryover_expires_day + (gelesen in absences.py für Resturlaub-Verfall-Berechnung). + """ + model_config = ConfigDict(extra="forbid") + + carryover_expires_month: int | None = Field(None, ge=1, le=12) + carryover_expires_day: int | None = Field(None, ge=1, le=31) + + class CompanyOut(BaseModel): model_config = {"from_attributes": True} @@ -37,7 +50,7 @@ class CompanyOut(BaseModel): class CompanyUpdate(BaseModel): name: str | None = Field(None, min_length=2, max_length=255) state: str | None = Field(None, max_length=10) - settings: dict | None = None + settings: CompanySettingsUpdate | None = None personnel_number_required: bool | None = None personnel_number_mode: PersonnelNumberModeT | None = None personnel_number_next: int | None = Field(None, ge=1) diff --git a/backend/app/services/caldav_service.py b/backend/app/services/caldav_service.py index 8c45a7e..87ec6cf 100644 --- a/backend/app/services/caldav_service.py +++ b/backend/app/services/caldav_service.py @@ -121,16 +121,21 @@ _BLOCKED_NETWORKS = [ ] -def _validate_caldav_url(url: str) -> None: +async def _validate_caldav_url(url: str) -> str: """ - Prüft eine CalDAV-URL auf SSRF-Risiken. - Wirft ValueError mit sprechender Meldung bei Problemen. + Prüft eine CalDAV-URL auf SSRF-Risiken und gibt die aufgelöste IP zurück. + + Die zurückgegebene IP wird beim eigentlichen HTTP-Request via PinnedIPTransport + fest eingesetzt, um DNS-Rebinding zwischen Validierung und Request zu verhindern + (TTL=0-Angriff: zweite DNS-Auflösung durch httpx könnte andere IP liefern). Prüfungen: 1. Schema muss http:// oder https:// sein (kein file://, ftp://, etc.) 2. Hostname muss vorhanden sein 3. Keine expliziten privaten IP-Adressen im URL 4. DNS-Auflösung + Prüfung ob die aufgelöste IP intern ist (DNS-Rebinding-Schutz) + + Gibt die erste aufgelöste (und erlaubte) IP als String zurück. """ try: parsed = urlparse(url) @@ -150,23 +155,39 @@ def _validate_caldav_url(url: str) -> None: try: ip = ipaddress.ip_address(hostname) _check_ip_blocked(ip, hostname) + # hostname ist bereits eine IP-Literal → direkt zurückgeben + return str(ip) except ValueError: pass # Kein gültiges IP-Literal → ist ein Hostname, DNS-Auflösung folgt - # DNS auflösen und alle aufgelösten Adressen prüfen (DNS-Rebinding-Schutz) + # DNS auflösen und alle aufgelösten Adressen prüfen (DNS-Rebinding-Schutz). + # socket.getaddrinfo ist blockierend → in Executor auslagern damit der Event-Loop + # nicht blockiert wird. + loop = asyncio.get_event_loop() + port = parsed.port or (443 if parsed.scheme == "https" else 80) try: - infos = socket.getaddrinfo(hostname, parsed.port or (443 if parsed.scheme == "https" else 80)) + infos = await loop.run_in_executor( + None, socket.getaddrinfo, hostname, port + ) except socket.gaierror as exc: raise ValueError(f"Hostname '{hostname}' konnte nicht aufgelöst werden: {exc}") from exc + first_allowed_ip: str | None = None for _family, _type, _proto, _canonname, sockaddr in infos: ip_str = sockaddr[0] try: ip = ipaddress.ip_address(ip_str) _check_ip_blocked(ip, hostname) + if first_allowed_ip is None: + first_allowed_ip = ip_str except ValueError as exc: raise ValueError(str(exc)) from exc + if first_allowed_ip is None: + raise ValueError(f"Hostname '{hostname}' konnte nicht auf eine erlaubte IP aufgelöst werden.") + + return first_allowed_ip + def _get_allowed_networks() -> list[ipaddress.IPv4Network | ipaddress.IPv6Network]: """Gibt die konfigurierten Whitelist-Netzwerke zurück (CALDAV_ALLOWED_CIDRS).""" @@ -203,6 +224,38 @@ def _check_ip_blocked(ip: ipaddress.IPv4Address | ipaddress.IPv6Address, hostnam ) +# ── DNS-Rebinding-sicherer Transport ────────────────────────────────────────── + +class PinnedIPTransport(httpx.AsyncHTTPTransport): + """ + Sicherheits-Transport: Nutzt eine vorab aufgelöste IP statt erneuter DNS-Auflösung. + Verhindert DNS-Rebinding zwischen URL-Validierung und eigentlichem HTTP-Request + (Angriff mit TTL=0: zweite DNS-Auflösung durch httpx könnte andere IP liefern). + + Der originale Host-Header bleibt erhalten, damit TLS-SNI und + vHost-Routing des Servers korrekt funktionieren. + """ + + def __init__(self, pinned_ip: str, **kwargs): + super().__init__(**kwargs) + self._pinned_ip = pinned_ip + + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + # URL mit aufgelöster IP statt Hostname umschreiben + new_url = request.url.copy_with(host=self._pinned_ip) + # Neues Request-Objekt mit ersetzter URL; alle Header (inkl. Host) bleiben + # erhalten – httpx setzt Host automatisch auf den original-Hostnamen wenn + # er nicht explizit überschrieben wird, was SNI und vHost korrekt hält. + request = request.__class__( + method=request.method, + url=new_url, + headers=request.headers, + stream=request.stream, + extensions=request.extensions, + ) + return await super().handle_async_request(request) + + # ── HTTP helpers ─────────────────────────────────────────────────────────────── def _event_url(calendar_url: str, uid: str) -> str: @@ -228,10 +281,11 @@ async def _http_put( ical: bytes, verify_ssl: bool, ) -> str: """PUT event. Returns ETag (empty string if server doesn't send one).""" - _validate_caldav_url(calendar_url) + pinned_ip = await _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: + transport = PinnedIPTransport(pinned_ip=pinned_ip, verify=verify_ssl) + async with httpx.AsyncClient(transport=transport, verify=verify_ssl, timeout=15) as client: resp = await client.put( url, content=ical, headers={"Content-Type": "text/calendar; charset=utf-8"}, @@ -244,10 +298,11 @@ async def _http_put( async def _http_delete( calendar_url: str, username: str, password: str, uid: str, verify_ssl: bool, ) -> None: - _validate_caldav_url(calendar_url) + pinned_ip = await _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: + transport = PinnedIPTransport(pinned_ip=pinned_ip, verify=verify_ssl) + async with httpx.AsyncClient(transport=transport, verify=verify_ssl, timeout=15) as client: resp = await client.delete(url, auth=(username, password)) if resp.status_code not in (200, 204, 404): resp.raise_for_status() @@ -257,10 +312,11 @@ async def _http_propfind( calendar_url: str, username: str, password: str, verify_ssl: bool, ) -> int: """Einfacher Verbindungstest via PROPFIND Depth:0. Gibt HTTP-Status zurück.""" - _validate_caldav_url(calendar_url) + pinned_ip = await _validate_caldav_url(calendar_url) verify_ssl = _effective_verify_ssl(verify_ssl) body = b'' - async with httpx.AsyncClient(verify=verify_ssl, timeout=10) as client: + transport = PinnedIPTransport(pinned_ip=pinned_ip, verify=verify_ssl) + async with httpx.AsyncClient(transport=transport, verify=verify_ssl, timeout=10) as client: resp = await client.request( "PROPFIND", calendar_url.rstrip("/") + "/", content=body, diff --git a/backend/app/services/kiosk_service.py b/backend/app/services/kiosk_service.py index 185d6bc..014c149 100644 --- a/backend/app/services/kiosk_service.py +++ b/backend/app/services/kiosk_service.py @@ -119,9 +119,13 @@ class KioskService: ) -> None: """ Heartbeat-Daten vom Kiosk-Gerät verarbeiten. - last_heartbeat_at wird bereits in verify_kiosk_request gesetzt – - hier werden nur die zusätzlichen Felder aktualisiert. + + last_heartbeat_at wird hier gesetzt (H-7 Fix: nicht in verify_kiosk_request, + da der Timestamp erst nach erfolgreicher Route-Logik committed werden soll). """ + # Heartbeat-Timestamp erst jetzt setzen – nach erfolgreicher Auth + Route-Logik + device.last_heartbeat_at = datetime.now(timezone.utc) + if data.client_version is not None: device.client_version = data.client_version