chore: prepare for PR
This commit is contained in:
parent
8a5ccd6323
commit
839635e9e8
7 changed files with 202 additions and 20 deletions
|
|
@ -37,7 +37,7 @@ public static function ensureMultiplexedConnection(Server $server): bool
|
||||||
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
|
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
|
||||||
$checkCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
$checkCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
||||||
}
|
}
|
||||||
$checkCommand .= "{$server->user}@{$server->ip}";
|
$checkCommand .= self::escapedUserAtHost($server);
|
||||||
$process = Process::run($checkCommand);
|
$process = Process::run($checkCommand);
|
||||||
|
|
||||||
if ($process->exitCode() !== 0) {
|
if ($process->exitCode() !== 0) {
|
||||||
|
|
@ -80,7 +80,7 @@ public static function establishNewMultiplexedConnection(Server $server): bool
|
||||||
$establishCommand .= ' -o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
$establishCommand .= ' -o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
||||||
}
|
}
|
||||||
$establishCommand .= self::getCommonSshOptions($server, $sshKeyLocation, $connectionTimeout, $serverInterval);
|
$establishCommand .= self::getCommonSshOptions($server, $sshKeyLocation, $connectionTimeout, $serverInterval);
|
||||||
$establishCommand .= "{$server->user}@{$server->ip}";
|
$establishCommand .= self::escapedUserAtHost($server);
|
||||||
$establishProcess = Process::run($establishCommand);
|
$establishProcess = Process::run($establishCommand);
|
||||||
if ($establishProcess->exitCode() !== 0) {
|
if ($establishProcess->exitCode() !== 0) {
|
||||||
return false;
|
return false;
|
||||||
|
|
@ -101,7 +101,7 @@ public static function removeMuxFile(Server $server)
|
||||||
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
|
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
|
||||||
$closeCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
$closeCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
||||||
}
|
}
|
||||||
$closeCommand .= "{$server->user}@{$server->ip}";
|
$closeCommand .= self::escapedUserAtHost($server);
|
||||||
Process::run($closeCommand);
|
Process::run($closeCommand);
|
||||||
|
|
||||||
// Clear connection metadata from cache
|
// Clear connection metadata from cache
|
||||||
|
|
@ -141,9 +141,9 @@ public static function generateScpCommand(Server $server, string $source, string
|
||||||
|
|
||||||
$scp_command .= self::getCommonSshOptions($server, $sshKeyLocation, config('constants.ssh.connection_timeout'), config('constants.ssh.server_interval'), isScp: true);
|
$scp_command .= self::getCommonSshOptions($server, $sshKeyLocation, config('constants.ssh.connection_timeout'), config('constants.ssh.server_interval'), isScp: true);
|
||||||
if ($server->isIpv6()) {
|
if ($server->isIpv6()) {
|
||||||
$scp_command .= "{$source} {$server->user}@[{$server->ip}]:{$dest}";
|
$scp_command .= "{$source} ".escapeshellarg($server->user).'@['.escapeshellarg($server->ip)."]:{$dest}";
|
||||||
} else {
|
} else {
|
||||||
$scp_command .= "{$source} {$server->user}@{$server->ip}:{$dest}";
|
$scp_command .= "{$source} ".self::escapedUserAtHost($server).":{$dest}";
|
||||||
}
|
}
|
||||||
|
|
||||||
return $scp_command;
|
return $scp_command;
|
||||||
|
|
@ -189,13 +189,18 @@ public static function generateSshCommand(Server $server, string $command, bool
|
||||||
$delimiter = base64_encode($delimiter);
|
$delimiter = base64_encode($delimiter);
|
||||||
$command = str_replace($delimiter, '', $command);
|
$command = str_replace($delimiter, '', $command);
|
||||||
|
|
||||||
$ssh_command .= "{$server->user}@{$server->ip} 'bash -se' << \\$delimiter".PHP_EOL
|
$ssh_command .= self::escapedUserAtHost($server)." 'bash -se' << \\$delimiter".PHP_EOL
|
||||||
.$command.PHP_EOL
|
.$command.PHP_EOL
|
||||||
.$delimiter;
|
.$delimiter;
|
||||||
|
|
||||||
return $ssh_command;
|
return $ssh_command;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static function escapedUserAtHost(Server $server): string
|
||||||
|
{
|
||||||
|
return escapeshellarg($server->user).'@'.escapeshellarg($server->ip);
|
||||||
|
}
|
||||||
|
|
||||||
private static function isMultiplexingEnabled(): bool
|
private static function isMultiplexingEnabled(): bool
|
||||||
{
|
{
|
||||||
return config('constants.ssh.mux_enabled') && ! config('constants.coolify.is_windows_docker_desktop');
|
return config('constants.ssh.mux_enabled') && ! config('constants.coolify.is_windows_docker_desktop');
|
||||||
|
|
@ -224,9 +229,9 @@ private static function getCommonSshOptions(Server $server, string $sshKeyLocati
|
||||||
|
|
||||||
// Bruh
|
// Bruh
|
||||||
if ($isScp) {
|
if ($isScp) {
|
||||||
$options .= "-P {$server->port} ";
|
$options .= '-P '.escapeshellarg((string) $server->port).' ';
|
||||||
} else {
|
} else {
|
||||||
$options .= "-p {$server->port} ";
|
$options .= '-p '.escapeshellarg((string) $server->port).' ';
|
||||||
}
|
}
|
||||||
|
|
||||||
return $options;
|
return $options;
|
||||||
|
|
@ -245,7 +250,7 @@ public static function isConnectionHealthy(Server $server): bool
|
||||||
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
|
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
|
||||||
$healthCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
$healthCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
|
||||||
}
|
}
|
||||||
$healthCommand .= "{$server->user}@{$server->ip} 'echo \"health_check_ok\"'";
|
$healthCommand .= self::escapedUserAtHost($server)." 'echo \"health_check_ok\"'";
|
||||||
|
|
||||||
$process = Process::run($healthCommand);
|
$process = Process::run($healthCommand);
|
||||||
$isHealthy = $process->exitCode() === 0 && str_contains($process->output(), 'health_check_ok');
|
$isHealthy = $process->exitCode() === 0 && str_contains($process->output(), 'health_check_ok');
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@
|
||||||
use App\Models\PrivateKey;
|
use App\Models\PrivateKey;
|
||||||
use App\Models\Project;
|
use App\Models\Project;
|
||||||
use App\Models\Server as ModelsServer;
|
use App\Models\Server as ModelsServer;
|
||||||
|
use App\Rules\ValidServerIp;
|
||||||
use Illuminate\Http\Request;
|
use Illuminate\Http\Request;
|
||||||
use OpenApi\Attributes as OA;
|
use OpenApi\Attributes as OA;
|
||||||
use Stringable;
|
use Stringable;
|
||||||
|
|
@ -472,10 +473,10 @@ public function create_server(Request $request)
|
||||||
$validator = customApiValidator($request->all(), [
|
$validator = customApiValidator($request->all(), [
|
||||||
'name' => 'string|max:255',
|
'name' => 'string|max:255',
|
||||||
'description' => 'string|nullable',
|
'description' => 'string|nullable',
|
||||||
'ip' => 'string|required',
|
'ip' => ['string', 'required', new ValidServerIp],
|
||||||
'port' => 'integer|nullable',
|
'port' => 'integer|nullable|between:1,65535',
|
||||||
'private_key_uuid' => 'string|required',
|
'private_key_uuid' => 'string|required',
|
||||||
'user' => 'string|nullable',
|
'user' => ['string', 'nullable', 'regex:/^[a-zA-Z0-9_-]+$/'],
|
||||||
'is_build_server' => 'boolean|nullable',
|
'is_build_server' => 'boolean|nullable',
|
||||||
'instant_validate' => 'boolean|nullable',
|
'instant_validate' => 'boolean|nullable',
|
||||||
'proxy_type' => 'string|nullable',
|
'proxy_type' => 'string|nullable',
|
||||||
|
|
@ -637,10 +638,10 @@ public function update_server(Request $request)
|
||||||
$validator = customApiValidator($request->all(), [
|
$validator = customApiValidator($request->all(), [
|
||||||
'name' => 'string|max:255|nullable',
|
'name' => 'string|max:255|nullable',
|
||||||
'description' => 'string|nullable',
|
'description' => 'string|nullable',
|
||||||
'ip' => 'string|nullable',
|
'ip' => ['string', 'nullable', new ValidServerIp],
|
||||||
'port' => 'integer|nullable',
|
'port' => 'integer|nullable|between:1,65535',
|
||||||
'private_key_uuid' => 'string|nullable',
|
'private_key_uuid' => 'string|nullable',
|
||||||
'user' => 'string|nullable',
|
'user' => ['string', 'nullable', 'regex:/^[a-zA-Z0-9_-]+$/'],
|
||||||
'is_build_server' => 'boolean|nullable',
|
'is_build_server' => 'boolean|nullable',
|
||||||
'instant_validate' => 'boolean|nullable',
|
'instant_validate' => 'boolean|nullable',
|
||||||
'proxy_type' => 'string|nullable',
|
'proxy_type' => 'string|nullable',
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@
|
||||||
use App\Enums\ProxyTypes;
|
use App\Enums\ProxyTypes;
|
||||||
use App\Models\Server;
|
use App\Models\Server;
|
||||||
use App\Models\Team;
|
use App\Models\Team;
|
||||||
|
use App\Rules\ValidServerIp;
|
||||||
use App\Support\ValidationPatterns;
|
use App\Support\ValidationPatterns;
|
||||||
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
||||||
use Livewire\Attributes\Locked;
|
use Livewire\Attributes\Locked;
|
||||||
|
|
@ -55,8 +56,8 @@ protected function rules(): array
|
||||||
'new_private_key_value' => 'nullable|string',
|
'new_private_key_value' => 'nullable|string',
|
||||||
'name' => ValidationPatterns::nameRules(),
|
'name' => ValidationPatterns::nameRules(),
|
||||||
'description' => ValidationPatterns::descriptionRules(),
|
'description' => ValidationPatterns::descriptionRules(),
|
||||||
'ip' => 'required|string',
|
'ip' => ['required', 'string', new ValidServerIp],
|
||||||
'user' => 'required|string',
|
'user' => ['required', 'string', 'regex:/^[a-zA-Z0-9_-]+$/'],
|
||||||
'port' => 'required|integer|between:1,65535',
|
'port' => 'required|integer|between:1,65535',
|
||||||
'is_build_server' => 'required|boolean',
|
'is_build_server' => 'required|boolean',
|
||||||
];
|
];
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@
|
||||||
use App\Events\ServerReachabilityChanged;
|
use App\Events\ServerReachabilityChanged;
|
||||||
use App\Models\CloudProviderToken;
|
use App\Models\CloudProviderToken;
|
||||||
use App\Models\Server;
|
use App\Models\Server;
|
||||||
|
use App\Rules\ValidServerIp;
|
||||||
use App\Services\HetznerService;
|
use App\Services\HetznerService;
|
||||||
use App\Support\ValidationPatterns;
|
use App\Support\ValidationPatterns;
|
||||||
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
||||||
|
|
@ -106,9 +107,9 @@ protected function rules(): array
|
||||||
return [
|
return [
|
||||||
'name' => ValidationPatterns::nameRules(),
|
'name' => ValidationPatterns::nameRules(),
|
||||||
'description' => ValidationPatterns::descriptionRules(),
|
'description' => ValidationPatterns::descriptionRules(),
|
||||||
'ip' => 'required',
|
'ip' => ['required', new ValidServerIp],
|
||||||
'user' => 'required',
|
'user' => ['required', 'regex:/^[a-zA-Z0-9_-]+$/'],
|
||||||
'port' => 'required',
|
'port' => 'required|integer|between:1,65535',
|
||||||
'validationLogs' => 'nullable',
|
'validationLogs' => 'nullable',
|
||||||
'wildcardDomain' => 'nullable|url',
|
'wildcardDomain' => 'nullable|url',
|
||||||
'isReachable' => 'required',
|
'isReachable' => 'required',
|
||||||
|
|
|
||||||
|
|
@ -913,6 +913,9 @@ public function port(): Attribute
|
||||||
return Attribute::make(
|
return Attribute::make(
|
||||||
get: function ($value) {
|
get: function ($value) {
|
||||||
return (int) preg_replace('/[^0-9]/', '', $value);
|
return (int) preg_replace('/[^0-9]/', '', $value);
|
||||||
|
},
|
||||||
|
set: function ($value) {
|
||||||
|
return (int) preg_replace('/[^0-9]/', '', (string) $value);
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -922,6 +925,9 @@ public function user(): Attribute
|
||||||
return Attribute::make(
|
return Attribute::make(
|
||||||
get: function ($value) {
|
get: function ($value) {
|
||||||
return preg_replace('/[^A-Za-z0-9\-_]/', '', $value);
|
return preg_replace('/[^A-Za-z0-9\-_]/', '', $value);
|
||||||
|
},
|
||||||
|
set: function ($value) {
|
||||||
|
return preg_replace('/[^A-Za-z0-9\-_]/', '', $value);
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -931,6 +937,9 @@ public function ip(): Attribute
|
||||||
return Attribute::make(
|
return Attribute::make(
|
||||||
get: function ($value) {
|
get: function ($value) {
|
||||||
return preg_replace('/[^0-9a-zA-Z.:%-]/', '', $value);
|
return preg_replace('/[^0-9a-zA-Z.:%-]/', '', $value);
|
||||||
|
},
|
||||||
|
set: function ($value) {
|
||||||
|
return preg_replace('/[^0-9a-zA-Z.:%-]/', '', $value);
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
40
app/Rules/ValidServerIp.php
Normal file
40
app/Rules/ValidServerIp.php
Normal file
|
|
@ -0,0 +1,40 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
namespace App\Rules;
|
||||||
|
|
||||||
|
use Closure;
|
||||||
|
use Illuminate\Contracts\Validation\ValidationRule;
|
||||||
|
|
||||||
|
class ValidServerIp implements ValidationRule
|
||||||
|
{
|
||||||
|
/**
|
||||||
|
* Accepts a valid IPv4 address, IPv6 address, or RFC 1123 hostname.
|
||||||
|
*/
|
||||||
|
public function validate(string $attribute, mixed $value, Closure $fail): void
|
||||||
|
{
|
||||||
|
if (empty($value)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
$trimmed = trim($value);
|
||||||
|
|
||||||
|
if (filter_var($trimmed, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (filter_var($trimmed, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Delegate hostname validation to ValidHostname
|
||||||
|
$hostnameRule = new ValidHostname;
|
||||||
|
$failed = false;
|
||||||
|
$hostnameRule->validate($attribute, $trimmed, function () use (&$failed) {
|
||||||
|
$failed = true;
|
||||||
|
});
|
||||||
|
|
||||||
|
if ($failed) {
|
||||||
|
$fail('The :attribute must be a valid IPv4 address, IPv6 address, or hostname.');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
125
tests/Unit/SshCommandInjectionTest.php
Normal file
125
tests/Unit/SshCommandInjectionTest.php
Normal file
|
|
@ -0,0 +1,125 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use App\Helpers\SshMultiplexingHelper;
|
||||||
|
use App\Rules\ValidHostname;
|
||||||
|
use App\Rules\ValidServerIp;
|
||||||
|
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
// ValidServerIp rule
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
|
it('accepts a valid IPv4 address', function () {
|
||||||
|
$rule = new ValidServerIp;
|
||||||
|
$failed = false;
|
||||||
|
$rule->validate('ip', '192.168.1.1', function () use (&$failed) {
|
||||||
|
$failed = true;
|
||||||
|
});
|
||||||
|
expect($failed)->toBeFalse();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts a valid IPv6 address', function () {
|
||||||
|
$rule = new ValidServerIp;
|
||||||
|
$failed = false;
|
||||||
|
$rule->validate('ip', '2001:db8::1', function () use (&$failed) {
|
||||||
|
$failed = true;
|
||||||
|
});
|
||||||
|
expect($failed)->toBeFalse();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts a valid hostname', function () {
|
||||||
|
$rule = new ValidServerIp;
|
||||||
|
$failed = false;
|
||||||
|
$rule->validate('ip', 'my-server.example.com', function () use (&$failed) {
|
||||||
|
$failed = true;
|
||||||
|
});
|
||||||
|
expect($failed)->toBeFalse();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects injection payloads in server ip', function (string $payload) {
|
||||||
|
$rule = new ValidServerIp;
|
||||||
|
$failed = false;
|
||||||
|
$rule->validate('ip', $payload, function () use (&$failed) {
|
||||||
|
$failed = true;
|
||||||
|
});
|
||||||
|
expect($failed)->toBeTrue("ValidServerIp should reject: $payload");
|
||||||
|
})->with([
|
||||||
|
'semicolon' => ['192.168.1.1; rm -rf /'],
|
||||||
|
'pipe' => ['192.168.1.1 | cat /etc/passwd'],
|
||||||
|
'backtick' => ['192.168.1.1`id`'],
|
||||||
|
'dollar subshell' => ['192.168.1.1$(id)'],
|
||||||
|
'ampersand' => ['192.168.1.1 & id'],
|
||||||
|
'newline' => ["192.168.1.1\nid"],
|
||||||
|
'null byte' => ["192.168.1.1\0id"],
|
||||||
|
]);
|
||||||
|
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
// Server model setter casts
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
|
it('strips dangerous characters from server ip on write', function () {
|
||||||
|
$server = new App\Models\Server;
|
||||||
|
$server->ip = '192.168.1.1;rm -rf /';
|
||||||
|
// Regex [^0-9a-zA-Z.:%-] removes ; space and /; hyphen is allowed
|
||||||
|
expect($server->ip)->toBe('192.168.1.1rm-rf');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('strips dangerous characters from server user on write', function () {
|
||||||
|
$server = new App\Models\Server;
|
||||||
|
$server->user = 'root$(id)';
|
||||||
|
expect($server->user)->toBe('rootid');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('strips non-numeric characters from server port on write', function () {
|
||||||
|
$server = new App\Models\Server;
|
||||||
|
$server->port = '22; evil';
|
||||||
|
expect($server->port)->toBe(22);
|
||||||
|
});
|
||||||
|
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
// escapeshellarg() in generated SSH commands (source-level verification)
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
|
it('has escapedUserAtHost private static helper in SshMultiplexingHelper', function () {
|
||||||
|
$reflection = new ReflectionClass(SshMultiplexingHelper::class);
|
||||||
|
expect($reflection->hasMethod('escapedUserAtHost'))->toBeTrue();
|
||||||
|
|
||||||
|
$method = $reflection->getMethod('escapedUserAtHost');
|
||||||
|
expect($method->isPrivate())->toBeTrue();
|
||||||
|
expect($method->isStatic())->toBeTrue();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('wraps port with escapeshellarg in getCommonSshOptions', function () {
|
||||||
|
$reflection = new ReflectionClass(SshMultiplexingHelper::class);
|
||||||
|
$source = file_get_contents($reflection->getFileName());
|
||||||
|
|
||||||
|
expect($source)->toContain('escapeshellarg((string) $server->port)');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('has no raw user@ip string interpolation in SshMultiplexingHelper', function () {
|
||||||
|
$reflection = new ReflectionClass(SshMultiplexingHelper::class);
|
||||||
|
$source = file_get_contents($reflection->getFileName());
|
||||||
|
|
||||||
|
expect($source)->not->toContain('{$server->user}@{$server->ip}');
|
||||||
|
});
|
||||||
|
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
// ValidHostname rejects shell metacharacters
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
|
it('rejects semicolon in hostname', function () {
|
||||||
|
$rule = new ValidHostname;
|
||||||
|
$failed = false;
|
||||||
|
$rule->validate('hostname', 'example.com;id', function () use (&$failed) {
|
||||||
|
$failed = true;
|
||||||
|
});
|
||||||
|
expect($failed)->toBeTrue();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects backtick in hostname', function () {
|
||||||
|
$rule = new ValidHostname;
|
||||||
|
$failed = false;
|
||||||
|
$rule->validate('hostname', 'example.com`id`', function () use (&$failed) {
|
||||||
|
$failed = true;
|
||||||
|
});
|
||||||
|
expect($failed)->toBeTrue();
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue