Merge pull request #56070 from phlogistonjohn/jjm-cephadm-sudo-cmd-auditing

cephadm: sudo cmd auditing

Reviewed-by: Adam King <adking@redhat.com>

doc/cephadm/operations.rst

@@ -658,6 +658,51 @@ For example, to distribute configs to hosts with the ``bare_config`` label, run
 
 (See :ref:`orchestrator-cli-placement-spec` for more information about placement specs.)
 
+
+Limiting Password-less sudo Access
+==================================
+
+By default, the cephadm install guide recommends enabling password-less
+``sudo`` for the cephadm user. This option is the most flexible and
+future-proof but may not be preferred in all environments. An administrator can
+restrict ``sudo`` to only running an exact list of commands without password
+access.  Note that this list may change between Ceph versions and
+administrators choosing this option should read the release notes and review
+this list in the destination version of the Ceph documentation. If the list
+differs one must extend the list of password-less ``sudo`` commands prior to
+upgrade.
+
+Commands requiring password-less sudo support:
+
+  - ``chmod``
+  - ``chown``
+  - ``ls``
+  - ``mkdir``
+  - ``mv``
+  - ``rm``
+  - ``sysctl``
+  - ``touch``
+  - ``true``
+  - ``which`` (see note)
+  - ``/usr/bin/cephadm`` or python executable (see note)
+
+.. note:: Typically cephadm will execute ``which`` to determine what python3
+   command is available and then use the command returned by ``which`` in
+   subsequent commands.
+   Before configuring ``sudo`` run ``which python3`` to determine what
+   python command to add to the ``sudo`` configuration.
+   In some rare configurations ``/usr/bin/cephadm`` will be used instead.
+
+
+Configuring the ``sudoers`` file can be performed using a tool like ``visudo``
+and adding or replacing a user configuration line such as the following:
+
+.. code-block::
+
+  # assuming the cephadm user is named "ceph"
+  ceph ALL=(ALL) NOPASSWD:/usr/bin/chmod,/usr/bin/chown,/usr/bin/ls,/usr/bin/mkdir,/usr/bin/mv,/usr/bin/rm,/usr/sbin/sysctl,/usr/bin/touch,/usr/bin/true,/usr/bin/which,/usr/bin/cephadm,/usr/bin/python3
+
+
 Purging a cluster
 =================
 

src/pybind/mgr/cephadm/offline_watcher.py

@@ -4,6 +4,8 @@ from typing import List, Optional, TYPE_CHECKING
 import multiprocessing as mp
 import threading
 
+from . import ssh
+
 if TYPE_CHECKING:
     from cephadm.module import CephadmOrchestrator
 
@@ -38,7 +40,8 @@ class OfflineHostWatcher(threading.Thread):
     def check_host(self, host: str) -> None:
         if host not in self.mgr.offline_hosts:
             try:
-                self.mgr.ssh.check_execute_command(host, ['true'], log_command=self.mgr.log_refresh_metadata)
+                rcmd = ssh.RemoteCommand(ssh.Executables.TRUE)
+                self.mgr.ssh.check_execute_command(host, rcmd, log_command=self.mgr.log_refresh_metadata)
             except Exception:
                 logger.debug(f'OfflineHostDetector: detected {host} to be offline')
                 # kick serve loop in case corrective action must be taken for offline host

src/pybind/mgr/cephadm/serve.py

@@ -34,6 +34,7 @@ from mgr_util import format_bytes, verify_tls, get_cert_issuer_info, ServerConfi
 
 from . import utils
 from . import exchange
+from . import ssh
 
 if TYPE_CHECKING:
     from cephadm.module import CephadmOrchestrator
@@ -42,6 +43,9 @@ logger = logging.getLogger(__name__)
 
 REQUIRES_POST_ACTIONS = ['grafana', 'iscsi', 'prometheus', 'alertmanager', 'rgw', 'nvmeof']
 
