From a0514a039dc28eb42f68493ba7adad621d5d69a1 Mon Sep 17 00:00:00 2001
From: "Daniel A. Wozniak" <dwozniak@saltstack.com>
Date: Wed, 16 Sep 2020 00:25:10 +0000
Subject: [PATCH] Fix CVE-2020-25592 and add tests (bsc#1178319)

---
 salt/client/ssh/shell.py | 27 +++++++++++----
 salt/modules/tls.py      | 22 ++++++------
 salt/netapi/__init__.py  | 72 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 104 insertions(+), 17 deletions(-)

diff --git a/salt/client/ssh/shell.py b/salt/client/ssh/shell.py
index 613660fe34..e1669bb6ca 100644
--- a/salt/client/ssh/shell.py
+++ b/salt/client/ssh/shell.py
@@ -8,6 +8,8 @@ from __future__ import absolute_import
 import re
 import os
 import json
+import sys
+import shlex
 import time
 import logging
 import subprocess
@@ -40,10 +42,10 @@ def gen_key(path):
     '''
     Generate a key for use with salt-ssh
     '''
-    cmd = 'ssh-keygen -P "" -f {0} -t rsa -q'.format(path)
+    cmd = ["ssh-keygen", "-P", '""', "-f", path, "-t", "rsa", "-q"]
     if not os.path.isdir(os.path.dirname(path)):
         os.makedirs(os.path.dirname(path))
-    subprocess.call(cmd, shell=True)
+    subprocess.call(cmd)
 
 
 class Shell(object):
@@ -269,8 +271,7 @@ class Shell(object):
         '''
         try:
             proc = salt.utils.nb_popen.NonBlockingPopen(
-                cmd,
-                shell=True,
+                self._split_cmd(cmd),
                 stderr=subprocess.PIPE,
                 stdout=subprocess.PIPE,
             )
@@ -349,14 +350,28 @@ class Shell(object):
 
         return self._run_cmd(cmd)
 
+    def _split_cmd(self, cmd):
+        """
+        Split a command string so that it is suitable to pass to Popen without
+        shell=True. This prevents shell injection attacks in the options passed
+        to ssh or some other command.
+        """
+        try:
+            ssh_part, cmd_part = cmd.split("/bin/sh")
+        except ValueError:
+            cmd_lst = shlex.split(cmd)
+        else:
+            cmd_lst = shlex.split(ssh_part)
+            cmd_lst.append("/bin/sh {}".format(cmd_part))
+        return cmd_lst
+
     def _run_cmd(self, cmd, key_accept=False, passwd_retries=3):
         '''
         Execute a shell command via VT. This is blocking and assumes that ssh
         is being run
         '''
         term = salt.utils.vt.Terminal(
-                cmd,
-                shell=True,
+                self._split_cmd(cmd),
                 log_stdout=True,
                 log_stdout_level='trace',
                 log_stderr=True,
diff --git a/salt/modules/tls.py b/salt/modules/tls.py
index d20367e0fb..f589d5df65 100644
--- a/salt/modules/tls.py
+++ b/salt/modules/tls.py
@@ -735,11 +735,12 @@ def create_ca(ca_name,
                 write_key = False
             else:
                 log.info('Saving old CA ssl key in {0}'.format(bck))
-                with salt.utils.fopen(bck, 'w') as bckf:
+                fp = os.open(bck, os.O_CREAT | os.O_RDWR, 0o600)
+                with os.fdopen(fp, 'w') as bckf:
                     bckf.write(old_key)
-                    os.chmod(bck, 0o600)
     if write_key:
-        with salt.utils.fopen(ca_keyp, 'w') as ca_key:
+        fp = os.open(ca_keyp, os.O_CREAT | os.O_RDWR, 0o600)
+        with os.fdopen(fp, 'w') as ca_key:
             ca_key.write(keycontent)
 
     with salt.utils.fopen(certp, 'w') as ca_crt:
@@ -1034,8 +1035,9 @@ def create_csr(ca_name,
     req.sign(key, digest)
 
     # Write private key and request
-    with salt.utils.fopen('{0}/{1}.key'.format(csr_path,
-                                               csr_filename), 'w+') as priv_key:
+    priv_keyp = '{0}/{1}.key'.format(csr_path, csr_filename)
+    fp = os.open(priv_keyp, os.O_CREAT | os.O_RDWR, 0o600)
+    with os.fdopen(fp, 'w+') as priv_key:
         priv_key.write(
             OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key)
         )
@@ -1175,11 +1177,11 @@ def create_self_signed_cert(tls_dir='tls',
     cert.sign(key, digest)
 
     # Write private key and cert
-    with salt.utils.fopen(
-        '{0}/{1}/certs/{2}.key'.format(cert_base_path(),
-                                       tls_dir, cert_filename),
-        'w+'
-    ) as priv_key:
+    priv_key_path = '{0}/{1}/certs/{2}.key'.format(cert_base_path(),
+                                                   tls_dir,
+                                                   cert_filename)
+    fp = os.open(priv_key_path, os.O_CREAT | os.O_RDWR, 0o600)
+    with os.fdopen(fp, 'w+') as priv_key:
         priv_key.write(
             OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key)
         )
diff --git a/salt/netapi/__init__.py b/salt/netapi/__init__.py
index 9821d5bb1d..44120f7d01 100644
--- a/salt/netapi/__init__.py
+++ b/salt/netapi/__init__.py
@@ -2,21 +2,36 @@
 '''
 Make api awesomeness
 '''
-from __future__ import absolute_import
+from __future__ import absolute_import, print_function, unicode_literals
+
+import copy
+
 # Import Python libs
 import inspect
+import logging
 import os
 
 # Import Salt libs
 import salt.log  # pylint: disable=W0611
+import salt.auth
 import salt.client
 import salt.config
+import salt.daemons.masterapi
 import salt.runner
 import salt.syspaths
 import salt.wheel
 import salt.utils
 import salt.client.ssh.client
 import salt.exceptions
+import salt.utils.args
+import salt.utils.minions
+import salt.wheel
+from salt.defaults import DEFAULT_TARGET_DELIM
+
+# Import third party libs
+from salt.ext import six
+
+log = logging.getLogger(__name__)
 
 
 class NetapiClient(object):
@@ -31,6 +46,15 @@ class NetapiClient(object):
 
     def __init__(self, opts):
         self.opts = opts
+        apiopts = copy.deepcopy(self.opts)
+        apiopts["enable_ssh_minions"] = True
+        apiopts["cachedir"] = os.path.join(opts["cachedir"], "saltapi")
+        if not os.path.exists(apiopts["cachedir"]):
+            os.makedirs(apiopts["cachedir"])
+        self.resolver = salt.auth.Resolver(apiopts)
+        self.loadauth = salt.auth.LoadAuth(apiopts)
+        self.key = salt.daemons.masterapi.access_keys(apiopts)
+        self.ckminions = salt.utils.minions.CkMinions(apiopts)
 
     def _is_master_running(self):
         '''
@@ -47,6 +71,49 @@ class NetapiClient(object):
             self.opts['sock_dir'],
             ipc_file))
 
