diff --git a/app/Provisioning/Jobs/ScanForIntrusions.php b/app/Provisioning/Jobs/ScanForIntrusions.php index cc0f53d..68c71d2 100644 --- a/app/Provisioning/Jobs/ScanForIntrusions.php +++ b/app/Provisioning/Jobs/ScanForIntrusions.php @@ -13,12 +13,25 @@ use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\RateLimiter; /** * Der Melder: liest gescheiterte Anmeldungen von jeder aktiven Instanz und * jedem eingerichteten Host, sperrt ab der Schwelle über `BlockAddress`, und * trägt danach jede noch gültige Sperre erneut in die Firewall ein. * + * Die Zusage lautet "10 Fehlversuche in 10 Minuten" — nicht "10 Fehlversuche + * im Zuwachs eines einzelnen Laufs". Jeder Lauf liest von `FailedLoginReader` + * nur, was seit dem letzten Mal neu dazugekommen ist (typisch: eine Minute + * Protokoll), und wer das direkt gegen die Schwelle hielte, würde aus "10 in + * 10 Minuten" faktisch "10 in einer Minute" machen — ein Angreifer mit drei + * Versuchen je Minute käme auf dreißig in zehn Minuten und liefe nie über die + * Schwelle. Deshalb wandert jeder frisch gelesene Treffer in einen + * Zählstand je Subjekt und Adresse, der über Läufe hinweg hält und nach zehn + * Minuten von selbst verfällt (`RateLimiter`, cache-gestützt, wie + * `App\Livewire\Auth\OperatorLogin` es für Anmeldeversuche schon vormacht) — + * erst DER Stand wird gegen die Schwelle gehalten. + * * Das Wiedereintragen ist kein Aufräumen nebenbei: die nftables-Menge lebt im * Speicher des Hosts, ein Neustart leert sie, während unsere Datenbank die * Sperre weiterführt. Mit der RESTLAUFZEIT (`now()` bis `expires_at`), nicht @@ -37,6 +50,15 @@ class ScanForIntrusions implements ShouldQueue /** Ab wie vielen Fehlversuchen im Fenster gesperrt wird. */ private const THRESHOLD = 10; + /** + * Das Fenster, ueber das der Zaehlstand je Subjekt und Adresse haelt — + * dieselben zehn Minuten wie die Schwelle selbst, nur jetzt als + * Cache-Ablaufzeit statt als Zeitstempel-Filter innerhalb einer einzelnen + * Protokoll-Zeile (das bleibt zusaetzlich in FailedLoginReader, gegen ein + * lange nicht gelesenes Protokoll). + */ + private const WINDOW_SECONDS = 600; + public function __construct() { // Nicht als `public $queue`-Eigenschaft: die kollidiert mit der @@ -82,9 +104,14 @@ class ScanForIntrusions implements ShouldQueue continue; } - foreach ($result['addresses'] as $ip => $attempts) { - if ($attempts >= self::THRESHOLD) { - $blocker->forInstance($instance, $ip, $attempts); + $subject = 'instance:'.$instance->id; + + foreach ($result['addresses'] as $ip => $freshAttempts) { + $total = $this->accumulate($subject, $ip, $freshAttempts); + + if ($total >= self::THRESHOLD) { + $blocker->forInstance($instance, $ip, $total); + $this->resetAccumulator($subject, $ip); } } @@ -108,9 +135,14 @@ class ScanForIntrusions implements ShouldQueue continue; } - foreach ($result['addresses'] as $ip => $attempts) { - if ($attempts >= self::THRESHOLD) { - $blocker->forHost($host, $ip, $attempts); + $subject = 'host:'.$host->id; + + foreach ($result['addresses'] as $ip => $freshAttempts) { + $total = $this->accumulate($subject, $ip, $freshAttempts); + + if ($total >= self::THRESHOLD) { + $blocker->forHost($host, $ip, $total); + $this->resetAccumulator($subject, $ip); } } @@ -118,6 +150,33 @@ class ScanForIntrusions implements ShouldQueue } } + /** + * Addiert frisch gelesene Treffer auf den laufübergreifenden Stand und + * gibt den neuen Gesamtstand zurück. Der Schlüssel trägt Subjekt UND + * Adresse: zwei Instanzen, die von derselben Adresse Versuche sehen, + * dürfen sich nicht gegenseitig hochzählen, und eine Adresse darf nicht + * von den Versuchen einer anderen mitgerissen werden. + */ + private function accumulate(string $subject, string $ip, int $freshAttempts): int + { + return RateLimiter::increment($this->tallyKey($subject, $ip), self::WINDOW_SECONDS, $freshAttempts); + } + + /** + * Nach dem Sperren zurücksetzen. Ohne das stünde der Stand weiter bei der + * Schwelle, und sobald die Sperre abläuft oder der Inhaber sie aufhebt, + * löste der nächste einzelne Fehlversuch sofort die nächste Sperre aus. + */ + private function resetAccumulator(string $subject, string $ip): void + { + RateLimiter::clear($this->tallyKey($subject, $ip)); + } + + private function tallyKey(string $subject, string $ip): string + { + return 'intrusion-scan:'.$subject.':'.$ip; + } + private function reapplyActiveBlocks(HostFirewall $firewall): void { $blocks = SecurityBlock::active()->with(['host', 'instance.host'])->get(); diff --git a/tests/Feature/Security/ScanForIntrusionsTest.php b/tests/Feature/Security/ScanForIntrusionsTest.php index 863ec87..d1a9fb6 100644 --- a/tests/Feature/Security/ScanForIntrusionsTest.php +++ b/tests/Feature/Security/ScanForIntrusionsTest.php @@ -90,6 +90,56 @@ it('sperrt nicht, wenn sich die Versuche ueber zwei Fenster verteilen', function expect(SecurityBlock::count())->toBe(0); }); +it('sperrt auch, wenn sich die zehn Versuche ueber mehrere Laeufe verteilen', function () { + // Der geduldige Angreifer. Drei Versuche je Lauf, vier Laeufe — zwoelf + // Versuche innerhalb des Zehn-Minuten-Fensters. Zaehlte jeder Lauf nur fuer + // sich (nur den eigenen Zuwachs seit dem letzten Mal), kaeme dieser + // Angreifer nie ueber die Schwelle, und die Zusage "10 in 10 Minuten" + // waere in Wahrheit "10 im Zuwachs eines einzelnen Laufs" — meist eine + // Minute. + aktiveInstanz(); + $pve = new FakeProxmoxClient; + app()->instance(\App\Services\Proxmox\ProxmoxClient::class, $pve); + + $offset = 0; + foreach (range(1, 4) as $lauf) { + // Je Lauf NEUE Zeilen im Protokoll, nicht dieselben noch einmal — + // 'stat -c %s' zuerst registriert (wie im Rotations-Test), damit die + // Groessen-Abfrage nicht denselben Treffer wie 'tail' bekommt. + $neu = protokollZeilen('203.0.113.7', 3); + $pve->guestScripts['stat -c %s'] = ['out-data' => ($offset + strlen($neu))."\n", 'exitcode' => 0]; + $pve->guestScripts['nextcloud.log'] = ['out-data' => $neu, 'exitcode' => 0]; + + app(ScanForIntrusions::class)->handle(); + + $offset += strlen($neu); + } + + expect(SecurityBlock::where('ip', '203.0.113.7')->count())->toBe(1); +}); + +it('sperrt nicht, wenn neun Versuche ueber mehrere Laeufe unter der Schwelle bleiben', function () { + // Die Gegenprobe zum Test oben: ohne sie waere der auch dann gruen, wenn + // ScanForIntrusions gar nicht mehr zaehlte, sondern bei jedem Lauf ab dem + // ersten Treffer sperrte. + aktiveInstanz(); + $pve = new FakeProxmoxClient; + app()->instance(\App\Services\Proxmox\ProxmoxClient::class, $pve); + + $offset = 0; + foreach ([2, 2, 2, 3] as $anzahl) { + $neu = protokollZeilen('203.0.113.7', $anzahl); + $pve->guestScripts['stat -c %s'] = ['out-data' => ($offset + strlen($neu))."\n", 'exitcode' => 0]; + $pve->guestScripts['nextcloud.log'] = ['out-data' => $neu, 'exitcode' => 0]; + + app(ScanForIntrusions::class)->handle(); + + $offset += strlen($neu); + } + + expect(SecurityBlock::count())->toBe(0); +}); + it('faengt bei einem rotierten Protokoll wieder bei null an', function () { // Ist die Datei kleiner als der gemerkte Versatz, wurde rotiert. Ohne diese // Behandlung liest der nächste Lauf ins Leere und sieht nie wieder etwas.