fix: Ohne Anmeldung liessen sich beliebige Dateien vom Server lesen
Die Auslieferung des Frontends nahm den Pfad roh aus der Adresse: "Path /
'../../x'" verlaesst den Ordner klaglos, und die Route liegt VOR jeder
Anmeldepruefung. Gegen die eigene Instanz nachgestellt – roh und
prozentkodiert kam beides Mal die Datei heraus:
GET /../../GEHEIM.txt -> "geheim"
GET /../../backend/app/config.py -> der Quelltext
Damit war alles lesbar, was der Dienst lesen darf: Quelltext, Konfiguration,
und mit der passenden Zahl von Ebenen die Datenbank samt verschluesseltem
Postfach-Passwort und Bank-Schluessel. Der Pfad wird jetzt aufgeloest und
muss im Auslieferungs-Ordner liegen.
Zweitens prueft der Postfach-Abruf jetzt das Zertifikat. Ohne eigenen
SSL-Kontext tut imaplib das NICHT – die Verschluesselung schuetzt dann vor
niemandem, der sich dazwischenhaengt, und das Postfach-Passwort geht mit.
Gilt fuer beide Wege, SSL und STARTTLS.
Drittens warnt der Start, wenn das Sitzungs-Cookie ohne "Secure" ausgeliefert
wird. Die Vorgabe bleibt bewusst aus: Sie umzustellen sperrt jeden aus, der
Certain im Heimnetz ueber HTTP aufruft – und ein Ausgesperrter saehe die
Ursache nirgends.
Die Befunde stammen aus dem /security-review dieser Sitzung und waren seither
offen. Sie wiegen schwerer, seit die Anwendung fuer die Bankanbindung unter
einer oeffentlichen HTTPS-Adresse erreichbar sein muss.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+24
-3
@@ -54,6 +54,8 @@ settings = get_settings()
|
|||||||
# Pfad zum gebauten Frontend (im Dev von Vite selbst ausgeliefert -> fehlt hier).
|
# Pfad zum gebauten Frontend (im Dev von Vite selbst ausgeliefert -> fehlt hier).
|
||||||
REPO_ROOT = Path(__file__).resolve().parent.parent.parent
|
REPO_ROOT = Path(__file__).resolve().parent.parent.parent
|
||||||
DIST_DIR = REPO_ROOT / "web" / "dist"
|
DIST_DIR = REPO_ROOT / "web" / "dist"
|
||||||
|
# Einmal aufgeloest: Gegen diesen Pfad wird jede Anfrage geprueft.
|
||||||
|
DIST_WURZEL = DIST_DIR.resolve()
|
||||||
|
|
||||||
|
|
||||||
@asynccontextmanager
|
@asynccontextmanager
|
||||||
@@ -66,6 +68,17 @@ async def lifespan(app: FastAPI):
|
|||||||
finally:
|
finally:
|
||||||
db.close()
|
db.close()
|
||||||
|
|
||||||
|
# Das Sitzungs-Cookie ohne „Secure" wandert auch über eine unverschlüsselte
|
||||||
|
# Verbindung – wer mitliest, ist angemeldet. Die Vorgabe bleibt trotzdem
|
||||||
|
# aus: Sie umzustellen sperrt jeden aus, der die App im Heimnetz über HTTP
|
||||||
|
# aufruft, und ein Ausgesperrter sieht die Ursache nirgends.
|
||||||
|
if not settings.cookie_secure:
|
||||||
|
log.warning(
|
||||||
|
"APP_COOKIE_SECURE ist aus – das Sitzungs-Cookie geht auch über "
|
||||||
|
"unverschlüsselte Verbindungen. Wenn Certain über HTTPS erreichbar "
|
||||||
|
"ist (und für die Bankanbindung MUSS es das), setz es auf 1."
|
||||||
|
)
|
||||||
|
|
||||||
# EZB-Kurse im HINTERGRUND laden – der Start darf nie darauf warten
|
# EZB-Kurse im HINTERGRUND laden – der Start darf nie darauf warten
|
||||||
# (langsames/geblocktes Netz oder hängendes DNS würde den Dienst sonst
|
# (langsames/geblocktes Netz oder hängendes DNS würde den Dienst sonst
|
||||||
# blockieren und der Health-Check nie grün werden).
|
# blockieren und der Health-Check nie grün werden).
|
||||||
@@ -263,7 +276,15 @@ if DIST_DIR.exists():
|
|||||||
# /api-Pfade, die keine echte Route trafen, sind echte 404 (kein SPA).
|
# /api-Pfade, die keine echte Route trafen, sind echte 404 (kein SPA).
|
||||||
if full_path.startswith("api"):
|
if full_path.startswith("api"):
|
||||||
raise HTTPException(status_code=404, detail="Not Found")
|
raise HTTPException(status_code=404, detail="Not Found")
|
||||||
candidate = DIST_DIR / full_path
|
# Aufgeloest UND geprueft, ob die Datei noch im Auslieferungs-Ordner
|
||||||
if full_path and candidate.is_file():
|
# liegt. Ohne das liefert "/../../etc/passwd" genau das aus: Der Pfad
|
||||||
return FileResponse(candidate)
|
# kommt roh aus der Adresse, und "Path / '../x'" verlaesst den Ordner
|
||||||
|
# klaglos. Ohne Anmeldung, denn diese Route liegt vor jeder Pruefung.
|
||||||
|
try:
|
||||||
|
kandidat = (DIST_DIR / full_path).resolve()
|
||||||
|
except (OSError, ValueError):
|
||||||
|
kandidat = None
|
||||||
|
if (full_path and kandidat is not None and kandidat.is_file()
|
||||||
|
and kandidat.is_relative_to(DIST_WURZEL)):
|
||||||
|
return FileResponse(kandidat)
|
||||||
return FileResponse(DIST_DIR / "index.html", headers={"Cache-Control": "no-cache"})
|
return FileResponse(DIST_DIR / "index.html", headers={"Cache-Control": "no-cache"})
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ import email
|
|||||||
import email.policy
|
import email.policy
|
||||||
import email.utils
|
import email.utils
|
||||||
import imaplib
|
import imaplib
|
||||||
|
import ssl
|
||||||
import logging
|
import logging
|
||||||
from datetime import date, datetime, timedelta, timezone
|
from datetime import date, datetime, timedelta, timezone
|
||||||
|
|
||||||
@@ -49,12 +50,17 @@ def _verbinden(quelle: MailSource) -> imaplib.IMAP4:
|
|||||||
try:
|
try:
|
||||||
if art == "ssl":
|
if art == "ssl":
|
||||||
# TLS ab der ersten Zeile – der Normalfall auf Port 993.
|
# TLS ab der ersten Zeile – der Normalfall auf Port 993.
|
||||||
verbindung = imaplib.IMAP4_SSL(quelle.host, quelle.port)
|
# MIT eigenem Kontext: Ohne einen prueft imaplib das Zertifikat
|
||||||
|
# nicht, und die Verschluesselung schuetzt dann vor niemandem, der
|
||||||
|
# sich dazwischenhaengt – das Postfach-Passwort geht mit.
|
||||||
|
verbindung = imaplib.IMAP4_SSL(quelle.host, quelle.port,
|
||||||
|
ssl_context=ssl.create_default_context())
|
||||||
else:
|
else:
|
||||||
verbindung = imaplib.IMAP4(quelle.host, quelle.port)
|
verbindung = imaplib.IMAP4(quelle.host, quelle.port)
|
||||||
if art == "starttls":
|
if art == "starttls":
|
||||||
# Erst im Klartext verbinden, dann verschlüsseln (Port 143).
|
# Erst im Klartext verbinden, dann verschlüsseln (Port 143) –
|
||||||
verbindung.starttls()
|
# ebenfalls mit geprüftem Zertifikat, aus demselben Grund.
|
||||||
|
verbindung.starttls(ssl.create_default_context())
|
||||||
except OSError as e:
|
except OSError as e:
|
||||||
raise MailFehler(f"Keine Verbindung zu {quelle.host}:{quelle.port} – {e}") from e
|
raise MailFehler(f"Keine Verbindung zu {quelle.host}:{quelle.port} – {e}") from e
|
||||||
except imaplib.IMAP4.error as e:
|
except imaplib.IMAP4.error as e:
|
||||||
|
|||||||
@@ -25,3 +25,71 @@ def test_users_are_isolated(admin):
|
|||||||
assert len(other.get("/api/categories").json()) >= 6
|
assert len(other.get("/api/categories").json()) >= 6
|
||||||
# Kein Admin: darf keine Nutzer listen.
|
# Kein Admin: darf keine Nutzer listen.
|
||||||
assert other.get("/api/users").status_code == 403
|
assert other.get("/api/users").status_code == 403
|
||||||
|
|
||||||
|
|
||||||
|
def test_kein_pfadwechsel_aus_dem_auslieferungs_ordner(client, tmp_path):
|
||||||
|
"""Diese Route liegt VOR jeder Anmeldepruefung. Ohne Begrenzung liefert
|
||||||
|
"/../../etc/passwd" genau das aus – der Pfad kommt roh aus der Adresse,
|
||||||
|
und "Path / '../x'" verlaesst den Ordner klaglos."""
|
||||||
|
from app import main
|
||||||
|
|
||||||
|
if not main.DIST_DIR.exists():
|
||||||
|
import pytest
|
||||||
|
pytest.skip("kein gebautes Frontend vorhanden")
|
||||||
|
|
||||||
|
geheim = main.DIST_DIR.parent.parent / "PRUEF-GEHEIM.txt"
|
||||||
|
geheim.write_text("streng geheim", encoding="utf-8")
|
||||||
|
try:
|
||||||
|
for pfad in ("/../../PRUEF-GEHEIM.txt",
|
||||||
|
"/%2e%2e%2f%2e%2e%2fPRUEF-GEHEIM.txt",
|
||||||
|
"/../../backend/app/config.py"):
|
||||||
|
r = client.get(pfad)
|
||||||
|
assert "streng geheim" not in r.text, f"{pfad} gab die Datei heraus"
|
||||||
|
assert "APP_DATA_DIR" not in r.text, f"{pfad} gab Quelltext heraus"
|
||||||
|
finally:
|
||||||
|
geheim.unlink(missing_ok=True)
|
||||||
|
|
||||||
|
|
||||||
|
def test_echte_dateien_werden_weiterhin_ausgeliefert(client):
|
||||||
|
"""Die Begrenzung darf die Auslieferung nicht kaputtmachen."""
|
||||||
|
from app import main
|
||||||
|
|
||||||
|
if not (main.DIST_DIR / "index.html").exists():
|
||||||
|
import pytest
|
||||||
|
pytest.skip("kein gebautes Frontend vorhanden")
|
||||||
|
assert client.get("/").status_code == 200
|
||||||
|
assert "<!doctype html" in client.get("/irgendeine/spa/route").text.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_imap_prueft_das_zertifikat(monkeypatch):
|
||||||
|
"""Ohne eigenen Kontext prueft imaplib das Zertifikat NICHT – die
|
||||||
|
Verschluesselung schuetzt dann vor niemandem, der sich dazwischenhaengt,
|
||||||
|
und das Postfach-Passwort geht mit."""
|
||||||
|
import ssl
|
||||||
|
|
||||||
|
from app.services import mail_import
|
||||||
|
|
||||||
|
gesehen = {}
|
||||||
|
|
||||||
|
class FakeSSL:
|
||||||
|
def __init__(self, host, port, ssl_context=None, **kw):
|
||||||
|
gesehen["context"] = ssl_context
|
||||||
|
|
||||||
|
def login(self, *a, **kw):
|
||||||
|
return "OK", []
|
||||||
|
|
||||||
|
def __getattr__(self, name):
|
||||||
|
return lambda *a, **kw: ("OK", [])
|
||||||
|
|
||||||
|
monkeypatch.setattr(mail_import.imaplib, "IMAP4_SSL", FakeSSL)
|
||||||
|
monkeypatch.setattr(mail_import.geheimnis, "entschluesseln", lambda x: "pw")
|
||||||
|
|
||||||
|
class Quelle:
|
||||||
|
password_enc, host, port, security = "x", "imap.example", 993, "ssl"
|
||||||
|
username = "wer@example.com"
|
||||||
|
|
||||||
|
mail_import._verbinden(Quelle())
|
||||||
|
kontext = gesehen["context"]
|
||||||
|
assert kontext is not None, "ohne Kontext prueft imaplib nichts"
|
||||||
|
assert kontext.verify_mode == ssl.CERT_REQUIRED
|
||||||
|
assert kontext.check_hostname is True
|
||||||
|
|||||||
Reference in New Issue
Block a user