X-Forwarded-For nur noch dem Proxy glauben, Durchsicht vom 14.09. notiert

Das CMD startete uvicorn mit --forwarded-allow-ips "*": jeder, der den
Container-Port direkt erreicht, konnte die Absender-IP frei erfinden und
bekam pro erfundener IP einen frischen Rate-Limit-Zaehler - und die
Limits sind das Einzige vor dem frei zugaenglichen Reservieren-Endpunkt.
Zusaetzlich liess sich so die HTTPS-Erkennung fuers Sitzungs-Cookie
faelschen.

Jetzt liest uvicorn die Grenze aus FORWARDED_ALLOW_IPS (ohne Wert: nur
127.0.0.1, also keinem externen Absender trauen). docker-compose.betrieb.yml
reicht die Variable mit Erklaerung durch; das README beschreibt, welche
Adresse hineingehoert und dass der Port nur fuer den Proxy erreichbar
sein darf. Wird die Variable nicht gesetzt, teilen sich alle Besucher
die Zaehler ueber die Proxy-IP - das kleinere Uebel.

Plan-Verbesserungen.md: Ergebnis der erneuten Durchsicht vom 14.09.2026
ergaenzt (GZip/Cache/Sicherung vom 13.09. nachgeprueft und fuer gut
befunden; drei P3-Funde: oeffentlicher 500er ueber ?limit=abc, nicht
atomares Reservieren, 500 statt Fehlermeldung in Betreiber-Formularen).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-14 17:19:12 +02:00
co-authored by Claude Fable 5
parent f3ac3d526e
commit 33c728411f
4 changed files with 119 additions and 1 deletions
+9 -1
View File
@@ -67,5 +67,13 @@ ENTRYPOINT ["kleiderboerse-entrypoint.sh"]
# --proxy-headers: hinter einem Reverse-Proxy stünde sonst dessen IP in # --proxy-headers: hinter einem Reverse-Proxy stünde sonst dessen IP in
# jedem Rate-Limit, und ein einzelner Besucher sperrte alle anderen aus. # jedem Rate-Limit, und ein einzelner Besucher sperrte alle anderen aus.
# Ausserdem erkennt die App darüber, ob der Browser per HTTPS verbunden ist. # 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", \ CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000", \
"--proxy-headers", "--forwarded-allow-ips", "*"] "--proxy-headers"]
+93
View File
@@ -112,3 +112,96 @@ Die Sicherung wurde gegen den laufenden Container erprobt: 4 Einträge,
Sicherungsordner. Gegenprobe mit entferntem Bild wird erkannt. Sicherungsordner. Gegenprobe mit entferntem Bild wird erkannt.
Offen bleibt P4 (Indizes auf `gender`/`season`) — ohne praktische Bedeutung. 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.**
+7
View File
@@ -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 die App nicht, dass der Browser verschlüsselt verbunden ist, und alle
Rate-Limits zählen auf die Adresse des Proxys ein einzelner Besucher Rate-Limits zählen auf die Adresse des Proxys ein einzelner Besucher
sperrte damit alle anderen aus. 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. - `COOKIE_HTTPS_ONLY` auf `"true"` lassen.
Die Galerie ist frei zugänglich, wird aber per `noindex` und `robots.txt` Die Galerie ist frei zugänglich, wird aber per `noindex` und `robots.txt`
+10
View File
@@ -42,6 +42,16 @@ services:
RATE_LIMIT_RESERVE: "5/hour" RATE_LIMIT_RESERVE: "5/hour"
RATE_LIMIT_LOGIN: "10/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: volumes:
# Hier liegen Datenbank und Bilder. Ohne diese beiden Zeilen wäre nach # Hier liegen Datenbank und Bilder. Ohne diese beiden Zeilen wäre nach
# einem Neustart alles weg. # einem Neustart alles weg.