From 118f0203c2aa9b20b5d725b61c070c9d914c815e Mon Sep 17 00:00:00 2001 From: nexxo Date: Mon, 3 Aug 2026 13:58:55 +0200 Subject: [PATCH] Fix-Runde 1: der Zaehlstand haelt jetzt ueber Laeufe hinweg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ScanForIntrusions hielt bisher nur den Zuwachs eines einzelnen Laufs gegen die Schwelle — bei einem Lauf pro Minute wurde aus "10 in 10 Minuten" faktisch "10 in einer Minute", und der geduldige Angreifer mit wenigen Versuchen je Minute lief nie darueber. Ein Zaehlstand je Subjekt und Adresse ueber Laravels RateLimiter (cache-gestuetzt, 600s, wie OperatorLogin es fuer Anmeldeversuche schon vormacht) addiert jeden Lauf auf den bestehenden Stand und wird nach dem Sperren zurueckgesetzt. --- app/Provisioning/Jobs/ScanForIntrusions.php | 71 +++++++++++++++++-- .../Security/ScanForIntrusionsTest.php | 50 +++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-) 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.