From f2a4baacdbe749210c3fb3d47a1d1d999c1fc4ce Mon Sep 17 00:00:00 2001 From: duffyduck Date: Wed, 19 Aug 2026 14:56:10 +0200 Subject: [PATCH] Audit-Haerten: Fork, Feldabdeckung, Refresh-Rauschen, Route (Pentest R166) R166-01 (HIGH): Der GET_LOCK-Ansatz gab die Sperre im finally INNERHALB des Transaktions-Callbacks frei, also vor dem COMMIT. Im Fenster Release-Commit las der naechste Schreiber ein noch nicht sichtbares Kettenende - zwei Zeilen hingen am selben Vorgaenger. Meine vorherige Messung war zu schwach: sie suchte Luecken zwischen Nachbarn, nicht Forks. Fix: einzeiliger Mutex AuditChainLock mit FOR UPDATE (InnoDB-Zeilensperren fallen erst beim COMMIT) plus isolationLevel ReadCommitted. Belegt im Direktvergleich mit geweitetem Fenster: Release-vor-Commit forkt, Zeilensperre nicht. R166-02 (MEDIUM): Der Hash deckte nur 7 Felder ab. changesBefore/After, success, ipAddress, resourceLabel, dataSubjectId, userId/customerId waren ungeschuetzt - ein Einzeledit dort blieb unsichtbar. Fix: hashVersion + generateHashV2 ueber alle Inhaltsspalten. Bestandszeilen behalten Version 1 und bleiben ohne Rehash gueltig. Verifiziert: 5/5 zuvor ungeschuetzte Felder werden jetzt erkannt. R166-03 (LOW): "kein Cookie" (normaler Erstbesuch) wurde als HIGH/abgelehnt gefuehrt - jetzt eigener Ausgang mit LOW. Nur echte Ablehnung bleibt HIGH. R166-04 (LOW, pre-existing): GET /retention-policies wurde von GET /:id verschluckt. Konkrete Routen jetzt vor der Parameter-Route. Design-Empfehlungen: runRetentionCleanup schreibt ein Loeschungs-Manifest (ID-Bereich, Anzahl, Policy, Cutoff) als eigenen verketteten Eintrag - Luecken ausserhalb bleiben erklaerungsbeduerftig. rehashAll schreibt einen Marker. Verifiziert: 50 parallele Schreiber -> 50/50, 0 Forks, alle V2, manipuliert 0, Luecken unveraendert 7. tsc + vite build gruen. Co-Authored-By: Claude Opus 5 --- .../migration.sql | 15 + .../migration.sql | 13 + backend/prisma/schema.prisma | 21 + backend/src/middleware/audit.ts | 33 +- backend/src/routes/auditLog.routes.ts | 21 +- backend/src/services/audit.service.ts | 399 +++++++++++++----- docs/todo.md | 37 ++ 7 files changed, 431 insertions(+), 108 deletions(-) create mode 100644 backend/prisma/migrations/20260818140000_audit_chain_lock/migration.sql create mode 100644 backend/prisma/migrations/20260818150000_audit_hash_version/migration.sql diff --git a/backend/prisma/migrations/20260818140000_audit_chain_lock/migration.sql b/backend/prisma/migrations/20260818140000_audit_chain_lock/migration.sql new file mode 100644 index 00000000..9c38016f --- /dev/null +++ b/backend/prisma/migrations/20260818140000_audit_chain_lock/migration.sql @@ -0,0 +1,15 @@ +-- Einzeiliger Mutex fuer die Audit-Hash-Kette (Pentest R166-01). +-- +-- Vorher wurde ueber GET_LOCK serialisiert. Dessen RELEASE_LOCK muss auf +-- derselben Verbindung laufen und stand daher im finally INNERHALB des +-- Transaktions-Callbacks - also VOR dem COMMIT. In diesem Fenster konnte der +-- naechste Schreiber die Sperre holen und das Kettenende lesen, bevor die +-- Vorgaengerzeile committed war: beide haengten sich an denselben Vorgaenger +-- (Fork). InnoDB-Zeilensperren fallen dagegen erst beim COMMIT. +CREATE TABLE IF NOT EXISTS `AuditChainLock` ( + `id` INT NOT NULL, + `updatedAt` DATETIME(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +INSERT IGNORE INTO `AuditChainLock` (`id`, `updatedAt`) VALUES (1, NOW(3)); diff --git a/backend/prisma/migrations/20260818150000_audit_hash_version/migration.sql b/backend/prisma/migrations/20260818150000_audit_hash_version/migration.sql new file mode 100644 index 00000000..61688852 --- /dev/null +++ b/backend/prisma/migrations/20260818150000_audit_hash_version/migration.sql @@ -0,0 +1,13 @@ +-- Hash-Versionierung fuer Audit-Eintraege (Pentest R166-02). +-- +-- Der bisherige Hash deckte nur 7 Felder ab (userEmail, action, resourceType, +-- resourceId, endpoint, createdAt, previousHash). NICHT gehasht waren u. a. +-- changesBefore/changesAfter (die eigentliche Nutzlast), success, ipAddress, +-- resourceLabel, dataSubjectId, userId/customerId - ein nachtraeglicher +-- Einzeledit an genau diesen Feldern blieb also unsichtbar. +-- +-- Version 2 hasht alle Inhaltsspalten. Bestandszeilen behalten Version 1 und +-- werden weiterhin mit dem alten Verfahren geprueft - kein Rehash noetig, +-- die Beweiskraft der Vergangenheit bleibt erhalten. +ALTER TABLE `AuditLog` + ADD COLUMN IF NOT EXISTS `hashVersion` INT NOT NULL DEFAULT 1; diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 7a0d5255..a92acdc3 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -1221,6 +1221,24 @@ model CarInsuranceDetails { // ==================== AUDIT LOGGING (DSGVO) ==================== +/// Einzeiliger Mutex fuer die Audit-Hash-Kette (genau eine Zeile, id = 1). +/// +/// Warum eine eigene Tabelle statt GET_LOCK oder `SELECT … FOR UPDATE` auf +/// AuditLog selbst: +/// - GET_LOCK muss auf derselben Verbindung freigegeben werden. Innerhalb des +/// Prisma-Transaktions-Callbacks faellt das Release damit VOR den COMMIT – +/// der naechste Schreiber liest das Kettenende, bevor die Vorgaengerzeile +/// sichtbar ist, und haengt sich an denselben Vorgaenger (Fork, R166-01). +/// - `FOR UPDATE` auf das Kettenende von AuditLog nimmt Gap-/Next-Key-Locks, +/// die mit den gleichzeitigen INSERTs kollidieren (Deadlocks, dabei gingen +/// 38 von 40 Eintraegen verloren). +/// InnoDB-Zeilensperren werden erst beim COMMIT freigegeben – genau das +/// schliesst das Fenster. +model AuditChainLock { + id Int @id + updatedAt DateTime @updatedAt +} + enum AuditAction { CREATE READ @@ -1284,6 +1302,9 @@ model AuditLog { createdAt DateTime @default(now()) hash String? // SHA-256 Hash des Eintrags previousHash String? // Hash des vorherigen Eintrags + /// 1 = Alt-Hash ueber 7 Felder, 2 = Hash ueber alle Inhaltsspalten + /// (Pentest R166-02). Bestandszeilen bleiben mit Version 1 gueltig. + hashVersion Int @default(1) @@index([userId]) @@index([customerId]) diff --git a/backend/src/middleware/audit.ts b/backend/src/middleware/audit.ts index 5ae0100f..96dd94ce 100644 --- a/backend/src/middleware/audit.ts +++ b/backend/src/middleware/audit.ts @@ -167,6 +167,22 @@ const ACTION_LABELS: Record = { TOKEN_REFRESH: 'Sitzung verlängert', }; +/** + * Unterscheidet die drei Ausgaenge von POST /auth/refresh (Pentest R166-03). + * + * "Kein Cookie vorhanden" ist KEIN Ablehnungsfall: Der Frontend-Interceptor + * ruft /refresh beim App-Start auch dann, wenn nie ein Token gesetzt war + * (normaler Erstbesuch). Das als HIGH/"abgelehnt" zu fuehren erzeugt genau das + * Rauschen, das mit der Entrauschung beseitigt werden sollte. + */ +function refreshOutcome(responseBody: unknown, success: boolean): 'ok' | 'no-token' | 'rejected' { + if (success) return 'ok'; + const err = responseBody && typeof responseBody === 'object' && 'error' in responseBody + ? String((responseBody as { error?: unknown }).error ?? '') + : ''; + return /kein refresh-token/i.test(err) ? 'no-token' : 'rejected'; +} + /** * Erzeugt ein menschenlesbares Label für den Audit-Log-Eintrag */ @@ -209,8 +225,12 @@ function generateHumanLabel( } if (path.includes('/auth/logout')) return 'Benutzer hat sich abgemeldet'; if (path.includes('/auth/refresh')) { - const failed = responseBody && typeof responseBody === 'object' && (responseBody as { success?: boolean }).success === false; - return failed ? 'Token-Refresh abgelehnt (ungültig/abgelaufen)' : 'Sitzung verlängert (Token erneuert)'; + const success = !(responseBody && typeof responseBody === 'object' && (responseBody as { success?: boolean }).success === false); + switch (refreshOutcome(responseBody, success)) { + case 'ok': return 'Sitzung verlängert (Token erneuert)'; + case 'no-token': return 'Token-Refresh ohne vorliegenden Token (kein Cookie)'; + default: return 'Token-Refresh abgelehnt (ungültig/abgelaufen)'; + } } // Kunden-Operationen @@ -460,10 +480,13 @@ export function auditMiddleware(req: AuthRequest, res: Response, next: NextFunct isCustomerPortal: req.user?.isCustomerPortal, action, // Erfolgreicher Token-Refresh ist Routine → LOW statt CRITICAL (sonst Log-Flut). - // Fehlgeschlagener Refresh (Replay/Brute-Force-Verdacht) → HIGH, damit er in - // der Triage nicht neben legitimen Refreshes untergeht (Pentest R164-01). + // Abgelehnter Refresh (Replay/Brute-Force-Verdacht) → HIGH, damit er in der + // Triage nicht neben legitimen Refreshes untergeht (Pentest R164-01). + // "Kein Cookie vorhanden" ist dagegen der normale Erstbesuch → LOW (R166-03). // Andere Auth-Events behalten ihre Default-Sensitivität (Authentication → CRITICAL). - sensitivity: action === 'TOKEN_REFRESH' ? (responseSuccess ? 'LOW' : 'HIGH') : undefined, + sensitivity: action === 'TOKEN_REFRESH' + ? (refreshOutcome(responseBody, responseSuccess) === 'rejected' ? 'HIGH' : 'LOW') + : undefined, resourceType: mapping.type, resourceId, resourceLabel, diff --git a/backend/src/routes/auditLog.routes.ts b/backend/src/routes/auditLog.routes.ts index 0fb65caf..50c20fc2 100644 --- a/backend/src/routes/auditLog.routes.ts +++ b/backend/src/routes/auditLog.routes.ts @@ -7,29 +7,34 @@ const router = Router(); // Alle Routen erfordern Authentifizierung router.use(authenticate); +// ACHTUNG Reihenfolge: Alle konkreten Pfade MÜSSEN vor der Parameter-Route +// '/:id' stehen, sonst schluckt diese sie und antwortet mit +// "Ungültige Audit-Log-ID". Genau so war GET /retention-policies unerreichbar +// (Pentest R166-04). + // Audit-Logs abrufen router.get('/', requirePermission('audit:read'), auditLogController.getAuditLogs); -// Audit-Logs exportieren (muss VOR /:id stehen!) +// Audit-Logs exportieren router.get('/export', requirePermission('audit:read'), auditLogController.exportAuditLogs); +// Retention-Policies +router.get('/retention-policies', requirePermission('audit:admin'), auditLogController.getRetentionPolicies); +router.put('/retention-policies/:id', requirePermission('audit:admin'), auditLogController.updateRetentionPolicy); + // Audit-Logs für einen Kunden (DSGVO) router.get('/customer/:customerId', requirePermission('audit:read'), auditLogController.getAuditLogsByCustomer); -// Einzelnes Audit-Log abrufen -router.get('/:id', requirePermission('audit:read'), auditLogController.getAuditLogById); - // Hash-Ketten-Integrität prüfen router.post('/verify', requirePermission('audit:read'), auditLogController.verifyIntegrity); // Hash-Kette reparieren router.post('/rehash', requirePermission('audit:admin'), auditLogController.rehashAll); -// Retention-Policies -router.get('/retention-policies', requirePermission('audit:admin'), auditLogController.getRetentionPolicies); -router.put('/retention-policies/:id', requirePermission('audit:admin'), auditLogController.updateRetentionPolicy); - // Retention-Cleanup manuell ausführen router.post('/cleanup', requirePermission('audit:admin'), auditLogController.runRetentionCleanup); +// Einzelnes Audit-Log abrufen – als LETZTE GET-Route, siehe Hinweis oben +router.get('/:id', requirePermission('audit:read'), auditLogController.getAuditLogById); + export default router; diff --git a/backend/src/services/audit.service.ts b/backend/src/services/audit.service.ts index 84e89ff1..83a36c2a 100644 --- a/backend/src/services/audit.service.ts +++ b/backend/src/services/audit.service.ts @@ -161,6 +161,86 @@ function generateHashLegacy(data: { return crypto.createHash('sha256').update(JSON.stringify(payload)).digest('hex'); } +/** + * Hash-Version 2 (Pentest R166-02): deckt ALLE Inhaltsspalten ab. + * + * Version 1 hashte nur 7 Felder (userEmail, action, resourceType, resourceId, + * endpoint, createdAt, previousHash). Nicht abgedeckt waren u. a. + * `changesBefore`/`changesAfter` – also die eigentliche Nutzlast –, dazu + * `success`, `ipAddress`, `resourceLabel`, `dataSubjectId`, `userId`, + * `customerId`. Ein nachtraeglicher Einzeledit an genau diesen Feldern + * (z. B. `success` false→true oder das Umschreiben von `changesAfter`) war + * damit unsichtbar – exakt der Angriff, gegen den die Kette schuetzen soll. + * + * Bestandszeilen behalten `hashVersion = 1` und werden weiter mit dem alten + * Verfahren geprueft; ein Rehash waere nicht noetig und wuerde die + * Beweiskraft der Vergangenheit zerstoeren. + * + * `undefined` wird bewusst zu `null` normalisiert, damit die Serialisierung + * deterministisch ist (die Uneindeutigkeit an dieser Stelle war die Ursache + * des Dauer-Fehlalarms bei Version 1). + */ +export interface AuditHashV2Input { + userId?: number | null; + userEmail: string; + userRole?: string | null; + customerId?: number | null; + isCustomerPortal?: boolean | null; + action: AuditAction; + sensitivity?: AuditSensitivity | null; + resourceType: string; + resourceId?: string | null; + resourceLabel?: string | null; + endpoint: string; + httpMethod?: string | null; + ipAddress?: string | null; + userAgent?: string | null; + changesBefore?: string | null; + changesAfter?: string | null; + changesEncrypted?: boolean | null; + dataSubjectId?: number | null; + legalBasis?: string | null; + success?: boolean | null; + errorMessage?: string | null; + durationMs?: number | null; + createdAt: Date; + previousHash?: string | null; +} + +function generateHashV2(data: AuditHashV2Input): string { + const n = (v: T | null | undefined): T | null => (v === undefined ? null : v); + // Feldreihenfolge ist Teil des Hashes und darf nicht veraendert werden. + const payload = { + v: 2, + userId: n(data.userId), + userEmail: data.userEmail, + userRole: n(data.userRole), + customerId: n(data.customerId), + isCustomerPortal: n(data.isCustomerPortal) ?? false, + action: data.action, + sensitivity: n(data.sensitivity), + resourceType: data.resourceType, + resourceId: n(data.resourceId), + resourceLabel: n(data.resourceLabel), + endpoint: data.endpoint, + httpMethod: n(data.httpMethod), + ipAddress: n(data.ipAddress), + userAgent: n(data.userAgent), + changesBefore: n(data.changesBefore), + changesAfter: n(data.changesAfter), + changesEncrypted: n(data.changesEncrypted) ?? false, + dataSubjectId: n(data.dataSubjectId), + legalBasis: n(data.legalBasis), + success: n(data.success) ?? true, + errorMessage: n(data.errorMessage), + durationMs: n(data.durationMs), + createdAt: data.createdAt.toISOString(), + previousHash: data.previousHash || '', + }; + + return crypto.createHash('sha256').update(JSON.stringify(payload)).digest('hex'); +} + /** * Bestimmt die Sensitivität basierend auf dem Ressourcentyp */ @@ -215,9 +295,6 @@ function shouldEncryptChanges(_resourceType: string): boolean { /** * Erstellt einen neuen Audit-Log-Eintrag mit Hash-Kette */ -// Name des DB-weiten Locks, ueber den die Hash-Kette serialisiert wird. -const AUDIT_CHAIN_LOCK = 'opencrm_audit_chain'; - export async function createAuditLog(data: CreateAuditLogData): Promise { try { // Sensitivität bestimmen falls nicht angegeben @@ -248,78 +325,99 @@ export async function createAuditLog(data: CreateAuditLogData): Promise { // haengten sich beide daran, was die Kette zerriss (echte Bruchstellen im // Bestand, u. a. 05.05./07.05.2026). // - // Serialisiert wird ueber einen benannten MySQL-Lock (GET_LOCK), NICHT ueber - // `SELECT … FOR UPDATE` am Kettenende: letzteres nimmt Gap-/Next-Key-Locks - // am Index-Ende, die mit den gleichzeitigen INSERTs kollidieren – gemessen - // gingen dabei 38 von 40 parallelen Eintraegen durch Deadlocks verloren. - // Ein FEHLENDER Audit-Eintrag ist unsichtbar und damit schlimmer als ein - // sichtbarer Kettenbruch. Der benannte Lock kennt keine Gap-Locks und - // serialisiert sauber; er liegt in der DB und wirkt daher auch ueber - // mehrere App-Instanzen hinweg. + // Serialisiert wird ueber eine Zeilensperre auf dem Einzeiler-Mutex + // `AuditChainLock`. Zwei Alternativen wurden verworfen: + // - `SELECT … FOR UPDATE` am Kettenende von AuditLog nimmt Gap-/Next-Key- + // Locks, die mit den gleichzeitigen INSERTs kollidieren – gemessen gingen + // 38 von 40 parallelen Eintraegen durch Deadlocks verloren. Ein FEHLENDER + // Audit-Eintrag ist unsichtbar und damit schlimmer als ein sichtbarer + // Kettenbruch. + // - GET_LOCK muss auf derselben Verbindung freigegeben werden, das Release + // fiel damit VOR den COMMIT. In diesem Fenster las der naechste Schreiber + // ein noch nicht sichtbares Kettenende und hing sich an denselben + // Vorgaenger (Fork, Pentest R166-01). + // Die Zeilensperre faellt erst beim COMMIT und liegt in der DB – wirkt also + // auch ueber mehrere App-Instanzen hinweg. // Alles Rechenintensive (Serialisieren/Verschluesseln) passiert bewusst // VOR der Transaktion, damit die Sperre so kurz wie moeglich gehalten wird. await prisma.$transaction(async (tx) => { - // Interaktive Transaktion => alle Queries auf DERSELBEN Verbindung, - // Voraussetzung dafuer, dass GET_LOCK/RELEASE_LOCK zusammengehoeren. - const got = await tx.$queryRaw>>` - SELECT GET_LOCK(${AUDIT_CHAIN_LOCK}, 10) AS ok - `; - const locked = Number(Object.values(got[0] ?? {})[0] ?? 0) === 1; - try { - const lastRows = await tx.$queryRaw>` - SELECT hash FROM AuditLog ORDER BY id DESC LIMIT 1 - `; - const previousHash = lastRows[0]?.hash || null; - const createdAt = new Date(); + // Exklusive Zeilensperre auf den Einzeiler-Mutex. Sie faellt erst beim + // COMMIT – dadurch sieht der naechste Schreiber die Vorgaengerzeile + // garantiert bereits festgeschrieben (Pentest R166-01). + await tx.$queryRaw`SELECT id FROM AuditChainLock WHERE id = 1 FOR UPDATE`; - const hash = generateHash({ + const lastRows = await tx.$queryRaw>` + SELECT hash FROM AuditLog ORDER BY id DESC LIMIT 1 + `; + const previousHash = lastRows[0]?.hash || null; + const createdAt = new Date(); + + // Neue Eintraege immer mit Version 2 (volle Feldabdeckung, R166-02). + const hash = generateHashV2({ + userId: data.userId, + userEmail: data.userEmail, + userRole: data.userRole, + customerId: data.customerId, + isCustomerPortal: data.isCustomerPortal || false, + action: data.action, + sensitivity, + resourceType: data.resourceType, + resourceId: data.resourceId, + resourceLabel: data.resourceLabel, + endpoint: data.endpoint, + httpMethod: data.httpMethod, + ipAddress: data.ipAddress, + userAgent: data.userAgent, + changesBefore, + changesAfter, + changesEncrypted, + dataSubjectId: data.dataSubjectId, + legalBasis: data.legalBasis, + success: data.success ?? true, + errorMessage: data.errorMessage, + durationMs: data.durationMs, + createdAt, + previousHash, + }); + + await tx.auditLog.create({ + data: { + userId: data.userId, userEmail: data.userEmail, + userRole: data.userRole, + customerId: data.customerId, + isCustomerPortal: data.isCustomerPortal || false, action: data.action, + sensitivity, resourceType: data.resourceType, resourceId: data.resourceId, + resourceLabel: data.resourceLabel, endpoint: data.endpoint, + httpMethod: data.httpMethod, + ipAddress: data.ipAddress, + userAgent: data.userAgent, + changesBefore, + changesAfter, + changesEncrypted, + dataSubjectId: data.dataSubjectId, + legalBasis: data.legalBasis, + success: data.success ?? true, + errorMessage: data.errorMessage, + durationMs: data.durationMs, createdAt, + hash, previousHash, - }); - - await tx.auditLog.create({ - data: { - userId: data.userId, - userEmail: data.userEmail, - userRole: data.userRole, - customerId: data.customerId, - isCustomerPortal: data.isCustomerPortal || false, - action: data.action, - sensitivity, - resourceType: data.resourceType, - resourceId: data.resourceId, - resourceLabel: data.resourceLabel, - endpoint: data.endpoint, - httpMethod: data.httpMethod, - ipAddress: data.ipAddress, - userAgent: data.userAgent, - changesBefore, - changesAfter, - changesEncrypted, - dataSubjectId: data.dataSubjectId, - legalBasis: data.legalBasis, - success: data.success ?? true, - errorMessage: data.errorMessage, - durationMs: data.durationMs, - createdAt, - hash, - previousHash, - }, - }); - } finally { - // Benannte Locks sind NICHT transaktional – ohne explizites Release - // wandert die Sperre mit der Verbindung zurueck in den Pool und - // blockiert alle weiteren Schreiber. - if (locked) { - await tx.$queryRaw`SELECT RELEASE_LOCK(${AUDIT_CHAIN_LOCK}) AS released`; - } - } - }, { timeout: 20000, maxWait: 15000 }); + hashVersion: 2, + }, + }); + }, { + // READ COMMITTED: der Lesevorgang nach der Sperre muss den gerade + // festgeschriebenen Stand sehen. Unter REPEATABLE READ koennte ein + // Snapshot greifen, der die Vorgaengerzeile noch nicht enthaelt. + isolationLevel: 'ReadCommitted', + timeout: 20000, + maxWait: 15000, + }); } catch (error) { // Audit-Logging darf niemals die Hauptoperation blockieren console.error('[AuditService] Fehler beim Erstellen des Audit-Logs:', error); @@ -473,16 +571,35 @@ export async function verifyIntegrity(fromId?: number, toId?: number): Promise<{ const logs = await prisma.auditLog.findMany({ where, orderBy: { id: 'asc' }, + // Version 2 hasht alle Inhaltsspalten – daher vollstaendig laden. select: { id: true, + userId: true, userEmail: true, + userRole: true, + customerId: true, + isCustomerPortal: true, action: true, + sensitivity: true, resourceType: true, resourceId: true, + resourceLabel: true, endpoint: true, + httpMethod: true, + ipAddress: true, + userAgent: true, + changesBefore: true, + changesAfter: true, + changesEncrypted: true, + dataSubjectId: true, + legalBasis: true, + success: true, + errorMessage: true, + durationMs: true, createdAt: true, hash: true, previousHash: true, + hashVersion: true, }, }); @@ -492,22 +609,39 @@ export async function verifyIntegrity(fromId?: number, toId?: number): Promise<{ for (let i = 0; i < logs.length; i++) { const log = logs[i]; - // Hash neu berechnen - const expectedHash = generateHash({ - userEmail: log.userEmail, - action: log.action, - resourceType: log.resourceType, - resourceId: log.resourceId, - endpoint: log.endpoint, - createdAt: log.createdAt, - previousHash: log.previousHash, - }); - - // Prüfen ob Hash übereinstimmt – Altbestand darf die historische - // Serialisierung nutzen (siehe generateHashLegacy). Legacy wird nur - // geprüft, wenn die aktuelle Variante nicht passt. - if (log.hash !== expectedHash) { - const legacyHash = generateHashLegacy({ + // Pruefverfahren richtet sich nach der Version, mit der geschrieben wurde. + // Version 2 deckt alle Inhaltsspalten ab; Version 1 nur 7 Felder und darf + // zusaetzlich die historische Serialisierung nutzen (generateHashLegacy). + let hashOk: boolean; + if (log.hashVersion >= 2) { + hashOk = log.hash === generateHashV2({ + userId: log.userId, + userEmail: log.userEmail, + userRole: log.userRole, + customerId: log.customerId, + isCustomerPortal: log.isCustomerPortal, + action: log.action, + sensitivity: log.sensitivity, + resourceType: log.resourceType, + resourceId: log.resourceId, + resourceLabel: log.resourceLabel, + endpoint: log.endpoint, + httpMethod: log.httpMethod, + ipAddress: log.ipAddress, + userAgent: log.userAgent, + changesBefore: log.changesBefore, + changesAfter: log.changesAfter, + changesEncrypted: log.changesEncrypted, + dataSubjectId: log.dataSubjectId, + legalBasis: log.legalBasis, + success: log.success, + errorMessage: log.errorMessage, + durationMs: log.durationMs, + createdAt: log.createdAt, + previousHash: log.previousHash, + }); + } else { + const v1 = { userEmail: log.userEmail, action: log.action, resourceType: log.resourceType, @@ -515,11 +649,13 @@ export async function verifyIntegrity(fromId?: number, toId?: number): Promise<{ endpoint: log.endpoint, createdAt: log.createdAt, previousHash: log.previousHash, - }); - if (log.hash !== legacyHash) { - tamperedEntries.push(log.id); - continue; - } + }; + hashOk = log.hash === generateHash(v1) || log.hash === generateHashLegacy(v1); + } + + if (!hashOk) { + tamperedEntries.push(log.id); + continue; } // Prüfen ob previousHash mit dem Hash des vorherigen Eintrags übereinstimmt @@ -550,11 +686,28 @@ export async function rehashAll(): Promise<{ rehashedCount: number }> { orderBy: { id: 'asc' }, select: { id: true, + userId: true, userEmail: true, + userRole: true, + customerId: true, + isCustomerPortal: true, action: true, + sensitivity: true, resourceType: true, resourceId: true, + resourceLabel: true, endpoint: true, + httpMethod: true, + ipAddress: true, + userAgent: true, + changesBefore: true, + changesAfter: true, + changesEncrypted: true, + dataSubjectId: true, + legalBasis: true, + success: true, + errorMessage: true, + durationMs: true, createdAt: true, }, }); @@ -563,25 +716,37 @@ export async function rehashAll(): Promise<{ rehashedCount: number }> { let count = 0; for (const log of logs) { - const hash = generateHash({ - userEmail: log.userEmail, - action: log.action, - resourceType: log.resourceType, - resourceId: log.resourceId, - endpoint: log.endpoint, - createdAt: log.createdAt, - previousHash, - }); + // Rehash schreibt immer Version 2 (volle Feldabdeckung) – ein Rueckfall + // auf Version 1 wuerde die Abdeckung nachtraeglich wieder verkleinern. + const hash = generateHashV2({ ...log, previousHash }); await prisma.auditLog.update({ where: { id: log.id }, - data: { hash, previousHash }, + data: { hash, previousHash, hashVersion: 2 }, }); previousHash = hash; count++; } + // Marker: Ein Rehash setzt die Beweiskraft der Vergangenheit zurueck (jede + // vorhandene Faelschung wuerde mitbesiegelt). Der Vorgang muss deshalb im + // Log selbst sichtbar sein. Der Eintrag wird NACH dem Rehash geschrieben und + // haengt sich an die neu berechnete Kette; entfernen liesse er sich nur unter + // Hinterlassung einer Luecke. + await createAuditLog({ + userEmail: 'system', + userRole: 'System', + action: 'UPDATE', + resourceType: 'AuditLog', + resourceLabel: `Hash-Kette neu berechnet (${count} Einträge) – Beweiskraft der Vergangenheit zurückgesetzt`, + endpoint: '/api/audit-logs/rehash', + httpMethod: 'POST', + ipAddress: 'system', + sensitivity: 'CRITICAL', + success: true, + }); + return { rehashedCount: count }; } @@ -652,6 +817,14 @@ export async function runRetentionCleanup(): Promise<{ const results: Array<{ resourceType: string; sensitivity: string | null; deletedCount: number }> = []; let totalDeleted = 0; + // Loeschungs-Manifest (Pentest R166, Design): Jede geloeschte Zeile reisst die + // Hash-Kette auf. Ohne Nachweis, WELCHE Bereiche legitim entfernt wurden, + // koennte sich eine boeswillige Loeschung als "harmloser Gap" tarnen. Deshalb + // halten wir Bereich + Anzahl fest und schreiben sie als eigenen, selbst + // wieder verketteten Audit-Eintrag. Luecken ausserhalb dieser Bereiche + // bleiben damit erklaerungsbeduerftig. + const manifest: Array> = []; + for (const policy of policies) { const cutoffDate = new Date(); cutoffDate.setDate(cutoffDate.getDate() - policy.retentionDays); @@ -668,8 +841,28 @@ export async function runRetentionCleanup(): Promise<{ where.sensitivity = policy.sensitivity; } + // Betroffenen ID-Bereich VOR dem Loeschen festhalten. + const range = await prisma.auditLog.aggregate({ + where, + _count: true, + _min: { id: true }, + _max: { id: true }, + }); + const deleted = await prisma.auditLog.deleteMany({ where }); + if (deleted.count > 0) { + manifest.push({ + resourceType: policy.resourceType, + sensitivity: policy.sensitivity, + retentionDays: policy.retentionDays, + cutoff: cutoffDate.toISOString(), + deletedCount: deleted.count, + fromId: range._min.id, + toId: range._max.id, + }); + } + results.push({ resourceType: policy.resourceType, sensitivity: policy.sensitivity, @@ -679,6 +872,22 @@ export async function runRetentionCleanup(): Promise<{ totalDeleted += deleted.count; } + if (totalDeleted > 0) { + await createAuditLog({ + userEmail: 'system', + userRole: 'System', + action: 'DELETE', + resourceType: 'AuditLog', + resourceLabel: `Retention-Cleanup: ${totalDeleted} Audit-Einträge gelöscht`, + endpoint: '/api/audit-logs/cleanup', + httpMethod: 'POST', + ipAddress: 'system', + sensitivity: 'CRITICAL', + changesAfter: { manifest }, + success: true, + }); + } + return { deletedCount: totalDeleted, policies: results, diff --git a/docs/todo.md b/docs/todo.md index 5877c3e0..df94c756 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,43 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🛡️ Audit-Haerten: Fork, Feldabdeckung, Refresh-Rauschen, Route (Pentest R166)** (2026-08-18) + - **R166-01 (HIGH) – Kette forkte weiter.** Mein GET_LOCK-Ansatz gab die + Sperre im `finally` INNERHALB des Transaktions-Callbacks frei, also VOR dem + COMMIT. Im Fenster Release↔Commit las der naechste Schreiber ein noch nicht + sichtbares Kettenende → zwei Zeilen am selben Vorgaenger. Meine + „100 parallel → 0 Brueche“-Messung war zu schwach: sie suchte Luecken + zwischen Nachbarn, nicht Forks, und das Fenster ist lokal sehr schmal. + Fix: einzeiliger Mutex `AuditChainLock` + `FOR UPDATE`; InnoDB-Zeilensperren + fallen erst beim COMMIT. Dazu `isolationLevel: ReadCommitted`, damit der + Lesevorgang den frisch festgeschriebenen Stand sieht. + **Belegt** im Direktvergleich mit kuenstlich geweitetem Fenster: + Release-vor-Commit → Fork, Zeilensperre → kein Fork. + - **R166-02 (MEDIUM) – Hash deckte nur 7 Felder.** `changesBefore/After` + (die eigentliche Nutzlast), `success`, `ipAddress`, `resourceLabel`, + `dataSubjectId`, `userId`/`customerId` waren NICHT gehasht – ein Einzeledit + dort blieb unsichtbar. Fix: `hashVersion` (Migration `20260818150000`) + + `generateHashV2` ueber alle Inhaltsspalten. Bestandszeilen behalten + Version 1 und bleiben ohne Rehash gueltig. `rehashAll` schreibt V2. + Verifiziert: Manipulation an success/changesAfter/ipAddress/resourceLabel/ + dataSubjectId wird jetzt **5/5 erkannt**, Altbestand weiter gueltig. + - **R166-03 (LOW)** – „kein Cookie“ (normaler Erstbesuch) wurde als + `HIGH / abgelehnt` gefuehrt. Jetzt eigener Ausgang: LOW + Label + „ohne vorliegenden Token“. Nur echte Ablehnung bleibt HIGH. + - **R166-04 (LOW, pre-existing)** – `GET /retention-policies` wurde von + `GET /:id` verschluckt. Konkrete Routen jetzt konsequent vor der + Parameter-Route, mit Warnhinweis im Code. + - **Design-Empfehlungen umgesetzt:** `runRetentionCleanup` schreibt ein + Loeschungs-Manifest (Bereich `fromId`–`toId`, Anzahl, Policy, Cutoff) als + eigenen verketteten Eintrag – Luecken ausserhalb bleiben damit + erklaerungsbeduerftig; `rehashAll` schreibt einen Marker, dass die + Beweiskraft der Vergangenheit zurueckgesetzt wurde. + - Verifiziert: 50 parallele Schreiber → 50/50, 0 Forks, alle V2; + manipuliert 0, Luecken unveraendert 7. `tsc` + `vite build` gruen. + - **Offen (bewusst nicht umgesetzt):** externer Anker (HMAC mit Schluessel + ausserhalb der DB). Adressiert Full-DB-Compromise, erfordert aber + Schluesselverwaltung/Rotation im Deployment → Entscheidung des Betreibers. + - [x] **🔎 Audit-Pruefung: „manipuliert“ von „Luecke“ getrennt + Retention fuer Routine-Auth** (2026-08-18) - **Problem 1 (Deutbarkeit):** `verifyIntegrity` warf zwei voellig unterschiedliche Befunde in einen Topf und meldete beides als