diff --git a/ironic/drivers/modules/console_utils.py b/ironic/drivers/modules/console_utils.py index b59d099c7c..f43435f2f9 100644 --- a/ironic/drivers/modules/console_utils.py +++ b/ironic/drivers/modules/console_utils.py @@ -23,7 +23,6 @@ import fcntl import ipaddress import os -import shlex import signal import socket import subprocess @@ -388,13 +387,38 @@ def get_socat_console_url(port): 'port': port} +_SOCAT_METACHARACTERS = frozenset(',:!\'"()[]{}\\') + + +def _escape_socat_console_cmd(console_cmd): + """Escape socat's metacharacters in an ``EXEC:`` command line. + + socat lexes the command line twice, once when parsing the address + specification and again when splitting the ``EXEC:`` command line into + arguments, and each pass consumes one level of escaping. Spaces are left + alone, since they are what the second pass separates arguments at. No + shell is involved anywhere. + + Address syntax (separators, quoting, nesting and escaping): + http://www.dest-unreach.org/socat/doc/socat.html#ADDRESS_SPECIFICATIONS + ``EXEC:``, whose arguments are separated by single spaces: + http://www.dest-unreach.org/socat/doc/socat.html#ADDRESS_EXEC + + :param console_cmd: the command line for socat to execute + :return: the command line with socat's metacharacters escaped + """ + return ''.join('\\' * 3 + char if char in _SOCAT_METACHARACTERS else char + for char in console_cmd) + + def start_socat_console(node_uuid, port, console_cmd, env_variables=None): """Open the serial console for a node. :param node_uuid: the uuid of the node :param port: the terminal port for the node - :param console_cmd: the shell command that will be executed by socat to - establish console to the node + :param console_cmd: the command that will be executed by socat to + establish console to the node. Arguments are separated by single + spaces; socat runs the command directly, without a shell. :param env_variables: optional dict of environment variables to pass to the subprocess (e.g. IPMI_PASSWORD for ipmitool -E). :raises ConsoleError: if the directory for the PID file or the PID file @@ -430,8 +454,8 @@ def start_socat_console(node_uuid, port, console_cmd, env_variables=None): args.append(arg % {'host': console_host, 'port': port}) - quoted_cmd = shlex.quote(console_cmd) - args.append('EXEC:"%s",pty,stderr' % quoted_cmd) + escaped_cmd = _escape_socat_console_cmd(console_cmd) + args.append('EXEC:"%s",pty,stderr' % escaped_cmd) # run the command as a subprocess try: diff --git a/ironic/tests/unit/drivers/modules/test_console_utils.py b/ironic/tests/unit/drivers/modules/test_console_utils.py index c89bd3acad..d499a256b7 100644 --- a/ironic/tests/unit/drivers/modules/test_console_utils.py +++ b/ironic/tests/unit/drivers/modules/test_console_utils.py @@ -589,11 +589,24 @@ def _test_start_socat_console_check_arg(self, mock_timer_start, mock_popen.assert_called_once_with(mock.ANY, stderr=subprocess.PIPE) return mock_popen.call_args[0][0] + def test_start_socat_console_check_arg_command(self): + command = ('ipmitool -I lanplus -H 192.0.2.1 -L ADMINISTRATOR ' + '-U root -f /tmp/pw sol activate') + args = self._test_start_socat_console_check_arg(console_cmd=command) + self.assertEqual('EXEC:"%s",pty,stderr' % command, args[-1]) + def test_escape_start_socat_console_command(self): - command = ";cat /etc/passwd; && echo it\'s tricky" - quoted_command = ';cat /etc/passwd; && echo it\'"\'"\'s tricky' + command = ';cat /etc/passwd; && echo it\'s "tricky",su=root' + escaped_command = (';cat /etc/passwd; && echo it\\\\\\\'s ' + '\\\\\\"tricky\\\\\\"\\\\\\,su=root') args = self._test_start_socat_console_check_arg(console_cmd=command) - self.assertIn(quoted_command, args[-1]) + self.assertEqual('EXEC:"%s",pty,stderr' % escaped_command, args[-1]) + + def test_escape_socat_console_cmd_ipv6_address(self): + self.assertEqual( + 'ipmitool -H 2001\\\\\\:db8\\\\\\:\\\\\\:1 sol activate', + console_utils._escape_socat_console_cmd( + 'ipmitool -H 2001:db8::1 sol activate')) def test_start_socat_console_check_arg_default_timeout(self): args = self._test_start_socat_console_check_arg() diff --git a/releasenotes/notes/fix-socat-console-escaping-6468e69791f1325d.yaml b/releasenotes/notes/fix-socat-console-escaping-6468e69791f1325d.yaml new file mode 100644 index 0000000000..32182c90bb --- /dev/null +++ b/releasenotes/notes/fix-socat-console-escaping-6468e69791f1325d.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixes the ``socat`` serial console failing to start. The console command + was quoted for a shell, but ``socat`` executes it directly without one, so + it tried to run the whole quoted command line as a single program name and + failed with ``No such file or directory``. The command is now escaped for + ``socat``'s own address syntax instead, which keeps the arguments + separated while still preventing ``socat`` options from being injected + through it.