From 1d163cc299e1d4bf2232ebb4539f3aed1edc97f2 Mon Sep 17 00:00:00 2001 From: patrick Date: Thu, 10 Sep 2026 15:33:12 +0200 Subject: [PATCH] =?UTF-8?q?fix(security):=20Audit-Nachzug=20=E2=80=93=20Ra?= =?UTF-8?q?te-Limit,=20X-Real-IP-Vertrauen,=20Indizes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security-Audit 2026-09-10 (siehe Ed25519-Reinstall-Commits): - Rate-Limit 10/min auf POST /kiosk/my-qr-token (Q-1) – kompromittiertes Employee-JWT konnte sonst unbegrenzt QR-Login-Tokens erzeugen. - _get_client_ip vertraut X-Real-IP nur noch, wenn die Verbindung selbst von 127.0.0.1 kommt (I-1) – sonst könnte ein Client den Header selbst setzen und die Kiosk-IP-Whitelist umgehen. - Migration 0047: Unique-Index auf kiosk_devices.device_token_hash (B-2, war Seq-Scan pro Bearer-Request), Index auf time_entries.kiosk_device_id (K-1). RLS auf kiosk_devices bereits seit Migration 0024 vorhanden, K-1-Verdacht einer RLS-Lücke damit ausgeräumt. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Dzdm3EWeNdYFUuXFRJryA1 --- backend/app/core/kiosk_security.py | 21 ++++++----- backend/app/routers/kiosk.py | 6 ++-- .../0047_kiosk_device_token_hash_index.py | 35 +++++++++++++++++++ 3 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 backend/migrations/versions/0047_kiosk_device_token_hash_index.py diff --git a/backend/app/core/kiosk_security.py b/backend/app/core/kiosk_security.py index dcfe532..dcf3656 100644 --- a/backend/app/core/kiosk_security.py +++ b/backend/app/core/kiosk_security.py @@ -116,18 +116,23 @@ def _load_ed25519_public_key(public_key_str: str) -> Ed25519PublicKey: # ── Client-IP ermitteln (nginx-Proxy-sicher) ───────────────────────────────── +_TRUSTED_PROXY_IPS = {"127.0.0.1", "::1"} + + def _get_client_ip(request: Request) -> str: """Liest die echte Client-IP auch hinter nginx-Proxy. - nginx setzt X-Real-IP auf die echte Client-IP. - Ohne diesen Header würde request.client.host hinter nginx immer - 127.0.0.1 zurückgeben, womit IP-Whitelisting wirkungslos wäre. + nginx setzt X-Real-IP auf die echte Client-IP. Der Header wird NUR + akzeptiert, wenn die Verbindung selbst von nginx auf demselben Host + kommt (127.0.0.1) – sonst könnte ein Client den Header selbst setzen + und die IP-Whitelist trivial umgehen (Security-Audit 2026-09-10, I-1). """ - real_ip = request.headers.get("X-Real-IP") - if real_ip: - return real_ip.strip() - # Fallback: direkte Verbindungs-IP (lokal/dev) - return request.client.host if request.client else "unknown" + direct_ip = request.client.host if request.client else None + if direct_ip in _TRUSTED_PROXY_IPS: + real_ip = request.headers.get("X-Real-IP") + if real_ip: + return real_ip.strip() + return direct_ip or "unknown" # ── IP-Whitelist prüfen ─────────────────────────────────────────────────────── diff --git a/backend/app/routers/kiosk.py b/backend/app/routers/kiosk.py index b121333..f598c64 100644 --- a/backend/app/routers/kiosk.py +++ b/backend/app/routers/kiosk.py @@ -2,12 +2,13 @@ import uuid as _uuid from datetime import datetime, timezone from uuid import UUID -from fastapi import APIRouter, Body, Depends, HTTPException, Query +from fastapi import APIRouter, Body, Depends, HTTPException, Query, Request from sqlalchemy.ext.asyncio import AsyncSession from app.core.database import get_db from app.core.dependencies import CurrentUser, require_role from app.core.kiosk_security import verify_kiosk_request +from app.core.limiter import limiter from app.models.company import Company from app.models.kiosk_device import KioskDevice, KioskDeviceStatus from app.models.user import User, UserRole @@ -49,7 +50,8 @@ _admin_roles = (UserRole.COMPANY_ADMIN, UserRole.SUPER_ADMIN) # ── Mitarbeiter-QR-Login (normale JWT-Auth, für die Employee-App) ──────────── @router.post("/my-qr-token", response_model=KioskQrTokenResponse) -async def get_my_qr_token(current_user: CurrentUser): +@limiter.limit("10/minute") +async def get_my_qr_token(request: Request, current_user: CurrentUser): """Erzeugt einen 5 min gültigen, einmalig einlösbaren QR-Login-Token für den eingeloggten Mitarbeiter – die Employee-App zeigt ihn als QR-Code, der Kiosk scannt ihn (POST /kiosk/auth/qr) und meldet den Mitarbeiter an.""" diff --git a/backend/migrations/versions/0047_kiosk_device_token_hash_index.py b/backend/migrations/versions/0047_kiosk_device_token_hash_index.py new file mode 100644 index 0000000..4a3f29d --- /dev/null +++ b/backend/migrations/versions/0047_kiosk_device_token_hash_index.py @@ -0,0 +1,35 @@ +"""kiosk_devices.device_token_hash: Unique-Index + +Revision ID: 0047 +Revises: 0046 +Create Date: 2026-09-10 + +Security-Audit 2026-09-10 (B-2): Lookup in _verify_bearer_device lief bisher +ohne Index (Seq-Scan pro Kiosk-Request), zudem keine Eindeutigkeitsgarantie. +Partial Index (nur wo NOT NULL, da die meisten Geräte ed25519 nutzen und +dort kein Token gesetzt ist). +""" +from alembic import op + +revision = "0047" +down_revision = "0046" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute( + "CREATE UNIQUE INDEX ix_kiosk_devices_device_token_hash " + "ON kiosk_devices (device_token_hash) WHERE device_token_hash IS NOT NULL" + ) + # Gleicher Audit-Fund (K-1): kiosk_device_id auf time_entries hatte + # ebenfalls keinen Index (Migration 0046). + op.execute( + "CREATE INDEX ix_time_entries_kiosk_device_id " + "ON time_entries (kiosk_device_id) WHERE kiosk_device_id IS NOT NULL" + ) + + +def downgrade() -> None: + op.execute("DROP INDEX IF EXISTS ix_time_entries_kiosk_device_id") + op.execute("DROP INDEX IF EXISTS ix_kiosk_devices_device_token_hash")