From 13b333cbec8156cf33229edf630b7789baffd5fd Mon Sep 17 00:00:00 2001 From: Bernd Steckmeister Date: Sat, 4 Jul 2026 18:56:42 +0200 Subject: [PATCH] =?UTF-8?q?security:=20server.py=20Restbefunde=20aus=20Rev?= =?UTF-8?q?iew=20(s)=20=E2=80=93=20ban/unban-Body=20vor=20ADMIN=5FLOCK,=20?= =?UTF-8?q?generische=20500er=20auf=20oeffentlichen=20Routen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- PROGRESS.md | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++ ROADMAP.md | 4 ++++ 2 files changed, 68 insertions(+) diff --git a/PROGRESS.md b/PROGRESS.md index 46626d6..1bbcc6b 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -13,6 +13,55 @@ Schritt (und beim Abbruch mitten im Schritt den Zwischenstand). Format: --- +## 2026-07-04 – Fable-5-Review (s): (r)-Fixes geprüft – korrekt, 2 kleinere Restbefunde gefunden und gefixt + +**Erledigt:** Review-Pass über die jüngste erledigte Arbeit – die drei +(r)-Restbefund-Fixes (Commit 5ada0e5) kritisch gegengelesen und als Punkt (s) +im Fable-5-Review-Abschnitt abgehakt. Der (r)-Diff selbst ist korrekt (gegen +Backup `server.py.bak-20260704-review-r` verifiziert, Dienst lief mit dem +aktuellen Code). Beim Gegenlesen der GANZEN Datei (Lehre aus (o)) zwei +kleinere Restbefunde in `server.py` gefunden und gefixt: + +1. **ban/unban las den Body INNERHALB des `ADMIN_LOCK`:** Das 4-KB-Limit aus + (r) deckelt nur die Größe – ein LAN-Client, der `Content-Length` ankündigt + und nichts schickt, hielt den Lock trotzdem bis zum 30-s-Socket-Timeout + fest (der (r)-Kommentar behauptete fälschlich, der Deckel verhindere das). + Seit (r) wartet auch der öffentliche GET-Leser von `registration-status` + auf diesen Lock → der hing mit. Fix: Body-Lesen/Validieren VOR den Lock + gezogen (Handler-Logik identisch, nur verschoben). +2. **Öffentliche Routen leakten Exception-Texte:** `livekit-token` und + `e2ee-diagnostics` antworteten Unauthentifizierten bei Fehlern mit + `str(e)` – das kann interne Pfade (`livekit.yaml`-Pfad bei + FileNotFoundError) oder interne Adressen preisgeben. Fix: generische + 500-Antwort („Interner Fehler"), Details per stderr ins Journal. Die + Admin-Routen behalten `str(e)` (LAN-only, für Bernd nützlich). + +**Verifiziert (headless, nach Dienst-Neustart per kill/`Restart=always`):** +`py_compile` sauber, Diff gegen Backup minimal wie geplant +(`server.py.bak-20260704-review-s`). 8 Routen-Checks wie in (r) alle +unverändert (stats 200, registration-status 200, ban ohne Nutzer 400, +8-KB-Body 413, `Content-Length: -5` 413, livekit-token ungültig 401, +Diagnose falscher Token 403, gespoofter Cf-Header 403). **Lock-Beweis (neu): +ban-Request mit angekündigtem, nie gesendetem Body + paralleler +`registration-status`-GET → Antwort in 15 ms statt Lock-Blockade** (vorher +hätte der GET bis zu 30 s gewartet). **500-Beweis (neu):** ungültiges JSON an +livekit-token → `{"error": "Interner Fehler"}`, Detail +(`JSONDecodeError(...)`) steht im Journal. Test-Deny-Zeile aus +`security_alerts.log` wieder entfernt. + +**Offen/Nächster Schritt:** Unverändert – alle offenen ROADMAP-Punkte brauchen +PC/Gerät (SQLCipher, LiveKit-Client-Umstellung, Härtetest) oder Bernd +(Secret-Rotation, Dashboard-Fernzugriff A/B, Call-Pilot-Go). + +**Stolperfallen:** (1) Ein Größen-Limit ist KEIN Zeit-Limit: `read(n)` mit +kleinem n blockiert trotzdem bis zum Socket-Timeout, wenn der Client nichts +schickt – blockierende I/O gehört grundsätzlich VOR einen Lock, nicht +hinein. (2) `str(e)` in Fehlerantworten öffentlicher Endpoints ist ein +Info-Leak (Pfade, interne Hosts) – Details gehören ins Journal, nach draußen +nur Generisches. + +--- + ## 2026-07-04 – Fable-5-Review (r): ADMIN_LOCK-Umbau (q) geprüft – korrekt, aber 3 kleinere Restbefunde gefunden und gefixt **Erledigt:** Review-Pass über die jüngste erledigte Arbeit – den ADMIN_LOCK-Umbau @@ -813,6 +862,21 @@ schließt den Review ab. schaltet Registrierung frei). Headless verifiziert (8 Checks inkl. 8-KB-Body → 413, `Content-Length: -5` → sofort 413 statt Hänger). Details im PROGRESS-Eintrag „(r)" oben. +- [x] **(s) (r)-Fixes (5ada0e5) geprüft – korrekt, 2 kleinere Restbefunde gefixt + (2026-07-04).** (r)-Diff gegen Backup verifiziert (GET-Leser-Lock, + Content-Length-Härtung, secrets-Token – alles wie protokolliert), Dienst lief + mit aktuellem Code, kein unprotokollierter Rest im CHANGES-Hook-Log. Befunde + beim Ganzdatei-Gegenlesen: (1) ban/unban las den Body innerhalb des + `ADMIN_LOCK` – ein Client mit angekündigtem, nie gesendetem Body hielt den + Lock bis zum 30-s-Socket-Timeout (das 4-KB-Limit deckelt nur die Größe, nicht + die Wartezeit; seit (r) hing damit auch der `registration-status`-Leser mit); + Fix: Body-Lesen/Validieren vor den Lock gezogen, Handler-Logik unverändert. + (2) `livekit-token`/`e2ee-diagnostics` leakten bei Fehlern `str(e)` an + Unauthentifizierte (interne Pfade/Adressen); Fix: generisches + `{"error": "Interner Fehler"}`, Details ins Journal (stderr). Headless + verifiziert: 8 Routen-Checks unverändert, Lock-Beweis (hängender ban-Body, + paralleler registration-status-GET in 15 ms), 500-Beweis (ungültiges JSON → + generisch, Detail im Journal). Details im PROGRESS-Eintrag „(s)" oben. **Fazit:** Die Sonnet-5-Arbeit war handwerklich sauber; die zwei echten Lücken (verlorene Kommentare, fehlender Soft-Logout-Guard) hatte die abgebrochene diff --git a/ROADMAP.md b/ROADMAP.md index 892334f..27748c4 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -83,6 +83,10 @@ Punkte abhaken (`[x]`), wenn erledigt UND in `PROGRESS.md` protokolliert. `registration-status`-Leser jetzt unter ADMIN_LOCK (Truncate-Write-Race), negative/übergroße `Content-Length` überall abgelehnt (kein Thread-Hänger, ban/unban 4-KB-Deckel), Einladungs-Token per `secrets` statt `random`. + Zusatz (Fable-5-Review (s), 2026-07-04): ban/unban-Body wird jetzt VOR dem + ADMIN_LOCK gelesen (hängender Body-Read konnte den Lock 30 s festhalten), + und die öffentlichen Routen antworten bei internen Fehlern generisch statt + mit Exception-Text (Info-Leak); Details ins Journal. Headless verifiziert. - [x] **LiveKit-Token-Route server-seitig gebaut + verifiziert** (Fable-5-Review (n), PROGRESS.md 2026-07-04). `POST /api/livekit-token` läuft live in `server.py` (whoami-Auth, stdlib-HS256-Minting aus `livekit.yaml`), headless geprüft