+WHICH = ssh.RemoteExecutable('which')
+CEPHADM_EXE = ssh.RemoteExecutable('/usr/bin/cephadm')
+
 
 class CephadmServe:
     """
@@ -1272,7 +1276,7 @@ class CephadmServe:
             if path == '/etc/ceph/ceph.conf':
                 continue
             self.log.info(f'Removing {host}:{path}')
-            cmd = ['rm', '-f', path]
+            cmd = ssh.RemoteCommand(ssh.Executables.RM, ['-f', path])
             self.mgr.ssh.check_execute_command(host, cmd)
             updated_files = True
             self.mgr.cache.removed_client_file(host, path)
@@ -1621,15 +1625,24 @@ class CephadmServe:
             if stdin and 'agent' not in str(entity):
                 self.log.debug('stdin: %s' % stdin)
 
-            cmd = ['which', 'python3']
+            cmd = ssh.RemoteCommand(WHICH, ['python3'])
             python = await self.mgr.ssh._check_execute_command(host, cmd, addr=addr)
-            cmd = [python, self.mgr.cephadm_binary_path] + final_args
+            # N.B. because the python3 executable is based on the results of the
+            # which command we can not know it ahead of time and must be converted
+            # into a RemoteExecutable.
+            cmd = ssh.RemoteCommand(
+                ssh.RemoteExecutable(python),
+                [self.mgr.cephadm_binary_path] + final_args
+            )
 
             try:
                 out, err, code = await self.mgr.ssh._execute_command(
                     host, cmd, stdin=stdin, addr=addr)
                 if code == 2:
-                    ls_cmd = ['ls', self.mgr.cephadm_binary_path]
+                    ls_cmd = ssh.RemoteCommand(
+                        ssh.Executables.LS,
+                        [self.mgr.cephadm_binary_path]
+                    )
                     out_ls, err_ls, code_ls = await self.mgr.ssh._execute_command(host, ls_cmd, addr=addr,
                                                                                   log_command=log_output)
                     if code_ls == 2:
@@ -1650,7 +1663,7 @@ class CephadmServe:
 
         elif self.mgr.mode == 'cephadm-package':
             try:
-                cmd = ['/usr/bin/cephadm'] + final_args
+                cmd = ssh.RemoteCommand(CEPHADM_EXE, final_args)
                 out, err, code = await self.mgr.ssh._execute_command(
                     host, cmd, stdin=stdin, addr=addr)
             except Exception as e:

src/pybind/mgr/cephadm/ssh.py

@@ -1,3 +1,4 @@
+import enum
 import logging
 import os
 import asyncio
@@ -44,6 +45,77 @@ Host *
 """
 
 
+class RemoteExecutable(str):
+    pass
+
+
+class RemoteCommand:
+    exe: RemoteExecutable
+    args: List[str]
+
+    def __init__(self, exe: RemoteExecutable, args: Optional[List[str]] = None) -> None:
+        self.exe = exe
+        self.args = args or []
+
+    def __iter__(self) -> Iterator[str]:
+        yield str(self.exe)
+        for arg in self.args:
+            yield arg
+
+    def quoted(self) -> Iterator[str]:
+        return (quote(a) for a in self)
+
+    def __str__(self) -> str:
+        return " ".join(self.quoted())
+
+    def __repr__(self) -> str:
+        # handy when debugging tests
+        return f'<RemoteCommand>({self.exe!r}, {self.args!r})'
+
+    def __eq__(self, other: object) -> bool:
+        # handy when working with unit tests
+        if not isinstance(other, self.__class__):
+            return NotImplemented
+        return other.exe == self.exe and other.args == self.args
+
+
+class RemoteSudoCommand(RemoteCommand):
+    use_sudo: bool = True
+
+    def __init__(
+        self, exe: RemoteExecutable, args: List[str], use_sudo: bool = True
+    ) -> None:
+        super().__init__(exe, args)
+        self.use_sudo = use_sudo
+
+    def __iter__(self) -> Iterator[str]:
+        if self.use_sudo:
+            yield 'sudo'
+        for a in super().__iter__():
+            yield a
+
+    @classmethod
+    def wrap(
+        cls, other: RemoteCommand, use_sudo: bool = True
+    ) -> 'RemoteSudoCommand':
+        return cls(other.exe, other.args, use_sudo)
+
+
+class Executables(RemoteExecutable, enum.Enum):
+    CHMOD = RemoteExecutable('chmod')
+    CHOWN = RemoteExecutable('chown')
+    LS = RemoteExecutable('ls')
+    MKDIR = RemoteExecutable('mkdir')
+    MV = RemoteExecutable('mv')
+    RM = RemoteExecutable('rm')
+    SYSCTL = RemoteExecutable('sysctl')
+    TOUCH = RemoteExecutable('touch')
+    TRUE = RemoteExecutable('true')
+
+    def __str__(self) -> str:
+        return self.value
+
+
 class EventLoopThread(Thread):
 
     def __init__(self) -> None:
