From 6b804cdc825b3021ef50d467b00f08089018fe9d Mon Sep 17 00:00:00 2001 From: duffyduck Date: Fri, 1 May 2026 07:59:19 +0200 Subject: [PATCH] Security-Hardening Runde 8: DNS-Rebinding + Per-File-Ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loose Ends aus Runde 5/7 abgearbeitet. 🛡 DNS-Rebinding-Schutz in SSRF-Guard - safeResolveHost() löst Hostname zu IPv4+IPv6 auf, prüft jede IP gegen die Block-Liste, gibt {ip, servername} zurück. - Caller (test-connection, test-mail-access) übergibt host=ip plus servername=hostname an die Mail-Services. Damit kann ein zweiter DNS-Lookup zur Connection-Zeit nicht plötzlich auf interne IPs umlenken (rebound-Attack). - ImapCredentials/SmtpCredentials um optionales servername-Feld erweitert; Services nutzen es als TLS-SNI / Cert-Validation-Hint. 🔒 Per-File-Ownership-Check (DSGVO-Härtung) - express.static('/api/uploads') ersetzt durch GET /api/files/download mit Pfad→Resource→Owner-Mapping in fileDownload.service.ts. - 12 subDir-Mappings (bank-cards, documents, contract-documents, invoices, cancellation-*, authorizations, business-/commercial-/ privacy-, pdf-templates). - canAccessCustomer / canAccessContract / Permission-Check je nach Owner-Typ. Portal-User sieht jetzt nur eigene Dateien, selbst wenn er fremde Filenames kennt. - Backwards-Compat: /api/uploads/* bleibt als Shim erhalten, ruft intern denselben Owner-Check. - Frontend fileUrl() zeigt auf /api/files/download?path=...&token=... Live-verifiziert: - Eigene Datei: 200, random Pfad: 404, ../etc/passwd: 400, kein Token: 401, Backwards-Compat-Shim: 200. - DNS-Rebinding: nip.io-Hostname mit interner Target-IP wird via DNS-Lookup geblockt; gmail.com (legitim) geht durch. Bewusst nicht gemacht: - Signierte URLs mit kurzlebigen Download-Tokens – v1.2-Item, da invasiv für -Flows ohne JS. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../controllers/emailProvider.controller.ts | 31 +++-- .../controllers/fileDownload.controller.ts | 84 ++++++++++++ backend/src/index.ts | 29 +++- backend/src/services/fileDownload.service.ts | 126 ++++++++++++++++++ backend/src/services/imapService.ts | 9 ++ backend/src/services/smtpService.ts | 16 ++- backend/src/utils/ssrfGuard.ts | 50 +++++++ backend/todo.md | 33 +++++ frontend/src/utils/fileUrl.ts | 23 ++-- 9 files changed, 372 insertions(+), 29 deletions(-) create mode 100644 backend/src/controllers/fileDownload.controller.ts create mode 100644 backend/src/services/fileDownload.service.ts diff --git a/backend/src/controllers/emailProvider.controller.ts b/backend/src/controllers/emailProvider.controller.ts index 11288d52..28301b91 100644 --- a/backend/src/controllers/emailProvider.controller.ts +++ b/backend/src/controllers/emailProvider.controller.ts @@ -7,7 +7,7 @@ import { ApiResponse } from '../types/index.js'; import { testImapConnection, ImapCredentials } from '../services/imapService.js'; import { testSmtpConnection, SmtpCredentials } from '../services/smtpService.js'; import { decrypt } from '../utils/encryption.js'; -import { assertAllowedHost } from '../utils/ssrfGuard.js'; +import { assertAllowedHost, safeResolveHost } from '../utils/ssrfGuard.js'; import { PrismaClient } from '@prisma/client'; const prisma = new PrismaClient(); @@ -119,13 +119,15 @@ export async function testConnection(req: Request, res: Response): Promise domain: req.body.domain, } : undefined; - // SSRF-Guard: testData.apiUrl-Hostname prüfen + // SSRF-Guard inkl. DNS-Rebinding: testData.apiUrl-Hostname zu IP auflösen + // und prüfen. Wenn DNS auf eine geblockte IP zeigt, abbrechen – ohne dass + // ein zweiter Lookup zur Connection-Zeit eine andere IP liefern könnte. if (testData?.apiUrl) { try { const url = new URL(testData.apiUrl); - assertAllowedHost(url.hostname, 'apiUrl-Host'); + await safeResolveHost(url.hostname, 'apiUrl-Host'); } catch (err) { - if (err instanceof Error && err.message.includes('geblockte')) { + if (err instanceof Error && (err.message.includes('geblockte') || err.message.includes('DNS'))) { res.status(400).json({ success: false, error: err.message } as ApiResponse); return; } @@ -229,12 +231,17 @@ export async function testMailAccess(req: Request, res: Response): Promise return; } - // SSRF-Guard: Wenn der Host vom Body kommt, blockieren wir Cloud-Metadata - // und Reserved-Ranges. Loopback/Private-Ranges bleiben erlaubt für - // legitime Plesk/Postfix-Setups. + // SSRF-Guard inkl. DNS-Rebinding: Hostnames pre-resolven und gegen + // geblockte IPs prüfen. Connection läuft danach gegen die IP, der + // ursprüngliche Hostname wird als TLS-servername gesetzt – damit kann + // ein zweiter DNS-Lookup keine andere IP unterschieben. + let smtpResolved: { ip: string; servername: string }; + let imapResolved: { ip: string; servername: string }; try { - assertAllowedHost(smtpServer, 'SMTP-Server'); - assertAllowedHost(imapServer, 'IMAP-Server'); + [smtpResolved, imapResolved] = await Promise.all([ + safeResolveHost(smtpServer, 'SMTP-Server'), + safeResolveHost(imapServer, 'IMAP-Server'), + ]); } catch (err) { res.status(400).json({ success: false, @@ -245,22 +252,24 @@ export async function testMailAccess(req: Request, res: Response): Promise // IMAP testen const imapCredentials: ImapCredentials = { - host: imapServer, + host: imapResolved.ip, port: imapPort, user: emailAddress, password, encryption: imapEncryption, allowSelfSignedCerts, + servername: imapResolved.servername, }; // SMTP testen const smtpCredentials: SmtpCredentials = { - host: smtpServer, + host: smtpResolved.ip, port: smtpPort, user: emailAddress, password, encryption: smtpEncryption, allowSelfSignedCerts, + servername: smtpResolved.servername, }; let imapResult: { success: boolean; error?: string } = { success: false }; diff --git a/backend/src/controllers/fileDownload.controller.ts b/backend/src/controllers/fileDownload.controller.ts new file mode 100644 index 00000000..eb949931 --- /dev/null +++ b/backend/src/controllers/fileDownload.controller.ts @@ -0,0 +1,84 @@ +import { Response } from 'express'; +import path from 'path'; +import fs from 'fs'; +import { AuthRequest } from '../types/index.js'; +import { findUploadOwner } from '../services/fileDownload.service.js'; +import { canAccessCustomer, canAccessContract } from '../utils/accessControl.js'; + +/** + * Authentifizierter Download-Endpoint mit Per-File-Ownership-Check. + * Ersetzt das ungeschützte `express.static('/api/uploads')`. + * + * Aufruf: GET /api/files/download?path=/uploads// + * + * Schritte: + * 1. Pfad-Format prüfen (muss mit /uploads/ beginnen, kein Traversal) + * 2. Owner via DB-Lookup ermitteln (welcher Customer/Contract gehört dazu?) + * 3. canAccessCustomer / canAccessContract / Permission-Check + * 4. Datei senden (mit korrektem Content-Type) + * + * Sicherheitsgewinn ggü. dem alten static-Handler: ein eingeloggter + * Portal-Kunde kann jetzt nur seine eigenen Files (oder die seiner + * vertretenen Kunden mit Vollmacht) herunterladen – nicht mehr beliebige + * Pfade von fremden Kunden, selbst wenn er die Filenames irgendwo + * mitgeschnitten hätte. + */ +export async function downloadFile(req: AuthRequest, res: Response): Promise { + const requested = typeof req.query.path === 'string' ? req.query.path : ''; + if (!requested) { + res.status(400).json({ success: false, error: 'path-Parameter fehlt' }); + return; + } + + // Format-Validierung (Traversal-Schutz) + if (!requested.startsWith('/uploads/') || requested.includes('..') || requested.includes('\0')) { + res.status(400).json({ success: false, error: 'Ungültiger Pfad' }); + return; + } + + // Owner ermitteln + const owner = await findUploadOwner(requested); + if (!owner) { + res.status(404).json({ success: false, error: 'Datei nicht gefunden' }); + return; + } + + // Access-Check je nach Owner-Typ + if (owner.kind === 'customer') { + if (!(await canAccessCustomer(req, res, owner.customerId))) return; + } else if (owner.kind === 'contract') { + if (!(await canAccessContract(req, res, owner.contractId))) return; + } else if (owner.kind === 'admin') { + // PDF-Vorlagen: nur Mitarbeiter mit settings:read + const perms = req.user?.permissions || []; + if (!perms.includes('settings:read') && !perms.includes('settings:update')) { + res.status(403).json({ success: false, error: 'Keine Berechtigung' }); + return; + } + } else if (owner.kind === 'gdpr-admin') { + const perms = req.user?.permissions || []; + if (!perms.includes('gdpr:admin')) { + res.status(403).json({ success: false, error: 'Keine Berechtigung' }); + return; + } + } + + // Datei vom Disk lesen + // requested startet mit /uploads/, wir mappen das auf process.cwd()/uploads/... + const relative = requested.substring('/uploads/'.length); + const absolute = path.join(process.cwd(), 'uploads', relative); + // Letzter Pfad-Sicherheitscheck: absolute Path muss noch unter uploads/ liegen. + const uploadsRoot = path.join(process.cwd(), 'uploads') + path.sep; + if (!absolute.startsWith(uploadsRoot)) { + res.status(400).json({ success: false, error: 'Ungültiger Pfad' }); + return; + } + if (!fs.existsSync(absolute)) { + res.status(404).json({ success: false, error: 'Datei nicht gefunden' }); + return; + } + + // Content-Type aus Extension bestimmen (konservativ – Express macht das eh) + res.setHeader('X-Content-Type-Options', 'nosniff'); + res.sendFile(absolute); +} diff --git a/backend/src/index.ts b/backend/src/index.ts index ee0474fc..595802e7 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -34,6 +34,7 @@ import emailLogRoutes from './routes/emailLog.routes.js'; import pdfTemplateRoutes from './routes/pdfTemplate.routes.js'; import birthdayRoutes from './routes/birthday.routes.js'; import factoryDefaultsRoutes from './routes/factoryDefaults.routes.js'; +import { downloadFile } from './controllers/fileDownload.controller.js'; import { startBirthdayScheduler } from './services/birthdayScheduler.service.js'; import { startContractStatusScheduler } from './services/contractStatusScheduler.service.js'; import { auditContextMiddleware } from './middleware/auditContext.js'; @@ -101,12 +102,28 @@ app.use(express.json({ limit: '5mb' })); app.use(auditContextMiddleware); app.use(auditMiddleware); -// Statische Dateien für Uploads – NUR für authentifizierte User. -// authenticate-Middleware unterstützt ?token=... Query-Parameter für direkte -// -Downloads, bei denen der Browser keinen Authorization-Header sendet. -// Ohne diesen Schutz könnte jeder per Datei-Name-Enumeration sensible PDFs -// (Ausweise, Kündigungsbestätigungen, Bankkarten) abrufen – DSGVO-GAU. -app.use('/api/uploads', authenticate as any, express.static(path.join(process.cwd(), 'uploads'))); +// Datei-Download mit Per-File-Ownership-Check (ersetzt das alte +// `/api/uploads/*` express.static). +// Frontend-URLs gehen jetzt über GET /api/files/download?path=/uploads/... +// Der Controller mappt den Pfad auf eine Resource (BankCard, Contract, etc.) +// und prüft canAccessCustomer/canAccessContract – damit kann ein Portal-Kunde +// nur seine eigenen Dateien laden, selbst wenn er fremde Filenames kennt. +// +// Kompatibilität: das alte /api/uploads/* bleibt erhalten, leitet aber jeden +// Request über denselben Owner-Check (kein freier static-Handler mehr). + +// Authentifizierter Datei-Download mit Per-File-Ownership-Check. +// Akzeptiert Pfade wie /uploads/bank-cards/ – egal ob als +// Query-Parameter oder im Pfad-Suffix. Beide gehen über denselben Handler, +// der DB-basiert prüft, ob der eingeloggte User die Resource sehen darf. +app.get('/api/files/download', authenticate as any, downloadFile as any); +// Backwards-compatibility shim: `/api/uploads/*` sieht weiter aus wie früher +// für Bestandsclients/Bookmarks, ruft aber denselben Owner-Check-Handler. +app.get('/api/uploads/*', authenticate as any, (req, res, next) => { + // Pfad in Query-Param umschreiben, dann an downloadFile weiterreichen + req.query.path = req.originalUrl.replace(/^\/api/, '').split('?')[0]; + return (downloadFile as any)(req, res, next); +}); // Öffentliche Routes (OHNE Authentifizierung) app.use('/api/public/consent', consentPublicRoutes); diff --git a/backend/src/services/fileDownload.service.ts b/backend/src/services/fileDownload.service.ts new file mode 100644 index 00000000..93497895 --- /dev/null +++ b/backend/src/services/fileDownload.service.ts @@ -0,0 +1,126 @@ +/** + * Pfad → Resource → Owner Mapping für `/api/files/download`. + * + * Jeder Upload-Subdirectory ist mit genau einem Prisma-Model + Path-Field + * verknüpft. Wir suchen den Record, der diesen Path referenziert, und + * leiten daraus den zuständigen Customer/Contract ab. canAccessCustomer / + * canAccessContract entscheidet danach über Zugriff. + * + * Pfade werden 1:1 mit dem in der DB gespeicherten Wert verglichen + * (z.B. `/uploads/bank-cards/12345.pdf`). Damit ist Path-Traversal + * automatisch ausgeschlossen – ein konstruierter Pfad findet keinen Record. + */ +import prisma from '../lib/prisma.js'; + +export type FileOwner = + | { kind: 'customer'; customerId: number } + | { kind: 'contract'; contractId: number } + | { kind: 'admin' } + | { kind: 'gdpr-admin' }; + +export async function findUploadOwner(uploadPath: string): Promise { + // Format-Check: muss mit /uploads// beginnen, kein Traversal. + if (!uploadPath.startsWith('/uploads/')) return null; + if (uploadPath.includes('..') || uploadPath.includes('\0')) return null; + + const parts = uploadPath.split('/'); + // ['', 'uploads', '', ''] + if (parts.length < 4) return null; + const subDir = parts[2]; + + switch (subDir) { + case 'bank-cards': { + const r = await prisma.bankCard.findFirst({ + where: { documentPath: uploadPath }, + select: { customerId: true }, + }); + return r ? { kind: 'customer', customerId: r.customerId } : null; + } + + case 'documents': { + const r = await prisma.identityDocument.findFirst({ + where: { documentPath: uploadPath }, + select: { customerId: true }, + }); + return r ? { kind: 'customer', customerId: r.customerId } : null; + } + + case 'business-registrations': { + const r = await prisma.customer.findFirst({ + where: { businessRegistrationPath: uploadPath }, + select: { id: true }, + }); + return r ? { kind: 'customer', customerId: r.id } : null; + } + + case 'commercial-registers': { + const r = await prisma.customer.findFirst({ + where: { commercialRegisterPath: uploadPath }, + select: { id: true }, + }); + return r ? { kind: 'customer', customerId: r.id } : null; + } + + case 'privacy-policies': { + const r = await prisma.customer.findFirst({ + where: { privacyPolicyPath: uploadPath }, + select: { id: true }, + }); + return r ? { kind: 'customer', customerId: r.id } : null; + } + + case 'authorizations': { + const r = await prisma.representativeAuthorization.findFirst({ + where: { documentPath: uploadPath }, + select: { customerId: true }, + }); + return r ? { kind: 'customer', customerId: r.customerId } : null; + } + + case 'contract-documents': { + const r = await prisma.contractDocument.findFirst({ + where: { documentPath: uploadPath }, + select: { contractId: true }, + }); + return r ? { kind: 'contract', contractId: r.contractId } : null; + } + + case 'invoices': { + const r = await prisma.invoice.findFirst({ + where: { documentPath: uploadPath }, + select: { contractId: true }, + }); + return r?.contractId ? { kind: 'contract', contractId: r.contractId } : null; + } + + case 'cancellation-letters': + case 'cancellation-confirmations': + case 'cancellation-letters-options': + case 'cancellation-confirmations-options': { + const fieldMap: Record = { + 'cancellation-letters': 'cancellationLetterPath', + 'cancellation-confirmations': 'cancellationConfirmationPath', + 'cancellation-letters-options': 'cancellationLetterOptionsPath', + 'cancellation-confirmations-options': 'cancellationConfirmationOptionsPath', + }; + const field = fieldMap[subDir]; + const r = await prisma.contract.findFirst({ + where: { [field]: uploadPath }, + select: { id: true }, + }); + return r ? { kind: 'contract', contractId: r.id } : null; + } + + case 'pdf-templates': { + // Admin-only Resource: Vorlagen gehören keinem Customer. + const r = await prisma.pdfTemplate.findFirst({ + where: { templatePath: uploadPath }, + select: { id: true }, + }); + return r ? { kind: 'admin' } : null; + } + + default: + return null; + } +} diff --git a/backend/src/services/imapService.ts b/backend/src/services/imapService.ts index 88ba25ac..3bb5afbe 100644 --- a/backend/src/services/imapService.ts +++ b/backend/src/services/imapService.ts @@ -14,6 +14,9 @@ export interface ImapCredentials { password: string; encryption?: MailEncryption; // SSL, STARTTLS oder NONE (Standard: SSL) allowSelfSignedCerts?: boolean; // Selbstsignierte Zertifikate erlauben + // DNS-Rebinding-Schutz: Caller hat den Hostname zu IP aufgelöst und + // setzt host=IP, servername=originalHostname für TLS-SNI / Cert-Validation. + servername?: string; } /** @@ -29,6 +32,12 @@ function buildTlsOptions(credentials: ImapCredentials): Record const rejectUnauthorized = !credentials.allowSelfSignedCerts; const options: Record = { rejectUnauthorized }; + // DNS-Rebinding-Schutz: wenn host eine IP ist und der ursprüngliche + // Hostname als servername mitgeliefert wird, nutze ihn für SNI/Cert. + if (credentials.servername) { + options.servername = credentials.servername; + } + if (credentials.allowSelfSignedCerts) { options.minVersion = 'TLSv1'; options.ciphers = 'DEFAULT:@SECLEVEL=0'; diff --git a/backend/src/services/smtpService.ts b/backend/src/services/smtpService.ts index 9d2222fd..59060ee4 100644 --- a/backend/src/services/smtpService.ts +++ b/backend/src/services/smtpService.ts @@ -15,6 +15,10 @@ export interface SmtpCredentials { password: string; encryption?: MailEncryption; // SSL, STARTTLS oder NONE (Standard: SSL) allowSelfSignedCerts?: boolean; // Selbstsignierte Zertifikate erlauben + // DNS-Rebinding-Schutz: Caller hat den Hostname zu IP aufgelöst und + // setzt host=IP, servername=originalHostname für TLS-SNI / Cert-Validation. + // Damit kann ein zweiter DNS-Lookup nicht plötzlich auf eine interne IP zeigen. + servername?: string; } // Anhang-Interface @@ -94,7 +98,7 @@ export async function sendEmail( port: number; secure: boolean; auth: { user: string; pass: string }; - tls?: { rejectUnauthorized: boolean; minVersion?: string; ciphers?: string }; + tls?: { rejectUnauthorized: boolean; minVersion?: string; ciphers?: string; servername?: string }; ignoreTLS?: boolean; requireTLS?: boolean; connectionTimeout: number; @@ -116,6 +120,11 @@ export async function sendEmail( // TLS-Optionen nur wenn nicht NONE if (encryption !== 'NONE') { transportOptions.tls = { rejectUnauthorized }; + // DNS-Rebinding-Schutz: wenn host eine IP ist, der ursprüngliche + // Hostname für SNI/Cert-Validation explizit setzen. + if (credentials.servername) { + transportOptions.tls.servername = credentials.servername; + } if (credentials.allowSelfSignedCerts) { // Auch ältere TLS-Versionen + legacy Cipher-Suites für alte Server zulassen transportOptions.tls.minVersion = 'TLSv1'; @@ -303,7 +312,7 @@ export async function testSmtpConnection(credentials: SmtpCredentials): Promise< port: number; secure: boolean; auth: { user: string; pass: string }; - tls?: { rejectUnauthorized: boolean; minVersion?: string; ciphers?: string }; + tls?: { rejectUnauthorized: boolean; minVersion?: string; ciphers?: string; servername?: string }; ignoreTLS?: boolean; connectionTimeout: number; greetingTimeout: number; @@ -321,6 +330,9 @@ export async function testSmtpConnection(credentials: SmtpCredentials): Promise< if (encryption !== 'NONE') { transportOptions.tls = { rejectUnauthorized }; + if (credentials.servername) { + transportOptions.tls.servername = credentials.servername; + } } else { transportOptions.ignoreTLS = true; } diff --git a/backend/src/utils/ssrfGuard.ts b/backend/src/utils/ssrfGuard.ts index bf2255dc..0ac49902 100644 --- a/backend/src/utils/ssrfGuard.ts +++ b/backend/src/utils/ssrfGuard.ts @@ -55,3 +55,53 @@ export function assertAllowedHost(host: string | null | undefined, label = 'Host throw new Error(`${label} verweist auf eine geblockte Adresse (Cloud-Metadata / Link-Local / Reserved).`); } } + +import { promises as dns } from 'dns'; +import net from 'net'; + +/** + * DNS-Rebinding-Schutz: löst den Hostname zu allen IPs auf und prüft jede + * gegen die Block-Liste. Wirft wenn IRGENDEINE IP geblockt ist. + * + * Das Resultat enthält die erste (geprüfte) IP plus den Original-Hostname + * als `servername` für TLS-SNI / Cert-Validation. Der Caller muss die + * Connection mit `host=ip` und `tls.servername=hostname` aufbauen, damit + * ein zweiter DNS-Lookup keine andere (geblockte) IP liefern kann. + * + * Wenn der Host bereits eine IP-Literal ist, wird er direkt geprüft. + */ +export async function safeResolveHost(host: string | null | undefined, label = 'Host'): Promise<{ ip: string; servername: string }> { + if (!host || !host.trim()) { + throw new Error(`${label} fehlt`); + } + const trimmed = host.trim(); + + // IP-Literal? Direkt prüfen, kein DNS nötig. + if (net.isIP(trimmed)) { + assertAllowedHost(trimmed, label); + return { ip: trimmed, servername: trimmed }; + } + + // Hostname → resolve to IPv4 + IPv6 + let ips: string[] = []; + try { + const v4 = await dns.resolve4(trimmed).catch(() => [] as string[]); + const v6 = await dns.resolve6(trimmed).catch(() => [] as string[]); + ips = [...v4, ...v6]; + } catch { + throw new Error(`${label}: DNS-Auflösung fehlgeschlagen für ${trimmed}`); + } + + if (ips.length === 0) { + throw new Error(`${label}: keine IP-Adresse für ${trimmed} gefunden`); + } + + // Alle aufgelösten IPs prüfen – schon eine geblockte reicht für Ablehnung. + for (const ip of ips) { + if (isBlockedSsrfHost(ip)) { + throw new Error(`${label} ${trimmed} löst auf geblockte Adresse ${ip} auf`); + } + } + + return { ip: ips[0], servername: trimmed }; +} diff --git a/backend/todo.md b/backend/todo.md index 2ee2d436..f756910d 100644 --- a/backend/todo.md +++ b/backend/todo.md @@ -141,6 +141,39 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung - Provider/Tariff-GETs: `requirePermission('providers:read')` (Portal-Kunden sehen Provider-Liste nicht mehr) - SMTP-Header-Injection: zentrale CRLF-Validierung in `smtpService.sendEmail` (schützt alle Caller) - bcrypt cost 10 → 12 (OWASP 2026) + - **Runde 8 – Loose Ends (DNS-Rebinding + Per-File-Ownership):** + - **DNS-Rebinding-Schutz** in test-connection / test-mail-access: + Hostnames werden vor Connect via `dns.resolve4/6` aufgelöst und + jede IP gegen die SSRF-Block-Liste geprüft. Connection läuft + anschließend gegen die IP, der ursprüngliche Hostname als + `tls.servername` für SNI/Cert-Validation. Ein zweiter DNS-Lookup + kann keine geblockte IP unterschieben. + - **Per-File-Ownership-Check** statt freiem static-Handler: + `app.use('/api/uploads', authenticate, express.static)` wird + ersetzt durch `GET /api/files/download?path=...`. Der + Controller mappt den Pfad via DB-Lookup auf Customer/Contract + und delegiert an `canAccessCustomer`/`canAccessContract` – + ein eingeloggter Portal-Kunde kann jetzt nur seine eigenen + (oder vertretene mit Vollmacht) Dateien laden, selbst wenn + er fremde Filenames irgendwo mitgeschnitten hätte. + `/api/uploads/*` bleibt als Backwards-Compat-Shim erhalten, + ruft aber denselben Owner-Check. + - 12 subDir-Mappings: bank-cards, documents, business-/commercial-/ + privacy-, authorizations, contract-documents, invoices, alle + 4 cancellation-* + pdf-templates (admin-only). + - Frontend `fileUrl()` zeigt jetzt auf den neuen Endpoint. + Path-Traversal wird sowohl per Format-Validation (begin /uploads/, + no '..') als auch durch absoluten Path-Vergleich gegen uploadsRoot + geblockt. + - Live-verifiziert: Portal-User lädt eigene Contract-Datei (200), + random Pfad (404), Traversal (400), kein Token (401), Backwards- + Compat-Shim (200). + + **Bewusst NICHT gemacht (für v1.2):** + - Signierte URLs mit kurzlebigen Download-Tokens statt JWT-im-Query + (verhindert Token-Leak via Logs/Referrer). Nicht trivial wegen + -Downloads ohne JS, lassen wir bis später. + - **Runde 7 – Letzter Schliff (SSRF + Logout):** - **SSRF-Schutz** in `test-connection` und `test-mail-access`: ein Admin-User konnte über die Plesk-API-URL bzw. SMTP/IMAP-Server-Felder diff --git a/frontend/src/utils/fileUrl.ts b/frontend/src/utils/fileUrl.ts index 2f9c2ed1..da9eeae7 100644 --- a/frontend/src/utils/fileUrl.ts +++ b/frontend/src/utils/fileUrl.ts @@ -1,20 +1,23 @@ /** * Baut eine Download-URL für ein im Backend gespeichertes Upload-File. * - * `/api/uploads/*` läuft hinter authenticate-Middleware, aber und - * window.open senden keinen Authorization-Header. Darum hängen wir das JWT - * als Query-Parameter an. Die authenticate-Middleware akzeptiert - * `?token=` neben dem Header. + * Geht über `GET /api/files/download?path=...` – der Backend-Controller + * macht einen Per-File-Ownership-Check (Pfad → Resource → canAccessCustomer + * / canAccessContract). Damit kann auch ein eingeloggter User keine + * fremden Dateien abrufen, selbst wenn er den Pfad kennen würde. * - * Trade-off: Tokens in URLs landen potenziell in Logs/Referrer. Für eine - * saubere Lösung (kurzlebige Download-Tokens) wäre ein separater Endpoint - * nötig – TODO für v1.1. + * und window.open senden keinen Authorization-Header, daher + * Token als Query-Parameter (auth-Middleware akzeptiert `?token=`). + * + * Trade-off: Tokens in URLs können in Logs/Referrer landen. Eine + * sauberere Lösung mit kurzlebigen Download-Tokens (signierte URLs) + * wäre v1.1-Item. */ export function fileUrl(path: string | null | undefined): string { if (!path) return ''; const token = localStorage.getItem('token'); - const base = `/api${path.startsWith('/') ? path : '/' + path}`; + const normalizedPath = path.startsWith('/') ? path : '/' + path; + const base = `/api/files/download?path=${encodeURIComponent(normalizedPath)}`; if (!token) return base; - const separator = base.includes('?') ? '&' : '?'; - return `${base}${separator}token=${encodeURIComponent(token)}`; + return `${base}&token=${encodeURIComponent(token)}`; }