From 5a9b6d317fcd7ec5695e9d92ad74cb9e73683cd8 Mon Sep 17 00:00:00 2001 From: boban Date: Fri, 12 Jun 2026 21:55:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(ssh):=20address=20adversarial=20review=20?= =?UTF-8?q?=E2=80=94=20host-key=20pinning,=20escaping,=20parsers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve all 12 confirmed findings from the SSH-integration review. Security: - TOFU host-key pinning (new VerifiesHostKey trait + servers.ssh_host_key column): SshClient/Sftp record the server host key on first connect and refuse the connection on any later mismatch — phpseclib does not pin host keys itself. - PollMetrics keeps one long-lived SSH connection per server and reuses it across ticks (a single login, not one per interval) — avoids fail2ban/auth-log churn; dead connections are dropped and re-established next tick. - @js() escaping for every server-controlled value interpolated into wire:click (file names, paths, service names, SSH-key comments/fingerprints) — prevents JS-string breakage / injection from untrusted remote data. Correctness: - parseKeys regex makes the key comment optional (keys without a comment were silently dropped). - parseVolumes pops the three trailing single-token df fields and rejoins the rest, so mount points containing spaces parse correctly. - Sftp gains a disconnect() method to match SshClient (explicit cleanup). Rules: - Services "Start" -> "Starten" (R9, German). - Files breadcrumb buttons get min-w-11 + padding (R7, >=44px touch target). Verified live: host key stored + mismatch refused + correct key reconnects; poller reuse polls ok; all pages still render real data. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/Console/Commands/PollMetrics.php | 32 +++++++++++-- app/Services/FleetService.php | 47 ++++++++++++++----- app/Support/Ssh/Sftp.php | 11 +++++ app/Support/Ssh/SshClient.php | 4 ++ app/Support/Ssh/VerifiesHostKey.php | 38 +++++++++++++++ ..._12_120000_add_ssh_host_key_to_servers.php | 23 +++++++++ .../views/livewire/files/index.blade.php | 8 ++-- .../views/livewire/servers/show.blade.php | 2 +- .../views/livewire/services/index.blade.php | 8 ++-- 9 files changed, 149 insertions(+), 24 deletions(-) create mode 100644 app/Support/Ssh/VerifiesHostKey.php create mode 100644 database/migrations/2026_06_12_120000_add_ssh_host_key_to_servers.php diff --git a/app/Console/Commands/PollMetrics.php b/app/Console/Commands/PollMetrics.php index 0c6f9b4..58c58dd 100644 --- a/app/Console/Commands/PollMetrics.php +++ b/app/Console/Commands/PollMetrics.php @@ -5,6 +5,8 @@ namespace App\Console\Commands; use App\Events\MetricsTicked; use App\Models\Server; use App\Services\FleetService; +use App\Support\Ssh\CredentialVault; +use App\Support\Ssh\SshClient; use Illuminate\Console\Command; use Throwable; @@ -12,7 +14,11 @@ use Throwable; * Replaces the dev mock emitter: polls REAL CPU/MEM/disk over SSH for every * server that has a stored credential, persists the values onto the Server row, * and broadcasts MetricsTicked so the dashboard chart moves with real data. - * Unreachable servers are flagged offline; the loop never dies on one failure. + * + * One long-lived SSH connection is kept per server and reused across ticks (a + * single login, not one per interval — avoids fail2ban/auth-log churn). A dead + * connection is dropped and re-established on the next tick; one server failing + * never kills the loop. */ class PollMetrics extends Command { @@ -22,10 +28,13 @@ class PollMetrics extends Command protected $description = 'Poll real metrics over SSH for credentialed servers, persist + broadcast'; - public function handle(FleetService $fleet): int + public function handle(FleetService $fleet, CredentialVault $vault): int { $interval = max(2, (int) $this->option('interval')); + /** @var array $clients */ + $clients = []; + do { $servers = Server::has('credential')->get(); @@ -35,20 +44,35 @@ class PollMetrics extends Command foreach ($servers as $server) { try { - $m = $fleet->refresh($server); + $clients[$server->id] ??= (new SshClient($vault, timeout: 12))->connect($server); + + $m = $fleet->readMetrics($clients[$server->id]); + $fleet->applyMetrics($server, $m); broadcast(new MetricsTicked($m['cpu'], $m['mem'], $server->name)); + $this->line("ok {$server->name} cpu={$m['cpu']} mem={$m['mem']} disk={$m['disk']} load={$m['load']}"); } catch (Throwable $e) { + // drop the (possibly dead) connection so the next tick reconnects + if (isset($clients[$server->id])) { + $clients[$server->id]->disconnect(); + unset($clients[$server->id]); + } $server->forceFill(['status' => 'offline'])->save(); $this->warn("offline {$server->name}: ".$e->getMessage()); } } if ($this->option('once')) { - return self::SUCCESS; + break; } sleep($interval); } while (true); + + foreach ($clients as $client) { + $client->disconnect(); + } + + return self::SUCCESS; } } diff --git a/app/Services/FleetService.php b/app/Services/FleetService.php index 954b658..1bbea10 100644 --- a/app/Services/FleetService.php +++ b/app/Services/FleetService.php @@ -53,6 +53,22 @@ class FleetService public function metrics(Server $server): array { $ssh = $this->client($server); + + try { + return $this->readMetrics($ssh); + } finally { + $ssh->disconnect(); + } + } + + /** + * Run the metrics command on an already-connected client. The poller reuses + * one long-lived connection per server (a single login, not one per tick). + * + * @return array{cpu:int,mem:int,disk:int,load:float} + */ + public function readMetrics(SshClient $ssh): array + { $cmd = 'export LC_ALL=C; ' .'echo '.self::MARK.'mem===; grep -E "MemTotal|MemAvailable" /proc/meminfo; ' .'echo '.self::MARK.'stat1===; head -1 /proc/stat; sleep 1; ' @@ -60,11 +76,7 @@ class FleetService .'echo '.self::MARK.'load===; cat /proc/loadavg; ' .'echo '.self::MARK.'disk===; df -B1 --output=pcent / | tail -1'; - try { - $s = $this->sections($ssh->exec($cmd)); - } finally { - $ssh->disconnect(); - } + $s = $this->sections($ssh->exec($cmd)); return [ 'cpu' => $this->cpuPercent($s['stat1'] ?? '', $s['stat2'] ?? ''), @@ -78,7 +90,14 @@ class FleetService public function refresh(Server $server): array { $m = $this->metrics($server); + $this->applyMetrics($server, $m); + return $m; + } + + /** Persist a metrics reading onto the Server row. */ + public function applyMetrics(Server $server, array $m): void + { $server->forceFill([ 'cpu' => $m['cpu'], 'mem' => $m['mem'], @@ -86,8 +105,6 @@ class FleetService 'status' => $this->statusFor($m), 'last_seen_at' => now(), ])->save(); - - return $m; } private function statusFor(array $m): string @@ -299,12 +316,20 @@ class FleetService private function parseVolumes(string $body): array { $out = []; - foreach ($this->lines($body) as $i => $line) { + foreach ($this->lines($body) as $line) { + // df --output=target,fstype,size,pcent; the mount (target) can contain + // spaces, so pop the three trailing single-token fields and rejoin the rest. $p = preg_split('/\s+/', trim($line)); - if ($i === 0 || count($p) < 4 || ! str_starts_with($p[0], '/')) { + if (count($p) < 4 || ! str_starts_with($p[0], '/')) { continue; // header / noise } - [$mount, $fs, $size, $pcent] = $p; + $pcent = array_pop($p); + $size = array_pop($p); + $fs = array_pop($p); + $mount = implode(' ', $p); + if (! ctype_digit($size)) { + continue; + } $out[] = [ 'mount' => $mount, 'fs' => $fs, @@ -379,7 +404,7 @@ class FleetService $out = []; foreach ($this->lines($body) as $line) { // "3072 SHA256:xxxx comment with spaces (RSA)" - if (! preg_match('/^(\d+)\s+(\S+)\s+(.*)\s+\(([^)]+)\)\s*$/', $line, $m)) { + if (! preg_match('/^(\d+)\s+(\S+)\s+(.*?)\s*\(([^)]+)\)\s*$/', $line, $m)) { continue; } $algo = strtolower($m[4]); diff --git a/app/Support/Ssh/Sftp.php b/app/Support/Ssh/Sftp.php index b8679f5..5fe6549 100644 --- a/app/Support/Ssh/Sftp.php +++ b/app/Support/Ssh/Sftp.php @@ -11,6 +11,8 @@ use RuntimeException; */ class Sftp { + use VerifiesHostKey; + private ?SftpProtocol $sftp = null; public function __construct(private readonly CredentialVault $vault) {} @@ -18,6 +20,9 @@ class Sftp public function connect(Server $server): self { $sftp = new SftpProtocol($server->ip, $server->ssh_port ?: 22); + + $this->verifyHostKey($sftp, $server); + $login = $this->vault->resolve($server); if (! $sftp->login($login['username'], $login['secret'])) { @@ -71,6 +76,12 @@ class Sftp return $this->client()->delete($remote); } + public function disconnect(): void + { + $this->sftp?->disconnect(); + $this->sftp = null; + } + private function client(): SftpProtocol { return $this->sftp ?? throw new RuntimeException('Nicht verbunden — connect() zuerst aufrufen.'); diff --git a/app/Support/Ssh/SshClient.php b/app/Support/Ssh/SshClient.php index 918e548..9d6312b 100644 --- a/app/Support/Ssh/SshClient.php +++ b/app/Support/Ssh/SshClient.php @@ -12,6 +12,8 @@ use RuntimeException; */ class SshClient { + use VerifiesHostKey; + private ?SSH2 $ssh = null; public function __construct( @@ -24,6 +26,8 @@ class SshClient $ssh = new SSH2($server->ip, $server->ssh_port ?: 22); $ssh->setTimeout($this->timeout); + $this->verifyHostKey($ssh, $server); + $login = $this->vault->resolve($server); if (! $ssh->login($login['username'], $login['secret'])) { diff --git a/app/Support/Ssh/VerifiesHostKey.php b/app/Support/Ssh/VerifiesHostKey.php new file mode 100644 index 0000000..bfe7427 --- /dev/null +++ b/app/Support/Ssh/VerifiesHostKey.php @@ -0,0 +1,38 @@ +getServerPublicHostKey(); + + if ($hostKey === false) { + throw new RuntimeException("Host-Key von {$server->name} ({$server->ip}) nicht lesbar — Verbindung abgebrochen."); + } + + $known = (string) ($server->ssh_host_key ?? ''); + + if ($known !== '') { + if (! hash_equals($known, $hostKey)) { + throw new RuntimeException("Host-Key-Mismatch fuer {$server->name} ({$server->ip}) — moegliches MITM. Verbindung abgebrochen."); + } + + return; + } + + // trust on first use + $server->forceFill(['ssh_host_key' => $hostKey])->save(); + } +} diff --git a/database/migrations/2026_06_12_120000_add_ssh_host_key_to_servers.php b/database/migrations/2026_06_12_120000_add_ssh_host_key_to_servers.php new file mode 100644 index 0000000..71bb205 --- /dev/null +++ b/database/migrations/2026_06_12_120000_add_ssh_host_key_to_servers.php @@ -0,0 +1,23 @@ +text('ssh_host_key')->nullable()->after('ssh_port'); + }); + } + + public function down(): void + { + Schema::table('servers', function (Blueprint $table) { + $table->dropColumn('ssh_host_key'); + }); + } +}; diff --git a/resources/views/livewire/files/index.blade.php b/resources/views/livewire/files/index.blade.php index b798221..3306628 100644 --- a/resources/views/livewire/files/index.blade.php +++ b/resources/views/livewire/files/index.blade.php @@ -37,8 +37,8 @@ @if (! $loop->first) / @endif - @@ -78,7 +78,7 @@ @if ($isDir) - @else @@ -117,7 +117,7 @@ Bearbeiten {{-- R5: destructive → wire-elements/modal confirm + audit --}} - diff --git a/resources/views/livewire/servers/show.blade.php b/resources/views/livewire/servers/show.blade.php index c0b11d5..ad74f2f 100644 --- a/resources/views/livewire/servers/show.blade.php +++ b/resources/views/livewire/servers/show.blade.php @@ -170,7 +170,7 @@ {{-- R5: destructive → wire-elements/modal confirm + audit --}} + >Starten