@@ -152,24 +224,27 @@ class SSHManager:
 
     async def _execute_command(self,
                                host: str,
-                               cmd_components: List[str],
+                               cmd_components: RemoteCommand,
                                stdin: Optional[str] = None,
                                addr: Optional[str] = None,
                                log_command: Optional[bool] = True,
                                ) -> Tuple[str, str, int]:
 
         conn = await self._remote_connection(host, addr)
-        sudo_prefix = "sudo " if self.mgr.ssh_user != 'root' else ""
-        cmd = sudo_prefix + " ".join(quote(x) for x in cmd_components)
+        use_sudo = (self.mgr.ssh_user != 'root')
+        rcmd = RemoteSudoCommand.wrap(cmd_components, use_sudo=use_sudo)
         try:
             address = addr or self.mgr.inventory.get_addr(host)
         except Exception:
             address = host
         if log_command:
-            logger.debug(f'Running command: {cmd}')
+            logger.debug(f'Running command: {rcmd}')
         try:
-            r = await conn.run(f'{sudo_prefix}true', check=True, timeout=5)  # host quick check
-            r = await conn.run(cmd, input=stdin)
+            test_cmd = RemoteSudoCommand(
+                Executables.TRUE, [], use_sudo=use_sudo
+            )
+            r = await conn.run(str(test_cmd), check=True, timeout=5)  # host quick check
+            r = await conn.run(str(rcmd), input=stdin)
         # handle these Exceptions otherwise you might get a weird error like
         # TypeError: __init__() missing 1 required positional argument: 'reason' (due to the asyncssh error interacting with raise_if_exception)
         except asyncssh.ChannelOpenError as e:
@@ -179,13 +254,13 @@ class SSHManager:
             self.mgr.offline_hosts.add(host)
             raise HostConnectionError(f'Unable to reach remote host {host}. {str(e)}', host, address)
         except asyncssh.ProcessError as e:
-            msg = f"Cannot execute the command '{cmd}' on the {host}. {str(e.stderr)}."
+            msg = f"Cannot execute the command '{rcmd}' on the {host}. {str(e.stderr)}."
             logger.debug(msg)
             await self._reset_con(host)
             self.mgr.offline_hosts.add(host)
             raise HostConnectionError(msg, host, address)
         except Exception as e:
-            msg = f"Generic error while executing command '{cmd}' on the host {host}. {str(e)}."
+            msg = f"Generic error while executing command '{rcmd}' on the host {host}. {str(e)}."
             logger.debug(msg)
             await self._reset_con(host)
             self.mgr.offline_hosts.add(host)
