[Pkg-salt-team] Bug#949222: Bug#959684: salt: CVE-2020-11651 and CVE-2020-11652
Salvatore Bonaccorso
carnil at debian.org
Tue May 5 16:37:53 BST 2020
Hi Simon,
On Tue, May 05, 2020 at 03:01:45PM +0100, Simon McVittie wrote:
> On Mon, 04 May 2020 at 01:34:33 +0200, Guilhem Moulin wrote:
> > CVE-2020-11651
> > CVE-2020-11652
>
> I found myself needing to mitigate this for a salt deployment, so I
> tried backporting the upstream patches to buster.
>
> The attached are not at all thoroughly-tested and should be reviewed
> carefully by someone who knows the codebase, but they seem to work, and
> the proof-of-concept from
> https://github.com/rossengeorgiev/salt-security-backports no longer reports
> that the master is vulnerable. This was only a stopgap, because that
> deployment is now using the packages from saltstack.com instead, but it
> might be useful to the salt maintainers.
>
> There are also unofficial backports in
> https://github.com/rossengeorgiev/salt-security-backports - I tried doing
> the cherry-picks myself and then compared what I got with those, in an
> attempt to guard against mistakes (by either myself or the author of those
> backports).
>
> Note that patch 0003 contains unofficial workarounds for regressions in the
> release that fixed those CVEs, which you might prefer to exclude from an
> official update.
I did actually work on that already yesterday and uploaded the
attached debdiffs to security-master *but* I'm in need of someone
using salt to effectively ina good way to test them.
Do you have respective stretch and buster setups which you could
expose those packages to?
Regards,
Salvatore
-------------- next part --------------
diff -Nru salt-2016.11.2+ds/debian/changelog salt-2016.11.2+ds/debian/changelog
--- salt-2016.11.2+ds/debian/changelog 2018-04-20 14:33:54.000000000 +0200
+++ salt-2016.11.2+ds/debian/changelog 2020-05-04 14:29:16.000000000 +0200
@@ -1,3 +1,14 @@
+salt (2016.11.2+ds-1+deb9u3) stretch-security; urgency=high
+
+ * Non-maintainer upload by the Security Team.
+ * Address CVE-2020-11651 and CVE-2020-11652 (Closes: #959684)
+ Thanks to Daniel Wozniak <dwozniak at saltstack.com>
+ * Add note about log messages to hardening salt docs
+ * salt-api NET API with the ssh client enabled is vulnerable to command
+ injection (CVE-2019-17361) (Closes: #949222)
+
+ -- Salvatore Bonaccorso <carnil at debian.org> Mon, 04 May 2020 14:29:16 +0200
+
salt (2016.11.2+ds-1+deb9u2) stretch; urgency=medium
* Fix CVE-2017-8109: salt-ssh minion copied over configuration from the
diff -Nru salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch
--- salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch 1970-01-01 01:00:00.000000000 +0100
+++ salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch 2020-05-04 14:29:16.000000000 +0200
@@ -0,0 +1,41 @@
+From: "Daniel A. Wozniak" <dwozniak at saltstack.com>
+Date: Mon, 13 Apr 2020 07:01:07 +0000
+Subject: Add note about log messages to hardening salt docs
+Origin: https://github.com/saltstack/salt/commit/4631781376ddc9ee9d279f407ac3d0b78644fae7
+
+---
+ doc/topics/hardening.rst | 4 ++++
+ salt/master.py | 2 +-
+ 2 files changed, 5 insertions(+), 1 deletion(-)
+
+diff --git a/doc/topics/hardening.rst b/doc/topics/hardening.rst
+index c645b84128e4..569ad654af69 100644
+--- a/doc/topics/hardening.rst
++++ b/doc/topics/hardening.rst
+@@ -57,6 +57,10 @@ Salt hardening tips
+ particularly sensitive minions. There is also :ref:`salt-ssh` or the
+ :mod:`modules.sudo <salt.modules.sudo>` if you need to further restrict
+ a minion.
++- Monitor specific security releated log messages. Salt ``salt-master`` logs
++ attempts to access methods which are not exposed to network clients. These log
++ messages are logged at the ``error`` log level and start with ``Requested
++ method not exposed``.
+
+ .. _salt-users: https://groups.google.com/forum/#!forum/salt-users
+ .. _salt-announce: https://groups.google.com/forum/#!forum/salt-announce
+diff --git a/salt/master.py b/salt/master.py
+index 7d1444cf1221..aae55f1828e1 100644
+--- a/salt/master.py
++++ b/salt/master.py
+@@ -1162,7 +1162,7 @@ class TransportMethods(object):
+ try:
+ return getattr(self, name)
+ except AttributeError:
+- log.error("Expose method not found: %s", name)
++ log.error("Requested method not exposed: %s", name)
+ else:
+ log.error("Requested method not exposed: %s", name)
+
+--
+2.20.1
+
diff -Nru salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch
--- salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch 1970-01-01 01:00:00.000000000 +0100
+++ salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch 2020-05-04 14:29:16.000000000 +0200
@@ -0,0 +1,237 @@
+From 006219501bbb3a81a9fb64975035011016d5a7eb Mon Sep 17 00:00:00 2001
+From: "Gareth J. Greenaway" <gareth at wiked.org>
+Date: Fri, 1 May 2020 13:36:11 -0700
+Subject: [PATCH] CVE-2020-11651 and CVE-2020-11652
+
+---
+ salt/master.py | 63 +++++++++++++++++++++++++++++++++-------
+ salt/utils/verify.py | 45 ++++++++++++++++++++++++++++
+ salt/wheel/config.py | 5 ++++
+ salt/wheel/file_roots.py | 8 ++++-
+ 4 files changed, 109 insertions(+), 12 deletions(-)
+
+diff --git a/salt/master.py b/salt/master.py
+index 27f91613e5..0d4034ba88 100644
+--- a/salt/master.py
++++ b/salt/master.py
+@@ -896,10 +896,10 @@ class MWorker(SignalHandlingMultiprocessingProcess):
+ :return: The result of passing the load to a function in ClearFuncs corresponding to
+ the command specified in the load's 'cmd' key.
+ '''
+- log.trace('Clear payload received with command {cmd}'.format(**load))
+- if load['cmd'].startswith('__'):
+- return False
+- return getattr(self.clear_funcs, load['cmd'])(load), {'fun': 'send_clear'}
++ log.trace('Clear payload received with command %s', load['cmd'])
++ cmd = load['cmd']
++ method = self.clear_funcs.get_method(cmd)
++ return method(load), {'fun': 'send_clear'}
+
+ def _handle_aes(self, data):
+ '''
+@@ -912,9 +912,11 @@ class MWorker(SignalHandlingMultiprocessingProcess):
+ if 'cmd' not in data:
+ log.error('Received malformed command {0}'.format(data))
+ return {}
+- log.trace('AES payload received with command {0}'.format(data['cmd']))
+- if data['cmd'].startswith('__'):
+- return False
++ cmd = data['cmd']
++ log.trace('AES payload received with command %s', data['cmd'])
++ method = self.aes_funcs.get_method(cmd)
++ if not method:
++ return {}, {'fun': 'send'}
+ return self.aes_funcs.run_func(data['cmd'], data)
+
+ def run(self):
+@@ -931,13 +933,45 @@ class MWorker(SignalHandlingMultiprocessingProcess):
+ self.__bind()
+
+
++class TransportMethods(object):
++ '''
++ Expose methods to the transport layer, methods with their names found in
++ the class attribute 'expose_methods' will be exposed to the transport layer
++ via 'get_method'.
++ '''
++
++ expose_methods = ()
++
++ def get_method(self, name):
++ '''
++ Get a method which should be exposed to the transport layer
++ '''
++ if name in self.expose_methods:
++ try:
++ return getattr(self, name)
++ except AttributeError:
++ log.error("Expose method not found: %s", name)
++ else:
++ log.error("Requested method not exposed: %s", name)
++
++
+ # TODO: rename? No longer tied to "AES", just "encrypted" or "private" requests
+-class AESFuncs(object):
++class AESFuncs(TransportMethods):
+ '''
+ Set up functions that are available when the load is encrypted with AES
+ '''
+- # The AES Functions:
+- #
++
++ expose_methods = (
++ 'verify_minion', '_master_tops', '_ext_nodes', '_master_opts',
++ '_mine_get', '_mine', '_mine_delete', '_mine_flush', '_file_recv',
++ '_pillar', '_minion_event', '_handle_minion_event', '_return',
++ '_syndic_return', 'minion_runner', 'pub_ret', 'minion_pub',
++ 'minion_publish', 'revoke_auth', 'run_func', '_serve_file',
++ '_file_find', '_file_hash', '_file_find_and_stat', '_file_list',
++ '_file_list_emptydirs', '_dir_list', '_symlink_list', '_file_envs',
++ '_file_hash_and_stat',
++ )
++
+ def __init__(self, opts):
+ '''
+ Create a new AESFuncs
+@@ -1621,11 +1655,18 @@ class AESFuncs(object):
+ return ret, {'fun': 'send'}
+
+
+-class ClearFuncs(object):
++class ClearFuncs(TransportMethods):
+ '''
+ Set up functions that are safe to execute when commands sent to the master
+ without encryption and authentication
+ '''
++
++ # These methods will be exposed to the transport layer by
++ # MWorker._handle_clear
++ expose_methods = (
++ 'ping', 'publish', 'get_token', 'mk_token', 'wheel', 'runner',
++ )
++
+ # The ClearFuncs object encapsulates the functions that can be executed in
+ # the clear:
+ # publish (The publish from the LocalClient)
+diff --git a/salt/utils/verify.py b/salt/utils/verify.py
+index 5e320c3b59..38d723b303 100644
+--- a/salt/utils/verify.py
++++ b/salt/utils/verify.py
+@@ -28,6 +28,7 @@ from salt.exceptions import SaltClientError, SaltSystemExit, \
+ CommandExecutionError
+ import salt.defaults.exitcodes
+ import salt.utils
++import salt.ext.six
+
+ log = logging.getLogger(__name__)
+
+@@ -461,6 +462,50 @@ def check_max_open_files(opts):
+ log.log(level=level, msg=msg)
+
+
++def _realpath_darwin(path):
++ base = ''
++ for part in path.split(os.path.sep)[1:]:
++ if base != '':
++ if os.path.islink(os.path.sep.join([base, part])):
++ base = os.readlink(os.path.sep.join([base, part]))
++ else:
++ base = os.path.abspath(os.path.sep.join([base, part]))
++ else:
++ base = os.path.abspath(os.path.sep.join([base, part]))
++ return base
++
++
++def _realpath_windows(path):
++ base = ''
++ for part in path.split(os.path.sep):
++ if base != '':
++ try:
++ part = os.readlink(os.path.sep.join([base, part]))
++ base = os.path.abspath(part)
++ except OSError:
++ base = os.path.abspath(os.path.sep.join([base, part]))
++ else:
++ base = part
++ return base
++
++
++def _realpath(path):
++ '''
++ Cross platform realpath method. On Windows when python 3, this method
++ uses the os.readlink method to resolve any filesystem links. On Windows
++ when python 2, this method is a no-op. All other platforms and version use
++ os.realpath
++ '''
++ if sys.platform.startswith("darwin"):
++ return _realpath_darwin(path)
++ elif sys.platform.startswith("win"):
++ if salt.ext.six.PY3:
++ return _realpath_windows(path)
++ else:
++ return path
++ return os.path.realpath(path)
++
++
+ def clean_path(root, path, subdir=False):
+ '''
+ Accepts the root the path needs to be under and verifies that the path is
+diff --git a/salt/wheel/config.py b/salt/wheel/config.py
+index c4976fa45c..db6f98ca36 100644
+--- a/salt/wheel/config.py
++++ b/salt/wheel/config.py
+@@ -13,6 +13,8 @@ import yaml
+
+ # Import salt libs
+ import salt.config
++import salt.utils
++import salt.utils.verify
+
+ log = logging.getLogger(__name__)
+
+@@ -80,6 +82,9 @@ def update_config(file_name, yaml_contents):
+ os.makedirs(dir_path, 0o755)
+
+ file_path = os.path.join(dir_path, file_name)
++ if not salt.utils.verify.clean_path(dir_path, file_path):
++ return 'Invalid path'
++
+ with salt.utils.fopen(file_path, 'w') as fp_:
+ fp_.write(yaml_out)
+
+diff --git a/salt/wheel/file_roots.py b/salt/wheel/file_roots.py
+index ff13d3971b..e2baa0f8e8 100644
+--- a/salt/wheel/file_roots.py
++++ b/salt/wheel/file_roots.py
+@@ -9,6 +9,7 @@ import os
+
+ # Import salt libs
+ import salt.utils
++import salt.utils.verify
+
+ # Import 3rd-party libs
+ import salt.ext.six as six
+@@ -24,6 +25,8 @@ def find(path, saltenv='base'):
+ return ret
+ for root in __opts__['file_roots'][saltenv]:
+ full = os.path.join(root, path)
++ if not salt.utils.verify.clean_path(root, full):
++ continue
+ if os.path.isfile(full):
+ # Add it to the dict
+ with salt.utils.fopen(full, 'rb') as fp_:
+@@ -104,7 +107,10 @@ def write(data, path, saltenv='base', index=0):
+ if os.path.isabs(path):
+ return ('The path passed in {0} is not relative to the environment '
+ '{1}').format(path, saltenv)
+- dest = os.path.join(__opts__['file_roots'][saltenv][index], path)
++ root = __opts__['file_roots'][saltenv][index]
++ dest = os.path.join(root, path)
++ if not salt.utils.verify.clean_path(root, dest, subdir=True):
++ return 'Invalid path: {}'.format(path)
+ dest_dir = os.path.dirname(dest)
+ if not os.path.isdir(dest_dir):
+ os.makedirs(dest_dir)
+--
+2.26.2
+
diff -Nru salt-2016.11.2+ds/debian/patches/series salt-2016.11.2+ds/debian/patches/series
--- salt-2016.11.2+ds/debian/patches/series 2018-04-20 14:33:54.000000000 +0200
+++ salt-2016.11.2+ds/debian/patches/series 2020-05-04 14:29:16.000000000 +0200
@@ -9,3 +9,6 @@
Check_if_data_return_is_dict_type.patch
clean-doc-without-sphinx.patch
CVE-2017-8109.patch
+Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch
+Add-note-about-log-messages-to-hardening-salt-docs.patch
+various-netapi-fixes-and-tests.patch
diff -Nru salt-2016.11.2+ds/debian/patches/various-netapi-fixes-and-tests.patch salt-2016.11.2+ds/debian/patches/various-netapi-fixes-and-tests.patch
--- salt-2016.11.2+ds/debian/patches/various-netapi-fixes-and-tests.patch 1970-01-01 01:00:00.000000000 +0100
+++ salt-2016.11.2+ds/debian/patches/various-netapi-fixes-and-tests.patch 2020-05-04 14:29:16.000000000 +0200
@@ -0,0 +1,67 @@
+From: "Gareth J. Greenaway" <gareth at wiked.org>
+Date: Tue, 7 Jan 2020 11:22:31 -0800
+Subject: various netapi fixes and tests
+Origin: https://github.com/saltstack/salt/commit/bca115f3f00fbde564dd2f12bf036b5d2fd08387
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2019-17361
+Bug-Debian: https://bugs.debian.org/949222
+
+[Salvatore Bonaccorso: Backport to 2016.11.2, drop tests]
+---
+ conf/master | 6 ++
+ salt/config/__init__.py | 5 +
+ salt/netapi/__init__.py | 7 +-
+ tests/integration/netapi/test_client.py | 136 +++++++++++++++++++++++-
+ tests/support/helpers.py | 21 +++-
+ 5 files changed, 172 insertions(+), 3 deletions(-)
+
+--- a/conf/master
++++ b/conf/master
+@@ -1066,3 +1066,8 @@
+ # The value of the parameters can be either one module or a comma separated list of them.
+ #thin_extra_mods: foo,bar
+
++
++##### NetAPI settings #####
++############################################
++# Allow the raw_shell parameter to be used when calling Salt SSH client via API
++#netapi_allow_raw_shell: True
+--- a/salt/netapi/__init__.py
++++ b/salt/netapi/__init__.py
+@@ -62,10 +62,15 @@ class NetapiClient(object):
+ if low.get('client') not in CLIENTS:
+ raise salt.exceptions.SaltInvocationError('Invalid client specified')
+
+- if not ('token' in low or 'eauth' in low) and low['client'] != 'ssh':
++ if not ('token' in low or 'eauth' in low):
+ raise salt.exceptions.EauthAuthenticationError(
+ 'No authentication credentials given')
+
++ if low.get('raw_shell') and \
++ not self.opts.get('netapi_allow_raw_shell'):
++ raise salt.exceptions.EauthAuthenticationError(
++ 'Raw shell option not allowed.')
++
+ 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', {}))
+--- a/salt/config/__init__.py
++++ b/salt/config/__init__.py
+@@ -928,6 +928,10 @@ VALID_OPTS = {
+ # Note: to set enum arguments values like `cert_reqs` and `ssl_version` use constant names
+ # without ssl module prefix: `CERT_REQUIRED` or `PROTOCOL_SSLv23`.
+ 'ssl': (dict, type(None)),
++
++ # Allow raw_shell option when using the ssh
++ # client via the Salt API
++ 'netapi_allow_raw_shell': bool,
+ }
+
+ # default configurations
+@@ -1435,6 +1439,7 @@ DEFAULT_MASTER_OPTS = {
+ 'cache': 'localfs',
+ 'thin_extra_mods': '',
+ 'ssl': None,
++ 'netapi_allow_raw_shell': False,
+ }
+
+
-------------- next part --------------
diff -Nru salt-2018.3.4+dfsg1/debian/changelog salt-2018.3.4+dfsg1/debian/changelog
--- salt-2018.3.4+dfsg1/debian/changelog 2019-05-24 15:01:45.000000000 +0200
+++ salt-2018.3.4+dfsg1/debian/changelog 2020-05-03 21:11:01.000000000 +0200
@@ -1,3 +1,17 @@
+salt (2018.3.4+dfsg1-6+deb10u1) buster-security; urgency=high
+
+ * Non-maintainer upload by the Security Team.
+ * Fix CVE-2020-11651: Resolve issue which allows access to un-intended
+ methods in the ClearFuncs class of the salt-master process
+ (Closes: #959684)
+ * Fix CVE-2020-11652: Sanitize paths in ClearFuncs methods provided by
+ salt-master (Closes: #959684)
+ * Add note about log messages to hardening salt docs
+ * salt-api NET API with the ssh client enabled is vulnerable to command
+ injection (CVE-2019-17361) (Closes: #949222)
+
+ -- Salvatore Bonaccorso <carnil at debian.org> Sun, 03 May 2020 21:11:01 +0200
+
salt (2018.3.4+dfsg1-6) unstable; urgency=medium
* Revert changes that were rejected by the release team:
diff -Nru salt-2018.3.4+dfsg1/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch salt-2018.3.4+dfsg1/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch
--- salt-2018.3.4+dfsg1/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch 1970-01-01 01:00:00.000000000 +0100
+++ salt-2018.3.4+dfsg1/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch 2020-05-03 21:11:01.000000000 +0200
@@ -0,0 +1,41 @@
+From: "Daniel A. Wozniak" <dwozniak at saltstack.com>
+Date: Mon, 13 Apr 2020 07:01:07 +0000
+Subject: Add note about log messages to hardening salt docs
+Origin: https://github.com/saltstack/salt/commit/4631781376ddc9ee9d279f407ac3d0b78644fae7
+
+---
+ doc/topics/hardening.rst | 4 ++++
+ salt/master.py | 2 +-
+ 2 files changed, 5 insertions(+), 1 deletion(-)
+
+diff --git a/doc/topics/hardening.rst b/doc/topics/hardening.rst
+index c645b84128e4..569ad654af69 100644
+--- a/doc/topics/hardening.rst
++++ b/doc/topics/hardening.rst
+@@ -57,6 +57,10 @@ Salt hardening tips
+ particularly sensitive minions. There is also :ref:`salt-ssh` or the
+ :mod:`modules.sudo <salt.modules.sudo>` if you need to further restrict
+ a minion.
++- Monitor specific security releated log messages. Salt ``salt-master`` logs
++ attempts to access methods which are not exposed to network clients. These log
++ messages are logged at the ``error`` log level and start with ``Requested
++ method not exposed``.
+
+ .. _salt-users: https://groups.google.com/forum/#!forum/salt-users
+ .. _salt-announce: https://groups.google.com/forum/#!forum/salt-announce
+diff --git a/salt/master.py b/salt/master.py
+index 7d1444cf1221..aae55f1828e1 100644
+--- a/salt/master.py
++++ b/salt/master.py
+@@ -1162,7 +1162,7 @@ class TransportMethods(object):
+ try:
+ return getattr(self, name)
+ except AttributeError:
+- log.error("Expose method not found: %s", name)
++ log.error("Requested method not exposed: %s", name)
+ else:
+ log.error("Requested method not exposed: %s", name)
+
+--
+2.20.1
+
diff -Nru salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11651.patch salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11651.patch
--- salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11651.patch 1970-01-01 01:00:00.000000000 +0100
+++ salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11651.patch 2020-05-03 21:11:01.000000000 +0200
@@ -0,0 +1,302 @@
+From: "Daniel A. Wozniak" <dwozniak at saltstack.com>
+Date: Mon, 13 Apr 2020 06:41:04 +0000
+Subject: Fix CVE-2020-11651
+Origin: https://github.com/saltstack/salt/commit/f47e4856497231eb672da2ce0df3e641581d47e6
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2020-11651
+
+Resolve issue which allows access to un-intended methods in the
+ClearFuncs class of the salt-master process
+
+[Salvatore Bonaccorso: Fix typo in whitelisted method on on AESFuncs and
+replace '_minion_runner' with 'minion_runner'. Additional whitelist
+'_file_hash_and_stat']
+---
+ salt/master.py | 58 +++++++--
+ tests/integration/master/test_clear_funcs.py | 128 +++++++++++++++++++
+ tests/unit/test_master.py | 25 ++++
+ tests/whitelist.txt | 1 +
+ 4 files changed, 203 insertions(+), 9 deletions(-)
+ create mode 100644 tests/integration/master/test_clear_funcs.py
+
+--- a/salt/master.py
++++ b/salt/master.py
+@@ -1053,12 +1053,13 @@ class MWorker(salt.utils.process.SignalH
+ '''
+ log.trace('Clear payload received with command %s', load['cmd'])
+ cmd = load['cmd']
+- if cmd.startswith('__'):
+- return False
++ method = self.clear_funcs.get_method(cmd)
++ if not method:
++ return {}, {'fun': 'send_clear'}
+ if self.opts['master_stats']:
+ start = time.time()
+ self.stats[cmd]['runs'] += 1
+- ret = getattr(self.clear_funcs, cmd)(load), {'fun': 'send_clear'}
++ ret = method(load), {'fun': 'send_clear'}
+ if self.opts['master_stats']:
+ self._post_stats(start, cmd)
+ return ret
+@@ -1076,8 +1077,9 @@ class MWorker(salt.utils.process.SignalH
+ return {}
+ cmd = data['cmd']
+ log.trace('AES payload received with command %s', data['cmd'])
+- if cmd.startswith('__'):
+- return False
++ method = self.aes_funcs.get_method(cmd)
++ if not method:
++ return {}, {'fun': 'send'}
+ if self.opts['master_stats']:
+ start = time.time()
+ self.stats[cmd]['runs'] += 1
+@@ -1100,13 +1102,45 @@ class MWorker(salt.utils.process.SignalH
+ self.__bind()
+
+
++class TransportMethods(object):
++ '''
++ Expose methods to the transport layer, methods with their names found in
++ the class attribute 'expose_methods' will be exposed to the transport layer
++ via 'get_method'.
++ '''
++
++ expose_methods = ()
++
++ def get_method(self, name):
++ '''
++ Get a method which should be exposed to the transport layer
++ '''
++ if name in self.expose_methods:
++ try:
++ return getattr(self, name)
++ except AttributeError:
++ log.error("Expose method not found: %s", name)
++ else:
++ log.error("Requested method not exposed: %s", name)
++
++
+ # TODO: rename? No longer tied to "AES", just "encrypted" or "private" requests
+-class AESFuncs(object):
++class AESFuncs(TransportMethods):
+ '''
+ Set up functions that are available when the load is encrypted with AES
+ '''
+- # The AES Functions:
+- #
++
++ expose_methods = (
++ 'verify_minion', '_master_tops', '_ext_nodes', '_master_opts',
++ '_mine_get', '_mine', '_mine_delete', '_mine_flush', '_file_recv',
++ '_pillar', '_minion_event', '_handle_minion_event', '_return',
++ '_syndic_return', 'minion_runner', 'pub_ret', 'minion_pub',
++ 'minion_publish', 'revoke_auth', 'run_func', '_serve_file',
++ '_file_find', '_file_hash', '_file_find_and_stat', '_file_list',
++ '_file_list_emptydirs', '_dir_list', '_symlink_list', '_file_envs',
++ '_file_hash_and_stat',
++ )
++
+ def __init__(self, opts):
+ '''
+ Create a new AESFuncs
+@@ -1820,11 +1854,18 @@ class AESFuncs(object):
+ return ret, {'fun': 'send'}
+
+
+-class ClearFuncs(object):
++class ClearFuncs(TransportMethods):
+ '''
+ Set up functions that are safe to execute when commands sent to the master
+ without encryption and authentication
+ '''
++
++ # These methods will be exposed to the transport layer by
++ # MWorker._handle_clear
++ expose_methods = (
++ 'ping', 'publish', 'get_token', 'mk_token', 'wheel', 'runner',
++ )
++
+ # The ClearFuncs object encapsulates the functions that can be executed in
+ # the clear:
+ # publish (The publish from the LocalClient)
+--- /dev/null
++++ b/tests/integration/master/test_clear_funcs.py
+@@ -0,0 +1,128 @@
++# -*- coding: utf-8 -*-
++from __future__ import absolute_import, print_function, unicode_literals
++import getpass
++import os
++import tempfile
++import time
++
++import salt.master
++import salt.transport.client
++import salt.utils.platform
++import salt.utils.files
++import salt.utils.user
++
++from tests.support.case import TestCase
++from tests.support.mixins import AdaptedConfigurationTestCaseMixin
++from tests.support.runtests import RUNTIME_VARS
++
++
++def keyuser():
++ user = salt.utils.user.get_specific_user()
++ if user.startswith('sudo_'):
++ user = user[5:].replace('\\', '_')
++ return user
++
++
++class ClearFuncsAuthTestCase(TestCase):
++
++ def test_auth_info_not_allowed(self):
++ assert hasattr(salt.master.ClearFuncs, '_prep_auth_info')
++ master = '127.0.0.1'
++ ret_port = 64506
++ user = getpass.getuser()
++ keyfile = '.{}_key'.format(user)
++
++ keypath = os.path.join(RUNTIME_VARS.TMP, 'rootdir', 'cache', keyfile)
++
++ with salt.utils.files.fopen(keypath) as keyfd:
++ key = keyfd.read()
++
++ minion_config = {
++ 'transport': 'zeromq',
++ 'pki_dir': '/tmp',
++ 'id': 'root',
++ 'master_ip': master,
++ 'master_port': ret_port,
++ 'auth_timeout': 5,
++ 'auth_tries': 1,
++ 'master_uri': 'tcp://{0}:{1}'.format(master, ret_port)
++ }
++
++ clear_channel = salt.transport.client.ReqChannel.factory(
++ minion_config, crypt='clear')
++
++ msg = {'cmd': '_prep_auth_info'}
++ rets = clear_channel.send(msg, timeout=15)
++ ret_key = None
++ for ret in rets:
++ try:
++ ret_key = ret[user]
++ break
++ except (TypeError, KeyError):
++ pass
++ assert ret_key != key, 'Able to retrieve user key'
++
++
++class ClearFuncsPubTestCase(TestCase):
++
++ def setUp(self):
++ self.master = '127.0.0.1'
++ self.ret_port = 64506
++ self.tmpfile = os.path.join(tempfile.mkdtemp(), 'evil_file')
++ self.master_opts = AdaptedConfigurationTestCaseMixin.get_config('master')
++
++ def tearDown(self):
++ try:
++ os.remove(self.tmpfile + 'x')
++ except OSError:
++ pass
++ delattr(self, 'master')
++ delattr(self, 'ret_port')
++ delattr(self, 'tmpfile')
++
++ def test_pub_not_allowed(self):
++ assert hasattr(salt.master.ClearFuncs, '_send_pub')
++ assert not os.path.exists(self.tmpfile)
++ minion_config = {
++ 'transport': 'zeromq',
++ 'pki_dir': '/tmp',
++ 'id': 'root',
++ 'master_ip': self.master,
++ 'master_port': self.ret_port,
++ 'auth_timeout': 5,
++ 'auth_tries': 1,
++ 'master_uri': 'tcp://{0}:{1}'.format(self.master, self.ret_port),
++ }
++ clear_channel = salt.transport.client.ReqChannel.factory(
++ minion_config, crypt='clear')
++ jid = '202003100000000001'
++ msg = {
++ 'cmd': '_send_pub',
++ 'fun': 'file.write',
++ 'jid': jid,
++ 'arg': [self.tmpfile, 'evil contents'],
++ 'kwargs': {'show_jid': False, 'show_timeout': False},
++ 'ret': '',
++ 'tgt': 'minion',
++ 'tgt_type': 'glob',
++ 'user': 'root'
++ }
++ eventbus = salt.utils.event.get_event(
++ 'master',
++ sock_dir=self.master_opts['sock_dir'],
++ transport=self.master_opts['transport'],
++ opts=self.master_opts)
++ ret = clear_channel.send(msg, timeout=15)
++ if salt.utils.platform.is_windows():
++ time.sleep(30)
++ timeout = 30
++ else:
++ timeout = 5
++ ret_evt = None
++ start = time.time()
++ while time.time() - start <= timeout:
++ raw = eventbus.get_event(timeout, auto_reconnect=True)
++ if raw and 'jid' in raw and raw['jid'] == jid:
++ ret_evt = raw
++ break
++ assert not os.path.exists(self.tmpfile), 'Evil file created'
+--- a/tests/unit/test_master.py
++++ b/tests/unit/test_master.py
+@@ -15,6 +15,24 @@ from tests.support.mock import (
+ )
+
+
++class TransportMethodsTest(TestCase):
++
++ def test_transport_methods(self):
++
++ class Foo(salt.master.TransportMethods):
++ expose_methods = ['bar']
++
++ def bar(self):
++ pass
++
++ def bang(self):
++ pass
++
++ foo = Foo()
++ assert foo.get_method('bar') is not None
++ assert foo.get_method('bang') is None
++
++
+ class ClearFuncsTestCase(TestCase):
+ '''
+ TestCase for salt.master.ClearFuncs class
+@@ -24,6 +42,13 @@ class ClearFuncsTestCase(TestCase):
+ opts = salt.config.master_config(None)
+ self.clear_funcs = salt.master.ClearFuncs(opts, {})
+
++ def tearDown(self):
++ del self.clear_funcs
++
++ def test_get_method(self):
++ assert getattr(self.clear_funcs, '_send_pub', None) is not None
++ assert self.clear_funcs.get_method('_send_pub') is None
++
+ # runner tests
+
+ def test_runner_token_not_authenticated(self):
+--- a/tests/whitelist.txt
++++ b/tests/whitelist.txt
+@@ -9,6 +9,7 @@ integration.doc.test_man
+ integration.grains.test_core
+ integration.loader.test_ext_grains
+ integration.loader.test_ext_modules
++integration.master.test_clear_funcs
+ integration.minion.test_blackout
+ integration.minion.test_pillar
+ integration.minion.test_timeout
diff -Nru salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11652.patch salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11652.patch
--- salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11652.patch 1970-01-01 01:00:00.000000000 +0100
+++ salt-2018.3.4+dfsg1/debian/patches/Fix-CVE-2020-11652.patch 2020-05-03 21:11:01.000000000 +0200
@@ -0,0 +1,181 @@
+From: "Daniel A. Wozniak" <dwozniak at saltstack.com>
+Date: Mon, 13 Apr 2020 06:44:58 +0000
+Subject: Fix CVE-2020-11652
+Origin: https://github.com/saltstack/salt/commit/7bd0ab195fbec4f34523dad11149f741c154e2b7
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2020-11652
+
+Sanitize paths in ClearFuncs methods provided by salt-master. This
+ensures we do not allow access to un-intended files and directories.
+
+[Salvatore Bonaccorso: Drop added upstream tests for backport to 2018.3.4]
+---
+ salt/tokens/localfs.py | 3 +
+ salt/utils/verify.py | 57 +++++-
+ salt/wheel/config.py | 8 +-
+ salt/wheel/file_roots.py | 7 +-
+ tests/integration/master/test_clear_funcs.py | 182 +++++++++++++++++++
+ tests/unit/test_module_names.py | 1 +
+ tests/unit/utils/test_verify.py | 80 ++++++++
+ 7 files changed, 331 insertions(+), 7 deletions(-)
+
+diff --git a/salt/tokens/localfs.py b/salt/tokens/localfs.py
+index 3660ee318692..747f8eea1e45 100644
+--- a/salt/tokens/localfs.py
++++ b/salt/tokens/localfs.py
+@@ -12,6 +12,7 @@ import logging
+
+ import salt.utils.files
+ import salt.utils.path
++import salt.utils.verify
+ import salt.payload
+
+ from salt.ext import six
+@@ -61,6 +62,8 @@ def get_token(opts, tok):
+ :returns: Token data if successful. Empty dict if failed.
+ '''
+ t_path = os.path.join(opts['token_dir'], tok)
++ if not salt.utils.verify.clean_path(opts['token_dir'], t_path):
++ return {}
+ if not os.path.isfile(t_path):
+ return {}
+ serial = salt.payload.Serial(opts)
+diff --git a/salt/utils/verify.py b/salt/utils/verify.py
+index 583c5910f83a..9b74fd797aa5 100644
+--- a/salt/utils/verify.py
++++ b/salt/utils/verify.py
+@@ -31,6 +31,7 @@ import salt.utils.files
+ import salt.utils.path
+ import salt.utils.platform
+ import salt.utils.user
++import salt.ext.six
+
+ log = logging.getLogger(__name__)
+
+@@ -495,23 +496,69 @@ def check_max_open_files(opts):
+ log.log(level=level, msg=msg)
+
+
++def _realpath_darwin(path):
++ base = ''
++ for part in path.split(os.path.sep)[1:]:
++ if base != '':
++ if os.path.islink(os.path.sep.join([base, part])):
++ base = os.readlink(os.path.sep.join([base, part]))
++ else:
++ base = os.path.abspath(os.path.sep.join([base, part]))
++ else:
++ base = os.path.abspath(os.path.sep.join([base, part]))
++ return base
++
++
++def _realpath_windows(path):
++ base = ''
++ for part in path.split(os.path.sep):
++ if base != '':
++ try:
++ part = os.readlink(os.path.sep.join([base, part]))
++ base = os.path.abspath(part)
++ except OSError:
++ base = os.path.abspath(os.path.sep.join([base, part]))
++ else:
++ base = part
++ return base
++
++
++def _realpath(path):
++ '''
++ Cross platform realpath method. On Windows when python 3, this method
++ uses the os.readlink method to resolve any filesystem links. On Windows
++ when python 2, this method is a no-op. All other platforms and version use
++ os.realpath
++ '''
++ if salt.utils.platform.is_darwin():
++ return _realpath_darwin(path)
++ elif salt.utils.platform.is_windows():
++ if salt.ext.six.PY3:
++ return _realpath_windows(path)
++ else:
++ return path
++ return os.path.realpath(path)
++
++
+ def clean_path(root, path, subdir=False):
+ '''
+ Accepts the root the path needs to be under and verifies that the path is
+ under said root. Pass in subdir=True if the path can result in a
+ subdirectory of the root instead of having to reside directly in the root
+ '''
+- if not os.path.isabs(root):
++ real_root = _realpath(root)
++ if not os.path.isabs(real_root):
+ return ''
+ if not os.path.isabs(path):
+ path = os.path.join(root, path)
+ path = os.path.normpath(path)
++ real_path = _realpath(path)
+ if subdir:
+- if path.startswith(root):
+- return path
++ if real_path.startswith(real_root):
++ return real_path
+ else:
+- if os.path.dirname(path) == os.path.normpath(root):
+- return path
++ if os.path.dirname(real_path) == os.path.normpath(real_root):
++ return real_path
+ return ''
+
+
+diff --git a/salt/wheel/config.py b/salt/wheel/config.py
+index a8a93c53e56d..3984444f8f1f 100644
+--- a/salt/wheel/config.py
++++ b/salt/wheel/config.py
+@@ -75,13 +75,19 @@ def update_config(file_name, yaml_contents):
+ dir_path = os.path.join(__opts__['config_dir'],
+ os.path.dirname(__opts__['default_include']))
+ try:
+- yaml_out = salt.utils.yaml.safe_dump(yaml_contents, default_flow_style=False)
++ yaml_out = salt.utils.yaml.safe_dump(
++ yaml_contents,
++ default_flow_style=False,
++ )
+
+ if not os.path.exists(dir_path):
+ log.debug('Creating directory %s', dir_path)
+ os.makedirs(dir_path, 0o755)
+
+ file_path = os.path.join(dir_path, file_name)
++ if not salt.utils.verify.clean_path(dir_path, file_path):
++ return 'Invalid path'
++
+ with salt.utils.files.fopen(file_path, 'w') as fp_:
+ fp_.write(yaml_out)
+
+diff --git a/salt/wheel/file_roots.py b/salt/wheel/file_roots.py
+index 02cc8c5b3275..ad42335734e0 100644
+--- a/salt/wheel/file_roots.py
++++ b/salt/wheel/file_roots.py
+@@ -25,6 +25,8 @@ def find(path, saltenv='base'):
+ return ret
+ for root in __opts__['file_roots'][saltenv]:
+ full = os.path.join(root, path)
++ if not salt.utils.verify.clean_path(root, full):
++ continue
+ if os.path.isfile(full):
+ # Add it to the dict
+ with salt.utils.files.fopen(full, 'rb') as fp_:
+@@ -107,7 +109,10 @@ def write(data, path, saltenv='base', index=0):
+ if os.path.isabs(path):
+ return ('The path passed in {0} is not relative to the environment '
+ '{1}').format(path, saltenv)
+- dest = os.path.join(__opts__['file_roots'][saltenv][index], path)
++ root = __opts__['file_roots'][saltenv][index]
++ dest = os.path.join(root, path)
++ if not salt.utils.verify.clean_path(root, dest, subdir=True):
++ return 'Invalid path: {}'.format(path)
+ dest_dir = os.path.dirname(dest)
+ if not os.path.isdir(dest_dir):
+ os.makedirs(dest_dir)
+--
+2.20.1
+
diff -Nru salt-2018.3.4+dfsg1/debian/patches/series salt-2018.3.4+dfsg1/debian/patches/series
--- salt-2018.3.4+dfsg1/debian/patches/series 2019-05-24 15:01:45.000000000 +0200
+++ salt-2018.3.4+dfsg1/debian/patches/series 2020-05-03 21:11:01.000000000 +0200
@@ -22,3 +22,7 @@
0001-Import-tornado.gen-as-tornado_gen.patch
0002-Explicitly-import-attributes-from-tornado.patch
0003-Use-renamed-python3-tornado4.patch
+Fix-CVE-2020-11651.patch
+Fix-CVE-2020-11652.patch
+Add-note-about-log-messages-to-hardening-salt-docs.patch
+various-netapi-fixes-and-tests.patch
diff -Nru salt-2018.3.4+dfsg1/debian/patches/various-netapi-fixes-and-tests.patch salt-2018.3.4+dfsg1/debian/patches/various-netapi-fixes-and-tests.patch
--- salt-2018.3.4+dfsg1/debian/patches/various-netapi-fixes-and-tests.patch 1970-01-01 01:00:00.000000000 +0100
+++ salt-2018.3.4+dfsg1/debian/patches/various-netapi-fixes-and-tests.patch 2020-05-03 21:11:01.000000000 +0200
@@ -0,0 +1,68 @@
+From: "Gareth J. Greenaway" <gareth at wiked.org>
+Date: Tue, 7 Jan 2020 11:22:31 -0800
+Subject: various netapi fixes and tests
+Origin: https://github.com/saltstack/salt/commit/bca115f3f00fbde564dd2f12bf036b5d2fd08387
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2019-17361
+Bug-Debian: https://bugs.debian.org/949222
+
+[Salvatore Bonaccorso: Backport to 2018.3.4 for context changes; drop tests]
+---
+ conf/master | 6 ++
+ salt/config/__init__.py | 5 +
+ salt/netapi/__init__.py | 7 +-
+ tests/integration/netapi/test_client.py | 136 +++++++++++++++++++++++-
+ tests/support/helpers.py | 21 +++-
+ 5 files changed, 172 insertions(+), 3 deletions(-)
+
+--- a/conf/master
++++ b/conf/master
+@@ -1282,3 +1282,9 @@
+ # use OS defaults, typically 75 seconds on Linux, see
+ # /proc/sys/net/ipv4/tcp_keepalive_intvl.
+ #tcp_keepalive_intvl: -1
++
++
++##### NetAPI settings #####
++############################################
++# Allow the raw_shell parameter to be used when calling Salt SSH client via API
++#netapi_allow_raw_shell: True
+--- a/salt/config/__init__.py
++++ b/salt/config/__init__.py
+@@ -1196,6 +1196,10 @@ VALID_OPTS = {
+
+ # Enable calling ssh minions from the salt master
+ 'enable_ssh_minions': bool,
++
++ # Allow raw_shell option when using the ssh
++ # client via the Salt API
++ 'netapi_allow_raw_shell': bool,
+ }
+
+ # default configurations
+@@ -1837,6 +1841,7 @@ DEFAULT_MASTER_OPTS = {
+ 'auth_events': True,
+ 'minion_data_cache_events': True,
+ 'enable_ssh_minions': False,
++ 'netapi_allow_raw_shell': False,
+ }
+
+
+--- a/salt/netapi/__init__.py
++++ b/salt/netapi/__init__.py
+@@ -66,10 +66,15 @@ class NetapiClient(object):
+ raise salt.exceptions.SaltInvocationError(
+ 'Invalid client specified: \'{0}\''.format(low.get('client')))
+
+- if not ('token' in low or 'eauth' in low) and low['client'] != 'ssh':
++ if not ('token' in low or 'eauth' in low):
+ raise salt.exceptions.EauthAuthenticationError(
+ 'No authentication credentials given')
+
++ if low.get('raw_shell') and \
++ not self.opts.get('netapi_allow_raw_shell'):
++ raise salt.exceptions.EauthAuthenticationError(
++ 'Raw shell option not allowed.')
++
+ 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', {}))
More information about the pkg-salt-team
mailing list