Merge 3002.6 bugfix changes

CHANGELOG.md

@@ -22,6 +22,8 @@ Deprecated
 
 - Added deprecation warning for grains.get_or_set_hash (#59425)
 
+Salt 3002.6 (2021-03-10)
+========================
 
 Changed
 -------
@@ -30,6 +32,7 @@ Changed
 - Store git sha in salt/_version.py when installing from a tag so it can be found if needed later. (#59137)
 - Changed package manager detection in yumpkg module (#59201)
 - Updating the pkg beacon to fire the events when there are upgrades to packages, but also when watched packages are installed or removed. Breaking out the logic for listing pkgs from context into a separate function to aid in testing. Updating tests to ensure context is not used when use_context option to list_pkgs is False. (#59463)
+- Store git sha in salt/_version.py when installing from a tag so it can be found if needed later. (#59137)
 
 
 Fixed
@@ -167,6 +170,11 @@ Added
 - Add -B flag to FreeBSD pkgng.check() to regenerate the library dependency
   metadata for a package by extracting library requirement information from the
   binary ELF files in the package. (#59569)
+- Fix argument injection bug in restartcheck.restartcheck. This change hardens
+  the fix for CVE-2020-28243. (#200)
+- Allow "extra_filerefs" as sanitized kwargs for SSH client.
+  Fix regression on "cmd.run" when passing tuples as cmd. (#59664)
+- Allow all ssh kwargs as sanitized kwargs for SSH client. (#59748)
 
 
 Salt 3002.5 (2021-02-25)
@@ -480,6 +488,18 @@ Added
   This flag will be deprecated in the Phosphorus release when this functionality
   becomes the default. (#58652)
 
+Salt 3001.7 (2021-03-10)
+========================
+
+Fixed
+-----
+
+- Fix argument injection bug in restartcheck.restartcheck. This change hardens
+  the fix for CVE-2020-28243. (#200)
+- Allow "extra_filerefs" as sanitized kwargs for SSH client.
+  Fix regression on "cmd.run" when passing tuples as cmd. (#59664)
+- Allow all ssh kwargs as sanitized kwargs for SSH client. (#59748)
+
 Salt 3001.6 (2021-02-09)
 ========================
 
@@ -971,6 +991,18 @@ Added
 - [#56637](https://github.com/saltstack/salt/pull/56637) - Add ``win_wua.installed`` to the ``win_wua`` execution module
 - Clarify how to get the master fingerprint (#54699)
 
+Salt 3000.9 (2021-03-10)
+========================
+
+Fixed
+-----
+
+- Allow "extra_filerefs" as sanitized kwargs for SSH client.
+  Fix regression on "cmd.run" when passing tuples as cmd. (#59664)
+- Allow all ssh kwargs as sanitized kwargs for SSH client. (#59748)
+- Fix argument injection bug in restartcheck.restartcheck. This change hardens
+  the fix for CVE-2020-28243.
+
 Salt 3000.8 (2021-02-09)
 ========================
 

doc/topics/releases/3000.9.rst

@@ -0,0 +1,17 @@
+.. _release-3000-9:
+
+===========================
+Salt 3000.9 Release Notes
+===========================
+
+Version 3000.9 is a bug fix release for :ref:`3000 <release-3000>`.
+
+
+Fixed
+-----
+
+- Allow "extra_filerefs" as sanitized kwargs for SSH client.
+  Fix regression on "cmd.run" when passing tuples as cmd. (#59664)
+- Allow all ssh kwargs as sanitized kwargs for SSH client. (#59748)
+- Fix argument injection bug in restartcheck.restartcheck. This change hardens
+  the fix for CVE-2020-28243.

doc/topics/releases/3001.7.rst

@@ -0,0 +1,17 @@
+.. _release-3001-7:
+
+=========================
+Salt 3001.7 Release Notes
+=========================
+
+Version 3001.7 is a bug fix release for :ref:`3001 <release-3001>`.
+
+
+Fixed
+-----
+
+- Allow "extra_filerefs" as sanitized kwargs for SSH client.
+  Fix regression on "cmd.run" when passing tuples as cmd. (#59664)
+- Allow all ssh kwargs as sanitized kwargs for SSH client. (#59748)
+- Fix argument injection bug in restartcheck.restartcheck. This change hardens
+  the fix for CVE-2020-28243.

doc/topics/releases/3002.6.rst

@@ -0,0 +1,24 @@
+.. _release-3002-6:
+
+=========================
+Salt 3002.6 Release Notes
+=========================
+
+Version 3002.6 is a bug fix release for :ref:`3002 <release-3002>`.
+
+
+Changed
+-------
+
+- Store git sha in salt/_version.py when installing from a tag so it can be found if needed later. (#59137)
+
+
+Fixed
+-----
+
+- Fix argument injection bug in restartcheck.restartcheck. This change hardens
+  the fix for CVE-2020-28243. (#200)
+- Allow "extra_filerefs" as sanitized kwargs for SSH client.
+  Fix regression on "cmd.run" when passing tuples as cmd. (#59664)
+- Allow all ssh kwargs as sanitized kwargs for SSH client. (#59748)
+

salt/client/ssh/client.py

@@ -52,11 +52,34 @@ def sanitize_kwargs(self, kwargs):
             ("ssh_identities_only", bool),
             ("ssh_remote_port_forwards", str),
             ("ssh_options", list),
+            ("ssh_max_procs", int),
+            ("ssh_askpass", bool),
+            ("ssh_key_deploy", bool),
+            ("ssh_update_roster", bool),
+            ("ssh_scan_ports", str),
+            ("ssh_scan_timeout", int),
+            ("ssh_timeout", int),
+            ("ssh_log_file", str),
+            ("raw_shell", bool),
+            ("refresh_cache", bool),
+            ("roster", str),
             ("roster_file", str),
             ("rosters", list),
             ("ignore_host_keys", bool),
             ("raw_shell", bool),
             ("extra_filerefs", str),
+            ("min_extra_mods", str),
+            ("thin_extra_mods", str),
+            ("verbose", bool),
+            ("static", bool),
+            ("ssh_wipe", bool),
+            ("rand_thin_dir", bool),
+            ("regen_thin", bool),
+            ("python2_bin", str),
+            ("python3_bin", str),
+            ("ssh_run_pre_flight", bool),
+            ("no_host_keys", bool),
+            ("saltfile", str),
         ]
         sane_kwargs = {}
         for name, kind in roster_vals:

salt/modules/restartcheck.py

@@ -11,7 +11,6 @@
 """
 import os
 import re
-import shlex
 import subprocess
 import sys
 import time
@@ -495,17 +494,17 @@ def restartcheck(ignorelist=None, blacklist=None, excludepid=None, **kwargs):
     verbose = kwargs.pop("verbose", True)
     timeout = kwargs.pop("timeout", 5)
     if __grains__.get("os_family") == "Debian":
-        cmd_pkg_query = "dpkg-query --listfiles "
+        cmd_pkg_query = ["dpkg-query", "--listfiles"]
         systemd_folder = "/lib/systemd/system/"
         systemd = "/bin/systemd"
         kernel_versions = _kernel_versions_debian()
     elif __grains__.get("os_family") == "RedHat":
-        cmd_pkg_query = "repoquery -l "
+        cmd_pkg_query = ["repoquery", "-l"]
         systemd_folder = "/usr/lib/systemd/system/"
         systemd = "/usr/bin/systemctl"
         kernel_versions = _kernel_versions_redhat()
     elif __grains__.get("os_family") == NILRT_FAMILY_NAME:
-        cmd_pkg_query = "opkg files "
+        cmd_pkg_query = ["opkg", "files"]
         systemd = ""
         kernel_versions = _kernel_versions_nilrt()
     else:
@@ -613,8 +612,8 @@ def restartcheck(ignorelist=None, blacklist=None, excludepid=None, **kwargs):
 
     for package in packages:
         _check_timeout(start_time, timeout)
-        cmd = cmd_pkg_query + package
-        cmd = shlex.split(cmd)
+        cmd = cmd_pkg_query[:]
+        cmd.append(package)
         paths = subprocess.Popen(cmd, stdout=subprocess.PIPE)
 
         while True:

tests/pytests/unit/client/test_ssh.py

@@ -1,8 +1,11 @@
 import pytest
+import salt.client.ssh.client
 import salt.utils.msgpack
 from salt.client import ssh
 from tests.support.mock import MagicMock, patch
 
+pytestmark = [pytest.mark.skip_if_binaries_missing("ssh", "ssh-keygen", check_all=True)]
+
 
 @pytest.fixture
 def ssh_target(tmpdir):
@@ -57,3 +60,76 @@ def test_cmd_block_python_version_error(ssh_target):
     with patch_shim:
         ret = single.cmd_block()
         assert "ERROR: Python version error. Recommendation(s) follow:" in ret[0]
+
+
+@pytest.mark.parametrize(
+    "test_opts",
+    [
+        ("extra_filerefs", "salt://foobar", True),
+        ("host", "testhost", False),
+        ("ssh_user", "testuser", True),
+        ("ssh_passwd", "testpasswd", True),
+        ("ssh_port", 23, False),
+        ("ssh_sudo", True, True),
+        ("ssh_sudo_user", "sudouser", False),
+        ("ssh_priv", "test_priv", True),
+        ("ssh_priv_passwd", "sshpasswd", True),
+        ("ssh_identities_only", True, True),
+        ("ssh_remote_port_forwards", "test", True),
+        ("ssh_options", ["test1", "test2"], True),
+        ("ssh_max_procs", 2, True),
+        ("ssh_askpass", True, True),
+        ("ssh_key_deploy", True, True),
+        ("ssh_update_roster", True, True),
+        ("ssh_scan_ports", "test", True),
+        ("ssh_scan_timeout", 1.0, True),
+        ("ssh_timeout", 1, False),
+        ("ssh_log_file", "/tmp/test", True),
+        ("raw_shell", True, True),
+        ("refresh_cache", True, True),
+        ("roster", "/test", True),
+        ("roster_file", "/test1", True),
+        ("rosters", ["test1"], False),
+        ("ignore_host_keys", True, True),
+        ("min_extra_mods", "test", True),
+        ("thin_extra_mods", "test1", True),
+        ("verbose", True, True),
+        ("static", True, True),
+        ("ssh_wipe", True, True),
+        ("rand_thin_dir", True, True),
+        ("regen_thin", True, True),
+        ("python2_bin", "python2", True),
+        ("python3_bin", "python3", True),
+        ("ssh_run_pre_flight", True, True),
+        ("no_host_keys", True, True),
+        ("saltfile", "/tmp/test", True),
+        ("doesnotexist", None, False),
+    ],
+)
+def test_ssh_kwargs(test_opts):
+    """
+    test all ssh kwargs are not excluded from kwargs
+    when preparing the SSH opts
+    """
+    opt_key = test_opts[0]
+    opt_value = test_opts[1]
+    # Is the kwarg in salt.utils.parsers?
+    in_parser = test_opts[2]
+
+    opts = {
+        "eauth": "auto",
+        "username": "test",
+        "password": "test",
+        "client": "ssh",
+        "tgt": "localhost",
+        "fun": "test.ping",
+        opt_key: opt_value,
+    }
+    client = salt.client.ssh.client.SSHClient(disable_custom_roster=True)
+    if in_parser:
+        ssh_kwargs = salt.utils.parsers.SaltSSHOptionParser().defaults
+        assert opt_key in ssh_kwargs
+
+    with patch("salt.roster.get_roster_file", MagicMock(return_value="")):
+        ssh_obj = client._prep_ssh(**opts)
+        assert ssh_obj.opts.get(opt_key, None) == opt_value

tests/unit/modules/test_restartcheck.py

@@ -381,3 +381,50 @@ def test_valid_command(self):
                     "Found 1 processes using old versions of upgraded files", ret
                 )
             self.assertFalse(os.path.exists(create_file))
+
+    def test_valid_command_b(self):
+        """
+        test for CVE-2020-28243
+        """
+        create_file = os.path.join(RUNTIME_VARS.TMP, "created_file")
+
+        patch_kernel = patch(
+            "salt.modules.restartcheck._kernel_versions_redhat",
+            return_value=["3.10.0-1127.el7.x86_64"],
+        )
+        services = {
+            "NetworkManager": {"ExecMainPID": 123},
+            "auditd": {"ExecMainPID": 456},
+            "crond": {"ExecMainPID": 789},
+        }
+
+        patch_salt = patch.dict(
+            restartcheck.__salt__,
+            {
+                "cmd.run": MagicMock(
+                    return_value="Linux localhost.localdomain 3.10.0-1127.el7.x86_64"
+                ),
+                "service.get_running": MagicMock(return_value=list(services.keys())),
+                "service.show": MagicMock(side_effect=list(services.values())),
+                "pkg.owner": MagicMock(return_value=""),
+                "service.available": MagicMock(return_value=True),
+            },
+        )
+
+        patch_deleted = patch(
+            "salt.modules.restartcheck._deleted_files",
+            MagicMock(return_value=[("--admindir tmp dpkg", 123, "/root/ (deleted)")]),
+        )
+
+        patch_readlink = patch("os.readlink", return_value="--admindir tmp dpkg")
+
+        popen_mock = MagicMock()
+        popen_mock.return_value.stdout.readline.side_effect = ["/usr/bin\n", ""]
+        patch_popen = patch("subprocess.Popen", popen_mock)
+
+        patch_grains = patch.dict(restartcheck.__grains__, {"os_family": "RedHat"})
+        with patch_kernel, patch_salt, patch_deleted, patch_readlink, patch_grains, patch_popen:
+            ret = restartcheck.restartcheck()
+            self.assertIn("Found 1 processes using old versions of upgraded files", ret)
+            args, kwargs = popen_mock.call_args
+            assert args[0] == ["repoquery", "-l", "--admindir tmp dpkg"]

tests/unit/test_module_names.py

@@ -200,6 +200,7 @@ def test_module_name_source_match(self):
             "unit.utils.scheduler.test_run_job",
             "unit.utils.scheduler.test_schedule",
             "unit.utils.scheduler.test_skip",
+            "unit.auth.test_auth",
         )
         errors = []