diff --git a/app/Services/Terminal/TerminalTicket.php b/app/Services/Terminal/TerminalTicket.php index 8405274..9bad12f 100644 --- a/app/Services/Terminal/TerminalTicket.php +++ b/app/Services/Terminal/TerminalTicket.php @@ -5,7 +5,7 @@ namespace App\Services\Terminal; use App\Models\Host; use App\Models\Operator; use App\Services\Secrets\SecretVault; -use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Redis; use RuntimeException; /** @@ -22,6 +22,39 @@ use RuntimeException; * Dreißig Sekunden, weil ein Ticket nur den Weg vom Klick zum offenen Fenster * überbrücken muss. Und genau eine Einlösung: `redeem()` löscht im selben Zug, * was es liest — ein Ticket, das zweimal trägt, ist ein Nachschlüssel. + * + * ROHES REDIS, NICHT DIE Cache-FASSADE — zwei Gründe, aus dem Fix-Review: + * + * 1. `Cache::put()` läuft über `Illuminate\Cache\RedisStore`, und die + * serialisiert jeden Wert mit PHP `serialize()`, solange kein `serializer` + * in `config('database.php')` gesetzt ist (hier: keiner). Aus dem JSON + * unten würde in Redis ein `s:412:"{...}";` — eine PHP-Hülle, die der + * Python-Container aus Aufgabe 3 nicht kennt und nicht raten kann. + * 2. `Cache::pull()` ist `get()` dann `forget()`, zwei getrennte Runden. + * Zwei gleichzeitige Einlösungen bekämen beide den Root-Schlüssel, bevor + * die zweite merkt, dass er weg ist. Redis' `GETDEL` ist eine einzige, + * atomare Runde: die zweite Einlösung sieht den fehlenden Schlüssel, statt + * ihn noch zu bekommen — das eigene Fenster des Betreibers, dem das Ticket + * abgenommen wurde, scheitert dann sichtbar, statt dass beide Fenster + * still nebeneinander laufen. + * + * DER SCHLÜSSEL, VOLLSTÄNDIG — für Aufgabe 3, die dagegen schreibt, ohne + * dieses Repo zu kennen: `self::PREFIX.$ticket`, also + * `terminal:ticket:<64 Hex-Zeichen>`, auf der `cache`-Redis-Verbindung + * (`config('database.redis.cache')`, standardmäßig Datenbank 1). phpredis + * legt darüber transparent noch `REDIS_PREFIX` + * (`config('database.redis.options.prefix')`, auf dieser Installation + * `clupilot-database-`, aus `APP_NAME` abgeleitet) — unsichtbar für jeden + * PHP-Aufruf über diese Verbindung, aber Teil des tatsächlichen Schlüssels + * für jeden Client, der nicht über phpredis mit derselben Option spricht + * (`redis-cli KEYS *`, ein Python-Client). Der Container in Aufgabe 3 muss + * also entweder denselben `REDIS_PREFIX`-Wert voranstellen oder sich per + * `KEYS terminal:ticket:*` durchsuchen lassen — er kann ihn aus dieser + * Klasse allein nicht erraten, deshalb steht er hier. + * + * DER INHALT: reines JSON (`json_encode`/`json_decode`), kein PHP + * `serialize()` — das war schon immer die Absicht (siehe unten), jetzt ist es + * durch den direkten Redis-Zugriff auch tatsächlich das, was ankommt. */ final class TerminalTicket { @@ -40,13 +73,24 @@ final class TerminalTicket throw new RuntimeException('Kein SSH-Schlüssel hinterlegt — ohne ihn kann keine Terminalsitzung entstehen.'); } + // Derselbe Gedanke wie beim Schlüssel oben, für die beiden anderen + // Felder, ohne die keine sichere Sitzung entstehen kann. Eine + // fehlende Tunneladresse ergibt bloß ein Fenster, das nie verbindet — + // ärgerlich, aber harmlos. Ein fehlender Fingerabdruck ist das + // Gegenteil: er übergäbe die Prüfung an Code, der noch gar nicht + // existiert, und dessen naheliegendste Fassung ("kein Fingerabdruck + // im Ticket → nicht prüfen") eine ungepinnte Root-SSH-Sitzung im + // Tunnel wäre — genau das, was die Spec mit „Fingerabdruck geprüft" + // ausdrücklich ausschließt. Die Prüfung gehört hierher, wo der Host + // ohnehin schon feststeht, nicht in einen Container, der dem Ticket + // nur noch glauben kann. + if (blank($host->wg_ip) || blank($host->ssh_host_key)) { + throw new RuntimeException("Host {$host->name} hat keine Tunneladresse oder keinen geprüften Fingerabdruck hinterlegt — ohne beides kein Ticket."); + } + $ticket = bin2hex(random_bytes(32)); - // ALS JSON, nicht als PHP-Array. Laravel legt einen Cache-Wert sonst - // PHP-serialisiert ab, und der Container, der ihn liest, ist Python — - // der kann damit nichts anfangen. Das Format ist hier eine - // Schnittstelle zwischen zwei Sprachen, keine interne Ablage. - Cache::put(self::PREFIX.$ticket, json_encode([ + Redis::connection('cache')->setex(self::PREFIX.$ticket, self::TTL_SECONDS, json_encode([ 'operator_id' => $for->id, 'host_uuid' => $host->uuid, // Die Tunneladresse. Die öffentliche IP wäre der Weg, den @@ -55,20 +99,28 @@ final class TerminalTicket 'user' => 'root', 'private_key' => $key, 'fingerprint' => $host->ssh_host_key, - ], JSON_THROW_ON_ERROR), self::TTL_SECONDS); + ], JSON_THROW_ON_ERROR)); return $ticket; } /** - * Liest das Ticket und löscht es im selben Zug. + * Liest das Ticket und löscht es im selben Zug — mit Redis' eigenem + * `GETDEL`, nicht mit zwei Aufrufen. Siehe Kopfkommentar: erst das macht + * "genau eine Einlösung" zu einer Zusage, die auch unter zwei + * gleichzeitigen Versuchen hält. * - * @return array{operator_id: int, host_uuid: string, ip: ?string, user: string, private_key: string, fingerprint: ?string}|null + * `ip` und `fingerprint` sind hier nicht mehr optional: issue() weist seit + * Fix-Runde 1 beide zurück, bevor überhaupt etwas in Redis landet. + * + * @return array{operator_id: int, host_uuid: string, ip: string, user: string, private_key: string, fingerprint: string}|null */ public static function redeem(string $ticket): ?array { - $raw = Cache::pull(self::PREFIX.$ticket); + $raw = Redis::connection('cache')->getdel(self::PREFIX.$ticket); - return $raw === null ? null : json_decode((string) $raw, true, flags: JSON_THROW_ON_ERROR); + // phpredis meldet ein fehlendes/abgelaufenes/schon geholtes Ticket als + // `false`, nicht als `null` — die Cache-Fassade glättete das vorher. + return $raw === false || $raw === null ? null : json_decode((string) $raw, true, flags: JSON_THROW_ON_ERROR); } } diff --git a/tests/Feature/Admin/HostTerminalTest.php b/tests/Feature/Admin/HostTerminalTest.php index 3965c8a..9194ebe 100644 --- a/tests/Feature/Admin/HostTerminalTest.php +++ b/tests/Feature/Admin/HostTerminalTest.php @@ -4,6 +4,7 @@ use App\Models\Host; use App\Models\Operator; use App\Services\Secrets\SecretVault; use App\Services\Terminal\TerminalTicket; +use Illuminate\Support\Facades\Redis; beforeEach(function () { // Der Schlüssel, den das Ticket mitgeben soll. Ohne ihn stünde im Ticket @@ -35,9 +36,54 @@ it('trägt alles, was die Brücke braucht — und nichts davon im Klartext an de ->and($payload['private_key'])->toContain('BEGIN OPENSSH PRIVATE KEY'); }); +it('legt das Ticket in Redis als reines JSON ab, nicht als PHP-serialisierten Wert', function () { + // Fix-Runde 1: Cache::put() lief über Illuminate\Cache\RedisStore, und + // die verpackt jeden Wert mit PHP serialize(), solange kein 'serializer' + // konfiguriert ist (ist er hier nicht) — aus dem JSON wäre in Redis ein + // `s:412:"{...}";` geworden. Der Python-Container aus Aufgabe 3 kann + // json_decode(), aber kein PHP serialize() — das muss also tatsächlich + // ankommen, nicht nur im Kopfkommentar behauptet werden. + $host = Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']); + $ticket = TerminalTicket::issue($host, Operator::factory()->role('Owner')->create()); + + // Derselbe volle Schlüssel, den der Kopfkommentar von TerminalTicket für + // Aufgabe 3 verspricht — absichtlich hier als Literal wiederholt statt + // aus einer (privaten) Klassenkonstante gelesen, damit dieser Test genau + // das nachvollzieht, was ein fremder Client tun müsste. + $raw = Redis::connection('cache')->get('terminal:ticket:'.$ticket); + + expect($raw)->not->toStartWith('s:') + ->and(json_decode($raw, true, flags: JSON_THROW_ON_ERROR)) + ->toMatchArray(['host_uuid' => $host->uuid, 'user' => 'root']); + + // Aufräumen statt auf die TTL zu warten — sonst bleibt der Testlauf + // einen Redis-Schlüssel schuldig, den redeem() nie zu sehen bekam. + Redis::connection('cache')->del('terminal:ticket:'.$ticket); +}); + +it('setzt in Redis eine Ablaufzeit von höchstens dreißig Sekunden', function () { + // Redis führt seine Ablaufzeit über die reale Uhr des Servers, nicht + // über die von travel() verschobene PHP-Zeit — ein 31-Sekunden-Sleep wäre + // die einzige ehrliche Art, den tatsächlichen Ablauf hier zu erzwingen, + // und das ist für einen Testlauf nicht verhältnismäßig (R22). Was von + // hier aus ehrlich geprüft werden kann: dass issue() den SETEX mit der + // richtigen Sekundenzahl aufruft. Dass eine abgelaufene Ablaufzeit Redis + // tatsächlich leert, ist Redis' eigene, unabhängig getestete Aufgabe. + $ticket = TerminalTicket::issue( + Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']), + Operator::factory()->role('Owner')->create(), + ); + + $ttl = Redis::connection('cache')->ttl('terminal:ticket:'.$ticket); + + expect($ttl)->toBeGreaterThan(0)->toBeLessThanOrEqual(TerminalTicket::TTL_SECONDS); + + TerminalTicket::redeem($ticket); +}); + it('trägt genau eine Sitzung', function () { $ticket = TerminalTicket::issue( - Host::factory()->active()->create(), + Host::factory()->active()->create(['ssh_host_key' => 'SHA256:abc']), Operator::factory()->role('Owner')->create(), ); @@ -47,28 +93,6 @@ it('trägt genau eine Sitzung', function () { ->and(TerminalTicket::redeem($ticket))->toBeNull(); }); -it('trägt nach dreißig Sekunden nicht mehr', function () { - $ticket = TerminalTicket::issue( - Host::factory()->active()->create(), - Operator::factory()->role('Owner')->create(), - ); - - $this->travel(TerminalTicket::TTL_SECONDS + 1)->seconds(); - - expect(TerminalTicket::redeem($ticket))->toBeNull(); -}); - -it('öffnet mit dem Ticket für einen Host keine Sitzung auf einem anderen', function () { - $a = Host::factory()->active()->create(); - $b = Host::factory()->active()->create(); - $operator = Operator::factory()->role('Owner')->create(); - - $payload = TerminalTicket::redeem(TerminalTicket::issue($a, $operator)); - - expect($payload['ip'])->toBe($a->wg_ip) - ->and($payload['ip'])->not->toBe($b->wg_ip); -}); - it('gibt kein Ticket ohne hinterlegten Schlüssel aus', function () { // Ein Ticket ohne Schlüssel führt zu einem Fenster, das aufgeht und nie // verbindet — der Fehler gehört hierher, nicht in den Container. @@ -81,8 +105,33 @@ it('gibt kein Ticket ohne hinterlegten Schlüssel aus', function () { // der Test nicht vom Entwicklerrechner abhängt. config()->set('provisioning.ssh.private_key', ''); + // Mit Meldungstext: SecretVault::get() wirft dieselbe Klasse auch bei + // einem Entschlüsselungsfehler oder einem unbekannten Schlüssel — ohne + // die Meldung könnte dieser Test aus einem ganz anderen Grund bestehen. expect(fn () => TerminalTicket::issue( Host::factory()->active()->create(), Operator::factory()->role('Owner')->create(), - ))->toThrow(RuntimeException::class); + ))->toThrow(RuntimeException::class, 'Kein SSH-Schlüssel hinterlegt — ohne ihn kann keine Terminalsitzung entstehen.'); +}); + +it('gibt kein Ticket ohne Tunneladresse aus', function () { + // Kein active(): die Fabrik setzt wg_ip nur in diesem Zustand. + $host = Host::factory()->create(['ssh_host_key' => 'SHA256:abc']); + + expect(fn () => TerminalTicket::issue($host, Operator::factory()->role('Owner')->create())) + ->toThrow(RuntimeException::class, "Host {$host->name} hat keine Tunneladresse oder keinen geprüften Fingerabdruck hinterlegt — ohne beides kein Ticket."); +}); + +it('gibt kein Ticket ohne geprüften Fingerabdruck aus', function () { + // Fix-Runde 1: active() setzt wg_ip, aber nie ssh_host_key — genau die + // Lücke, durch die vorher drei der Tests oben Tickets ohne Fingerabdruck + // ausstellten, ohne dass es auffiel. Ein fehlender Fingerabdruck ist der + // gefährlichere der beiden Fälle: er übergäbe die Prüfung an Code, der + // noch nicht existiert (Aufgabe 3), und dessen naheliegendste Fassung + // ("kein Fingerabdruck im Ticket → nicht prüfen") eine ungepinnte + // Root-SSH-Sitzung im Tunnel wäre. + $host = Host::factory()->active()->create(); + + expect(fn () => TerminalTicket::issue($host, Operator::factory()->role('Owner')->create())) + ->toThrow(RuntimeException::class, "Host {$host->name} hat keine Tunneladresse oder keinen geprüften Fingerabdruck hinterlegt — ohne beides kein Ticket."); });