fix(vpn): a late revocation must not disconnect whoever holds the key now
tests / pest (push) Failing after 49s Details
tests / assets (push) Successful in 51s Details
tests / release (push) Has been skipped Details

Once a key is free, someone can legitimately create a new access with it. The
forced revocation looked the key up and would have deleted that person's access
and pulled their key off the hub. It now acts only on an adoption artifact — a
system peer with no owner and no creator — and leaves anything else alone.

Re-issuing also threw when VPN_CONFIG_KEY was missing, which is exactly the
situation the console tells people to fix by re-issuing. It now hands the new
config over once instead of storing it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/portal-design
nexxo 2026-07-26 01:24:55 +02:00
parent 4f9ca27732
commit 53d305340f
3 changed files with 52 additions and 3 deletions

View File

@ -251,7 +251,12 @@ class Vpn extends Component
$peer->forceFill([
'public_key' => $keypair->publicKey,
'present' => false,
'config_secret' => $peer->hasStoredConfig() ? ConfigVault::encrypt($config) : null,
// Only re-stored when the vault can actually be used: without the key
// this would throw, and the console tells people to re-issue
// precisely when a stored config has become unreadable.
'config_secret' => $peer->hasStoredConfig() && ConfigVault::available()
? ConfigVault::encrypt($config)
: null,
])->save();
// Old key off the hub first, then the new one on: the other order would

View File

@ -5,6 +5,7 @@ namespace App\Provisioning\Jobs;
use App\Models\VpnPeer;
use App\Services\Wireguard\WireguardHub;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Log;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
@ -70,14 +71,25 @@ class ApplyVpnPeer implements ShouldQueue
$enabled = ! $this->force && $peer !== null && ! $peer->trashed() && $peer->enabled;
$ip = $peer?->allowed_ip ?? $this->allowedIp;
// Checked before touching the hub: a forced revocation whose key has
// since been re-issued to someone else must not disconnect them.
if ($this->force && $peer !== null
&& ! ($peer->kind === VpnPeer::KIND_SYSTEM && $peer->created_by === null && $peer->user_id === null)) {
Log::info('forced revocation skipped: the key belongs to a live access again', [
'peer' => $peer->uuid,
]);
return;
}
if ($enabled && $ip !== null) {
$hub->addPeer($this->publicKey, $ip);
} else {
$hub->removePeer($this->publicKey);
}
// A row still carrying a forcibly revoked key can only be an adoption
// artifact — the access itself has moved on to a new key.
// Adoption artifact cleaned up; the guard above already sent anything
// else home.
if ($this->force && $peer !== null) {
$peer->forceDelete();

View File

@ -847,3 +847,35 @@ it('revokes the replaced key even after a sync adopted it', function () {
expect($hub->peerIps())->not->toHaveKey($oldKey)
->and(VpnPeer::withTrashed()->where('public_key', $oldKey)->exists())->toBeFalse();
});
it('does not let a late revocation delete an access that reused the key', function () {
$hub = vpnHub();
$owner = operator('Owner');
$key = Keypair::generate()->publicKey;
// Someone created a new access with a key that used to belong elsewhere.
$fresh = VpnPeer::factory()->ownedBy($owner)->create(['public_key' => $key]);
$hub->addPeer($key, $fresh->allowed_ip);
// The old revocation job finally runs.
(new ApplyVpnPeer($key, null, false, force: true))->handle($hub);
expect(VpnPeer::query()->whereKey($fresh->id)->exists())->toBeTrue()
->and($hub->peerIps())->toHaveKey($key); // and they stay connected
});
it('re-issues even when the config vault is unavailable', function () {
vpnHub();
Queue::fake();
$owner = operator('Owner');
$peer = VpnPeer::factory()->ownedBy($owner)->create([
'config_secret' => App\Services\Wireguard\ConfigVault::encrypt("[Interface]\n"),
]);
// The very situation the console tells people to fix by re-issuing.
config()->set('admin_access.vpn_config_key', '');
Livewire::actingAs($owner)->test(Vpn::class)->call('reissue', $peer->uuid);
expect($peer->fresh()->config_secret)->toBeNull(); // shown once instead
});