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
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 <dwozn...@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 <car...@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" <dwozn...@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" <gar...@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" <gar...@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,
+ }
+ 
+ 
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 <car...@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" <dwozn...@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" <dwozn...@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" <dwozn...@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" <gar...@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', {}))

Reply via email to