diff --git a/Changes.md b/Changes.md index ffbdce8..58898d8 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 2026-07-10 – Fix: Kein Schutz gegen Brute-Force am Login (Issue #122) + +- **Mittel:** `LoginController::authenticate()` prüfte Benutzername/Passwort ausschließlich per `password_verify()` – ohne Rate-Begrenzung, Lockout, Captcha oder Delay. Da `/login` als einzige Route bewusst ohne `Auth::require()` erreichbar ist und Passwörter nur eine Mindestlänge von 8 Zeichen benötigen, war ein Online-Brute-Force-/Credential-Stuffing-Angriff gegen einzelne Konten praktisch unlimitiert (CWE-307). +- **`migrations/086_login_throttle.sql`:** Neue Spalten `users.failed_attempts` und `users.locked_until`. +- **`src/Models/User.php`:** `registerFailedLogin()` zählt Fehlversuche pro Benutzer und sperrt das Konto ab 5 Fehlversuchen für ein mit jedem weiteren Fehlversuch steigendes Zeitfenster (`2^(Fehlversuche-5)` Minuten, gedeckelt auf 60 Minuten). `resetFailedLogins()` setzt den Zähler bei erfolgreichem Login zurück, `isLocked()` prüft die Sperre. +- **`src/Controllers/LoginController.php`:** `authenticate()` weist Login-Versuche für gesperrte Konten ab, bevor das Passwort geprüft wird, und registriert Fehlversuche bei falschem Passwort. +- **`tests/53_LoginThrottleTest.php`:** Neuer Test prüft Zählung der Fehlversuche, Sperre ab dem 5. Fehlversuch, steigendes Zeitfenster bei weiteren Fehlversuchen und Reset bei erfolgreichem Login. + ## 2026-07-08 – Fix: SMTP-/Nextcloud-Zugangsdaten im Klartext gespeichert und im HTML sichtbar (Issue #119) - **Hoch:** `mail_smtp_pass` (SMTP-Passwort) und `nextcloud_pass` (Nextcloud-App-Passwort) wurden unverschlüsselt in `company_settings` abgelegt, obwohl für vergleichbar sensible Felder (FinTS-PIN, Client-ID/Secret) bereits `Crypto::encrypt()`/`Crypto::decrypt()` (AES-256-GCM) etabliert ist. Zusätzlich schrieb `src/Views/settings/form.php` beide Klartext-Passwörter in das `value`-Attribut zurück, wodurch sie für jeden Betrachter des Seitenquelltexts sichtbar waren. diff --git a/migrations/086_login_throttle.sql b/migrations/086_login_throttle.sql new file mode 100644 index 0000000..734e4fd --- /dev/null +++ b/migrations/086_login_throttle.sql @@ -0,0 +1,8 @@ +-- 086_login_throttle.sql +-- Fix #122: Brute-Force-Schutz für /login. +-- Fehlversuche pro Benutzername zählen und Konto nach zu vielen +-- Fehlversuchen für ein steigendes Zeitfenster sperren. + +ALTER TABLE users + ADD COLUMN IF NOT EXISTS failed_attempts INT NOT NULL DEFAULT 0 AFTER active, + ADD COLUMN IF NOT EXISTS locked_until DATETIME NULL DEFAULT NULL AFTER failed_attempts; diff --git a/src/Controllers/LoginController.php b/src/Controllers/LoginController.php index 7db940e..0e8db0f 100644 --- a/src/Controllers/LoginController.php +++ b/src/Controllers/LoginController.php @@ -90,10 +90,20 @@ class LoginController $user = User::findByUsername($username); - if (!$user || !$user['active'] || !password_verify($password, $user['password_hash'])) { + if (!$user || !$user['active']) { return 'Ungültige Anmeldedaten oder Konto deaktiviert.'; } + if (User::isLocked($user)) { + return 'Konto vorübergehend gesperrt wegen zu vieler Fehlversuche. Bitte später erneut versuchen.'; + } + + if (!password_verify($password, $user['password_hash'])) { + User::registerFailedLogin((int)$user['id'], (int)$user['failed_attempts']); + return 'Ungültige Anmeldedaten oder Konto deaktiviert.'; + } + + User::resetFailedLogins((int)$user['id']); Auth::login((int)$user['id']); return null; } diff --git a/src/Models/User.php b/src/Models/User.php index efad6b0..764eccb 100644 --- a/src/Models/User.php +++ b/src/Models/User.php @@ -8,6 +8,9 @@ use Steuerkram\Database; class User { + private const LOCK_THRESHOLD = 5; + private const LOCK_MAX_MINUTES = 60; + public static function all(): array { return Database::fetchAll( @@ -26,7 +29,7 @@ class User public static function findByUsername(string $username): array|false { return Database::fetchOne( - 'SELECT id, username, password_hash, active FROM users WHERE username = ?', + 'SELECT id, username, password_hash, active, failed_attempts, locked_until FROM users WHERE username = ?', [$username] ); } @@ -69,6 +72,41 @@ class User ); } + /** + * Zählt einen Fehlversuch. Ab LOCK_THRESHOLD Fehlversuchen wird das Konto + * für ein mit jedem weiteren Fehlversuch steigendes Zeitfenster gesperrt. + */ + public static function registerFailedLogin(int $id, int $currentFailedAttempts): void + { + $attempts = $currentFailedAttempts + 1; + + if ($attempts >= self::LOCK_THRESHOLD) { + $lockMinutes = min(2 ** ($attempts - self::LOCK_THRESHOLD), self::LOCK_MAX_MINUTES); + Database::execute( + 'UPDATE users SET failed_attempts = ?, locked_until = DATE_ADD(NOW(), INTERVAL ? MINUTE) WHERE id = ?', + [$attempts, $lockMinutes, $id] + ); + } else { + Database::execute( + 'UPDATE users SET failed_attempts = ? WHERE id = ?', + [$attempts, $id] + ); + } + } + + public static function resetFailedLogins(int $id): void + { + Database::execute( + 'UPDATE users SET failed_attempts = 0, locked_until = NULL WHERE id = ?', + [$id] + ); + } + + public static function isLocked(array $user): bool + { + return !empty($user['locked_until']) && strtotime((string)$user['locked_until']) > time(); + } + public static function count(): int { $row = Database::fetchOne('SELECT COUNT(*) AS n FROM users'); diff --git a/tests/53_LoginThrottleTest.php b/tests/53_LoginThrottleTest.php new file mode 100644 index 0000000..e77fdc7 --- /dev/null +++ b/tests/53_LoginThrottleTest.php @@ -0,0 +1,51 @@ +assertEquals(0, $user['failed_attempts'], 'Neuer Benutzer startet ohne Fehlversuche'); + $this->assert(!User::isLocked($user), 'Neuer Benutzer ist nicht gesperrt'); + + // 4 Fehlversuche: noch keine Sperre + for ($i = 0; $i < 4; $i++) { + $user = User::findByUsername('throttletest'); + User::registerFailedLogin($id, (int)$user['failed_attempts']); + } + $user = User::findByUsername('throttletest'); + $this->assertEquals(4, $user['failed_attempts'], 'Nach 4 Fehlversuchen: Zähler steht auf 4'); + $this->assert(!User::isLocked($user), 'Nach 4 Fehlversuchen noch keine Sperre'); + + // 5. Fehlversuch: Konto wird gesperrt + User::registerFailedLogin($id, (int)$user['failed_attempts']); + $user = User::findByUsername('throttletest'); + $this->assertEquals(5, $user['failed_attempts'], 'Nach 5 Fehlversuchen: Zähler steht auf 5'); + $this->assert(User::isLocked($user), 'Nach 5 Fehlversuchen ist das Konto gesperrt'); + $this->assertNotEmpty($user['locked_until'], 'locked_until ist gesetzt'); + + // Weitere Fehlversuche verlängern die Sperre (steigendes Zeitfenster) + $firstLock = $user['locked_until']; + User::registerFailedLogin($id, (int)$user['failed_attempts']); + $user = User::findByUsername('throttletest'); + $this->assert( + strtotime((string)$user['locked_until']) >= strtotime((string)$firstLock), + 'Sperrzeitfenster wird bei weiteren Fehlversuchen nicht kürzer' + ); + + // Erfolgreicher Login-Reset hebt die Sperre auf + User::resetFailedLogins($id); + $user = User::findByUsername('throttletest'); + $this->assertEquals(0, $user['failed_attempts'], 'Reset setzt Zähler auf 0'); + $this->assert(!User::isLocked($user), 'Nach Reset ist das Konto nicht mehr gesperrt'); + } +}