@@ -209,7 +284,7 @@ class SSHManager:
 
     def execute_command(self,
                         host: str,
-                        cmd: List[str],
+                        cmd: RemoteCommand,
                         stdin: Optional[str] = None,
                         addr: Optional[str] = None,
                         log_command: Optional[bool] = True
@@ -219,7 +294,7 @@ class SSHManager:
 
     async def _check_execute_command(self,
                                      host: str,
-                                     cmd: List[str],
+                                     cmd: RemoteCommand,
                                      stdin: Optional[str] = None,
                                      addr: Optional[str] = None,
                                      log_command: Optional[bool] = True
@@ -233,7 +308,7 @@ class SSHManager:
 
     def check_execute_command(self,
                               host: str,
-                              cmd: List[str],
+                              cmd: RemoteCommand,
                               stdin: Optional[str] = None,
                               addr: Optional[str] = None,
                               log_command: Optional[bool] = True,
@@ -253,14 +328,22 @@ class SSHManager:
         try:
             cephadm_tmp_dir = f"/tmp/cephadm-{self.mgr._cluster_fsid}"
             dirname = os.path.dirname(path)
-            await self._check_execute_command(host, ['mkdir', '-p', dirname], addr=addr)
-            await self._check_execute_command(host, ['mkdir', '-p', cephadm_tmp_dir + dirname], addr=addr)
+            mkdir = RemoteCommand(Executables.MKDIR, ['-p', dirname])
+            await self._check_execute_command(host, mkdir, addr=addr)
+            mkdir2 = RemoteCommand(Executables.MKDIR, ['-p', cephadm_tmp_dir + dirname])
+            await self._check_execute_command(host, mkdir2, addr=addr)
             tmp_path = cephadm_tmp_dir + path + '.new'
-            await self._check_execute_command(host, ['touch', tmp_path], addr=addr)
+            touch = RemoteCommand(Executables.TOUCH, [tmp_path])
+            await self._check_execute_command(host, touch, addr=addr)
             if self.mgr.ssh_user != 'root':
                 assert self.mgr.ssh_user
-                await self._check_execute_command(host, ['chown', '-R', self.mgr.ssh_user, cephadm_tmp_dir], addr=addr)
-                await self._check_execute_command(host, ['chmod', str(644), tmp_path], addr=addr)
+                chown = RemoteCommand(
+                    Executables.CHOWN,
+                    ['-R', self.mgr.ssh_user, cephadm_tmp_dir]
+                )
+                await self._check_execute_command(host, chown, addr=addr)
+                chmod = RemoteCommand(Executables.CHMOD, [str(644), tmp_path])
+                await self._check_execute_command(host, chmod, addr=addr)
             with NamedTemporaryFile(prefix='cephadm-write-remote-file-') as f:
                 os.fchmod(f.fileno(), 0o600)
                 f.write(content)
@@ -270,9 +353,15 @@ class SSHManager:
                     await sftp.put(f.name, tmp_path)
             if uid is not None and gid is not None and mode is not None:
                 # shlex quote takes str or byte object, not int
-                await self._check_execute_command(host, ['chown', '-R', str(uid) + ':' + str(gid), tmp_path], addr=addr)
-                await self._check_execute_command(host, ['chmod', oct(mode)[2:], tmp_path], addr=addr)
-            await self._check_execute_command(host, ['mv', tmp_path, path], addr=addr)
+                chown = RemoteCommand(
+                    Executables.CHOWN,
+                    ['-R', str(uid) + ':' + str(gid), tmp_path]
+                )
+                await self._check_execute_command(host, chown, addr=addr)
+                chmod = RemoteCommand(Executables.CHMOD, [oct(mode)[2:], tmp_path])
+                await self._check_execute_command(host, chmod, addr=addr)
+            mv = RemoteCommand(Executables.MV, [tmp_path, path])
+            await self._check_execute_command(host, mv, addr=addr)
         except Exception as e:
             msg = f"Unable to write {host}:{path}: {e}"
             logger.exception(msg)

src/pybind/mgr/cephadm/tests/test_remote_executables.py

@@ -0,0 +1,262 @@
+#!/usr/bin/python3
+"""Test to ensure remote suodable executables are audited.
+
+This file can be used in one of two ways:
+* as a "unit test" executed by pytest
+* as a script that can report on or check expected remote executables
+
+It is designed to act as a method of ensuring that the executables that we run
+on remote nodes under sudo are explicitly known. The types defined in ssh.py
+act as both a way to audit the commands (via this script) and that we don't
+lose track of what can be run - by relying on mypy.
+
+The unit test mode integrates into pytest for convenience, it really acts as a
+static check that scans the source code of the cephadm mgr module.  NOTE: the
+file's test script mode is sensitive to it's location in the source tree. If
+files get moved this script may need to be updated.
+
+The test asserts that the `EXPECTED` list matches all tools that may be
+executed remotely under sudo.
+
+This file can also be run as a script. When supplied with a directory or list
+of files it will read the sources and report on all remote executables found.
+When run with the `--check` option it performs the same job as the unit test
+mode but outputs a report in a more human readable format.
+
+If the commands the manager module can execute remotely change the `EXPECTED`
+this must be updated to match to ensure the change is being done deliberately.
+Any corresponding documentation should also be updated.
+
+Note that ideally the EXPECTED list should shrink and not grow.  Any changes to
+the list may cause administrators of the ceph cluster to have to make manual
+changes to the system prior/during upgrade!
+"""
+import ast
+import os
+import pathlib
+import sys
+
+ssh_py = 'ssh.py'
+serve_py = 'serve.py'
+
+# IMPORTANT - please read the entire module docstring before changing
+# the list below.
+EXPECTED = [
+    # - value | is_constant | filename -
+    # constant executables
+    ('/usr/bin/cephadm', True, serve_py),
+    ('chmod', True, ssh_py),
+    ('chown', True, ssh_py),
+    ('ls', True, ssh_py),
+    ('mkdir', True, ssh_py),
+    ('mv', True, ssh_py),
+    ('rm', True, ssh_py),
+    ('sysctl', True, ssh_py),
+    ('touch', True, ssh_py),
+    ('true', True, ssh_py),
+    ('which', True, serve_py),
+    # variable executables
+    ('python', False, serve_py),
+]
+
+
+def test_expected_remote_executables():
+    import pytest
+
+    if sys.version_info < (3, 8):
+        pytest.skip("python 3.8 or later required")
+
+    self_path = pathlib.Path(__file__).resolve()
+    # test is sensitive to where it is in the source tree. it expects to be in
+    # a tests directory under the cephadm mgr module. if stuff gets moved
+    # around this test will likely start failing and need to be updated
+    remote_exes = find_sudoable_exes_in_files(
+        _file_paths([self_path.parent.parent])
+    )
+    unexpected, gone = diff_remote_exes(remote_exes)
+    unexpected_msgs = gone_msgs = []
+    if unexpected or gone:
+        unexpected_msgs, gone_msgs = format_diff(
+            unexpected, gone, remote_exes
+        )
+    assert not unexpected_msgs, unexpected_msgs
+    assert not gone_msgs, gone_msgs
+
+
+def _essential(v):
+    """Convert a remote exe dict to a tuple with only the essential fields."""
+    return (v["value"], v["is_constant"], v["filename"].split("/")[-1])
+
+
+def _names(node):
+    if isinstance(node, ast.Name):
+        return [node.id]
+    if isinstance(node, ast.Attribute):
+        vn = _names(node.value)
+        return vn + [node.attr]
+    if isinstance(node, ast.Call):
+        return _names(node.func)
+    if isinstance(node, ast.Constant):
+        return [repr(node.value)]
+    if isinstance(node, ast.JoinedStr):
+        return [f"<JoinedStr: {node.values!r}>"]
+    if isinstance(node, ast.Subscript):
+        return [f"<Subscript: {node.value}{node.slice}>"]
+    raise ValueError(f"_names: unexpected type: {node}")
+
+
+def _arg_kind(node):
+    assert isinstance(node, ast.Call)
+    assert len(node.args) == 1
+    arg = node.args[0]
+    if isinstance(arg, ast.Constant):
+        return str(arg.value), True
+    names = _names(arg)
+    return ".".join(names), False
+
+
+class ExecutableVisitor(ast.NodeVisitor):
+    def __init__(self):
+        super().__init__()
+        self.remote_executables = []
+
+    def visit_Call(self, node):
+        names = _names(node)
+        if names[-1] == 'RemoteExecutable':
+            value, is_constant = _arg_kind(node)
+            self.remote_executables.append(
+                dict(value=value, is_constant=is_constant, lineno=node.lineno)
+            )
+        self.generic_visit(node)
+
+
+def find_sudoable_exes(tree):
+    ev = ExecutableVisitor()
+    ev.visit(tree)
+    return ev.remote_executables
+
+
+def find_sudoable_exes_in_files(files):
+    out = []
+    for file in files:
+        with open(file) as fh:
+            source = fh.read()
+        tree = ast.parse(source, fh.name)
+        rmes = find_sudoable_exes(tree)
+        for rme in rmes:
+            rme['filename'] = str(file)
+        out.extend(rmes)
+    return out
+
+
+def diff_remote_exes(remote_exes):
+    expected = set(EXPECTED)
+    current = {_essential(v) for v in remote_exes}
+    unexpected = current - expected
+    gone = expected - current
+    return unexpected, gone
+
+
+def format_diff(unexpected, gone, remote_exes):
+    current = {_essential(v): v for v in remote_exes}
+    unexpected_msgs = []
+    for val, is_constant, fn in unexpected:
+        vn = 'constant' if is_constant else 'variable'
+        vq = repr(val) if is_constant else val
+        # info is needed for full filename/linenumber and is only relevant for
+        # found (unexpected) entries
+        info = current[(val, is_constant, fn)]
+        unexpected_msgs.append(
+            f'{vn} {vq} in {info["filename"]}:{info["lineno"]} not tracked'
+        )
+    gone_msgs = []
+    for val, is_constant, fn in gone:
+        vn = 'constant' if is_constant else 'variable'
+        vq = repr(val) if is_constant else val
+        gone_msgs.append(f"{vn} {vq} expected in {fn} not found")
+    return unexpected_msgs, gone_msgs
+
+
+def report_remote_exes(remote_exes):
+    for rme in remote_exes:
+        clabel = 'CONSTANT' if rme["is_constant"] else "VARIABLE"
+        print(
+            "{clabel:10} {value:10} {filename}:{lineno}".format(
+                clabel=clabel, **rme
+            )
+        )
+
+
+def report_compare_remote_exes(remote_exes):
+    import textwrap
+
+    unexpected, gone = diff_remote_exes(remote_exes)
+    if not (unexpected or gone):
+        print('No issues detected')
+        sys.exit(0)
+    unexpected_msgs, gone_msgs = format_diff(unexpected, gone, remote_exes)
+    if unexpected_msgs:
+        desc = textwrap.wrap(
+            "One or more remote executable has been detected in the source"
+            " files that is not tracked in the test. If this change is"
+            " intended you must update the test AND update any corresponding"
+            " documentation.",
+            76,
+        )
+        for line in desc:
+            print(line)
+        print('-' * 76)
+    for msg in unexpected_msgs:
+        print(f'* {msg}')
+    if unexpected_msgs:
+        print()
+    if gone_msgs:
+        desc = textwrap.wrap(
+            "One or more remote executable that is expected to appear"
+            " in the source files has not been detected."
+            " If this change is intended you must update the test AND update"
+            " any corresponding documentation.",
+            76,
+        )
+        for line in desc:
+            print(line)
+        print('-' * 76)
+    for msg in gone_msgs:
+        print(f'* {msg}')
+
+    sys.exit(1)
+
+
+def _file_paths(src_paths):
+    files = set()
+    for path in src_paths:
+        if path.is_file():
+            files.add(path)
+            continue
+        for d, ds, fs in os.walk(path):
+            if 'tests' in ds:
+                ds.remove('tests')
+            dpath = pathlib.Path(d)
+            for fn in fs:
+                if fn.endswith('.py'):
+                    files.add(dpath / fn)
+    return files
+
+
+def main():
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--check', action='store_true')
+    parser.add_argument('PATH', nargs='+', type=pathlib.Path)
+    cli = parser.parse_args()
+
+    remote_exes = find_sudoable_exes_in_files(_file_paths(cli.PATH))
+    if cli.check:
+        report_compare_remote_exes(remote_exes)
+    else:
+        report_remote_exes(remote_exes)
+
+
+if __name__ == '__main__':
+    main()

src/pybind/mgr/cephadm/tests/test_ssh.py

@@ -103,3 +103,14 @@ class TestWithSSH:
 class TestWithoutSSH:
     def test_can_run(self, cephadm_module: CephadmOrchestrator):
         assert cephadm_module.can_run() == (False, "loading asyncssh library:No module named 'asyncssh'")
+
+
+def test_remote_command():
+    from cephadm.ssh import RemoteCommand, Executables
+
+    assert list(RemoteCommand(Executables.TRUE)) == ['true']
+    assert list(RemoteCommand(Executables.RM, ['-rf', '/tmp/blat'])) == [
+        'rm',
+        '-rf',
+        '/tmp/blat',
+    ]

src/pybind/mgr/cephadm/tests/test_tuned_profiles.py

@@ -5,7 +5,7 @@ from cephadm.tuned_profiles import TunedProfileUtils, SYSCTL_DIR
 from cephadm.inventory import TunedProfileStore
 from ceph.utils import datetime_now
 from ceph.deployment.service_spec import TunedProfileSpec, PlacementSpec
-from cephadm.ssh import SSHManager
+from cephadm.ssh import SSHManager, RemoteCommand, Executables
 from orchestrator import HostSpec
 
 from typing import List, Dict
@@ -148,10 +148,26 @@ class TestTunedProfiles:
         tp = TunedProfileUtils(mgr)
         tp._remove_stray_tuned_profiles('a', self.profiles_to_calls(tp, [self.tspec1, self.tspec2]))
         calls = [
-            mock.call('a', ['ls', SYSCTL_DIR], log_command=False),
-            mock.call('a', ['rm', '-f', f'{SYSCTL_DIR}/p3-cephadm-tuned-profile.conf']),
-            mock.call('a', ['rm', '-f', f'{SYSCTL_DIR}/who-cephadm-tuned-profile.conf']),
-            mock.call('a', ['sysctl', '--system'])
+            mock.call(
+                'a', RemoteCommand(Executables.LS, [SYSCTL_DIR]), log_command=False
+            ),
+            mock.call(
+                'a',
+                RemoteCommand(
+                    Executables.RM,
+                    ['-f', f'{SYSCTL_DIR}/p3-cephadm-tuned-profile.conf']
+                )
+            ),
+            mock.call(
+                'a',
+                RemoteCommand(
+                    Executables.RM,
+                    ['-f', f'{SYSCTL_DIR}/who-cephadm-tuned-profile.conf']
+                )
+            ),
+            mock.call(
+                'a', RemoteCommand(Executables.SYSCTL, ['--system'])
+            ),
         ]
         _check_execute_command.assert_has_calls(calls, any_order=True)
 
@@ -170,7 +186,9 @@ class TestTunedProfiles:
                       profiles)
         tp = TunedProfileUtils(mgr)
         tp._write_tuned_profiles('a', self.profiles_to_calls(tp, [self.tspec1, self.tspec2]))
-        _check_execute_command.assert_called_with('a', ['sysctl', '--system'])
+        _check_execute_command.assert_called_with(
+            'a', RemoteCommand(Executables.SYSCTL, ['--system'])
+        )
         _write_remote_file.assert_called_with(
             'a', f'{SYSCTL_DIR}/p2-cephadm-tuned-profile.conf', tp._profile_to_str(self.tspec2).encode('utf-8'))
 

src/pybind/mgr/cephadm/tuned_profiles.py

@@ -3,6 +3,7 @@ from typing import Dict, List, TYPE_CHECKING
 from ceph.utils import datetime_now
 from .schedule import HostAssignment
 from ceph.deployment.service_spec import ServiceSpec, TunedProfileSpec
+from . import ssh
 
 if TYPE_CHECKING:
     from cephadm.module import CephadmOrchestrator
@@ -11,6 +12,8 @@ logger = logging.getLogger(__name__)
 
 SYSCTL_DIR = '/etc/sysctl.d'
 
+SYSCTL_SYSTEM_CMD = ssh.RemoteCommand(ssh.Executables.SYSCTL, ['--system'])
+
 
 class TunedProfileUtils():
     def __init__(self, mgr: "CephadmOrchestrator") -> None:
@@ -69,7 +72,7 @@ class TunedProfileUtils():
         """
         if self.mgr.cache.is_host_unreachable(host):
             return
-        cmd = ['ls', SYSCTL_DIR]
+        cmd = ssh.RemoteCommand(ssh.Executables.LS, [SYSCTL_DIR])
         found_files = self.mgr.ssh.check_execute_command(host, cmd, log_command=self.mgr.log_refresh_metadata).split('\n')
         found_files = [s.strip() for s in found_files]
         profile_names: List[str] = sum([[*p] for p in profiles], [])  # extract all profiles names
@@ -81,11 +84,11 @@ class TunedProfileUtils():
                 continue
             if file not in expected_files:
                 logger.info(f'Removing stray tuned profile file {file}')
-                cmd = ['rm', '-f', f'{SYSCTL_DIR}/{file}']
+                cmd = ssh.RemoteCommand(ssh.Executables.RM, ['-f', f'{SYSCTL_DIR}/{file}'])
                 self.mgr.ssh.check_execute_command(host, cmd)
                 updated = True
         if updated:
-            self.mgr.ssh.check_execute_command(host, ['sysctl', '--system'])
+            self.mgr.ssh.check_execute_command(host, SYSCTL_SYSTEM_CMD)
 
     def _write_tuned_profiles(self, host: str, profiles: List[Dict[str, str]]) -> None:
         if self.mgr.cache.is_host_unreachable(host):
@@ -99,5 +102,5 @@ class TunedProfileUtils():
                     self.mgr.ssh.write_remote_file(host, profile_filename, content.encode('utf-8'))
                     updated = True
         if updated:
-            self.mgr.ssh.check_execute_command(host, ['sysctl', '--system'])
+            self.mgr.ssh.check_execute_command(host, SYSCTL_SYSTEM_CMD)
         self.mgr.cache.last_tuned_profile_update[host] = datetime_now()