diff --git a/app/Provisioning/Jobs/ScanForIntrusions.php b/app/Provisioning/Jobs/ScanForIntrusions.php index 68c71d2..d974bee 100644 --- a/app/Provisioning/Jobs/ScanForIntrusions.php +++ b/app/Provisioning/Jobs/ScanForIntrusions.php @@ -14,6 +14,7 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\RateLimiter; +use Throwable; /** * Der Melder: liest gescheiterte Anmeldungen von jeder aktiven Instanz und @@ -97,27 +98,41 @@ class ScanForIntrusions implements ShouldQueue ->get(); foreach ($instances as $instance) { - $result = $reader->fromInstance($instance); + // Wie PingHosts nebenan: je Instanz einzeln umschlossen, nicht die + // ganze Schleife in einem Block. `FailedLoginReader::fromInstance()` + // fängt zwar schon jede Ausnahme von `guestExec()` selbst ab + // (Fix-Runde 2: es tat das vorher NICHT, und eine einzelne + // abgeschaltete VM riss den ganzen Lauf mit — die übrigen + // Instanzen wurden nicht mehr gelesen, scanHosts() lief nicht, + // und vor allem reapplyActiveBlocks() lief nicht, jede Minute + // erneut). Dieser zweite Ring bleibt als Netz für alles andere in + // dieser Runde (BlockAddress, save()), aus demselben Grund. + try { + $result = $reader->fromInstance($instance); - // Gast antwortet nicht — kein Vorfall, Versatz bleibt unangetastet. - if ($result === null) { - continue; - } - - $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); + // Gast antwortet nicht — kein Vorfall, Versatz bleibt unangetastet. + if ($result === null) { + continue; } - } - // Nur bei Erfolg speichern — genau die Bytes, die wirklich - // gelesen wurden. - $instance->forceFill(['security_log_offset' => $result['offset']])->save(); + $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); + } + } + + // Nur bei Erfolg speichern — genau die Bytes, die wirklich + // gelesen wurden. + $instance->forceFill(['security_log_offset' => $result['offset']])->save(); + } catch (Throwable) { + // Absichtlich still, wie PingHosts: der Cursor bleibt + // unangetastet, der nächste Lauf versucht es wieder. + } } } diff --git a/app/Services/Proxmox/FakeProxmoxClient.php b/app/Services/Proxmox/FakeProxmoxClient.php index c485cc5..1b0aa4b 100644 --- a/app/Services/Proxmox/FakeProxmoxClient.php +++ b/app/Services/Proxmox/FakeProxmoxClient.php @@ -255,10 +255,26 @@ class FakeProxmoxClient implements ProxmoxClient return $this; } + /** + * VMIDs whose guest agent calls throw instead of answering at all — a + * genuinely unreachable guest (powered off, agent not started yet), as + * opposed to `guestScripts`, which simulates a clean non-zero exit code. + * The real client's `guestExec()` calls `->throw()` on every HTTP + * response, so this is the failure mode a clean exit code cannot stand + * in for. + * + * @var array + */ + public array $guestThrows = []; + public function guestExec(string $node, int $vmid, string $command): array { $this->guestCommands[] = $command; + if (isset($this->guestThrows[$vmid])) { + throw $this->guestThrows[$vmid]; + } + foreach ($this->guestScripts as $substring => $result) { if (str_contains($command, $substring)) { return $result; diff --git a/app/Services/Security/FailedLoginReader.php b/app/Services/Security/FailedLoginReader.php index c237994..98bcdd7 100644 --- a/app/Services/Security/FailedLoginReader.php +++ b/app/Services/Security/FailedLoginReader.php @@ -50,26 +50,36 @@ class FailedLoginReader return null; } - $pve = $this->pve->forHost($host); $node = (string) ($host->node ?? 'pve'); $vmid = (int) $instance->vmid; $offset = (int) $instance->security_log_offset; - $size = $pve->guestExec($node, $vmid, NextcloudOcc::exec('stat -c %s data/nextcloud.log')); + try { + $pve = $this->pve->forHost($host); - if ((int) ($size['exitcode'] ?? 1) !== 0) { + $size = $pve->guestExec($node, $vmid, NextcloudOcc::exec('stat -c %s data/nextcloud.log')); + + if ((int) ($size['exitcode'] ?? 1) !== 0) { + return null; + } + + // Kleiner als der gemerkte Versatz heißt: rotiert. Ohne diese + // Behandlung liest der nächste Aufruf ins Leere und sieht nie + // wieder etwas — siehe den eigenen Testfall dafür. + if ((int) trim((string) ($size['out-data'] ?? '')) < $offset) { + $offset = 0; + } + + $tail = $pve->guestExec($node, $vmid, NextcloudOcc::exec('tail -c +'.($offset + 1).' data/nextcloud.log')); + } catch (Throwable) { + // Wie fromHost(): ein abgeschalteter Gast oder ein Agent, der noch + // nicht laeuft, wirft eine echte Ausnahme statt eines + // Fehlercodes — `guestExec()` ruft `->throw()` auf jede + // HTTP-Antwort. Genauso kein Vorfall wie ein sauberer + // `exitcode != 0`, und kein Grund, den ganzen Lauf mitzureissen. return null; } - // Kleiner als der gemerkte Versatz heißt: rotiert. Ohne diese - // Behandlung liest der nächste Aufruf ins Leere und sieht nie wieder - // etwas — siehe den eigenen Testfall dafür. - if ((int) trim((string) ($size['out-data'] ?? '')) < $offset) { - $offset = 0; - } - - $tail = $pve->guestExec($node, $vmid, NextcloudOcc::exec('tail -c +'.($offset + 1).' data/nextcloud.log')); - if ((int) ($tail['exitcode'] ?? 1) !== 0) { return null; } diff --git a/tests/Feature/Security/ScanForIntrusionsTest.php b/tests/Feature/Security/ScanForIntrusionsTest.php index d1a9fb6..60d628c 100644 --- a/tests/Feature/Security/ScanForIntrusionsTest.php +++ b/tests/Feature/Security/ScanForIntrusionsTest.php @@ -166,28 +166,84 @@ it('traegt eine noch gueltige Sperre mit der RESTLAUFZEIT wieder ein', function // Mikrosekunden, die `expires_at` nach dem Runden-durch-die-Datenbank // (die Spalte kennt keine Bruchteile) nicht mehr hat — genau die // Differenz, um die 1200s sonst auf 1199s abrundet. + // finally: bricht eine Zusicherung weiter unten ab, bleibt die Uhr sonst + // fuer alle Folgetests eingefroren stehen, und jemand sucht stundenlang + // am falschen Ende. Carbon::setTestNow(now()->startOfSecond()); - $shell = new \App\Services\Ssh\FakeRemoteShell; - app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + try { + $shell = new \App\Services\Ssh\FakeRemoteShell; + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); - // Der Auftragszettel legt die Sperre ohne Host an — SecurityBlockFactory - // haengt sie sonst an eine Instanz ohne host_id, und ohne Host gibt es - // nichts, wo HostFirewall::block() etwas eintragen koennte. Mit `forHost` - // (Aufgabe 3) direkt an einem echten Host, wie es eine host_ssh-Sperre - // ohnehin waere. - $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); - $block = SecurityBlock::factory()->forHost($host)->create([ - 'expires_at' => now()->addMinutes(20), - 'blocked_at' => now()->subMinutes(40), - ]); + // Der Auftragszettel legt die Sperre ohne Host an — SecurityBlockFactory + // haengt sie sonst an eine Instanz ohne host_id, und ohne Host gibt es + // nichts, wo HostFirewall::block() etwas eintragen koennte. Mit `forHost` + // (Aufgabe 3) direkt an einem echten Host, wie es eine host_ssh-Sperre + // ohnehin waere. + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + $block = SecurityBlock::factory()->forHost($host)->create([ + 'expires_at' => now()->addMinutes(20), + 'blocked_at' => now()->subMinutes(40), + ]); - app(ScanForIntrusions::class)->handle(); + app(ScanForIntrusions::class)->handle(); - expect($shell->ran('timeout 1200s'))->toBeTrue() - ->and($shell->ran('timeout 3600s'))->toBeFalse(); + expect($shell->ran('timeout 1200s'))->toBeTrue() + ->and($shell->ran('timeout 3600s'))->toBeFalse(); + } finally { + Carbon::setTestNow(); + } +}); - Carbon::setTestNow(); +it('laesst einen werfenden Gast den Lauf nicht abbrechen', function () { + // Ein abgeschalteter Gast wirft, statt einen Fehlercode zu liefern — + // guestExec() ruft ->throw() auf jede Antwort. Vorher riss das den ganzen + // Minutentakt mit: die uebrigen Instanzen, die Hosts, und vor allem das + // Wiedereintragen der noch gueltigen Sperren. + // + // Uhr eingefroren wie im Restlaufzeit-Test darueber, aus demselben Grund: + // die zweite Zusicherung prueft eine exakte Sekundenzahl. + Carbon::setTestNow(now()->startOfSecond()); + + try { + $pve = new FakeProxmoxClient; + app()->instance(\App\Services\Proxmox\ProxmoxClient::class, $pve); + + $shell = new \App\Services\Ssh\FakeRemoteShell; + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + + // Erste Instanz: guestExec wirft (abgeschalteter Gast, kein sauberer + // Fehlercode). + aktiveInstanz(['vmid' => 201]); + $pve->guestThrows[201] = new RuntimeException('guest agent unreachable'); + + // Zweite Instanz: zehn Fehlversuche im Protokoll — muss trotz der + // ersten weiterhin gelesen und gesperrt werden. + aktiveInstanz(['vmid' => 202]); + $pve->guestScripts['nextcloud.log'] = ['out-data' => protokollZeilen('203.0.113.7', 10), 'exitcode' => 0]; + + // Dazu eine noch gueltige, UNBETEILIGTE Sperre (andere Adresse, eigener + // Host), die wiedereingetragen werden muss — mit ihrer Restlaufzeit, + // nicht mit der Basisdauer einer frisch ausgeloesten Sperre. Nur so + // beweist die zweite Zusicherung wirklich, dass reapplyActiveBlocks() + // gelaufen ist, statt bloss zufaellig mit der neu ausgeloesten Sperre + // (die selbst schon "timeout" enthaelt) zusammenzufallen. + $andererHost = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:def']); + SecurityBlock::factory()->forHost($andererHost)->create([ + 'ip' => '198.51.100.9', + 'expires_at' => now()->addMinutes(20), + 'blocked_at' => now()->subMinutes(40), + ]); + + app(ScanForIntrusions::class)->handle(); + + // Der Lauf ist durchgelaufen: die zweite Instanz wurde gesperrt UND die + // bestehende, unbeteiligte Sperre wurde erneut eingetragen. + expect(SecurityBlock::where('ip', '203.0.113.7')->exists())->toBeTrue() + ->and($shell->ran('timeout 1200s'))->toBeTrue(); + } finally { + Carbon::setTestNow(); + } }); it('ueberspringt einen Gast, der nicht antwortet, ohne den Versatz zu verlieren', function () {