Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP4:GA
salt.6185
security-fixes-cve-2017-14695-and-cve-2017-1469...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File security-fixes-cve-2017-14695-and-cve-2017-14696.patch of Package salt.6185
From b7ef927ef4a2548217750ee26c09c649733001fd Mon Sep 17 00:00:00 2001 From: Erik Johnson <palehose@gmail.com> Date: Fri, 25 Aug 2017 14:15:58 -0500 Subject: [PATCH] Security fixes: CVE-2017-14695 and CVE-2017-14696 * Don't allow path separators in minion ID * Do not allow IDs with null bytes in decoded payloads --- salt/crypt.py | 3 ++ salt/transport/tcp.py | 109 ++++++++++++++++++++++++---------------- salt/transport/zeromq.py | 11 ++++ salt/utils/verify.py | 15 ++---- tests/unit/utils/verify_test.py | 10 ++++ 5 files changed, 94 insertions(+), 54 deletions(-) diff --git a/salt/crypt.py b/salt/crypt.py index 6658e46919..fbff832d0d 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -576,6 +576,9 @@ class AsyncAuth(object): raise tornado.gen.Return('retry') else: raise SaltClientError('Attempt to authenticate with the salt master failed with timeout error') + if not isinstance(payload, dict): + log.error('Sign-in attempt failed: %s', payload) + raise tornado.gen.Return(False) if 'load' in payload: if 'ret' in payload['load']: if not payload['load']['ret']: diff --git a/salt/transport/tcp.py b/salt/transport/tcp.py index 29ef0746d5..cf047a4744 100644 --- a/salt/transport/tcp.py +++ b/salt/transport/tcp.py @@ -611,49 +611,72 @@ class TCPReqServerChannel(salt.transport.mixins.auth.AESReqServerMixin, salt.tra Handle incoming messages from underylying tcp streams ''' try: - payload = self._decode_payload(payload) - except Exception: - stream.write(salt.transport.frame.frame_msg('bad load', header=header)) - raise tornado.gen.Return() - - # TODO helper functions to normalize payload? - if not isinstance(payload, dict) or not isinstance(payload.get('load'), dict): - yield stream.write(salt.transport.frame.frame_msg( - 'payload and load must be a dict', header=header)) - raise tornado.gen.Return() - - # intercept the "_auth" commands, since the main daemon shouldn't know - # anything about our key auth - if payload['enc'] == 'clear' and payload.get('load', {}).get('cmd') == '_auth': - yield stream.write(salt.transport.frame.frame_msg( - self._auth(payload['load']), header=header)) - raise tornado.gen.Return() - - # TODO: test - try: - ret, req_opts = yield self.payload_handler(payload) - except Exception as e: - # always attempt to return an error to the minion - stream.write('Some exception handling minion payload') - log.error('Some exception handling a payload from minion', exc_info=True) - stream.close() - raise tornado.gen.Return() - - req_fun = req_opts.get('fun', 'send') - if req_fun == 'send_clear': - stream.write(salt.transport.frame.frame_msg(ret, header=header)) - elif req_fun == 'send': - stream.write(salt.transport.frame.frame_msg(self.crypticle.dumps(ret), header=header)) - elif req_fun == 'send_private': - stream.write(salt.transport.frame.frame_msg(self._encrypt_private(ret, - req_opts['key'], - req_opts['tgt'], - ), header=header)) - else: - log.error('Unknown req_fun {0}'.format(req_fun)) - # always attempt to return an error to the minion - stream.write('Server-side exception handling payload') - stream.close() + try: + payload = self._decode_payload(payload) + except Exception: + stream.write(salt.transport.frame.frame_msg('bad load', header=header)) + raise tornado.gen.Return() + + # TODO helper functions to normalize payload? + if not isinstance(payload, dict) or not isinstance(payload.get('load'), dict): + yield stream.write(salt.transport.frame.frame_msg( + 'payload and load must be a dict', header=header)) + raise tornado.gen.Return() + + try: + id_ = payload['load'].get('id', '') + if '\0' in id_: + log.error('Payload contains an id with a null byte: %s', payload) + stream.send(self.serial.dumps('bad load: id contains a null byte')) + raise tornado.gen.Return() + except TypeError: + log.error('Payload contains non-string id: %s', payload) + stream.send(self.serial.dumps('bad load: id {0} is not a string'.format(id_))) + raise tornado.gen.Return() + + # intercept the "_auth" commands, since the main daemon shouldn't know + # anything about our key auth + if payload['enc'] == 'clear' and payload.get('load', {}).get('cmd') == '_auth': + yield stream.write(salt.transport.frame.frame_msg( + self._auth(payload['load']), header=header)) + raise tornado.gen.Return() + + # TODO: test + try: + ret, req_opts = yield self.payload_handler(payload) + except Exception as e: + # always attempt to return an error to the minion + stream.write('Some exception handling minion payload') + log.error('Some exception handling a payload from minion', exc_info=True) + stream.close() + raise tornado.gen.Return() + + req_fun = req_opts.get('fun', 'send') + if req_fun == 'send_clear': + stream.write(salt.transport.frame.frame_msg(ret, header=header)) + elif req_fun == 'send': + stream.write(salt.transport.frame.frame_msg(self.crypticle.dumps(ret), header=header)) + elif req_fun == 'send_private': + stream.write(salt.transport.frame.frame_msg(self._encrypt_private(ret, + req_opts['key'], + req_opts['tgt'], + ), header=header)) + else: + log.error('Unknown req_fun {0}'.format(req_fun)) + # always attempt to return an error to the minion + stream.write('Server-side exception handling payload') + stream.close() + except tornado.gen.Return: + raise + except tornado.iostream.StreamClosedError: + # Stream was closed. This could happen if the remote side + # closed the connection on its end (eg in a timeout or shutdown + # situation). + log.error('Connection was unexpectedly closed', exc_info=True) + except Exception as exc: # pylint: disable=broad-except + # Absorb any other exceptions + log.error('Unexpected exception occurred: {0}'.format(exc), exc_info=True) + raise tornado.gen.Return() diff --git a/salt/transport/zeromq.py b/salt/transport/zeromq.py index 48bece8344..58106120ba 100644 --- a/salt/transport/zeromq.py +++ b/salt/transport/zeromq.py @@ -605,6 +605,17 @@ class ZeroMQReqServerChannel(salt.transport.mixins.auth.AESReqServerMixin, salt. stream.send(self.serial.dumps('payload and load must be a dict')) raise tornado.gen.Return() + try: + id_ = payload['load'].get('id', '') + if '\0' in id_: + log.error('Payload contains an id with a null byte: %s', payload) + stream.send(self.serial.dumps('bad load: id contains a null byte')) + raise tornado.gen.Return() + except TypeError: + log.error('Payload contains non-string id: %s', payload) + stream.send(self.serial.dumps('bad load: id {0} is not a string'.format(id_))) + raise tornado.gen.Return() + # intercept the "_auth" commands, since the main daemon shouldn't know # anything about our key auth if payload['enc'] == 'clear' and payload.get('load', {}).get('cmd') == '_auth': diff --git a/salt/utils/verify.py b/salt/utils/verify.py index 8690f6fb61..45581f02ce 100644 --- a/salt/utils/verify.py +++ b/salt/utils/verify.py @@ -481,22 +481,15 @@ def clean_path(root, path, subdir=False): return '' -def clean_id(id_): - ''' - Returns if the passed id is clean. - ''' - if re.search(r'\.\.\{sep}'.format(sep=os.sep), id_): - return False - return True - - def valid_id(opts, id_): ''' Returns if the passed id is valid ''' try: - return bool(clean_path(opts['pki_dir'], id_)) and clean_id(id_) - except (AttributeError, KeyError) as e: + if any(x in id_ for x in ('/', '\\', '\0')): + return False + return bool(clean_path(opts['pki_dir'], id_)) + except (AttributeError, KeyError, TypeError): return False diff --git a/tests/unit/utils/verify_test.py b/tests/unit/utils/verify_test.py index 7e60f886d0..c3fa373290 100644 --- a/tests/unit/utils/verify_test.py +++ b/tests/unit/utils/verify_test.py @@ -60,6 +60,16 @@ class TestVerify(TestCase): opts = {'pki_dir': '/tmp/whatever'} self.assertFalse(valid_id(opts, None)) + def test_valid_id_pathsep(self): + ''' + Path separators in id should make it invalid + ''' + opts = {'pki_dir': '/tmp/whatever'} + # We have to test both path separators because os.path.normpath will + # convert forward slashes to backslashes on Windows. + for pathsep in ('/', '\\'): + self.assertFalse(valid_id(opts, pathsep.join(('..', 'foobar')))) + def test_zmq_verify(self): self.assertTrue(zmq_version()) -- 2.14.2
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