Skip to content

Commit 985a085

Browse files
authored
Fix known_hosts race condition and command injection in console subprocess calls (#2137)
Use per-invocation known_hosts files for clush to avoid race conditions with concurrent SSH connections. Replace shell=True subprocess calls with list form to prevent command injection, and apply shlex.quote() to user-controlled values in remote shell commands. AI-assisted: Claude Code Signed-off-by: Christian Berendt <berendt@osism.tech>
1 parent 60675bb commit 985a085

1 file changed

Lines changed: 70 additions & 15 deletions

File tree

osism/commands/console.py

Lines changed: 70 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
# SPDX-License-Identifier: Apache-2.0
22

33
import json
4+
import os
5+
import shlex
6+
import shutil
47
import socket
58
import subprocess
9+
import tempfile
610
from typing import Optional
711

812
from cliff.command import Command
@@ -196,15 +200,40 @@ def take_action(self, parsed_args):
196200
type_console = "clush"
197201
host = host[1:]
198202

199-
ssh_options = f"-o StrictHostKeyChecking=no -o LogLevel=ERROR -o UserKnownHostsFile={KNOWN_HOSTS_PATH}"
203+
ssh_options = [
204+
"-o",
205+
"StrictHostKeyChecking=no",
206+
"-o",
207+
"LogLevel=ERROR",
208+
"-o",
209+
f"UserKnownHostsFile={KNOWN_HOSTS_PATH}",
210+
]
200211

201212
if type_console == "ansible":
202-
subprocess.call(f"/run-ansible-console.sh {host}", shell=True)
213+
subprocess.call(["/run-ansible-console.sh", host])
203214
elif type_console == "clush":
204-
subprocess.call(
205-
f"/usr/local/bin/clush -l {settings.OPERATOR_USER} -g {host}",
206-
shell=True,
207-
)
215+
# Create a per-invocation known_hosts file to avoid race conditions
216+
# with fanout:64 concurrent SSH connections while still persisting
217+
# host keys during the session.
218+
fd, tmp_known_hosts = tempfile.mkstemp(prefix="clush_known_hosts_")
219+
try:
220+
os.close(fd)
221+
if os.path.exists(KNOWN_HOSTS_PATH):
222+
shutil.copy2(KNOWN_HOSTS_PATH, tmp_known_hosts)
223+
subprocess.call(
224+
[
225+
"/usr/local/bin/clush",
226+
"-l",
227+
settings.OPERATOR_USER,
228+
"-o",
229+
f"-o UserKnownHostsFile={tmp_known_hosts}",
230+
"-g",
231+
host,
232+
]
233+
)
234+
finally:
235+
if os.path.exists(tmp_known_hosts):
236+
os.unlink(tmp_known_hosts)
208237
elif type_console == "ssh":
209238
# Try to resolve as an inventory group
210239
group_hosts = get_hosts_from_group(host)
@@ -221,35 +250,61 @@ def take_action(self, parsed_args):
221250
resolved_host = resolve_host_with_fallback(host)
222251
# FIXME: use paramiko or something else more Pythonic + make operator user + key configurable
223252
subprocess.call(
224-
f"/usr/bin/ssh -i /ansible/secrets/id_rsa.operator {ssh_options} {settings.OPERATOR_USER}@{resolved_host}",
225-
shell=True,
253+
[
254+
"/usr/bin/ssh",
255+
"-i",
256+
"/ansible/secrets/id_rsa.operator",
257+
*ssh_options,
258+
f"{settings.OPERATOR_USER}@{resolved_host}",
259+
]
226260
)
227261
elif type_console == "container_prompt":
228262
while True:
229263
command = prompt(f"{host[:-1]}>>> ")
230264
if command in ["Exit", "exit", "EXIT"]:
231265
break
232266

233-
ssh_command = f"docker {command}"
267+
ssh_command = f"docker {shlex.quote(command)}"
234268
# Resolve hostname with Netbox fallback
235269
resolved_host = resolve_host_with_fallback(host[:-1])
236270
# FIXME: use paramiko or something else more Pythonic + make operator user + key configurable
237271
subprocess.call(
238-
f"/usr/bin/ssh -i /ansible/secrets/id_rsa.operator {ssh_options} {settings.OPERATOR_USER}@{resolved_host} {ssh_command}",
239-
shell=True,
272+
[
273+
"/usr/bin/ssh",
274+
"-i",
275+
"/ansible/secrets/id_rsa.operator",
276+
*ssh_options,
277+
f"{settings.OPERATOR_USER}@{resolved_host}",
278+
ssh_command,
279+
]
240280
)
241281
elif type_console == "container":
242282
target_containername = host.split("/")[1]
243283
target_host = host.split("/")[0]
244284
target_command = "bash"
245285

246-
ssh_command = f"docker exec -it {target_containername} {target_command}"
247-
ssh_options = f"-o RequestTTY=force -o StrictHostKeyChecking=no -o LogLevel=ERROR -o UserKnownHostsFile={KNOWN_HOSTS_PATH}"
286+
ssh_command = f"docker exec -it {shlex.quote(target_containername)} {shlex.quote(target_command)}"
287+
ssh_options = [
288+
"-o",
289+
"RequestTTY=force",
290+
"-o",
291+
"StrictHostKeyChecking=no",
292+
"-o",
293+
"LogLevel=ERROR",
294+
"-o",
295+
f"UserKnownHostsFile={KNOWN_HOSTS_PATH}",
296+
]
248297

249298
# Resolve hostname with Netbox fallback
250299
resolved_target_host = resolve_host_with_fallback(target_host)
251300
# FIXME: use paramiko or something else more Pythonic + make operator user + key configurable
252301
subprocess.call(
253-
f"/usr/bin/ssh -i /ansible/secrets/id_rsa.operator {ssh_options} {settings.OPERATOR_USER}@{resolved_target_host} {ssh_command}",
254-
shell=True,
302+
[
303+
"/usr/bin/ssh",
304+
"-i",
305+
"/ansible/secrets/id_rsa.operator",
306+
*ssh_options,
307+
f"{settings.OPERATOR_USER}@{resolved_target_host}",
308+
ssh_command,
309+
]
255310
)

0 commit comments

Comments
 (0)