Fix #122: Brute-Force-Schutz für /login (Account-Lockout nach Fehlversuchen)
This commit is contained in:
parent
6292c90fc5
commit
1d93ab8662
5 changed files with 117 additions and 2 deletions
|
|
@ -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)
|
## 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.
|
- **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.
|
||||||
|
|
|
||||||
8
migrations/086_login_throttle.sql
Normal file
8
migrations/086_login_throttle.sql
Normal file
|
|
@ -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;
|
||||||
|
|
@ -90,10 +90,20 @@ class LoginController
|
||||||
|
|
||||||
$user = User::findByUsername($username);
|
$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.';
|
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']);
|
Auth::login((int)$user['id']);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -8,6 +8,9 @@ use Steuerkram\Database;
|
||||||
|
|
||||||
class User
|
class User
|
||||||
{
|
{
|
||||||
|
private const LOCK_THRESHOLD = 5;
|
||||||
|
private const LOCK_MAX_MINUTES = 60;
|
||||||
|
|
||||||
public static function all(): array
|
public static function all(): array
|
||||||
{
|
{
|
||||||
return Database::fetchAll(
|
return Database::fetchAll(
|
||||||
|
|
@ -26,7 +29,7 @@ class User
|
||||||
public static function findByUsername(string $username): array|false
|
public static function findByUsername(string $username): array|false
|
||||||
{
|
{
|
||||||
return Database::fetchOne(
|
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]
|
[$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
|
public static function count(): int
|
||||||
{
|
{
|
||||||
$row = Database::fetchOne('SELECT COUNT(*) AS n FROM users');
|
$row = Database::fetchOne('SELECT COUNT(*) AS n FROM users');
|
||||||
|
|
|
||||||
51
tests/53_LoginThrottleTest.php
Normal file
51
tests/53_LoginThrottleTest.php
Normal file
|
|
@ -0,0 +1,51 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
use Steuerkram\Models\User;
|
||||||
|
|
||||||
|
class LoginThrottleTest extends TestCase
|
||||||
|
{
|
||||||
|
protected string $testName = 'Test 53: Brute-Force-Schutz beim Login (Konto-Lockout)';
|
||||||
|
|
||||||
|
public function run(): void
|
||||||
|
{
|
||||||
|
User::create('throttletest', 'password123');
|
||||||
|
$user = User::findByUsername('throttletest');
|
||||||
|
$id = (int)$user['id'];
|
||||||
|
|
||||||
|
$this->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');
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue