Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
systemsmanagement:saltstack:products:testing
py27-compat-salt
fix-cve-2020-25592-and-add-tests-bsc-1178319.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File fix-cve-2020-25592-and-add-tests-bsc-1178319.patch of Package py27-compat-salt
From e7514afcba4f57c5cb8599f561fcefdcc3db7314 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) Properly validate eauth credentials and tokens on SSH calls made by Salt API (bsc#1178319) (bsc#1178362) (bsc#1178361) (CVE-2020-25592) (CVE-2020-17490) (CVE-2020-16846) --- salt/client/ssh/shell.py | 26 ++- salt/modules/tls.py | 18 +- salt/netapi/__init__.py | 67 ++++++ tests/integration/netapi/test_client.py | 296 +++++++++++++++++++++++- 4 files changed, 388 insertions(+), 19 deletions(-) diff --git a/salt/client/ssh/shell.py b/salt/client/ssh/shell.py index bd55c514ee..27aba7b382 100644 --- a/salt/client/ssh/shell.py +++ b/salt/client/ssh/shell.py @@ -8,6 +8,7 @@ from __future__ import absolute_import, print_function, unicode_literals import re import os import sys +import shlex import time import logging import subprocess @@ -43,10 +44,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) def gen_shell(opts, **kwargs): @@ -289,8 +290,7 @@ class Shell(object): ''' try: proc = salt.utils.nb_popen.NonBlockingPopen( - cmd, - shell=True, + self._split_cmd(cmd), stderr=subprocess.PIPE, stdout=subprocess.PIPE, ) @@ -369,6 +369,21 @@ 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 @@ -378,8 +393,7 @@ class Shell(object): return '', 'No command or passphrase', 245 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 af845621a3..116b5fe379 100644 --- a/salt/modules/tls.py +++ b/salt/modules/tls.py @@ -798,12 +798,13 @@ def create_ca(ca_name, if old_key.strip() == keycontent.strip(): write_key = False else: - log.info('Saving old CA ssl key in %s', bck) - with salt.utils.files.fopen(bck, 'w') as bckf: + log.info('Saving old CA ssl key in {0}'.format(bck)) + 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.files.fopen(ca_keyp, 'wb') as ca_key: + fp = os.open(ca_keyp, os.O_CREAT | os.O_RDWR, 0o600) + with os.fdopen(fp, 'wb') as ca_key: ca_key.write(salt.utils.stringutils.to_bytes(keycontent)) with salt.utils.files.fopen(certp, 'wb') as ca_crt: @@ -1115,9 +1116,9 @@ def create_csr(ca_name, req.sign(key, salt.utils.stringutils.to_str(digest)) # Write private key and request - with salt.utils.files.fopen('{0}/{1}.key'.format(csr_path, - csr_filename), - 'wb+') 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, 'wb+') as priv_key: priv_key.write( salt.utils.stringutils.to_bytes( OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, @@ -1266,7 +1267,8 @@ def create_self_signed_cert(tls_dir='tls', priv_key_path = '{0}/{1}/certs/{2}.key'.format(cert_base_path(), tls_dir, cert_filename) - with salt.utils.files.fopen(priv_key_path, 'wb+') as priv_key: + fp = os.open(priv_key_path, os.O_CREAT | os.O_RDWR, 0o600) + with os.fdopen(fp, 'wb+') as priv_key: priv_key.write( salt.utils.stringutils.to_bytes( OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, diff --git a/salt/netapi/__init__.py b/salt/netapi/__init__.py index 31a24bb420..4e5b6b093a 100644 --- a/salt/netapi/__init__.py +++ b/salt/netapi/__init__.py @@ -3,24 +3,36 @@ Make api awesomeness ''' 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.args 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): ''' @@ -34,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): ''' @@ -55,6 +76,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 @@ -80,6 +144,9 @@ class NetapiClient(object): raise salt.exceptions.EauthAuthenticationError( 'Raw shell option not allowed.') + if low['client'] == 'ssh': + self._authorize_ssh(low) + l_fun = getattr(self, low['client']) f_call = salt.utils.args.format_call(l_fun, low) return l_fun(*f_call.get('args', ()), **f_call.get('kwargs', {})) diff --git a/tests/integration/netapi/test_client.py b/tests/integration/netapi/test_client.py index 08030f31ec..b99bdfe313 100644 --- a/tests/integration/netapi/test_client.py +++ b/tests/integration/netapi/test_client.py @@ -1,26 +1,30 @@ # encoding: utf-8 - # Import Python libs from __future__ import absolute_import, print_function, unicode_literals +import copy import logging import os import time +import salt.config +import salt.netapi +import salt.utils.files +import salt.utils.platform +import salt.utils.pycrypto + # Import Salt Testing libs from tests.support.paths import TMP_CONF_DIR, TMP from tests.support.runtests import RUNTIME_VARS from tests.support.unit import TestCase, skipIf from tests.support.mock import patch -from tests.support.case import SSHCase +from tests.support.case import ModuleCase, SSHCase +from salt.exceptions import EauthAuthenticationError from tests.support.helpers import ( Webserver, SaveRequestsPostHandler, requires_sshd_server ) -# Import Salt libs -import salt.config -import salt.netapi from salt.exceptions import ( EauthAuthenticationError @@ -174,6 +178,10 @@ class NetapiSSHClientTest(SSHCase): ''' opts = salt.config.client_config(os.path.join(TMP_CONF_DIR, 'master')) self.netapi = salt.netapi.NetapiClient(opts) + opts = salt.config.client_config(os.path.join(TMP_CONF_DIR, "master")) + naopts = copy.deepcopy(opts) + naopts["ignore_host_keys"] = True + self.netapi = salt.netapi.NetapiClient(naopts) self.priv_file = os.path.join(RUNTIME_VARS.TMP_CONF_DIR, 'key_test') self.rosters = os.path.join(RUNTIME_VARS.TMP_CONF_DIR) @@ -271,3 +279,281 @@ class NetapiSSHClientTest(SSHCase): self.assertEqual(ret, None) self.assertFalse(os.path.exists('badfile.txt')) + + @staticmethod + def cleanup_file(path): + try: + os.remove(path) + except OSError: + pass + + @staticmethod + def cleanup_dir(path): + try: + salt.utils.files.rm_rf(path) + except OSError: + pass + + def test_shell_inject_ssh_priv(self): + """ + Verify CVE-2020-16846 for ssh_priv variable + """ + # ZDI-CAN-11143 + path = "/tmp/test-11143" + self.addCleanup(self.cleanup_file, path) + self.addCleanup(self.cleanup_file, "aaa") + self.addCleanup(self.cleanup_file, "aaa.pub") + self.addCleanup(self.cleanup_dir, "aaa|id>") + low = { + "roster": "cache", + "client": "ssh", + "tgt": "www.zerodayinitiative.com", + "ssh_priv": "aaa|id>{} #".format(path), + "fun": "test.ping", + "eauth": "auto", + "username": "saltdev_auto", + "password": "saltdev", + } + ret = self.netapi.run(low) + self.assertFalse(os.path.exists(path)) + + def test_shell_inject_tgt(self): + """ + Verify CVE-2020-16846 for tgt variable + """ + # ZDI-CAN-11167 + path = "/tmp/test-11167" + self.addCleanup(self.cleanup_file, path) + low = { + "roster": "cache", + "client": "ssh", + "tgt": "root|id>{} #@127.0.0.1".format(path), + "roster_file": "/tmp/salt-tests-tmpdir/config/roaster", + "rosters": "/", + "fun": "test.ping", + "eauth": "auto", + "username": "saltdev_auto", + "password": "saltdev", + } + ret = self.netapi.run(low) + self.assertFalse(os.path.exists(path)) + + def test_shell_inject_ssh_options(self): + """ + Verify CVE-2020-16846 for ssh_options + """ + # ZDI-CAN-11169 + path = "/tmp/test-11169" + self.addCleanup(self.cleanup_file, path) + low = { + "roster": "cache", + "client": "ssh", + "tgt": "127.0.0.1", + "renderer": "cheetah", + "fun": "test.ping", + "eauth": "auto", + "username": "saltdev_auto", + "password": "saltdev", + "roster_file": "/tmp/salt-tests-tmpdir/config/roaster", + "rosters": "/", + "ssh_options": ["|id>{} #".format(path), "lol"], + } + ret = self.netapi.run(low) + self.assertFalse(os.path.exists(path)) + + def test_shell_inject_ssh_port(self): + """ + Verify CVE-2020-16846 for ssh_port variable + """ + # ZDI-CAN-11172 + path = "/tmp/test-11172" + self.addCleanup(self.cleanup_file, path) + low = { + "roster": "cache", + "client": "ssh", + "tgt": "127.0.0.1", + "renderer": "cheetah", + "fun": "test.ping", + "eauth": "auto", + "username": "saltdev_auto", + "password": "saltdev", + "roster_file": "/tmp/salt-tests-tmpdir/config/roaster", + "rosters": "/", + "ssh_port": "hhhhh|id>{} #".format(path), + } + ret = self.netapi.run(low) + self.assertFalse(os.path.exists(path)) + + def test_shell_inject_remote_port_forwards(self): + """ + Verify CVE-2020-16846 for remote_port_forwards variable + """ + # ZDI-CAN-11173 + path = "/tmp/test-1173" + self.addCleanup(self.cleanup_file, path) + low = { + "roster": "cache", + "client": "ssh", + "tgt": "127.0.0.1", + "renderer": "cheetah", + "fun": "test.ping", + "roster_file": "/tmp/salt-tests-tmpdir/config/roaster", + "rosters": "/", + "ssh_remote_port_forwards": "hhhhh|id>{} #, lol".format(path), + "eauth": "auto", + "username": "saltdev_auto", + "password": "saltdev", + } + ret = self.netapi.run(low) + self.assertFalse(os.path.exists(path)) + + +@requires_sshd_server +class NetapiSSHClientAuthTest(SSHCase): + + USERA = "saltdev" + USERA_PWD = "saltdev" + + def setUp(self): + """ + Set up a NetapiClient instance + """ + opts = salt.config.client_config(os.path.join(TMP_CONF_DIR, "master")) + naopts = copy.deepcopy(opts) + naopts["ignore_host_keys"] = True + self.netapi = salt.netapi.NetapiClient(naopts) + + self.priv_file = os.path.join(RUNTIME_VARS.TMP_CONF_DIR, "key_test") + self.rosters = os.path.join(RUNTIME_VARS.TMP_CONF_DIR) + # Initialize salt-ssh + self.run_function("test.ping") + self.mod_case = ModuleCase() + try: + add_user = self.mod_case.run_function( + "user.add", [self.USERA], createhome=False + ) + self.assertTrue(add_user) + if salt.utils.platform.is_darwin(): + hashed_password = self.USERA_PWD + else: + hashed_password = salt.utils.pycrypto.gen_hash(password=self.USERA_PWD) + add_pwd = self.mod_case.run_function( + "shadow.set_password", [self.USERA, hashed_password], + ) + self.assertTrue(add_pwd) + except AssertionError: + self.mod_case.run_function("user.delete", [self.USERA], remove=True) + self.skipTest("Could not add user or password, skipping test") + + def tearDown(self): + del self.netapi + self.mod_case.run_function("user.delete", [self.USERA], remove=True) + + @classmethod + def setUpClass(cls): + cls.post_webserver = Webserver(handler=SaveRequestsPostHandler) + cls.post_webserver.start() + cls.post_web_root = cls.post_webserver.web_root + cls.post_web_handler = cls.post_webserver.handler + + @classmethod + def tearDownClass(cls): + cls.post_webserver.stop() + del cls.post_webserver + + def test_ssh_auth_bypass(self): + """ + CVE-2020-25592 - Bogus eauth raises exception. + """ + low = { + "roster": "cache", + "client": "ssh", + "tgt": "127.0.0.1", + "renderer": "cheetah", + "fun": "test.ping", + "roster_file": "/tmp/salt-tests-tmpdir/config/roaster", + "rosters": "/", + "eauth": "xx", + } + with self.assertRaises(salt.exceptions.EauthAuthenticationError): + ret = self.netapi.run(low) + + def test_ssh_auth_valid(self): + """ + CVE-2020-25592 - Valid eauth works as expected. + """ + low = { + "client": "ssh", + "tgt": "localhost", + "fun": "test.ping", + "roster_file": "roster", + "rosters": [self.rosters], + "ssh_priv": self.priv_file, + "eauth": "pam", + "username": "saltdev", + "password": "saltdev", + } + ret = self.netapi.run(low) + assert "localhost" in ret + assert ret["localhost"]["return"] is True + + def test_ssh_auth_invalid(self): + """ + CVE-2020-25592 - Wrong password raises exception. + """ + low = { + "client": "ssh", + "tgt": "localhost", + "fun": "test.ping", + "roster_file": "roster", + "rosters": [self.rosters], + "ssh_priv": self.priv_file, + "eauth": "pam", + "username": "saltdev", + "password": "notvalidpassword", + } + with self.assertRaises(salt.exceptions.EauthAuthenticationError): + ret = self.netapi.run(low) + + def test_ssh_auth_invalid_acl(self): + """ + CVE-2020-25592 - Eauth ACL enforced. + """ + low = { + "client": "ssh", + "tgt": "localhost", + "fun": "at.at", + "args": ["12:05am", "echo foo"], + "roster_file": "roster", + "rosters": [self.rosters], + "ssh_priv": self.priv_file, + "eauth": "pam", + "username": "saltdev", + "password": "notvalidpassword", + } + with self.assertRaises(salt.exceptions.EauthAuthenticationError): + ret = self.netapi.run(low) + + def test_ssh_auth_token(self): + """ + CVE-2020-25592 - Eauth tokens work as expected. + """ + low = { + "eauth": "pam", + "username": "saltdev", + "password": "saltdev", + } + ret = self.netapi.loadauth.mk_token(low) + assert "token" in ret and ret["token"] + low = { + "client": "ssh", + "tgt": "localhost", + "fun": "test.ping", + "roster_file": "roster", + "rosters": [self.rosters], + "ssh_priv": self.priv_file, + "token": ret["token"], + } + ret = self.netapi.run(low) + assert "localhost" in ret + assert ret["localhost"]["return"] is True -- 2.28.0
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
