Fix #20: Befunde der Sicherheitsprüfung #21

Merged
florian.wilde merged 2 commits from fix/20-sicherheitspruefung into main 2026-08-24 07:41:08 +00:00
8 changed files with 180 additions and 22 deletions
Showing only changes of commit 1e0d04d0db - Show all commits
+7 -4
View File
@@ -14,12 +14,15 @@ BASE_URL=http://localhost:3000
APP_PORT=3000
# Anmeldung mit Benutzern, Gruppen und Rollen.
# false — jeder, der die App erreicht, darf alles (Einzelplatz/LAN).
# true — Anmeldung erforderlich. Beim ersten Aufruf legst du den ersten
# Administrator an.
# Administrator an. Das ist die Vorgabe, auch wenn die Zeile fehlt.
# false — jeder, der die App erreicht, darf alles: Verwaltung, Uploads,
# saemtliche Codes. Nur fuer Einzelplatz- oder LAN-Betrieb, und
# nur, wenn niemand Fremdes an den Rechner kommt. Der Container
# warnt beim Start deutlich, solange das so steht.
# Bewusst hier und nicht in der Oberfläche: so kann man sich nicht aussperren.
# Wer den Zugang verliert, setzt den Wert zurück auf false und startet neu.
AUTH_ENABLED=false
# Wer den Zugang verliert, setzt den Wert auf false und startet neu.
AUTH_ENABLED=true
# Groesste erlaubte Datei fuer die Typen "PDF" und "Bild", in Megabyte.
# Die Dateien liegen in der Datenbank — wer die sichert, hat alles. Sehr
+3 -1
View File
@@ -37,7 +37,9 @@ services:
BASE_URL: ${BASE_URL:-http://localhost:3000}
DATABASE_URL: ${DATABASE_URL}
# Anmeldung mit Benutzern, Gruppen und Rollen.
AUTH_ENABLED: ${AUTH_ENABLED:-false}
# Ohne Angabe: an. Abgeschaltet wird nur, wer ausdruecklich
# AUTH_ENABLED=false setzt — siehe .env.example.
AUTH_ENABLED: ${AUTH_ENABLED:-true}
MAX_UPLOAD_MB: ${MAX_UPLOAD_MB:-10}
SESSION_IDLE_DAYS: ${SESSION_IDLE_DAYS:-30}
SESSION_MAX_DAYS: ${SESSION_MAX_DAYS:-90}
+12
View File
@@ -5,5 +5,17 @@ set -e
echo "→ Datenbankmigrationen werden angewendet …"
node /app/node_modules/prisma/build/index.js migrate deploy
# Eine Anwendung ohne Anmeldung sieht genauso aus wie eine mit — deshalb
# muss sie es sagen. Wer das hier im Protokoll sieht und nicht erwartet hat,
# soll sofort wissen, woran er ist.
if [ "$(printf '%s' "${AUTH_ENABLED:-}" | tr '[:upper:]' '[:lower:]')" = "false" ]; then
echo "" >&2
echo " !! ACHTUNG: AUTH_ENABLED=false — die Anwendung laeuft OHNE Anmeldung." >&2
echo " !! Jeder, der sie erreicht, darf alles: Verwaltung, Uploads, saemtliche" >&2
echo " !! Codes anlegen, aendern und loeschen. Das ist nur fuer Einzelplatz-" >&2
echo " !! oder LAN-Betrieb gedacht und gehoert nicht ins offene Netz." >&2
echo "" >&2
fi
echo "→ Anwendung wird gestartet …"
exec "$@"
+112 -1
View File
@@ -1,7 +1,9 @@
# Anmeldung und Sitzungen — Durchsicht nach OWASP
Durchsicht der Anmeldung, der Sitzungsverwaltung und der Berechtigungen gegen
die einschlägigen OWASP-Merkblätter. Stand: 15.08.2026.
die einschlägigen OWASP-Merkblätter. Stand: 15.08.2026, ergänzt am 24.08.2026
um die Befunde einer externen Codeprüfung (Punkte 11 bis 14 und die drei
offenen Punkte am Ende).
Geprüft gegen:
@@ -163,6 +165,72 @@ Nutzung abgewiesen —, aber die Tabelle wuchs unbegrenzt.
**Behoben:** Beim Nachführen einer Sitzung (höchstens stündlich je Sitzung)
wird beiläufig aufgeräumt.
### 11. OIDC wertete eine unbestätigte E-Mail als bestätigt
**Befund:** Der schwerste der externen Prüfung. Findet die OIDC-Rückkehr ein
lokales Konto mit derselben E-Mail-Adresse, hängt sie die Identität des
Anbieters daran — aber nur, wenn die Adresse **bestätigt** ist. Genau das
prüfte der Code auch, nur war die Bestätigung falsch berechnet:
```ts
emailVerified: payload.email_verified !== false
```
Damit galt nicht nur `true` als bestätigt, sondern auch ein **fehlender
Claim**, `null`, `0` und jede Zeichenkette. Ein Anbieter, der
`email_verified` gar nicht mitschickt — das ist erlaubt und kommt vor —,
hätte die Verknüpfung geöffnet. Wer sich dort eine fremde Adresse eintragen
kann, hätte damit das hiesige Konto übernommen, im schlimmsten Fall das eines
Administrators.
**Behoben:** Nur ein ausdrückliches `true` zählt. Alles andere führt zur
bisherigen Meldung „der Anbieter hat sie nicht bestätigt“, und die
Verknüpfung unterbleibt.
**Was das nicht war:** kein Angriff ohne Zugriff auf den Anbieter — das
ID-Token muss weiterhin gültig signiert sein und Aussteller, Empfänger und
Nonce bestehen.
### 12. Eine fehlende Angabe schaltete die Anmeldung ab
**Befund:** `isAuthEnabled()` verlangte den Wert `true`; alles andere hieß
„aus“. Eine vergessene oder falsch geschriebene Variable ergab damit eine
Anwendung, in der jeder Besucher alles darf — und nichts sah danach aus. Die
Compose-Vorgabe stand zusätzlich auf `false`.
**Behoben:** Umgedreht. Abgeschaltet wird nur, wer ausdrücklich `false`
setzt; ein fehlender Wert bedeutet „an“. Die Vorgaben in `docker-compose.yml`
und `.env.example` stehen auf `true`. Steht die Anmeldung aus, schreibt der
Container beim Start eine mehrzeilige Warnung ins Protokoll.
Der offene Betrieb bleibt möglich — er ist für Einzelplatz und LAN gedacht
und dokumentiert. Er ist nur nicht mehr das, was man versehentlich bekommt.
### 13. Wettlauf beim ersten Administrator über OIDC
**Befund:** Der OIDC-Pfad las `user.count()` und legte danach an. Zwei
gleichzeitige erste Anmeldungen konnten sich beide für die erste halten und
beide als Instanz-Administrator entstehen. Die Passwort-Ersteinrichtung
sicherte denselben Ablauf längst mit einer serialisierbaren Transaktion.
**Behoben:** Derselbe Schutz auch hier — Zählen und Anlegen liegen in einer
Transaktion mit `isolationLevel: "Serializable"`.
### 14. Die Herkunftsprüfung verglich das Schema nicht
**Befund:** `isSameOrigin()` verglich nur den Host. `http://example.com` galt
damit als eigen, obwohl die Anwendung unter `https://example.com` läuft.
**Behoben:** Das Schema wird mitverglichen, und es kommt aus `BASE_URL` — der
einen Angabe zum Ursprung, über die der Aufrufer nicht mitbestimmt.
**Zur Einordnung:** Die Prüfung vergleicht weiterhin gegen den `Host`- bzw.
`X-Forwarded-Host`-Header der Anfrage. Aus einem Browser heraus ist das nicht
zu missbrauchen — `Host` darf Skriptcode nicht setzen, und ein selbst
gesetztes `X-Forwarded-Host` macht die Anfrage zu einer, die erst einen
CORS-Vorabflug bestehen müsste, den die Anwendung nicht beantwortet. Das
verbleibende Unbehagen ist das Vertrauensmodell insgesamt; dazu unten mehr.
---
## Was schon vorher stimmte
@@ -218,6 +286,49 @@ selbst ausgestellt, falscher Name — statt bloß daran zu scheitern.
Bewusst nicht umgesetzt, mit Begründung.
**Die Kontosperre kann den Besitzer aussperren.** Acht falsche Passwörter
sperren ein Konto für 15 Minuten, ohne dass der Anfragende belegen muss, wer
er ist. Wer die Adresse des Administrators kennt, kann die Sperre alle 15
Minuten erneuern und die Verwaltung praktisch dauerhaft blockieren.
Der naheliegende Ausweg — auch während der Sperre das Passwort prüfen und ein
richtiges durchlassen — wurde geprüft und **verworfen**: Eine Prüfung kostet
hier rund **128 MiB Arbeitsspeicher** (scrypt mit N = 2¹⁷). Wer sie beliebig
oft auslösen darf, legt damit den Server lahm. Die Sperre gegen ein Konto
einzutauschen wäre also eine Sperre gegen alle.
Was es stattdessen braucht, ist ein eigener Entwurf: eine ansteigende
Verzögerung, die keinen Speicher hält, oder eine billige Vorprüfung vor dem
teuren Hash — zusammen mit dem nächsten Punkt zu entscheiden.
**Die Bremsen lassen sich über `X-Forwarded-For` umgehen.** Der Absender wird
aus diesem Header gelesen. Steht kein Proxy davor — und die mitgelieferte
Compose-Konfiguration veröffentlicht den Port direkt —, wählt ihn jeder
Aufrufer selbst und umgeht damit Login-Bremse und Feedback-Bremse. Die Zähler
liegen zudem im Arbeitsspeicher eines Prozesses und sind nach einem Neustart
weg.
Die Anwendung läuft in Next.js-Routen und kommt an die Adresse der
Gegenstelle gar nicht heran — „im Zweifel die Socket-Adresse nehmen“ ist hier
also keine Möglichkeit. Nötig ist eine ausdrückliche Einstellung, welchem
Proxy zu trauen ist, und ein gemeinsamer Zähler, wenn mehr als eine Instanz
läuft. Beides gehört mit dem Punkt davor in einen Entwurf und nicht in eine
schnelle Änderung.
**Prisma hängt an einer Advisory.** `npm audit --omit=dev` meldet
[GHSA-ggr8-5vv4-36mx](https://github.com/advisories/GHSA-ggr8-5vv4-36mx) in
`deepmerge-ts@7.1.5`, mitgeschleppt über `@prisma/config`. Es geht um
Stapelüberlauf beim Zusammenführen rekursiver Objekte.
Aus HTTP-Eingaben entsteht hier keine Prisma-Konfiguration, die praktische
Ausnutzbarkeit ist also gering. Die Prisma-CLI liegt aber im Laufzeit-Abbild
und läuft bei jedem Containerstart für die Migrationen — deshalb steht der
Punkt hier und nicht unter „egal“. Eine Fassung ohne die Advisory gibt es
noch nicht: 7.9.1 ist die neueste und hängt weiterhin an `deepmerge-ts@7.1.5`.
`npm audit fix --force` würde auf Prisma 6 zurückstufen und ist deshalb keine
Lösung. Zu tun: beobachten und aktualisieren, sobald es eine passende Fassung
gibt.
**E-Mail-Verifizierung.** Es gibt keine — und sie würde derzeit auch nichts
bringen. Die Anwendung kennt **keine Selbstregistrierung**: Es gibt nur
`login`, `setup`, `logout` und `change-password`. Konten legt ein
+23 -14
View File
@@ -128,21 +128,30 @@ async function findeOderLegeAn(
// Der erste Mensch, der sich überhaupt anmeldet, wird Administrator —
// sonst gäbe es bei `OIDC_ONLY=true` niemanden, der die Verwaltung öffnen
// kann. Das entspricht der Ersteinrichtung bei der Passwort-Anmeldung.
const istErster = (await prisma.user.count()) === 0;
const angelegt = await prisma.user.create({
data: {
email: identity.email,
name: identity.name,
passwordHash: null,
oidcSubject: identity.subject,
isInstanceAdmin: istAdmin ?? istErster,
lastLoginAt: new Date(),
memberships: standardgruppe
? { create: { groupId: standardgruppe.id, role: config.defaultRole } }
: undefined,
//
// Serialisierbar, aus demselben Grund wie dort: Zwischen „gibt es schon
// jemanden?" und dem Anlegen könnte sonst eine zweite Anmeldung
// dazwischenkommen, und beide würden sich für die erste halten. Am Ende
// stünden zwei Administratoren da, von denen einer ungebeten ist.
const angelegt = await prisma.$transaction(
async (tx) => {
const istErster = (await tx.user.count()) === 0;
return tx.user.create({
data: {
email: identity.email,
name: identity.name,
passwordHash: null,
oidcSubject: identity.subject,
isInstanceAdmin: istAdmin ?? istErster,
lastLoginAt: new Date(),
memberships: standardgruppe
? { create: { groupId: standardgruppe.id, role: config.defaultRole } }
: undefined,
},
});
},
});
{ isolationLevel: "Serializable" },
);
return { id: angelegt.id, isActive: angelegt.isActive };
}
+6 -1
View File
@@ -23,9 +23,14 @@ export type Actor =
* Bewusst eine Umgebungsvariable und kein Schalter in der Oberfläche: so
* kann man sich nicht aussperren. Wer den Zugang verliert, setzt
* `AUTH_ENABLED=false` in der .env und startet neu.
*
* **Abgeschaltet wird nur auf ausdrückliche Ansage.** Ein fehlender oder
* leerer Wert bedeutet „an“. Vorher war es umgekehrt: Wer die Variable
* vergaß, bekam eine Anwendung, in der jeder Besucher alles darf — und
* merkte es nicht, weil nichts danach aussah.
*/
export function isAuthEnabled(): boolean {
return (process.env.AUTH_ENABLED ?? "").trim().toLowerCase() === "true";
return (process.env.AUTH_ENABLED ?? "").trim().toLowerCase() !== "false";
}
export async function getActor(): Promise<Actor> {
+8 -1
View File
@@ -326,7 +326,14 @@ export async function exchangeCode(options: {
return {
subject,
email,
emailVerified: payload.email_verified !== false,
// Nur ein ausdrückliches `true` zählt. Ein fehlender Claim, `null`, `0`
// oder die Zeichenkette „true" gelten als **nicht** bestätigt.
//
// Vorher stand hier `!== false`, und damit war das Gegenteil der Fall:
// Ein Anbieter, der den Claim gar nicht schickt, hätte die Verknüpfung
// mit einem bestehenden Konto erlaubt — genau das, was die Prüfung in
// der Callback-Route verhindern soll.
emailVerified: payload.email_verified === true,
name,
groups: readGroups(payload, config.groupsClaim),
};
+9
View File
@@ -26,6 +26,15 @@ export function isSameOrigin(headers: Headers): boolean {
return false;
}
// Das Schema kommt aus `BASE_URL`, nicht aus der Anfrage: Es ist die eine
// Angabe zum Ursprung, über die der Aufrufer nicht mitbestimmt. Ohne diesen
// Vergleich gälte `http://example.com` als eigen, obwohl die Anwendung nur
// unter `https://example.com` läuft.
const erwartetesSchema = (process.env.BASE_URL ?? "").startsWith("http://")
? "http:"
: "https:";
if (herkunft.protocol !== erwartetesSchema) return false;
const host = headers.get("x-forwarded-host") ?? headers.get("host");
return Boolean(host) && herkunft.host === host;
}