From c7d6b6de7ea44667e6c98f34a595806aa30819e4 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Wed, 19 Aug 2026 11:30:36 +0200 Subject: [PATCH] Audit-Pruefung: "manipuliert" von "Luecke" getrennt + Retention fuer Routine-Auth Problem 1 (Deutbarkeit): verifyIntegrity warf zwei voellig unterschiedliche Befunde in einen Topf und meldete beides als "N manipulierte Eintraege". Eine harmlose Verkettungsluecke sah damit aus wie ein Angriff - die Meldung war im Alltag nicht deutbar und dadurch wertlos, dasselbe Muster wie beim Refresh-Rauschen. Fix: Rueckgabe um tamperedEntries (Inhalt nachtraeglich veraendert, ernst) und chainGaps (Verkettung unterbrochen durch parallele Schreibvorgaenge oder geloeschte Zeilen, meist harmlos) erweitert. invalidEntries bleibt als Summe erhalten. Controller formuliert die Meldung eindeutig, Frontend-API-Typ nachgezogen. Problem 2 (Aufbewahrung): Token-Refreshes landen seit der Entrauschung als Authentication/LOW. Diese Kombination traf auf keine spezifische Regel und fiel in die Auffangregel * mit 3650 Tagen - das Rauschen waere 10 Jahre aufbewahrt worden, echte Logins nur 2. Fix: Regel Authentication/LOW mit 90 Tagen, als idempotente Migration und im Seed. Sensitivitaet steuert die Aufbewahrung und ist keine Alarmstufe - normale Logins und Zugriffe auf Bankdaten/Ausweise bleiben bewusst CRITICAL, ein Herabstufen wuerde still die Aufbewahrungsfrist verlaengern. Verifiziert: Live-Test gegen Dev-DB - echte Manipulation einer Zeile wird als manipuliert erkannt und nicht mit Luecken verwechselt, Ketten-Luecken bleiben bei 7, Originalzustand exakt wiederhergestellt. tsc + vite build gruen. Co-Authored-By: Claude Opus 5 --- .../migration.sql | 19 +++++++++++++ backend/prisma/seed.ts | 10 +++++++ .../src/controllers/auditLog.controller.ts | 23 +++++++++++++-- backend/src/services/audit.service.ts | 24 ++++++++++++++-- docs/todo.md | 28 +++++++++++++++++++ frontend/src/services/api.ts | 2 +- 6 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 backend/prisma/migrations/20260818130000_audit_retention_auth_low/migration.sql diff --git a/backend/prisma/migrations/20260818130000_audit_retention_auth_low/migration.sql b/backend/prisma/migrations/20260818130000_audit_retention_auth_low/migration.sql new file mode 100644 index 00000000..d45594f2 --- /dev/null +++ b/backend/prisma/migrations/20260818130000_audit_retention_auth_low/migration.sql @@ -0,0 +1,19 @@ +-- Aufbewahrungsregel fuer routinemaessige Auth-Eintraege (Token-Refresh). +-- +-- Seit der Entrauschung landen erfolgreiche Token-Refreshes als +-- `Authentication / LOW`. Diese Kombination traf auf KEINE spezifische Regel +-- (es gab nur `Authentication / CRITICAL`) und fiel damit in die Auffangregel +-- `*` mit 3650 Tagen. Ergebnis: Das Rauschen waere 10 Jahre aufbewahrt worden, +-- echte Logins dagegen nur 2 Jahre - genau verkehrt herum. +-- +-- 90 Tage reichen, um einen Refresh-Vorgang im Nachhinein nachzuvollziehen. +-- Idempotent: der Unique-Index (resourceType, sensitivity) verhindert Dubletten. +INSERT INTO `AuditRetentionPolicy` + (`resourceType`, `sensitivity`, `retentionDays`, `description`, `legalBasis`, `isActive`, `createdAt`, `updatedAt`) +VALUES + ('Authentication', 'LOW', 90, 'Routine-Auth (stiller Token-Refresh)', 'Betriebsnotwendigkeit / Datenminimierung (DSGVO Art. 5)', 1, NOW(3), NOW(3)) +ON DUPLICATE KEY UPDATE + `retentionDays` = VALUES(`retentionDays`), + `description` = VALUES(`description`), + `legalBasis` = VALUES(`legalBasis`), + `updatedAt` = NOW(3); diff --git a/backend/prisma/seed.ts b/backend/prisma/seed.ts index e4b9facb..4c673ef7 100644 --- a/backend/prisma/seed.ts +++ b/backend/prisma/seed.ts @@ -497,6 +497,16 @@ async function main() { description: 'Allgemeine Einstellungen', legalBasis: 'Verjährungsfrist (BGB §195)', }, + { + // Stiller Token-Refresh (Routine). Ohne eigene Regel fiele diese + // Kombination in die Auffangregel `*` mit 3650 Tagen – das Rauschen + // wäre dann länger aufbewahrt als echte Logins (730 Tage). + resourceType: 'Authentication', + sensitivity: 'LOW' as const, + retentionDays: 90, + description: 'Routine-Auth (stiller Token-Refresh)', + legalBasis: 'Betriebsnotwendigkeit / Datenminimierung (DSGVO Art. 5)', + }, ]; for (const policy of specificPolicies) { diff --git a/backend/src/controllers/auditLog.controller.ts b/backend/src/controllers/auditLog.controller.ts index dc189c93..7d97eb23 100644 --- a/backend/src/controllers/auditLog.controller.ts +++ b/backend/src/controllers/auditLog.controller.ts @@ -143,15 +143,32 @@ export async function verifyIntegrity(req: AuthRequest, res: Response) { toId ? parseInt(toId as string) : undefined ); + // Zwei sehr unterschiedliche Befunde sauber trennen – vorher wurde beides + // pauschal als "manipuliert" gemeldet, was strukturelle Lücken wie einen + // echten Angriff aussehen liess (und damit die Meldung entwertete). + const tampered = result.tamperedEntries.length; + const gaps = result.chainGaps.length; + + const message = tampered > 0 + ? `${tampered} MANIPULIERTE Einträge gefunden` + + (gaps > 0 ? ` (zusätzlich ${gaps} strukturelle Lücken)` : '') + : gaps > 0 + ? `Keine Manipulation. ${gaps} strukturelle Lücken in der Verkettung ` + + '(parallel geschriebene oder gelöschte Einträge) – Inhalte unverändert.' + : 'Alle Einträge sind unverändert und lückenlos verkettet'; + res.json({ success: true, data: { valid: result.valid, checkedCount: result.checkedCount, invalidEntries: result.invalidEntries, - message: result.valid - ? 'Alle Einträge sind valide' - : `${result.invalidEntries.length} manipulierte Einträge gefunden`, + // Ernst: Inhalt einer bestehenden Zeile wurde nachträglich verändert. + tamperedEntries: result.tamperedEntries, + // Meist harmlos: Verkettung unterbrochen, Inhalte selbst unversehrt. + chainGaps: result.chainGaps, + tampered: tampered > 0, + message, }, }); } catch (error) { diff --git a/backend/src/services/audit.service.ts b/backend/src/services/audit.service.ts index f116099c..84e89ff1 100644 --- a/backend/src/services/audit.service.ts +++ b/backend/src/services/audit.service.ts @@ -450,7 +450,20 @@ export async function getAuditLogsByDataSubject(customerId: number) { export async function verifyIntegrity(fromId?: number, toId?: number): Promise<{ valid: boolean; checkedCount: number; + /** Alle beanstandeten Zeilen (tampered + chainGaps) – Abwaertskompatibilitaet. */ invalidEntries: number[]; + /** + * ERNST: Der Inhalt der Zeile passt nicht mehr zu ihrem Hash – jemand hat + * einen bestehenden Eintrag nachtraeglich veraendert. + */ + tamperedEntries: number[]; + /** + * MEIST HARMLOS: Der Inhalt stimmt, aber die Verkettung zur Vorgaengerzeile + * passt nicht. Ursachen: parallel geschriebene Eintraege (bis zum Fix der + * Race-Condition) oder geloeschte Zeilen (Retention-Cleanup). Kein Hinweis + * auf Manipulation – die Zeilen selbst sind unveraendert. + */ + chainGaps: number[]; }> { const where: Prisma.AuditLogWhereInput = {}; @@ -473,7 +486,8 @@ export async function verifyIntegrity(fromId?: number, toId?: number): Promise<{ }, }); - const invalidEntries: number[] = []; + const tamperedEntries: number[] = []; + const chainGaps: number[] = []; for (let i = 0; i < logs.length; i++) { const log = logs[i]; @@ -503,7 +517,7 @@ export async function verifyIntegrity(fromId?: number, toId?: number): Promise<{ previousHash: log.previousHash, }); if (log.hash !== legacyHash) { - invalidEntries.push(log.id); + tamperedEntries.push(log.id); continue; } } @@ -512,15 +526,19 @@ export async function verifyIntegrity(fromId?: number, toId?: number): Promise<{ if (i > 0) { const previousLog = logs[i - 1]; if (log.previousHash !== previousLog.hash) { - invalidEntries.push(log.id); + chainGaps.push(log.id); } } } + const invalidEntries = [...tamperedEntries, ...chainGaps].sort((a, b) => a - b); + return { valid: invalidEntries.length === 0, checkedCount: logs.length, invalidEntries, + tamperedEntries, + chainGaps, }; } diff --git a/docs/todo.md b/docs/todo.md index 02309a9e..5877c3e0 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,34 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [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 + „N manipulierte Eintraege“. Eine harmlose Verkettungsluecke sah damit aus + wie ein Angriff – die Meldung war im Alltag nicht deutbar und wurde dadurch + wertlos (dasselbe Muster wie beim Refresh-Rauschen). + - Fix: Rueckgabe um `tamperedEntries` (Inhalt einer Zeile nachtraeglich + veraendert – **ernst**) und `chainGaps` (Verkettung unterbrochen durch + parallele Schreibvorgaenge oder geloeschte Zeilen – **meist harmlos**) + erweitert. `invalidEntries` bleibt als Summe erhalten + (Abwaertskompatibilitaet). Controller formuliert die Meldung entsprechend + eindeutig; Frontend-API-Typ nachgezogen. + - **Problem 2 (Aufbewahrung):** Seit der Entrauschung landen Token-Refreshes + als `Authentication / LOW`. Diese Kombination traf auf keine spezifische + Regel und fiel in die Auffangregel `*` mit 3650 Tagen – das **Rauschen + waere 10 Jahre** aufbewahrt worden, echte Logins nur 2 (730 Tage). + - Fix: Regel `Authentication / LOW` → 90 Tage. Als Migration + (`20260818130000`, idempotent per `ON DUPLICATE KEY`) **und** im Seed, damit + sie sowohl bestehende Installationen als auch Neuinstallationen erreicht. + - Hinweis zur Sensitivitaet: Sie steuert die Aufbewahrung, ist also **keine** + Alarmstufe. Normale Logins/Logouts sowie Zugriffe auf Bankdaten/Ausweise + bleiben bewusst CRITICAL. Ein Herabstufen „fuer eine ruhigere Liste“ wuerde + still die Aufbewahrungsfrist verlaengern – daher unterlassen. + - Verifiziert: Live-Test gegen Dev-DB – echte Manipulation einer Zeile + (`UPDATE … SET userEmail`) wird als **manipuliert** erkannt und nicht mit + Luecken verwechselt; Ketten-Luecken bleiben bei 7; Originalzustand exakt + wiederhergestellt (0 manipuliert danach). `tsc` + `vite build` gruen. + - [x] **🔗 Audit-Kette: Race beim Fortschreiben behoben (parallele Requests)** (2026-08-18) - `createAuditLog` las den Vorgaenger-Hash und schrieb den neuen Eintrag als zwei getrennte Schritte. Zwei parallele Requests lasen denselben letzten diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 5ac8e7cb..14e5ef01 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -1752,7 +1752,7 @@ export const auditLogApi = { return res.data; }, verifyIntegrity: async () => { - const res = await api.post>('/audit-logs/verify'); + const res = await api.post>('/audit-logs/verify'); return res.data; }, rehash: async () => {