diff --git a/.env.example b/.env.example index 7161f6e..5cc4282 100644 --- a/.env.example +++ b/.env.example @@ -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 diff --git a/docker-compose.yml b/docker-compose.yml index 18c4ecc..62584e5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -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} diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 6452bf1..564e918 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -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 "$@" diff --git a/docs/sicherheit.md b/docs/sicherheit.md index 366ab25..164b185 100644 --- a/docs/sicherheit.md +++ b/docs/sicherheit.md @@ -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 diff --git a/src/app/api/auth/oidc/callback/route.ts b/src/app/api/auth/oidc/callback/route.ts index 635ed9d..f37c75b 100644 --- a/src/app/api/auth/oidc/callback/route.ts +++ b/src/app/api/auth/oidc/callback/route.ts @@ -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 }; } diff --git a/src/lib/auth/actor.ts b/src/lib/auth/actor.ts index 2916db4..369e82e 100644 --- a/src/lib/auth/actor.ts +++ b/src/lib/auth/actor.ts @@ -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 { diff --git a/src/lib/auth/oidc.ts b/src/lib/auth/oidc.ts index 185a3c5..f6728fb 100644 --- a/src/lib/auth/oidc.ts +++ b/src/lib/auth/oidc.ts @@ -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), }; diff --git a/src/lib/http.ts b/src/lib/http.ts index 34a3b52..c9955c1 100644 --- a/src/lib/http.ts +++ b/src/lib/http.ts @@ -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; }