Fix-Runde 1: Ticket ueber rohes Redis (GETDEL, reines JSON) statt Cache-Fassade

Cache::put() serialisierte den JSON-Inhalt zusaetzlich mit PHP serialize()
(kein 'serializer' konfiguriert), und Cache::pull() war get()+forget() in
zwei Runden statt atomar. issue()/redeem() sprechen jetzt direkt ueber
Redis::connection('cache') (setex/getdel), der volle Schluessel inkl.
REDIS_PREFIX steht im Kopfkommentar fuer Aufgabe 3. issue() weist ausserdem
Hosts ohne wg_ip oder ohne ssh_host_key zurueck, statt die Pruefung an einen
noch nicht existierenden Container zu delegieren.
main
nexxo 2026-08-02 17:52:16 +02:00
parent 830af24b6c
commit 714b044ff1
2 changed files with 136 additions and 35 deletions

View File

@ -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);
}
}

View File

@ -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.");
});