feat(security): a way back into the console that does not need the console
The allowlist is managed in the console, which is fine until the address you manage it from changes — and then the page that would fix the problem is the page the problem blocks. Every gate needs a door that does not depend on itself, and on a server that door is a shell: php artisan clupilot:console-access show php artisan clupilot:console-access allow 203.0.113.7 php artisan clupilot:console-access open The address check moved to RestrictConsoleNetwork::isNetwork() so the command and the console apply the same rule. Codex caught the version that did not: a typo like 203.0.113.9/99 was stored, reported as success, and matched nothing — leaving whoever was recovering still locked out, now believing they were not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feat/portal-design tested-20260727-0125-0066b1c
parent
9ccd4f59d8
commit
0066b1c6a0
|
|
@ -0,0 +1,103 @@
|
|||
<?php
|
||||
|
||||
namespace App\Console\Commands;
|
||||
|
||||
use App\Http\Middleware\RestrictConsoleNetwork;
|
||||
use App\Support\Settings;
|
||||
use Illuminate\Console\Command;
|
||||
|
||||
/**
|
||||
* The way back in when the console has locked its owner out.
|
||||
*
|
||||
* The allowlist is managed in the console — which is fine until the address you
|
||||
* manage it from changes, and then the page that would fix the problem is the
|
||||
* page the problem blocks. Every gate needs a door that does not depend on
|
||||
* itself, and on a server that door is a shell.
|
||||
*/
|
||||
class ConsoleAccess extends Command
|
||||
{
|
||||
protected $signature = 'clupilot:console-access
|
||||
{action=show : show|allow|deny|open|close}
|
||||
{value? : an address or CIDR, for allow and deny}';
|
||||
|
||||
protected $description = 'Show or change who may reach the operator console';
|
||||
|
||||
public function handle(): int
|
||||
{
|
||||
$action = (string) $this->argument('action');
|
||||
$value = trim((string) $this->argument('value'));
|
||||
|
||||
return match ($action) {
|
||||
'show' => $this->show(),
|
||||
'allow' => $this->allow($value),
|
||||
'deny' => $this->deny($value),
|
||||
'open' => $this->setRestricted(false),
|
||||
'close' => $this->setRestricted(true),
|
||||
default => $this->refuse("Unknown action: {$action}"),
|
||||
};
|
||||
}
|
||||
|
||||
private function show(): int
|
||||
{
|
||||
$this->line(' restricted : '.(RestrictConsoleNetwork::isRestricted() ? 'yes' : 'no — anyone reaching the hostname gets in'));
|
||||
$this->line(' always : '.implode(', ', (array) config('admin_access.trusted_ranges', [])).' (VPN, not removable)');
|
||||
|
||||
$own = (array) Settings::get('console.allowed_ips', []);
|
||||
$this->line(' additional : '.($own === [] ? '(none)' : implode(', ', $own)));
|
||||
|
||||
return self::SUCCESS;
|
||||
}
|
||||
|
||||
private function allow(string $value): int
|
||||
{
|
||||
if ($value === '') {
|
||||
return $this->refuse('Give an address or range: clupilot:console-access allow 203.0.113.7');
|
||||
}
|
||||
|
||||
// An entry that matches nothing would be stored, reported as success,
|
||||
// and leave whoever is recovering still locked out.
|
||||
if (! RestrictConsoleNetwork::isNetwork($value)) {
|
||||
return $this->refuse("Not an address or a range: {$value}");
|
||||
}
|
||||
|
||||
$list = (array) Settings::get('console.allowed_ips', []);
|
||||
|
||||
if (! in_array($value, $list, true)) {
|
||||
$list[] = $value;
|
||||
Settings::set('console.allowed_ips', array_values($list));
|
||||
}
|
||||
|
||||
$this->info("{$value} may now reach the console.");
|
||||
|
||||
return $this->show();
|
||||
}
|
||||
|
||||
private function deny(string $value): int
|
||||
{
|
||||
Settings::set('console.allowed_ips', array_values(array_filter(
|
||||
(array) Settings::get('console.allowed_ips', []),
|
||||
fn ($entry) => $entry !== $value,
|
||||
)));
|
||||
|
||||
$this->info("{$value} removed.");
|
||||
|
||||
return $this->show();
|
||||
}
|
||||
|
||||
private function setRestricted(bool $on): int
|
||||
{
|
||||
// No lock-out check here on purpose: this command IS the recovery path,
|
||||
// and it is only reachable by someone who already has the server.
|
||||
Settings::set('console.network_restricted', $on);
|
||||
$this->info($on ? 'Console restricted to the VPN and the listed addresses.' : 'Restriction lifted.');
|
||||
|
||||
return $this->show();
|
||||
}
|
||||
|
||||
private function refuse(string $message): int
|
||||
{
|
||||
$this->error($message);
|
||||
|
||||
return self::FAILURE;
|
||||
}
|
||||
}
|
||||
|
|
@ -98,6 +98,31 @@ class RestrictConsoleNetwork
|
|||
return self::covers($ip, array_merge($vpn, $extra));
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a value is an address or a CIDR range at all.
|
||||
*
|
||||
* Shared by the console and the recovery command on purpose: an entry that
|
||||
* matches nothing is stored happily and reports success, and the operator
|
||||
* then switches the restriction on believing they are covered. That is the
|
||||
* exact situation the recovery command exists to get out of.
|
||||
*/
|
||||
public static function isNetwork(string $value): bool
|
||||
{
|
||||
[$address, $prefix] = array_pad(explode('/', $value, 2), 2, null);
|
||||
|
||||
if (filter_var($address, FILTER_VALIDATE_IP) === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($prefix === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$max = str_contains((string) $address, ':') ? 128 : 32;
|
||||
|
||||
return ctype_digit((string) $prefix) && (int) $prefix >= 0 && (int) $prefix <= $max;
|
||||
}
|
||||
|
||||
/** @param array<int, string> $ranges */
|
||||
private static function covers(string $ip, array $ranges): bool
|
||||
{
|
||||
|
|
|
|||
|
|
@ -251,7 +251,7 @@ class Settings extends Component
|
|||
|
||||
$value = trim($this->consoleIp);
|
||||
|
||||
if (! $this->looksLikeNetwork($value)) {
|
||||
if (! \App\Http\Middleware\RestrictConsoleNetwork::isNetwork($value)) {
|
||||
$this->addError('consoleIp', __('admin_settings.console_ip_invalid'));
|
||||
|
||||
return;
|
||||
|
|
@ -321,22 +321,6 @@ class Settings extends Component
|
|||
$this->dispatch('notify', message: __($on ? 'admin_settings.console_locked' : 'admin_settings.console_unlocked'));
|
||||
}
|
||||
|
||||
private function looksLikeNetwork(string $value): bool
|
||||
{
|
||||
[$address, $prefix] = array_pad(explode('/', $value, 2), 2, null);
|
||||
|
||||
if (filter_var($address, FILTER_VALIDATE_IP) === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($prefix === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$max = str_contains($address, ':') ? 128 : 32;
|
||||
|
||||
return ctype_digit($prefix) && (int) $prefix >= 0 && (int) $prefix <= $max;
|
||||
}
|
||||
|
||||
public function render()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -110,3 +110,22 @@ it('is re-checked on every Livewire action, not only on the page', function () {
|
|||
|
||||
expect($persistent)->toContain(RestrictConsoleNetwork::class);
|
||||
});
|
||||
|
||||
it('offers a way back in that does not depend on the console', function () {
|
||||
// Every gate needs a door that does not depend on itself: the list is
|
||||
// managed from the console, and the address you manage it from can change.
|
||||
Settings::set('console.network_restricted', true);
|
||||
Settings::set('console.allowed_ips', []);
|
||||
|
||||
$this->artisan('clupilot:console-access allow 203.0.113.42')->assertSuccessful();
|
||||
expect(Settings::get('console.allowed_ips'))->toBe(['203.0.113.42'])
|
||||
->and(RestrictConsoleNetwork::wouldStillAllow('203.0.113.42', ['203.0.113.42']))->toBeTrue();
|
||||
|
||||
$this->artisan('clupilot:console-access deny 203.0.113.42')->assertSuccessful();
|
||||
expect(Settings::get('console.allowed_ips'))->toBe([]);
|
||||
|
||||
// And the restriction itself can be lifted from the shell — the only place
|
||||
// that is reachable when the gate has shut on its own owner.
|
||||
$this->artisan('clupilot:console-access open')->assertSuccessful();
|
||||
expect(RestrictConsoleNetwork::isRestricted())->toBeFalse();
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue