From 17d7dc2ad9be089cb24f6a0ab03abf8c37e1583a Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 22:53:30 +0200 Subject: [PATCH] fix(vpn): store the config inside the creation transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Encryption and the write ran after the transaction had committed and the peer had been dispatched to the hub. A failure there left a live access whose config was never stored — unrecoverable, and the operator would create a second one not knowing why. Both now happen in the same insert; nothing is dispatched unless it succeeded. Re-verified in the browser afterwards: create with storage, wrong password refused, right password reveals the config. Co-Authored-By: Claude Opus 4.8 --- app/Livewire/Admin/Vpn.php | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/app/Livewire/Admin/Vpn.php b/app/Livewire/Admin/Vpn.php index e208bf2..2fc30c7 100644 --- a/app/Livewire/Admin/Vpn.php +++ b/app/Livewire/Admin/Vpn.php @@ -108,7 +108,11 @@ class Vpn extends Component // tables, so neither unique index can catch the other's insert — the // shared lock is what keeps a host and an access off the same tunnel IP. try { - $peer = Cache::lock('wireguard:allocate', 30)->block(10, fn () => DB::transaction(function () use ($hub, $publicKey) { + // Filled inside the transaction; only the handoff needs it afterwards. + $plainConfig = null; + + $peer = Cache::lock('wireguard:allocate', 30)->block(10, function () use ($hub, $publicKey, $keypair, &$plainConfig) { + return DB::transaction(function () use ($hub, $publicKey, $keypair, &$plainConfig) { // The owner row is locked for the whole insert, and revokeStaff() // locks the same row: without that, a revocation could commit // between the check and the insert, find no peer to remove, and @@ -135,17 +139,33 @@ class Vpn extends Component return 'host_key'; } + $ip = $hub->allocateIp(); + + // Built and encrypted here so the stored config is part of the + // same insert. Doing it afterwards could leave a live access + // whose config was never stored — unrecoverable, and the + // operator would create a second one not knowing why. + $secret = null; + if ($keypair !== null) { + $plainConfig = $this->clientConfig($keypair, $ip); + if ($this->storeConfig) { + $secret = ConfigVault::encrypt($plainConfig); + } + } + return VpnPeer::create([ 'name' => trim($this->name), 'kind' => VpnPeer::KIND_STAFF, 'user_id' => $this->ownerId, 'public_key' => $publicKey, - 'allowed_ip' => $hub->allocateIp(), + 'allowed_ip' => $ip, + 'config_secret' => $secret, 'enabled' => true, 'present' => false, 'created_by' => auth()->id(), ]); - })); + }); + }); } catch (QueryException) { // Backstop: the unique index caught a writer that did not take this // lock. Report it like any other duplicate instead of a 500. @@ -169,14 +189,9 @@ class Vpn extends Component $this->dismissConfig(); // Only a key we generated can be turned into a ready-to-use config. - if ($keypair !== null) { - $config = $this->clientConfig($keypair, $peer->allowed_ip); - $this->configToken = ConfigHandoff::put($config); + if ($plainConfig !== null) { + $this->configToken = ConfigHandoff::put($plainConfig); $this->newConfigName = $peer->name; - - if ($this->storeConfig) { - $peer->forceFill(['config_secret' => ConfigVault::encrypt($config)])->saveQuietly(); - } } $this->reset('name', 'publicKey', 'storeConfig');