From c8ff850c986669e9b33abb06dfce49fd7b6d9455 Mon Sep 17 00:00:00 2001 From: Bernd Steckmeister Date: Sat, 4 Jul 2026 14:20:42 +0200 Subject: [PATCH] security: server.py Admin-Routen serialisieren (ADMIN_LOCK, Review-Befund q) Threading-Umstellung aus (p) hatte eine Nebenlaeufigkeits-Race in den Admin-Routen erzeugt: conduit.toml Read-Modify-Write + docker restart und der send_admin-Readback-nach-sleep waren seit multithreaded race-faehig. ADMIN_LOCK serialisiert NUR die Admin-Mutationen; die oeffentlichen Routen (livekit-token, e2ee-diagnostics) laufen zuerst und bleiben nebenlaeufig, der Slow-Loris-Schutz aus (p) bleibt erhalten. Headless verifiziert. Co-Authored-By: Claude Fable 5 --- CHANGES.md | 4 ++++ PROGRESS.md | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++ ROADMAP.md | 5 +++++ 3 files changed, 70 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 8dfcc75..9bdc75e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1261,3 +1261,7 @@ 2026-07-04 13:51 [Edit] /home/steggi/pyramid/PROGRESS.md 2026-07-04 13:51 [Edit] /home/steggi/pyramid/ROADMAP.md 2026-07-04 13:51 [Edit] /home/steggi/.claude-pyramid/projects/-home-steggi-pyramid/memory/pyramid-arbeitsstand.md +2026-07-04 14:17 [Write] /home/steggi/matrix/server.py +2026-07-04 14:19 [Edit] /home/steggi/pyramid/PROGRESS.md +2026-07-04 14:20 [Edit] /home/steggi/pyramid/PROGRESS.md +2026-07-04 14:20 [Edit] /home/steggi/pyramid/ROADMAP.md diff --git a/PROGRESS.md b/PROGRESS.md index 0fb1306..b03220b 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -13,6 +13,53 @@ Schritt (und beim Abbruch mitten im Schritt den Zwischenstand). Format: --- +## 2026-07-04 – Fable-5-Review (q): Threading-Härtung (p) tiefer geprüft – echte Nebenläufigkeits-Race in den Admin-Routen gefunden und mit ADMIN_LOCK behoben + +**Erledigt:** Review-Pass über die jüngste erledigte Arbeit – die Threading-Umstellung +aus (p) (Commit 343545b, `ThreadingHTTPServer`) noch einmal kritisch gegengelesen und +als Punkt (q) im Fable-5-Review-Abschnitt abgehakt. Befund: Die Umstellung auf +multithreaded hat einen **echten Race erzeugt, den es vorher (single-threaded, alles +serialisiert) nicht geben konnte** – (p) hat nur den öffentlich beschreibbaren +Diagnose-Pfad per `DIAG_LOCK` abgesichert, die **Admin-Routen aber übersehen**: +- `toggle-registration` und `create-invite` machen ein Read-Modify-Write auf + `conduit.toml` **plus** `docker compose restart conduit` – zwei gleichzeitige Klicks + = verlorenes Update (lost update) + doppelter Container-Neustart. +- `send_admin()` schläft 4 s und liest danach die **letzten** Nachrichten aus dem + Admin-Raum zurück, um „die" Antwort zuzuordnen – zwei gleichzeitige Admin-Befehle + können sich vertauschte Antworten einfangen. + +**Fix (`server.py`):** Neues `ADMIN_LOCK` (threading.Lock), das **nur** die +Admin-Mutationen serialisiert. Dafür `do_POST` minimal umgebaut: die beiden +öffentlichen Routen (`/api/livekit-token`, `/api/e2ee-diagnostics`) werden jetzt +**zuerst** behandelt und returnen, danach folgt das Heimnetz-Gate und der komplette +Admin-Dispatch in einem `with ADMIN_LOCK:`. Damit bleiben die öffentlichen Pfade +**nebenläufig** – der Slow-Loris-Schutz aus (p) ist unangetastet. Handler-Logik +wortwörtlich unverändert (nur verschoben/eingerückt, per `diff` gegen Backup +verifiziert). `python3 -m py_compile` sauber; Backup `server.py.bak-20260704-adminlock`. + +**Verifiziert (headless, nach Dienst-Neustart per kill/`Restart=always`):** +8 Routen-Checks identisch wie vorher (stats 200; ban localhost → 400 „Kein Nutzer", +ban „aus dem Internet"/gespoofter Cf-Header → 403; livekit-token 401/400/413; +e2ee-diagnostics falscher Token 403; unbekannte Route 404). **Race-Beweis:** ein +`run-stats` allein 1388 ms, **zwei parallel 2633 ms** (≈ 2× → serialisiert; ohne +Lock wären es ~1400 ms). **Non-Blocking-Beweis:** ein öffentliches `livekit-token` +**während** ein Admin-`run-stats` den Lock hält → in **11 ms** beantwortet (nicht +blockiert). 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:** Eine Threading-Umstellung ist nie „nur" ein Server-Konstruktor- +Tausch – sie macht **jeden** geteilten Zustand (Dateien, Read-Modify-Write, +Readback-nach-sleep) plötzlich race-fähig. (p) hatte richtig den einen öffentlich +beschreibbaren Pfad (`DIAG_LOCK`) gesehen, aber die Admin-Routen mit `conduit.toml` +und dem Admin-Raum-Readback sind derselbe Fall. Lehre: Nach „Server ist jetzt +multithreaded" ALLE schreibenden/zustandsändernden Handler durchgehen, nicht nur +den einen, der den Anlass gab. + +--- + ## 2026-07-04 – Fable-5-Review (p): Interims-Gate geprüft (korrekt) + halbfertige Threading-Härtung einer abgebrochenen Session vollendet **Erledigt:** Review-Pass über die jüngste erledigte Arbeit (Interims-Gate, @@ -684,6 +731,20 @@ schließt den Review ab. Vollendet (Threading + Lock-Nutzung), Dienst neu gestartet, 11 Headless- Prüfungen grün inkl. Slow-Loris (parallele Anfrage 10 ms statt blockiert; hängender Body-Read nach 30 s gekappt). Details im PROGRESS-Eintrag „(p)" oben. +- [x] **(q) Threading-Umstellung aus (p) tiefer geprüft – echte Nebenläufigkeits-Race + in den Admin-Routen gefunden und mit `ADMIN_LOCK` behoben (2026-07-04).** (p) hatte + den öffentlich beschreibbaren Diagnose-Pfad per `DIAG_LOCK` abgesichert, aber die + Admin-Routen übersehen: `toggle-registration`/`create-invite` machen ein + Read-Modify-Write auf `conduit.toml` + `docker restart`, und `send_admin()` liest + nach 4 s die letzten Admin-Raum-Nachrichten zurück – beides seit multithreaded + race-fähig (lost update, doppelter Neustart, vertauschte Antworten). Fix: neues + `ADMIN_LOCK`, das **nur** die Admin-Mutationen serialisiert; die öffentlichen Routen + (`livekit-token`, `e2ee-diagnostics`) laufen zuerst und bleiben nebenläufig + (Slow-Loris-Schutz aus (p) unangetastet). Handler-Logik wortwörtlich unverändert + (per `diff` gegen Backup verifiziert). Headless verifiziert: 8 Routen-Checks wie + vorher; Race-Beweis (2 parallele `run-stats` 2633 ms ≈ 2× einzeln 1388 ms → + serialisiert); Non-Blocking-Beweis (öffentliches `livekit-token` während gehaltenem + Lock in 11 ms). Details im PROGRESS-Eintrag „(q)" 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 7dc9bc4..1640719 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -74,6 +74,11 @@ Punkte abhaken (`[x]`), wenn erledigt UND in `PROGRESS.md` protokolliert. Zusatz (Fable-5-Review (p), 2026-07-04): `server.py` läuft jetzt multithreaded mit 30-s-Socket-Timeout – ein einzelner langsamer Client kann den Server (und damit `/api/livekit-token`) nicht mehr blockieren; headless verifiziert. + Zusatz (Fable-5-Review (q), 2026-07-04): Der durch (p) eingeführte + Nebenläufigkeits-Race in den Admin-Routen (Read-Modify-Write auf `conduit.toml`, + `send_admin`-Readback) ist mit `ADMIN_LOCK` geschlossen, das NUR die + Admin-Mutationen serialisiert – die öffentlichen Routen bleiben nebenläufig; + headless verifiziert (2× parallel = 2× Laufzeit, öffentlicher Pfad 11 ms). - [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