+    def _prep_auth_info(self, clear_load):
+        sensitive_load_keys = []
+        key = None
+        if "token" in clear_load:
+            auth_type = "token"
+            err_name = "TokenAuthenticationError"
+            sensitive_load_keys = ["token"]
+            return auth_type, err_name, key, sensitive_load_keys
+        elif "eauth" in clear_load:
+            auth_type = "eauth"
+            err_name = "EauthAuthenticationError"
+            sensitive_load_keys = ["username", "password"]
+            return auth_type, err_name, key, sensitive_load_keys
+        raise salt.exceptions.EauthAuthenticationError(
+            "No authentication credentials given"
+        )
+
+    def _authorize_ssh(self, low):
+        auth_type, err_name, key, sensitive_load_keys = self._prep_auth_info(low)
+        auth_check = self.loadauth.check_authentication(low, auth_type, key=key)
+        auth_list = auth_check.get("auth_list", [])
+        error = auth_check.get("error")
+        if error:
+            raise salt.exceptions.EauthAuthenticationError(error)
+        delimiter = low.get("kwargs", {}).get("delimiter", DEFAULT_TARGET_DELIM)
+        _res = self.ckminions.check_minions(
+            low["tgt"], low.get("tgt_type", "glob"), delimiter
+        )
+        minions = _res.get("minions", list())
+        missing = _res.get("missing", list())
+        authorized = self.ckminions.auth_check(
+            auth_list,
+            low["fun"],
+            low.get("arg", []),
+            low["tgt"],
+            low.get("tgt_type", "glob"),
+            minions=minions,
+        )
+        if not authorized:
+            raise salt.exceptions.EauthAuthenticationError(
+                "Authorization error occurred."
+            )
+
     def run(self, low):
         '''
         Execute the specified function in the specified client by passing the
@@ -66,6 +133,9 @@ class NetapiClient(object):
             raise salt.exceptions.EauthAuthenticationError(
                     'No authentication credentials given')
 
+        if low['client'] == 'ssh':
+            self._authorize_ssh(low)
+
         l_fun = getattr(self, low['client'])
         f_call = salt.utils.format_call(l_fun, low)
         return l_fun(*f_call.get('args', ()), **f_call.get('kwargs', {}))
-- 
2.28.0
