fix(realtime): harden terminal command handling
This commit is contained in:
parent
51a8a97d87
commit
aff581043f
6 changed files with 221 additions and 4 deletions
|
|
@ -4,7 +4,7 @@
|
|||
'coolify' => [
|
||||
'version' => env('COOLIFY_VERSION') ?: '4.3.14',
|
||||
'helper_version' => '1.0.16',
|
||||
'realtime_version' => '1.0.17',
|
||||
'realtime_version' => '1.0.18',
|
||||
'railpack_version' => '0.23.0',
|
||||
'self_hosted' => env('SELF_HOSTED', true),
|
||||
'autoupdate' => env('AUTOUPDATE'),
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ services:
|
|||
retries: 10
|
||||
timeout: 2s
|
||||
soketi:
|
||||
image: '${REGISTRY_URL:-docker.io}/coollabsio/coolify-realtime:1.0.17'
|
||||
image: '${REGISTRY_URL:-docker.io}/coollabsio/coolify-realtime:1.0.18'
|
||||
ports:
|
||||
- "${SOKETI_PORT:-6001}:6001"
|
||||
- "6002:6002"
|
||||
|
|
|
|||
|
|
@ -97,7 +97,7 @@ services:
|
|||
retries: 10
|
||||
timeout: 2s
|
||||
soketi:
|
||||
image: 'ghcr.io/coollabsio/coolify-realtime:1.0.17'
|
||||
image: 'ghcr.io/coollabsio/coolify-realtime:1.0.18'
|
||||
pull_policy: always
|
||||
container_name: coolify-realtime
|
||||
restart: always
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ import {
|
|||
extractTimeout,
|
||||
getTerminalSessionTimeout,
|
||||
isAuthorizedTargetHost,
|
||||
sanitizeSshArgs,
|
||||
validateSshArgs,
|
||||
} from './terminal-utils.js';
|
||||
|
||||
async function postToCoolify(path, headers) {
|
||||
|
|
@ -384,6 +386,16 @@ async function handleCommand(ws, command, userId) {
|
|||
return;
|
||||
}
|
||||
|
||||
if (!validateSshArgs(sshArgs, userSession.authorizedIPs)) {
|
||||
logTerminal('warn', 'Rejecting terminal command because its SSH arguments are not allowed.', {
|
||||
userId,
|
||||
targetHost,
|
||||
});
|
||||
ws.send('Invalid SSH command: Unsupported SSH arguments');
|
||||
return;
|
||||
}
|
||||
const sanitizedSshArgs = sanitizeSshArgs(sshArgs);
|
||||
|
||||
const options = {
|
||||
name: 'xterm-color',
|
||||
cols: 80,
|
||||
|
|
@ -401,7 +413,7 @@ async function handleCommand(ws, command, userId) {
|
|||
commandTimeout,
|
||||
terminalSessionTimeout,
|
||||
});
|
||||
const ptyProcess = pty.spawn('ssh', sshArgs.concat([hereDocContent]), options);
|
||||
const ptyProcess = pty.spawn('ssh', sanitizedSshArgs.concat([hereDocContent]), options);
|
||||
|
||||
userSession.ptyProcess = ptyProcess;
|
||||
userSession.isActive = true;
|
||||
|
|
|
|||
|
|
@ -131,3 +131,133 @@ export function isAuthorizedTargetHost(targetHost, authorizedHosts = []) {
|
|||
.map(host => normalizeHostForAuthorization(host))
|
||||
.includes(normalizedTargetHost);
|
||||
}
|
||||
|
||||
const REQUIRED_SSH_OPTIONS = new Set([
|
||||
'StrictHostKeyChecking',
|
||||
'UserKnownHostsFile',
|
||||
'PasswordAuthentication',
|
||||
'ConnectTimeout',
|
||||
'ServerAliveInterval',
|
||||
'RequestTTY',
|
||||
'LogLevel',
|
||||
]);
|
||||
|
||||
function isAllowedSshOption(name, value) {
|
||||
const fixedOptions = {
|
||||
StrictHostKeyChecking: 'no',
|
||||
UserKnownHostsFile: '/dev/null',
|
||||
PasswordAuthentication: 'no',
|
||||
LogLevel: 'ERROR',
|
||||
ControlMaster: 'auto',
|
||||
ProxyCommand: 'cloudflared access ssh --hostname %h',
|
||||
};
|
||||
|
||||
if (Object.hasOwn(fixedOptions, name)) {
|
||||
return value === fixedOptions[name];
|
||||
}
|
||||
|
||||
if (name === 'RequestTTY') {
|
||||
return value === 'yes' || value === 'no';
|
||||
}
|
||||
|
||||
if (name === 'ConnectTimeout' || name === 'ServerAliveInterval' || name === 'ControlPersist') {
|
||||
return /^\d+$/.test(value) && Number(value) > 0;
|
||||
}
|
||||
|
||||
if (name === 'ControlPath') {
|
||||
return /^\/var\/www\/html\/storage\/app\/ssh\/mux\/mux_[a-zA-Z0-9_-]+$/.test(value);
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
export function validateSshArgs(sshArgs, authorizedHosts = []) {
|
||||
if (!Array.isArray(sshArgs) || sshArgs.length === 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const seenOptions = new Set();
|
||||
let hasIdentityFile = false;
|
||||
let hasPort = false;
|
||||
let targetHost = null;
|
||||
|
||||
for (let index = 0; index < sshArgs.length; index++) {
|
||||
const argument = sshArgs[index];
|
||||
|
||||
if (typeof argument !== 'string' || /[\0\r\n]/.test(argument)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (argument === '-i') {
|
||||
const identityFile = sshArgs[++index];
|
||||
if (hasIdentityFile || !/^\/var\/www\/html\/storage\/app\/ssh\/keys\/ssh_key@[a-zA-Z0-9_-]+$/.test(identityFile ?? '')) {
|
||||
return false;
|
||||
}
|
||||
hasIdentityFile = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (argument === '-p') {
|
||||
const port = sshArgs[++index];
|
||||
if (hasPort || !/^\d+$/.test(port ?? '') || Number(port) < 1 || Number(port) > 65535) {
|
||||
return false;
|
||||
}
|
||||
hasPort = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (argument === '-o') {
|
||||
const option = sshArgs[++index];
|
||||
const separator = option?.indexOf('=') ?? -1;
|
||||
if (separator < 1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const name = option.slice(0, separator);
|
||||
const value = option.slice(separator + 1);
|
||||
if (seenOptions.has(name) || !isAllowedSshOption(name, value)) {
|
||||
return false;
|
||||
}
|
||||
seenOptions.add(name);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (/^[a-zA-Z0-9_][a-zA-Z0-9._-]*@[^@]+$/.test(argument) && targetHost === null) {
|
||||
targetHost = extractTargetHost([argument]);
|
||||
continue;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
const hasRequiredOptions = [...REQUIRED_SSH_OPTIONS].every(option => seenOptions.has(option));
|
||||
const hasCompleteMultiplexingOptions =
|
||||
!['ControlMaster', 'ControlPath', 'ControlPersist'].some(option => seenOptions.has(option))
|
||||
|| ['ControlMaster', 'ControlPath', 'ControlPersist'].every(option => seenOptions.has(option));
|
||||
|
||||
return hasIdentityFile
|
||||
&& hasPort
|
||||
&& targetHost !== null
|
||||
&& hasRequiredOptions
|
||||
&& hasCompleteMultiplexingOptions
|
||||
&& isAuthorizedTargetHost(targetHost, authorizedHosts);
|
||||
}
|
||||
|
||||
export function sanitizeSshArgs(sshArgs) {
|
||||
const multiplexingOptions = new Set(['ControlMaster', 'ControlPath', 'ControlPersist']);
|
||||
const sanitizedArgs = [];
|
||||
|
||||
for (let index = 0; index < sshArgs.length; index++) {
|
||||
if (sshArgs[index] === '-o') {
|
||||
const optionName = sshArgs[index + 1]?.split('=', 1)[0];
|
||||
if (multiplexingOptions.has(optionName)) {
|
||||
index++;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
sanitizedArgs.push(sshArgs[index]);
|
||||
}
|
||||
|
||||
return sanitizedArgs;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@ import {
|
|||
getTerminalSessionTimeout,
|
||||
isAuthorizedTargetHost,
|
||||
normalizeHostForAuthorization,
|
||||
sanitizeSshArgs,
|
||||
validateSshArgs,
|
||||
} from './terminal-utils.js';
|
||||
|
||||
test('extractTargetHost normalizes quoted IPv4 hosts from generated ssh commands', () => {
|
||||
|
|
@ -48,6 +50,79 @@ test('isAuthorizedTargetHost rejects hosts that are not in the allowlist', () =>
|
|||
assert.equal(isAuthorizedTargetHost("'10.0.0.9'", ['10.0.0.5']), false);
|
||||
});
|
||||
|
||||
test('validateSshArgs accepts the SSH arguments generated by Coolify', () => {
|
||||
const sshArgs = extractSshArgs(
|
||||
"timeout 3600 ssh -i /var/www/html/storage/app/ssh/keys/ssh_key@cm123 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no -o ConnectTimeout=10 -o ServerAliveInterval=20 -o RequestTTY=no -o LogLevel=ERROR -p '22' 'root'@'10.0.0.5' 'bash -se' << \\$abc\necho hi\nabc"
|
||||
);
|
||||
|
||||
assert.equal(validateSshArgs(sshArgs, ['10.0.0.5']), true);
|
||||
});
|
||||
|
||||
test('validateSshArgs rejects an injected ProxyCommand', () => {
|
||||
const sshArgs = extractSshArgs(
|
||||
"timeout 300 ssh -o 'ProxyCommand=/bin/busybox id >/tmp/marker' root@10.0.0.5 'bash -se' << \\ENDSSH\nENDSSH"
|
||||
);
|
||||
|
||||
assert.equal(validateSshArgs(sshArgs, ['10.0.0.5']), false);
|
||||
});
|
||||
|
||||
test('validateSshArgs accepts only the fixed Cloudflare ProxyCommand', () => {
|
||||
const validArgs = extractSshArgs(
|
||||
"timeout 3600 ssh -o ProxyCommand='cloudflared access ssh --hostname %h' -i /var/www/html/storage/app/ssh/keys/ssh_key@cm123 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no -o ConnectTimeout=10 -o ServerAliveInterval=20 -o RequestTTY=no -o LogLevel=ERROR -p 22 root@example.com 'bash -se' << \\$abc\necho hi\nabc"
|
||||
);
|
||||
const maliciousArgs = [...validArgs];
|
||||
maliciousArgs[1] = 'ProxyCommand=cloudflared access ssh --hostname %h; id';
|
||||
|
||||
assert.equal(validateSshArgs(validArgs, ['example.com']), true);
|
||||
assert.equal(validateSshArgs(maliciousArgs, ['example.com']), false);
|
||||
});
|
||||
|
||||
test('validateSshArgs rejects unknown SSH options and key paths', () => {
|
||||
const baseArgs = extractSshArgs(
|
||||
"timeout 3600 ssh -i /var/www/html/storage/app/ssh/keys/ssh_key@cm123 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no -o ConnectTimeout=10 -o ServerAliveInterval=20 -o RequestTTY=no -o LogLevel=ERROR -p 22 root@10.0.0.5 'bash -se' << \\$abc\necho hi\nabc"
|
||||
);
|
||||
|
||||
assert.equal(validateSshArgs(['-F', '/tmp/config', ...baseArgs], ['10.0.0.5']), false);
|
||||
assert.equal(validateSshArgs(['-i', '/tmp/attacker-key', ...baseArgs.slice(2)], ['10.0.0.5']), false);
|
||||
});
|
||||
|
||||
test('validateSshArgs rejects a destination that begins with an option prefix', () => {
|
||||
const sshArgs = extractSshArgs(
|
||||
"timeout 3600 ssh -i /var/www/html/storage/app/ssh/keys/ssh_key@cm123 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no -o ConnectTimeout=10 -o ServerAliveInterval=20 -o RequestTTY=no -o LogLevel=ERROR -p 22 -evil@10.0.0.5 'bash -se' << \\$abc\necho hi\nabc"
|
||||
);
|
||||
|
||||
assert.equal(validateSshArgs(sshArgs, ['10.0.0.5']), false);
|
||||
});
|
||||
|
||||
test('sanitizeSshArgs removes SSH multiplexing options before spawning SSH', () => {
|
||||
const sshArgs = extractSshArgs(
|
||||
"timeout 3600 ssh -o ControlMaster=auto -o ControlPath=/var/www/html/storage/app/ssh/mux/mux_cm123 -o ControlPersist=3600 -i /var/www/html/storage/app/ssh/keys/ssh_key@cm123 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no -o ConnectTimeout=10 -o ServerAliveInterval=20 -o RequestTTY=no -o LogLevel=ERROR -p 22 root@10.0.0.5 'bash -se' << \\$abc\necho hi\nabc"
|
||||
);
|
||||
|
||||
assert.equal(validateSshArgs(sshArgs, ['10.0.0.5']), true);
|
||||
assert.deepEqual(sanitizeSshArgs(sshArgs), [
|
||||
'-i',
|
||||
'/var/www/html/storage/app/ssh/keys/ssh_key@cm123',
|
||||
'-o',
|
||||
'StrictHostKeyChecking=no',
|
||||
'-o',
|
||||
'UserKnownHostsFile=/dev/null',
|
||||
'-o',
|
||||
'PasswordAuthentication=no',
|
||||
'-o',
|
||||
'ConnectTimeout=10',
|
||||
'-o',
|
||||
'ServerAliveInterval=20',
|
||||
'-o',
|
||||
'RequestTTY=yes',
|
||||
'-o',
|
||||
'LogLevel=ERROR',
|
||||
'-p',
|
||||
'22',
|
||||
'root@10.0.0.5',
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
test('getTerminalSessionTimeout always enforces the maximum terminal session lifetime', () => {
|
||||
assert.equal(getTerminalSessionTimeout(null), MAX_TERMINAL_SESSION_TIMEOUT_SECONDS);
|
||||
|
|
|
|||
Loading…
Reference in a new issue