diff --git a/app/Provisioning/Jobs/ScanForIntrusions.php b/app/Provisioning/Jobs/ScanForIntrusions.php index d974bee..0645ce4 100644 --- a/app/Provisioning/Jobs/ScanForIntrusions.php +++ b/app/Provisioning/Jobs/ScanForIntrusions.php @@ -144,24 +144,39 @@ class ScanForIntrusions implements ShouldQueue ->get(); foreach ($hosts as $host) { - $result = $reader->fromHost($host); + // Derselbe Ring wie bei den Instanzen nebenan, aus demselben Grund + // (Fix-Runde 2 hat ihn dort eingezogen, diese Methode ging leer + // aus). `FailedLoginReader::fromHost()` fängt seine eigene + // SSH-Ausnahme zwar schon ab, aber alles danach nicht: + // `SecurityBlock::create()`, `Settings::set()` und die + // Empfängersuche in `BlockAddress` (Spaties + // `PermissionDoesNotExist`, wenn `hosts.manage` fehlt), dazu + // `$host->save()`. Wirft eines davon, bricht der ganze Lauf ab — + // die übrigen Hosts werden nicht mehr gelesen, und vor allem läuft + // `reapplyActiveBlocks()` nicht, jede Minute erneut. + try { + $result = $reader->fromHost($host); - if ($result === null) { - continue; - } - - $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); + if ($result === null) { + continue; } - } - $host->forceFill(['security_log_seen_at' => $result['seenAt']])->save(); + $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); + } + } + + $host->forceFill(['security_log_seen_at' => $result['seenAt']])->save(); + } catch (Throwable) { + // Absichtlich still, wie bei den Instanzen: der Zeitpunkt + // bleibt unangetastet, der nächste Lauf versucht es wieder. + } } } @@ -192,10 +207,28 @@ class ScanForIntrusions implements ShouldQueue return 'intrusion-scan:'.$subject.':'.$ip; } + /** + * Alle noch gültigen Sperren erneut eintragen — je Host EIN Aufruf, EINE + * Verbindung. + * + * Vorher lief das je Sperre einzeln, und jede Eintragung baute ihre eigene + * SSH-Sitzung auf. Bei dreissig Sperren waren das dreissig Handshakes pro + * Minute, seriell, auf dem einzigen Provisionierungs-Arbeiter — und + * dreissig Sperren gibt es genau dann, wenn gerade angegriffen wird, + * während derselbe Arbeiter die bezahlte Kundenbereitstellung fährt. + * + * Die RESTLAUFZEIT je Sperre, nicht die ursprüngliche Dauer, und auch + * nicht eine gemeinsame fürs ganze Bündel: `HostFirewall::blockMany()` + * nimmt sie einzeln entgegen. Sonst verlängerte sich jede Sperre bei jedem + * Neustart des Hosts. + */ private function reapplyActiveBlocks(HostFirewall $firewall): void { $blocks = SecurityBlock::active()->with(['host', 'instance.host'])->get(); + /** @var array}> */ + $perHost = []; + foreach ($blocks as $block) { $host = $block->host ?? $block->instance?->host; @@ -211,9 +244,22 @@ class ScanForIntrusions implements ShouldQueue // wuerde JEDE Restlaufzeit um bis zu eine Sekunde verkuerzen. $seconds = (int) round(now()->diffInSeconds($block->expires_at)); - if ($seconds > 0) { - $firewall->block($host, $block->ip, $seconds); + if ($seconds <= 0) { + continue; } + + $perHost[$host->id] ??= ['host' => $host, 'seconds' => []]; + + // Dieselbe Adresse an zwei Subjekten desselben Hosts (Instanz und + // Host, oder zwei Instanzen) ist EIN Element in der Menge. Die + // längere Restlaufzeit gewinnt — die kürzere zuletzt einzutragen + // würde die längere Sperre verkürzen. + $perHost[$host->id]['seconds'][$block->ip] = + max($perHost[$host->id]['seconds'][$block->ip] ?? 0, $seconds); + } + + foreach ($perHost as $entry) { + $firewall->blockMany($entry['host'], $entry['seconds']); } } } diff --git a/app/Services/Security/BlockAddress.php b/app/Services/Security/BlockAddress.php index 57d6021..dd7801b 100644 --- a/app/Services/Security/BlockAddress.php +++ b/app/Services/Security/BlockAddress.php @@ -7,6 +7,7 @@ use App\Models\Host; use App\Models\Instance; use App\Models\Operator; use App\Models\SecurityBlock; +use App\Support\DnsLookup; use App\Support\ProvisioningSettings; use App\Support\Settings; use Illuminate\Support\Facades\Mail; @@ -36,6 +37,9 @@ class BlockAddress private const MAX_SECONDS = 86400; + /** @var array|null einmal aufgelöst, siehe exemptRanges() */ + private ?array $exempt = null; + public function __construct(private HostFirewall $firewall) {} public function forInstance(Instance $instance, string $ip, int $attempts): ?SecurityBlock @@ -254,20 +258,83 @@ class BlockAddress * Verwaltungsnetz `10.66.0.0/24` erreicht CluPilot den Host überhaupt — * eine Sperre dort wäre das Ende der Fernwartung. * + * Einmal je Instanz dieser Klasse aufgelöst und dann gemerkt: ein Lauf + * holt sich `BlockAddress` einmal aus dem Container und fragt damit jede + * gesichtete Adresse, und der Endpunkt-Eintrag unten kann eine + * Namensauflösung kosten. + * * @return array */ private function exemptRanges(): array { - $ranges = ['10.66.0.0/24', '127.0.0.1', '::1']; + return $this->exempt ??= array_merge( + ['10.66.0.0/24', '127.0.0.1', '::1'], + $this->ownServerAddresses(), + ); + } - $endpoint = ProvisioningSettings::wgEndpoint(); + /** + * Die eigene öffentliche Adresse des CluPilot-Servers, aus dem + * WireGuard-Endpunkt. + * + * Der Endpunkt ist `host:port`, und `host` darf ein NAME sein — genau so + * steht es in `.env.example` (`vpn.clupilot.com:51820`). `IpUtils::checkIp()` + * sagt zu einem Namen immer `false`, also war die eigene Adresse in + * dokumentierter Konfiguration still NICHT ausgenommen: der Server hätte + * sich selbst aussperren können. Bei IPv6 blieben zusätzlich die Klammern + * stehen (`[2001:db8::1]`), mit demselben Ergebnis. + * + * Schlägt die Auflösung fehl, ist das kein Fehler — dann fällt der Eintrag + * weg wie bei leerem Endpunkt. Ein Name, der gerade nicht auflöst, darf + * nicht dazu führen, dass gar nichts mehr gesperrt wird. + * + * @return array + */ + private function ownServerAddresses(): array + { + $endpoint = trim(ProvisioningSettings::wgEndpoint()); // Leer ist kein Fehler (frische Installation ohne Endpoint) — dann // fällt genau dieser eine Eintrag der Liste weg. - if ($endpoint !== '') { - $ranges[] = Str::beforeLast($endpoint, ':'); + if ($endpoint === '') { + return []; } - return $ranges; + $host = $this->hostPartOf($endpoint); + + if ($host === '') { + return []; + } + + // Schon eine Adresse: nichts aufzulösen. Deckt beide Schreibweisen ab, + // die hostPartOf() liefern kann — die entklammerte IPv6 und die + // gewöhnliche IPv4. + if (filter_var($host, FILTER_VALIDATE_IP) !== false) { + return [$host]; + } + + // Ein Name steht für ALLE seine Adressen. Nur die erste zu nehmen + // hieße, den Server bei einem Wechsel im Round-Robin doch wieder + // aussperren zu können. + return app(DnsLookup::class)->addressesFor($host); + } + + /** + * Der Wirt aus `wirt:port` — mit den drei Formen, die wirklich vorkommen: + * `1.2.3.4:51820`, `[2001:db8::1]:51820` und `vpn.example.com:51820`. + * Eine nackte IPv6 ohne Klammern und ohne Port (`2001:db8::1`) bleibt + * ganz, weil `beforeLast(':')` sie sonst mitten im Wort abschnitte. + */ + private function hostPartOf(string $endpoint): string + { + if (preg_match('/^\[(.+)\](?::\d+)?$/', $endpoint, $match) === 1) { + return $match[1]; + } + + if (filter_var($endpoint, FILTER_VALIDATE_IP) !== false) { + return $endpoint; + } + + return str_contains($endpoint, ':') ? Str::beforeLast($endpoint, ':') : $endpoint; } } diff --git a/app/Services/Security/FailedLoginReader.php b/app/Services/Security/FailedLoginReader.php index 98bcdd7..690e733 100644 --- a/app/Services/Security/FailedLoginReader.php +++ b/app/Services/Security/FailedLoginReader.php @@ -52,7 +52,7 @@ class FailedLoginReader $node = (string) ($host->node ?? 'pve'); $vmid = (int) $instance->vmid; - $offset = (int) $instance->security_log_offset; + $stored = (int) $instance->security_log_offset; try { $pve = $this->pve->forHost($host); @@ -63,12 +63,29 @@ class FailedLoginReader return null; } + $bytes = (int) trim((string) ($size['out-data'] ?? '')); + + // Der ERSTE Lauf holt nur den Startwert und liest nichts. + // + // `security_log_offset` beginnt bei 0, und 0 heißt wörtlich "von + // vorn": ohne diesen Zweig tailt der allererste Lauf die KOMPLETTE + // Protokolldatei jeder Instanz durch den Gastagenten, alle + // Instanzen im selben Auftrag, in derselben Minute. Gesperrt würde + // dabei ohnehin nichts — das Zeitfenster wirft jede Zeile weg, die + // älter als zehn Minuten ist —, es wäre also eine große Übertragung + // für ein garantiert leeres Ergebnis. + // + // Bewusst getrennt vom Rotationsfall unten: dort ist die Datei + // NEUER als der Versatz und muss von vorn gelesen werden, weil das + // Neue am Anfang steht. + if ($stored === 0) { + return ['offset' => $bytes, 'addresses' => []]; + } + // 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; - } + $offset = $bytes < $stored ? 0 : $stored; $tail = $pve->guestExec($node, $vmid, NextcloudOcc::exec('tail -c +'.($offset + 1).' data/nextcloud.log')); } catch (Throwable) { @@ -164,6 +181,42 @@ class FailedLoginReader continue; } + // RIEGEL GEGEN BEFEHLSEINSCHLEUSUNG. `remoteAddr` ist eine + // beliebige Zeichenkette aus einer Protokolldatei IM GAST, und der + // Weg von hier führt über HostFirewall::block() in ein + // `nft`-Kommando, das PhpseclibRemoteShell als ROOT auf dem + // Proxmox-Host ausführt. Ein Semikolon darin wäre ein zweiter + // Befehl. Der SSH-Zweig unten begrenzt den Zeichenvorrat schon über + // sein `preg_match`; dieser Zweig hatte kein Gegenstück. + if (filter_var($ip, FILTER_VALIDATE_IP) === false) { + continue; + } + + // VORSICHTSMASSNAHME, nicht Sicherheitsgrenze — und nur hier, nicht + // im Host-Zweig. + // + // Traefik sitzt auf dem Host und leitet auf `guest_ip:80` weiter, + // aber `TRUSTED_PROXIES` wird in die .env des Gastes nie + // geschrieben. Was Nextcloud als `remoteAddr` protokolliert, ist + // damit entweder die Brückenadresse des Hosts (dann zählen ALLE + // Kunden unter einer Adresse und ein echter Angreifer nie) oder ein + // `X-Forwarded-For`, das der Client SELBST wählt (dann macht sich + // ein Angreifer mit `10.66.0.1` dauerhaft ausnahmefähig und sperrt + // mit einer fremden Adresse gezielt Unbeteiligte aus). Welcher der + // beiden Fälle zutrifft, ist ohne echte Instanz nicht zu + // entscheiden. + // + // Beide Schadensfälle sind privat oder reserviert; ein echter + // Angreifer sitzt auf einer öffentlichen Adresse. Bis die + // Vertrauenskette zum Gast sauber gesetzt und BEWIESEN ist + // (TRUSTED_PROXIES in die Gast-.env, dann `grep remoteAddr + // data/nextcloud.log` an einer echten Instanz), sperrt der + // Instanz-Zweig deshalb nur öffentliche Adressen. Diese Zeile + // gehört dann weg — nicht früher. + if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false) { + continue; + } + $entries[] = ['ip' => $ip, 'time' => Carbon::parse($decoded['time'])]; } @@ -214,6 +267,19 @@ class FailedLoginReader continue; } + // Derselbe Riegel wie im Instanz-Zweig. Der Zeichenvorrat oben + // schließt ein Semikolon zwar schon aus, aber er lässt "1.2.3" und + // "...." durch — und der Riegel darf nicht davon abhängen, dass + // jemand beim nächsten Ändern des Musters daran denkt. + // + // OHNE die Einschränkung auf öffentliche Adressen: hier stammt die + // Adresse aus dem SSH-Journal des Hosts und ist echt, und eine + // Sperre aus dem Verwaltungsnetz fängt die Ausnahmeliste in + // BlockAddress ohnehin ab. + if (filter_var($match[1], FILTER_VALIDATE_IP) === false) { + continue; + } + $counts[$match[1]] = ($counts[$match[1]] ?? 0) + 1; } diff --git a/app/Services/Security/HostFirewall.php b/app/Services/Security/HostFirewall.php index 629f98d..39e3ef6 100644 --- a/app/Services/Security/HostFirewall.php +++ b/app/Services/Security/HostFirewall.php @@ -5,6 +5,7 @@ namespace App\Services\Security; use App\Models\Host; use App\Services\Secrets\SecretVault; use App\Services\Ssh\RemoteShell; +use RuntimeException; use Throwable; /** @@ -15,8 +16,8 @@ use Throwable; * wir hier mit, und der Kernel selbst nimmt die Adresse wieder heraus — kein * eigener Aufräum-Lauf nötig für den Normalfall. * - * Wer eine Sperre AUSLÖST, gehört nicht hierher (eigene, spätere Aufgabe) — - * das hier ist nur der Griff: eintragen, herausnehmen. + * Wer eine Sperre AUSLÖST, gehört nicht hierher — das hier ist nur der Griff: + * eintragen, herausnehmen. * * Baut die Verbindung direkt über die WireGuard-Adresse des Hosts auf, nicht * über HostStep::keyLogin()s Rückfallpfad auf die öffentliche Adresse: diese @@ -24,11 +25,30 @@ use Throwable; * bewiesen ist, nie während der Inbetriebnahme selbst. * * block() und release() werfen absichtlich NICHT, wenn der Host nicht - * erreichbar ist — sie geben `false` zurück. Eine spätere Aufgabe legt den - * Sperrdatensatz in der Datenbank trotzdem an und trägt ihn beim nächsten Lauf - * erneut ein (Wiedereintragen). Eine Sperre, die nur in der Datenbank steht, - * ist sichtbar und wird nachgeholt; eine Ausnahme hier würde stattdessen den - * ganzen Zeitplan-Auftrag mitreißen, der diese Methode aufruft. + * erreichbar ist — sie geben `false` zurück. `BlockAddress` legt den + * Sperrdatensatz trotzdem an und `ScanForIntrusions` trägt ihn beim nächsten + * Lauf erneut ein (Wiedereintragen). Eine Sperre, die nur in der Datenbank + * steht, ist sichtbar und wird nachgeholt; eine Ausnahme hier würde + * stattdessen den ganzen Zeitplan-Auftrag mitreißen, der diese Methode + * aufruft. + * + * `false` ist deshalb kein Grund zum Werfen, aber sehr wohl einer zum + * MELDEN: auf einem Host, der sein Regelwerk noch ohne die Mengen trägt, + * scheitert `nft add element` bei JEDEM Versuch, und beide Aufrufer verwerfen + * den Rückgabewert. Die Sperre stünde dann in Datenbank, Portal, Konsole und + * in der Mail an den Kunden als aktiv und in der Firewall nie. Darum meldet + * `apply()` jeden Misserfolg selbst — an der einen Stelle, an der er anfällt, + * statt an jedem Aufrufer, der ihn vergessen kann. Nachziehen lässt sich so + * ein Host mit `php artisan clupilot:refresh-host-firewall`. + * + * Jede Adresse geht vor dem `sprintf` durch `filter_var(…, FILTER_VALIDATE_IP)`. + * Das ist kein doppelter Boden für einen ohnehin sauberen Aufrufer, sondern + * die Bedingung dafür, dass dieser Dienst überhaupt eine Wurzel-Shell füttern + * darf: `PhpseclibRemoteShell::run()` reicht die zusammengebaute Zeile an + * `/bin/sh` als root auf dem Proxmox-Host, und ein Semikolon in der Adresse + * wäre dort ein zweiter Befehl. Der Leser prüft dasselbe (FailedLoginReader); + * ein Dienst, der eine Wurzel-Shell füttert, darf sich nicht darauf verlassen, + * dass sein Aufrufer sauber war. */ class HostFirewall { @@ -36,21 +56,91 @@ class HostFirewall public function block(Host $host, string $ip, int $seconds): bool { - return $this->apply($host, sprintf( - 'nft add element inet clupilot_filter %s { %s timeout %ds }', - $this->setFor($ip), - $ip, - $seconds, - )); + return $this->blockMany($host, [$ip => $seconds]); + } + + /** + * Mehrere Adressen desselben Hosts in EINER SSH-Sitzung. + * + * `ScanForIntrusions::reapplyActiveBlocks()` trug bisher je aktiver Sperre + * einzeln ein, und jede Eintragung baute ihre eigene Verbindung auf: bei + * dreissig Sperren dreissig Handshakes pro Minute, seriell, auf dem + * einzigen Provisionierungs-Arbeiter — also genau WÄHREND eines Angriffs + * auf demselben Arbeiter, der die bezahlte Kundenbereitstellung fährt. + * + * `nft` nimmt mehrere Elemente in einem Aufruf: `{ a timeout 60s, b + * timeout 90s }`. Je Adressfamilie einer, weil v4 und v6 in getrennten + * Mengen leben — also höchstens zwei Befehle, eine Verbindung. + * + * Jede Adresse behält ihre EIGENE Restlaufzeit. Eine gemeinsame Dauer für + * das ganze Bündel wäre bequemer und würde genau die Zusicherung + * verwässern, für die es einen eigenen Test gibt. + * + * @param array $seconds Adresse => Restlaufzeit in Sekunden + */ + public function blockMany(Host $host, array $seconds): bool + { + /** @var array> */ + $elements = []; + + foreach ($seconds as $ip => $duration) { + $ip = (string) $ip; + + if (! $this->isWellFormed($host, $ip)) { + return false; + } + + $elements[$this->setFor($ip)][] = sprintf('%s timeout %ds', $ip, (int) $duration); + } + + if ($elements === []) { + return true; + } + + $commands = []; + + foreach ($elements as $set => $entries) { + $commands[] = sprintf( + 'nft add element inet clupilot_filter %s { %s }', + $set, + implode(', ', $entries), + ); + } + + return $this->apply($host, $commands, implode(', ', array_keys($seconds))); } public function release(Host $host, string $ip): bool { - return $this->apply($host, sprintf( + if (! $this->isWellFormed($host, $ip)) { + return false; + } + + return $this->apply($host, [sprintf( 'nft delete element inet clupilot_filter %s { %s }', $this->setFor($ip), $ip, + )], $ip); + } + + /** + * Wohlgeformt genug, um in eine Wurzel-Shell zu wandern? + * + * Meldet und verweigert statt still weiterzumachen: eine Adresse, die hier + * ankommt und keine ist, ist entweder ein Versuch oder ein Fehler weiter + * oben — beides will man sehen. + */ + private function isWellFormed(Host $host, string $ip): bool + { + if (filter_var($ip, FILTER_VALIDATE_IP) !== false) { + return true; + } + + report(new RuntimeException( + "HostFirewall: abgewiesene Adresse [{$ip}] fuer Host [{$host->name}] — keine gueltige IP, nichts ausgefuehrt." )); + + return false; } /** Die Adressfamilie entscheidet über die Menge — v4 und v6 leben getrennt. */ @@ -59,7 +149,10 @@ class HostFirewall return str_contains($ip, ':') ? 'clupilot_blocked6' : 'clupilot_blocked'; } - private function apply(Host $host, string $command): bool + /** + * @param array $commands alle in EINER Sitzung + */ + private function apply(Host $host, array $commands, string $subject): bool { try { $this->shell->connectWithKey( @@ -69,9 +162,25 @@ class HostFirewall $host->ssh_host_key, // gepinnt bei EstablishSshTrust ); - return $this->shell->run($command)->ok(); + $ok = true; + + foreach ($commands as $command) { + // Nicht abbrechen beim ersten Fehlschlag: die Verbindung steht + // ohnehin schon, und v6 soll nicht deshalb ausfallen, weil v4 + // gerade nicht ging. + $ok = $this->shell->run($command)->ok() && $ok; + } } catch (Throwable) { - return false; + $ok = false; } + + if (! $ok) { + report(new RuntimeException( + "HostFirewall: [{$subject}] auf Host [{$host->name}] nicht in die Firewall geschrieben " + .'— der Sperrdatensatz steht, die Regel nicht.' + )); + } + + return $ok; } } diff --git a/app/Support/DnsLookup.php b/app/Support/DnsLookup.php new file mode 100644 index 0000000..32e676c --- /dev/null +++ b/app/Support/DnsLookup.php @@ -0,0 +1,42 @@ +instance(DnsLookup::class, …)`, wie FakeRemoteShell und + * FakeProxmoxClient es für SSH und Proxmox schon vormachen). Ein Test, der für + * seine Zusicherung echtes DNS bräuchte, prüft nicht die Regel, sondern das + * Netz des Rechners, auf dem er gerade läuft. + * + * Ein Name, der sich nicht auflösen lässt, ergibt ein leeres Feld — kein + * Fehler. Der einzige Aufrufer (BlockAddress::exemptRanges()) behandelt das + * wie einen leeren Eintrag. + */ +class DnsLookup +{ + /** @return array */ + public function addressesFor(string $name): array + { + $v4 = gethostbynamel($name); + + // Getrennt geholt, weil gethostbynamel() nur A-Records kennt. Ein + // Endpunkt, der ausschließlich per AAAA existiert, wäre sonst still + // nicht ausgenommen — genau der Fehler, den dieser Umbau schließt, nur + // eine Adressfamilie weiter. + $records = @dns_get_record($name, DNS_AAAA) ?: []; + + $v6 = array_values(array_filter(array_map( + fn (array $record): string => (string) ($record['ipv6'] ?? ''), + $records, + ))); + + return array_values(array_unique(array_merge( + is_array($v4) ? $v4 : [], + $v6, + ))); + } +} diff --git a/tests/Feature/Security/BlockAddressTest.php b/tests/Feature/Security/BlockAddressTest.php index 0d97e8e..029408c 100644 --- a/tests/Feature/Security/BlockAddressTest.php +++ b/tests/Feature/Security/BlockAddressTest.php @@ -4,6 +4,7 @@ use App\Models\Host; use App\Models\Instance; use App\Models\SecurityBlock; use App\Services\Security\BlockAddress; +use App\Support\DnsLookup; use Illuminate\Support\Carbon; it('sperrt beim ersten Mal fuer eine Stunde', function () { @@ -83,3 +84,61 @@ it('sperrt niemals die eigene oeffentliche Adresse des CluPilot-Servers', functi // gesperrt wird: der Nachbar in derselben Zeile ist NICHT ausgenommen. expect(app(BlockAddress::class)->forHost($host, '203.0.113.11', 999))->not->toBeNull(); }); + +it('nimmt auch einen NAMEN als Endpunkt aus, mit allen seinen Adressen', function () { + // `.env.example` dokumentiert genau diese Form (vpn.clupilot.com:51820). + // `Str::beforeLast(':')` lieferte damit einen Hostnamen, und + // `IpUtils::checkIp()` sagt zu einem Hostnamen immer false — die eigene + // Adresse war in dokumentierter Konfiguration still NICHT ausgenommen. + config()->set('provisioning.wireguard.endpoint', 'vpn.example.com:51820'); + + // Nicht das echte DNS: ein Test, der dafuer aufloesen muesste, prueft das + // Netz des Rechners, auf dem er gerade laeuft, nicht die Regel. + app()->instance(DnsLookup::class, new class extends DnsLookup + { + public function addressesFor(string $name): array + { + return $name === 'vpn.example.com' ? ['203.0.113.10', '2001:db8:ff::5'] : []; + } + }); + + $host = Host::factory()->create(); + + // ALLE Ergebnisse, nicht nur das erste: sonst sperrte sich der Server bei + // einem Wechsel im Round-Robin doch wieder selbst aus. + expect(app(BlockAddress::class)->forHost($host, '203.0.113.10', 999))->toBeNull() + ->and(app(BlockAddress::class)->forHost($host, '2001:db8:ff::5', 999))->toBeNull() + ->and(SecurityBlock::count())->toBe(0) + // Gegenprobe: der Nachbar ist es nicht. + ->and(app(BlockAddress::class)->forHost($host, '203.0.113.11', 999))->not->toBeNull(); +}); + +it('streift bei einem IPv6-Endpunkt die Klammern ab', function () { + // `[2001:db8::1]:51820` — beforeLast(':') liess die Klammern stehen, und + // `[2001:db8::1]` ist fuer IpUtils keine Adresse. + config()->set('provisioning.wireguard.endpoint', '[2001:db8::1]:51820'); + + $host = Host::factory()->create(); + + expect(app(BlockAddress::class)->forHost($host, '2001:db8::1', 999))->toBeNull() + ->and(app(BlockAddress::class)->forHost($host, '2001:db8::2', 999))->not->toBeNull(); +}); + +it('sperrt weiter, wenn sich der Endpunkt-Name gerade nicht aufloesen laesst', function () { + // Schlaegt die Aufloesung fehl, faellt der Eintrag weg wie bei leerem + // Endpunkt. Ein Name, der gerade nicht aufloest, darf nicht dazu fuehren, + // dass gar nichts mehr gesperrt wird. + config()->set('provisioning.wireguard.endpoint', 'vpn.example.invalid:51820'); + + app()->instance(DnsLookup::class, new class extends DnsLookup + { + public function addressesFor(string $name): array + { + return []; + } + }); + + $host = Host::factory()->create(); + + expect(app(BlockAddress::class)->forHost($host, '203.0.113.7', 999))->not->toBeNull(); +}); diff --git a/tests/Feature/Security/HostFirewallTest.php b/tests/Feature/Security/HostFirewallTest.php index 810515b..7ece6a4 100644 --- a/tests/Feature/Security/HostFirewallTest.php +++ b/tests/Feature/Security/HostFirewallTest.php @@ -4,6 +4,7 @@ use App\Models\Host; use App\Provisioning\Steps\Host\SecureHostFirewall; use App\Services\Security\HostFirewall; use App\Services\Ssh\FakeRemoteShell; +use Illuminate\Support\Facades\Exceptions; it('stellt die Sperrregel UNTER die Regel fuer bestehende Verbindungen', function () { // Das ist die eigentliche Zusage des ganzen Systems: „wer drin ist, bleibt @@ -83,3 +84,123 @@ it('gibt false zurueck statt zu werfen, wenn der Host beim Entsperren nicht erre expect($result)->toBeFalse(); }); + +// ---- Eine Sperre, die nur in der Datenbank steht, darf nicht schweigen ---- + +it('meldet, wenn die Regel nicht auf den Host kam', function () { + // Auf einem Host, der noch das alte Regelwerk ohne die Mengen traegt, + // scheitert `nft add element` bei JEDEM Versuch — und beide Aufrufer + // verwerfen den Rueckgabewert. Die Sperre stuende dann in Datenbank, + // Portal, Konsole und in der Mail an den Kunden als aktiv und in der + // Firewall nie. + Exceptions::fake(); + + $shell = new FakeRemoteShell; + $shell->defaultResult(\App\Services\Ssh\CommandResult::failure(1, 'Error: No such file or directory')); + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + $host = Host::factory()->active()->create(['name' => 'pve-fsn-07', 'ssh_host_key' => 'SHA256:abc']); + + expect(app(HostFirewall::class)->block($host, '203.0.113.7', 3600))->toBeFalse(); + + // Konkrete Klasse, nicht Throwable: der Fake vergleicht den Typ des + // Closure-Parameters EXAKT mit get_class() — mit Throwable trifft er nie + // (siehe pest-toThrow-Falle, dasselbe Muster). + Exceptions::assertReported(fn (RuntimeException $e) => str_contains($e->getMessage(), 'pve-fsn-07') + && str_contains($e->getMessage(), '203.0.113.7')); +}); + +it('meldet nichts, solange die Regel ankommt', function () { + // Die Gegenprobe: eine Meldung, die immer kommt, ist keine. + Exceptions::fake(); + + app()->instance(\App\Services\Ssh\RemoteShell::class, new FakeRemoteShell); + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + + app(HostFirewall::class)->block($host, '203.0.113.7', 3600); + + Exceptions::assertNothingReported(); +}); + +// ---- Der Riegel: was hier hineingeht, wird als root auf dem Host ausgefuehrt ---- + +it('fuehrt bei einer Adresse mit Semikolon gar nichts aus und gibt false zurueck', function () { + // `PhpseclibRemoteShell::run()` reicht die zusammengebaute Zeile an /bin/sh + // als ROOT auf dem Proxmox-Host. Ein Semikolon in der Adresse waere dort + // ein zweiter Befehl — der Dienst darf sich nicht darauf verlassen, dass + // sein Aufrufer sauber war. + $shell = new FakeRemoteShell; + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + + $result = app(HostFirewall::class)->block($host, '1.2.3.4; rm -rf /', 3600); + + expect($result)->toBeFalse() + ->and($shell->recorded())->toBe([]) + // Nicht einmal verbunden: was nicht ausgefuehrt werden darf, braucht + // auch keine Wurzel-Sitzung. + ->and($shell->connections)->toBe([]); +}); + +it('weist auch beim Entsperren eine Adresse ab, die keine ist', function () { + $shell = new FakeRemoteShell; + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + + expect(app(HostFirewall::class)->release($host, '$(id)'))->toBeFalse() + ->and($shell->recorded())->toBe([]); +}); + +// ---- Buendeln: eine Verbindung je Host, nicht eine je Sperre ---- + +it('traegt drei Adressen desselben Hosts in EINER Verbindung ein', function () { + // Bei dreissig Sperren waren das dreissig Handshakes pro Minute, seriell, + // auf demselben Arbeiter, der die bezahlte Kundenbereitstellung faehrt — + // und dreissig Sperren gibt es genau waehrend eines Angriffs. + $shell = new FakeRemoteShell; + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + + $ok = app(HostFirewall::class)->blockMany($host, [ + '203.0.113.7' => 1200, + '203.0.113.8' => 600, + '198.51.100.9' => 3600, + ]); + + expect($ok)->toBeTrue() + ->and($shell->connections)->toHaveCount(1) + ->and($shell->recorded())->toHaveCount(1); + + // Und jede Adresse behaelt ihre EIGENE Restlaufzeit — eine gemeinsame + // Dauer fuers Buendel waere bequemer und wuerde genau die Zusicherung + // verwaessern, fuer die es einen eigenen Test gibt. + expect($shell->ran('203.0.113.7 timeout 1200s'))->toBeTrue() + ->and($shell->ran('203.0.113.8 timeout 600s'))->toBeTrue() + ->and($shell->ran('198.51.100.9 timeout 3600s'))->toBeTrue(); +}); + +it('trennt v4 und v6 in zwei Befehle, aber nicht in zwei Verbindungen', function () { + $shell = new FakeRemoteShell; + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + + app(HostFirewall::class)->blockMany($host, ['203.0.113.7' => 60, '2001:db8::1' => 60]); + + expect($shell->connections)->toHaveCount(1) + ->and($shell->recorded())->toHaveCount(2) + ->and($shell->ran('clupilot_blocked { 203.0.113.7 timeout 60s }'))->toBeTrue() + ->and($shell->ran('clupilot_blocked6 { 2001:db8::1 timeout 60s }'))->toBeTrue(); +}); + +it('fuehrt aus einem Buendel gar nichts aus, wenn eine einzige Adresse keine ist', function () { + $shell = new FakeRemoteShell; + app()->instance(\App\Services\Ssh\RemoteShell::class, $shell); + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + + $ok = app(HostFirewall::class)->blockMany($host, [ + '203.0.113.7' => 60, + '1.2.3.4; nft flush ruleset' => 60, + ]); + + expect($ok)->toBeFalse() + ->and($shell->recorded())->toBe([]); +}); diff --git a/tests/Feature/Security/ScanForIntrusionsTest.php b/tests/Feature/Security/ScanForIntrusionsTest.php index 60d628c..81dcdcc 100644 --- a/tests/Feature/Security/ScanForIntrusionsTest.php +++ b/tests/Feature/Security/ScanForIntrusionsTest.php @@ -45,6 +45,12 @@ function aktiveInstanz(array $attributes = []): Instance 'status' => 'active', 'vmid' => 101, 'host_id' => Host::factory()->active()->create()->id, + // Versatz 1 statt 0, und das ist kein Detail: 0 heisst seit der + // Fix-Welle woertlich „noch nie gelesen" und laesst den Lauf NUR den + // Startwert holen, ohne das Protokoll zu uebertragen (siehe den + // eigenen Testfall dafuer weiter unten). Eine Instanz, die schon + // einmal gelesen wurde, ist der Normalfall dieser Datei. + 'security_log_offset' => 1, ], $attributes)); } @@ -246,6 +252,137 @@ it('laesst einen werfenden Gast den Lauf nicht abbrechen', function () { } }); +it('liest beim ERSTEN Lauf gar nichts, sondern merkt sich nur die Groesse', function () { + // Versatz 0 heisst woertlich „von vorn": ohne diesen Zweig tailt der + // allererste Lauf die KOMPLETTE Protokolldatei jeder Instanz durch den + // Gastagenten, alle Instanzen im selben Auftrag, in derselben Minute. + // Gesperrt wuerde dabei ohnehin nichts — das Zehn-Minuten-Fenster wirft + // jede aeltere Zeile weg —, es waere also eine grosse Uebertragung fuer ein + // garantiert leeres Ergebnis. + $pve = new FakeProxmoxClient; + $pve->guestScripts['stat -c %s'] = ['out-data' => "8123\n", 'exitcode' => 0]; + $pve->guestScripts['nextcloud.log'] = ['out-data' => protokollZeilen('203.0.113.7', 50), 'exitcode' => 0]; + app()->instance(\App\Services\Proxmox\ProxmoxClient::class, $pve); + + $instance = aktiveInstanz(['security_log_offset' => 0]); + app(ScanForIntrusions::class)->handle(); + + expect($instance->fresh()->security_log_offset)->toBe(8123) + ->and(SecurityBlock::count())->toBe(0) + // Und die Uebertragung selbst hat nicht stattgefunden — sonst waere + // dieser Test auch dann gruen, wenn nur das Zaehlen unterbliebe. + ->and($pve->guestRan('tail -c'))->toBeFalse(); +}); + +it('faengt beim ZWEITEN Lauf dort an, wo der erste aufgehoert hat', function () { + // Die Gegenprobe: der Startwert darf die Instanz nicht dauerhaft blind + // machen. Nach dem ersten Lauf wird wieder gelesen und gesperrt. + $pve = new FakeProxmoxClient; + $pve->guestScripts['stat -c %s'] = ['out-data' => "8123\n", 'exitcode' => 0]; + app()->instance(\App\Services\Proxmox\ProxmoxClient::class, $pve); + + aktiveInstanz(['security_log_offset' => 0]); + app(ScanForIntrusions::class)->handle(); + + $neu = protokollZeilen('203.0.113.7', 10); + $pve->guestScripts['stat -c %s'] = ['out-data' => (8123 + strlen($neu))."\n", 'exitcode' => 0]; + $pve->guestScripts['nextcloud.log'] = ['out-data' => $neu, 'exitcode' => 0]; + + app(ScanForIntrusions::class)->handle(); + + expect(SecurityBlock::where('ip', '203.0.113.7')->exists())->toBeTrue(); +}); + +// ---- Was aus dem Gast-Protokoll kommt, ist keine Adresse, bis es geprueft ist ---- + +it('sperrt nicht auf eine Adresse, in der ein Semikolon steht, und fuehrt nichts aus', function () { + // `remoteAddr` ist eine beliebige Zeichenkette aus einer Datei IM GAST. Der + // Weg von hier fuehrt in ein `nft`-Kommando, das als root auf dem Host + // laeuft — ein Semikolon darin waere ein zweiter Befehl. + $boese = '203.0.113.7; nft flush ruleset'; + + $shell = new FakeRemoteShell; + app()->instance(RemoteShell::class, $shell); + + $pve = new FakeProxmoxClient; + $pve->guestScripts['nextcloud.log'] = ['out-data' => protokollZeilen($boese, 20), 'exitcode' => 0]; + app()->instance(\App\Services\Proxmox\ProxmoxClient::class, $pve); + + aktiveInstanz(); + app(ScanForIntrusions::class)->handle(); + + expect(SecurityBlock::count())->toBe(0) + ->and($shell->ran('nft'))->toBeFalse() + ->and($shell->ran('flush ruleset'))->toBeFalse(); +}); + +it('sperrt im Instanz-Zweig keine private Adresse, eine oeffentliche schon', function () { + // Vorsichtsmassnahme, nicht Sicherheitsgrenze: TRUSTED_PROXIES steht in der + // .env des Gastes nicht, waehrend Traefik auf dem Host sitzt. Was Nextcloud + // als remoteAddr protokolliert, ist damit entweder die Brueckenadresse des + // Hosts oder ein X-Forwarded-For, das der Client selbst waehlt — beides ist + // privat oder reserviert, ein echter Angreifer ist es nicht. + $pve = new FakeProxmoxClient; + $pve->guestScripts['nextcloud.log'] = [ + 'out-data' => protokollZeilen('172.17.0.1', 20)."\n".protokollZeilen('203.0.113.7', 10), + 'exitcode' => 0, + ]; + app()->instance(\App\Services\Proxmox\ProxmoxClient::class, $pve); + + aktiveInstanz(); + app(ScanForIntrusions::class)->handle(); + + expect(SecurityBlock::where('ip', '172.17.0.1')->exists())->toBeFalse() + ->and(SecurityBlock::where('ip', '203.0.113.7')->exists())->toBeTrue(); +}); + +it('faengt einen werfenden Host ab, statt den ganzen Lauf abzubrechen', function () { + // Derselbe Ring wie bei den Instanzen. Wirft hier etwas, liefe + // reapplyActiveBlocks() nicht — jede Minute erneut. + Carbon::setTestNow(now()->startOfSecond()); + + try { + $shell = new FakeRemoteShell; + app()->instance(RemoteShell::class, $shell); + + // Ein Host, dessen Lesen wirft — stellvertretend fuer alles, was + // NACH FailedLoginReader::fromHost()s eigenem catch noch schiefgehen + // kann: SecurityBlock::create(), Settings::set(), Spaties + // PermissionDoesNotExist, $host->save(). + Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + + app()->bind(\App\Services\Security\FailedLoginReader::class, fn () => new class extends \App\Services\Security\FailedLoginReader + { + public function __construct() {} + + public function fromInstance(Instance $instance): ?array + { + return null; + } + + public function fromHost(Host $host): ?array + { + throw new RuntimeException('Host-Lesen kaputt'); + } + }); + + // Eine noch gueltige, unbeteiligte Sperre, die trotzdem wieder + // eingetragen werden muss. + $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(); + + expect($shell->ran('timeout 1200s'))->toBeTrue(); + } finally { + Carbon::setTestNow(); + } +}); + it('ueberspringt einen Gast, der nicht antwortet, ohne den Versatz zu verlieren', function () { $pve = new FakeProxmoxClient; $pve->guestScripts['nextcloud.log'] = ['exitcode' => 1]; diff --git a/tests/Feature/Security/SecurityConsoleTest.php b/tests/Feature/Security/SecurityConsoleTest.php index 127a7eb..e7a08fa 100644 --- a/tests/Feature/Security/SecurityConsoleTest.php +++ b/tests/Feature/Security/SecurityConsoleTest.php @@ -19,6 +19,16 @@ use Spatie\Permission\Models\Role; * sich EIN Modal (App\Livewire\Admin\ConfirmReleaseBlock), das nach dem * Subjekt der Sperre autorisiert. */ + +// Aufheben ruft HostFirewall::release(), und das verbindet sich per SSH mit +// dem Host. Ohne gebundenen Fake lief der erste Test dieser Datei gegen eine +// echte, nicht erreichbare Adresse und haengte gut zwei Minuten am +// TCP-Verbindungsaufbau — allein, mehr als die gesamte uebrige Suite dieses +// Ordners zusammen. +beforeEach(fn () => app()->instance( + \App\Services\Ssh\RemoteShell::class, + new \App\Services\Ssh\FakeRemoteShell +)); it('zeigt einem Betreiber mit hosts.manage die Sperre eines Hosts und laesst ihn sie aufheben', function () { $host = Host::factory()->active()->create(); $block = SecurityBlock::factory()->forHost($host)->create(['ip' => '203.0.113.20']);