diff --git a/Dockerfile b/Dockerfile index 32e9f33..ee1ee08 100644 --- a/Dockerfile +++ b/Dockerfile @@ -67,5 +67,13 @@ ENTRYPOINT ["kleiderboerse-entrypoint.sh"] # --proxy-headers: hinter einem Reverse-Proxy stünde sonst dessen IP in # jedem Rate-Limit, und ein einzelner Besucher sperrte alle anderen aus. # Ausserdem erkennt die App darüber, ob der Browser per HTTPS verbunden ist. +# +# WEM die X-Forwarded-*-Kopfzeilen geglaubt werden, steuert +# FORWARDED_ALLOW_IPS - uvicorn liest die Variable selbst aus der Umgebung +# (ohne Wert: nur 127.0.0.1). Früher stand hier "--forwarded-allow-ips *": +# damit durfte jeder, der den Port direkt erreicht, die Absender-IP frei +# erfinden - pro erfundener IP ein frischer Rate-Limit-Zähler, und die +# Limits sind das Einzige vor dem Reservieren-Endpunkt. Im Betrieb die +# Adresse des Proxys eintragen (siehe docker-compose.betrieb.yml). CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000", \ - "--proxy-headers", "--forwarded-allow-ips", "*"] + "--proxy-headers"] diff --git a/Plan-Verbesserungen.md b/Plan-Verbesserungen.md index 8fd266c..2aed07d 100644 --- a/Plan-Verbesserungen.md +++ b/Plan-Verbesserungen.md @@ -112,3 +112,96 @@ Die Sicherung wurde gegen den laufenden Container erprobt: 4 Einträge, Sicherungsordner. Gegenprobe mit entferntem Bild wird erkannt. Offen bleibt P4 (Indizes auf `gender`/`season`) — ohne praktische Bedeutung. + +--- + +## Durchsicht 14.09.2026 + +Erneute Durchsicht des gesamten Repos, mit Schwerpunkt auf den seit dem +13.09. gemergten Änderungen (`c986161`, Branch `verbesserungen`) — die +waren bisher ungeprüft. + +**Ergebnis der Nachprüfung:** GZip, Cache-Header und Sicherungsskript sind +korrekt umgesetzt. Tests lokal ausgeführt: **80 von 81 grün** — der eine +rote (`test_heic_vom_iphone_wird_angenommen`) liegt allein daran, dass +`pillow-heif` in der improvisierten Umgebung fehlte (Python 3.14, kein +Wheel); im Docker-Testimage ist das Paket enthalten. Kein Code-Fehler. + +### P2 — `--forwarded-allow-ips "*"` macht die Rate-Limits umgehbar ✅ ERLEDIGT 14.09.2026 + +Das `CMD` im Dockerfile vertraut `X-Forwarded-For` von **jedem** Absender. +Wer den Container-Port direkt erreicht (das Compose veröffentlicht +`8090:8000` auf dem Host), setzt den Header selbst und bekommt pro +erfundener IP einen frischen Zähler — die Rate-Limits sind laut eigenem +Kommentar „das Einzige, was zwischen einem Skript und dem Bestand steht", +und genau die fallen damit. Zusätzlich lässt sich so die HTTPS-Erkennung +fürs Cookie fälschen. + +Hinter einem sauber konfigurierten Proxy, der den Header **ersetzt**, und +mit einem von aussen unerreichbaren Port 8000 ist alles gut — aber darauf +verlässt sich das Image derzeit stillschweigend. + +**Vorgehen:** `FORWARDED_ALLOW_IPS` als Umgebungsvariable durchreichen +(Standard: leer = keinem trauen) und in `docker-compose.betrieb.yml` die +Proxy-IP eintragen; dort ausserdem den Hinweis ergänzen, dass der +veröffentlichte Port nur für den Proxy erreichbar sein darf. + +### P3 — Öffentlicher 500er über `?limit=abc` + +`pages.py` (`galerie`, `liste_ausschnitt`) rechnet +`int(request.query_params.get("limit") or SEITE)` ohne Prüfung — `/?limit=x` +liefert jedem anonymen Besucher einen internen Serverfehler, `?limit=-1` +bedeutet in SQLite „alles". Die API-Seite macht es mit +`Query(ge=1, le=200)` längst richtig; dieselbe Grenze hier nachziehen +(ungültig → Standardwert). + +### P3 — Betreiber-Formulare: 500 statt Fehlermeldung + +In `erfassen` und `bearbeiten` (`pages.py`) wird `int(category_id)` ohne +Prüfung gerechnet, und `ItemAnlegen(...)` wirft bei ungültigen Werten eine +ValidationError **im** Handler — beides ergibt einen 500er statt der sonst +üblichen freundlichen Meldung. Erreichbar nur angemeldet, und die +HTML-Formulare (Dropdown, `maxlength`) verhindern es auf dem normalen Weg — +es bricht aber das Muster, das `reservieren` mit try/except vormacht. + +### P3 — Reservieren: Prüfen und Setzen sind nicht atomar + +`reservieren` prüft erst `status == available` und schreibt dann. Zwei +gleichzeitige Anfragen können beide die Prüfung passieren; der zweite +Commit überschreibt Name und Token der ersten Person, ohne dass die es +erfährt — und genau dieser Fall (zwei wollen dasselbe Stück) ist der +wahrscheinlichste Konflikt der ganzen App. Robust wäre ein +`UPDATE … SET status='reserved' … WHERE id=? AND status='available'` mit +Prüfung der betroffenen Zeilen. + +### P4 — Kleinigkeiten (neu) + +* **HEIC-Test** könnte mit `pytest.importorskip("pillow_heif")` sauber + übersprungen werden, wenn das Paket fehlt — dann bleibt die Suite auch + ausserhalb des Docker-Images aussagekräftig. +* **`description` ohne Längenbegrenzung** (`schemas.py`) — Titel und + Grösse haben eine, das Beschreibungsfeld nicht. Nur vom Betreiber + befüllbar. +* **Freigabe-Link trägt das Token in der URL** (GET `/kleid/…/freigeben`) + — landet damit in Proxy-Logs und Browserverlauf. Für den Zweck + (einmalige Selbst-Freigabe) hingenommen, der Vollständigkeit halber + notiert. + +### Stand der alten Punkte + +P4 (Indizes auf `gender`/`season`) bleibt offen und bleibt bedeutungslos. + +### Umsetzung des P2 am 14.09.2026 + +`--forwarded-allow-ips "*"` ist aus dem `CMD` entfernt; uvicorn liest die +Grenze jetzt aus der Umgebungsvariable `FORWARDED_ALLOW_IPS` (ohne Wert: +nur 127.0.0.1, also keinem externen Absender trauen — fail closed). +`docker-compose.betrieb.yml` reicht die Variable mit Erklärung durch, das +README beschreibt, welche Adresse hineingehört und dass der Port nur für +den Proxy erreichbar sein darf. + +Bewusste Folge der sicheren Vorgabe: Wer die Variable nicht setzt, hat +funktionierende Rate-Limits, aber alle Besucher teilen sich die Zähler +über die Proxy-IP. Das ist das kleinere Übel gegenüber frei erfindbaren +Absenderadressen. **Beim nächsten Deployment die Proxy-IP eintragen und +einmal prüfen, dass `request.client.host` die echte Besucher-IP zeigt.** diff --git a/README.md b/README.md index 6fb7eda..f9c4fd1 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,13 @@ Die App gehört hinter einen Reverse-Proxy, der HTTPS beendet. Wichtig dabei: die App nicht, dass der Browser verschlüsselt verbunden ist, und alle Rate-Limits zählen auf die Adresse des Proxys – ein einzelner Besucher sperrte damit alle anderen aus. +- `FORWARDED_ALLOW_IPS` auf die Adresse setzen, mit der der Proxy im + Container ankommt (siehe Kommentar in `docker-compose.betrieb.yml`). + Ohne den Wert werden die Kopfzeilen ignoriert; mit `"*"` könnte jeder, + der den Container-Port direkt erreicht, die Absender-IP erfinden und + damit die Rate-Limits umgehen. +- Der veröffentlichte Port (`8090`) gehört hinter die Firewall: erreichbar + nur für den Proxy, nicht aus dem Internet. - `COOKIE_HTTPS_ONLY` auf `"true"` lassen. Die Galerie ist frei zugänglich, wird aber per `noindex` und `robots.txt` diff --git a/docker-compose.betrieb.yml b/docker-compose.betrieb.yml index 6328a07..e4200e1 100644 --- a/docker-compose.betrieb.yml +++ b/docker-compose.betrieb.yml @@ -42,6 +42,16 @@ services: RATE_LIMIT_RESERVE: "5/hour" RATE_LIMIT_LOGIN: "10/hour" + # Nur diesem Absender werden die X-Forwarded-*-Kopfzeilen geglaubt. + # Hier gehört die Adresse hin, mit der der Reverse-Proxy im Container + # ankommt - bei Docker meist das Bridge-Gateway (nachsehen mit: + # docker compose logs, oder "172.17.0.1" bzw. "172.18.0.1" probieren). + # Leer = keinem trauen: die App läuft trotzdem, aber alle Besucher + # teilen sich dann die Rate-Limits über die Adresse des Proxys. + # Niemals "*" eintragen: dann kann jeder, der den Port direkt + # erreicht, mit erfundenen Adressen die Rate-Limits umgehen. + FORWARDED_ALLOW_IPS: "" + volumes: # Hier liegen Datenbank und Bilder. Ohne diese beiden Zeilen wäre nach # einem Neustart alles weg.