valitydev/salt
14
0
Fork 0
mirror of https://github.com/valitydev/salt.git synced 2024-11-07 08:58:59 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #31626 from isbm/isbm-md5-to-sha1-dev

Browse Source 
Use SHA256 instead of MD5 (develop branch)
This commit is contained in:
Mike Place 2016-03-16 10:14:28 -06:00
commit 854d34aa30
9 changed files with 345 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
conf/master
View File

@ -472,9 +472,12 @@
# the master server. The default is md5, but sha1, sha224, sha256, sha384
# and sha512 are also supported.
#
# Prior to changing this value, the master should be stopped and all Salt
# WARNING: While md5 is also supported, do not use it due to the high chance
# of possible collisions and thus security breach.
#
# Prior to changing this value, the master should be stopped and all Salt
# caches should be cleared.
#hash_type: md5
#hash_type: sha256
# The buffer size in the file server can be adjusted here:
#file_buffer_size: 1048576

7
conf/minion
View File

@ -476,13 +476,16 @@
# is False.
#fileserver_limit_traversal: False
# The hash_type is the hash to use when discovering the hash of a file in
# The hash_type is the hash to use when discovering the hash of a file on
# the local fileserver. The default is md5, but sha1, sha224, sha256, sha384
# and sha512 are also supported.
#
# WARNING: While md5 is also supported, do not use it due to the high chance
# of possible collisions and thus security breach.
#
# Warning: Prior to changing this value, the minion should be stopped and all
# Salt caches should be cleared.
#hash_type: md5
#hash_type: sha256
# The Salt pillar is searched for locally if file_client is set to local. If
# this is the case, and pillar data is defined, then the pillar_roots need to

5
conf/proxy
View File

@ -422,9 +422,12 @@
# the local fileserver. The default is md5, but sha1, sha224, sha256, sha384
# and sha512 are also supported.
#
# WARNING: While md5 is also supported, do not use it due to the high chance
# of possible collisions and thus security breach.
#
# Warning: Prior to changing this value, the minion should be stopped and all
# Salt caches should be cleared.
#hash_type: md5
#hash_type: sha256
# The Salt pillar is searched for locally if file_client is set to local. If
# this is the case, and pillar data is defined, then the pillar_roots need to

174
salt/cli/daemons.py
View File

@ -57,7 +57,43 @@ from salt.exceptions import SaltSystemExit
log = salt.log.setup.logging.getLogger(__name__)
class Master(parsers.MasterOptionParser):
class DaemonsControlMixIn(object): # pylint: disable=no-init
'''
Uses the same functions for all daemons
'''
def verify_hash_type(self):
'''
Verify and display a nag-messsage to the log if vulnerable hash-type is used.
:return:
'''
if self.config['hash_type'].lower() in ['md5', 'sha1']:
log.warning('IMPORTANT: Do not use {h_type} hashing algorithm! Please set "hash_type" to '
'SHA256 in Salt {d_name} config!'.format(
h_type=self.config['hash_type'], d_name=self.__class__.__name__))
def action_log_info(self, action):
'''
Say daemon starting.
:param action
:return:
'''
log.info('{action} the Salt {d_name}'.format(d_name=self.__class__.__name__, action=action))
def environment_failure(self, error):
'''
Log environment failure for the daemon and exit with the error code.
:param error:
:return:
'''
log.exception('Failed to create environment for {d_name}: {reason}'.format(
d_name=self.__class__.__name__, reason=error.message))
self.shutdown(error.errno)
class Master(parsers.MasterOptionParser, DaemonsControlMixIn):
'''
Creates a master server
'''
@ -110,9 +146,7 @@ class Master(parsers.MasterOptionParser):
pki_dir=self.config['pki_dir'],
)
logfile = self.config['log_file']
if logfile is not None and not logfile.startswith(('tcp://',
'udp://',
'file://')):
if logfile is not None and not logfile.startswith(('tcp://', 'udp://', 'file://')):
# Logfile is not using Syslog, verify
current_umask = os.umask(0o027)
verify_files([logfile], self.config['user'])
@ -120,13 +154,12 @@ class Master(parsers.MasterOptionParser):
# Clear out syndics from cachedir
for syndic_file in os.listdir(self.config['syndic_dir']):
os.remove(os.path.join(self.config['syndic_dir'], syndic_file))
except OSError as err:
log.exception('Failed to prepare salt environment')
self.shutdown(err.errno)
except OSError as error:
self.environment_failure(error)
self.setup_logfile_logger()
verify_log(self.config)
log.info('Setting up the Salt Master')
self.action_log_info('Setting up')
# TODO: AIO core is separate from transport
if self.config['transport'].lower() in ('zeromq', 'tcp'):
@ -160,14 +193,15 @@ class Master(parsers.MasterOptionParser):
'''
super(Master, self).start()
if check_user(self.config['user']):
log.info('The salt master is starting up')
self.action_log_info('Starting up')
self.verify_hash_type()
self.master.start()
def shutdown(self, exitcode=0, exitmsg=None):
'''
If sub-classed, run any shutdown operations on this method.
'''
log.info('The salt master is shutting down..')
self.action_log_info('Shutting down')
msg = 'The salt master is shutdown. '
if exitmsg is not None:
exitmsg = msg + exitmsg
@ -176,7 +210,7 @@ class Master(parsers.MasterOptionParser):
super(Master, self).shutdown(exitcode, exitmsg)
class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
class Minion(parsers.MinionOptionParser, DaemonsControlMixIn): # pylint: disable=no-init
'''
Create a minion server
'''
@ -241,16 +275,13 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
pki_dir=self.config['pki_dir'],
)
logfile = self.config['log_file']
if logfile is not None and not logfile.startswith(('tcp://',
'udp://',
'file://')):
if logfile is not None and not logfile.startswith(('tcp://', 'udp://', 'file://')):
# Logfile is not using Syslog, verify
current_umask = os.umask(0o027)
verify_files([logfile], self.config['user'])
os.umask(current_umask)
except OSError as err:
log.exception('Failed to prepare salt environment')
self.shutdown(err.errno)
except OSError as error:
self.environment_failure(error)
self.setup_logfile_logger()
verify_log(self.config)
@ -263,7 +294,7 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
# Bail out if we find a process running and it matches out pidfile
if self.check_running():
log.exception('Salt minion is already running. Exiting.')
self.action_log_info('An instance is already running. Exiting')
self.shutdown(1)
# TODO: AIO core is separate from transport
@ -304,16 +335,17 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
super(Minion, self).start()
try:
if check_user(self.config['user']):
log.info('The salt minion is starting up')
self.action_log_info('Starting up')
self.verify_hash_type()
self.minion.tune_in()
except (KeyboardInterrupt, SaltSystemExit) as exc:
log.warning('Stopping the Salt Minion')
if isinstance(exc, KeyboardInterrupt):
except (KeyboardInterrupt, SaltSystemExit) as error:
self.action_log_info('Stopping')
if isinstance(error, KeyboardInterrupt):
log.warning('Exiting on Ctrl-c')
self.shutdown()
else:
log.error(str(exc))
self.shutdown(exc.code)
log.error(str(error))
self.shutdown(error.code)
def call(self, cleanup_protecteds):
'''
@ -335,7 +367,7 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
self.minion.opts['raet_cleanup_protecteds'] = cleanup_protecteds
self.minion.call_in()
except (KeyboardInterrupt, SaltSystemExit) as exc:
log.warning('Stopping the Salt Minion')
self.action_log_info('Stopping')
if isinstance(exc, KeyboardInterrupt):
log.warning('Exiting on Ctrl-c')
self.shutdown()
@ -346,20 +378,20 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
def shutdown(self, exitcode=0, exitmsg=None):
'''
If sub-classed, run any shutdown operations on this method.
:param exitcode
:param exitmsg
'''
log.info('The salt minion is shutting down..')
self.action_log_info('Shutting down')
if hasattr(self, 'minion'):
self.minion.destroy()
msg = 'The salt minion is shutdown. '
if exitmsg is not None:
exitmsg = msg + exitmsg
else:
exitmsg = msg.strip()
super(Minion, self).shutdown(exitcode, exitmsg)
super(Minion, self).shutdown(
exitcode, ('The Salt {0} is shutdown. {1}'.format(
self.__class__.__name__, (exitmsg or '')).strip()))
# pylint: enable=no-member
class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
class ProxyMinion(parsers.ProxyMinionOptionParser, DaemonsControlMixIn): # pylint: disable=no-init
'''
Create a proxy minion server
'''
@ -422,29 +454,21 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
permissive=self.config['permissive_pki_access'],
pki_dir=self.config['pki_dir'],
)
if 'proxy_log' in self.config:
logfile = self.config['proxy_log']
else:
logfile = self.config['log_file']
if logfile is not None and not logfile.startswith(('tcp://',
'udp://',
'file://')):
logfile = self.config.get('proxy_log') or self.config['log_file']
if logfile is not None and not logfile.startswith(('tcp://', 'udp://', 'file://')):
# Logfile is not using Syslog, verify
current_umask = os.umask(0o027)
verify_files([logfile], self.config['user'])
os.umask(current_umask)
except OSError as err:
log.exception('Failed to prepare salt environment')
self.shutdown(err.errno)
except OSError as error:
self.environment_failure(error)
self.setup_logfile_logger()
verify_log(self.config)
log.info(
'Setting up a Salt Proxy Minion "{0}"'.format(
self.config['id']
)
)
self.action_log_info('Setting up "{0}"'.format(self.config['id']))
migrations.migrate_paths(self.config)
# TODO: AIO core is separate from transport
if self.config['transport'].lower() in ('zeromq', 'tcp'):
@ -479,9 +503,11 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
try:
if check_user(self.config['user']):
log.info('The proxy minion is starting up')
self.verify_hash_type()
self.action_log_info('Starting up')
self.minion.tune_in()
except (KeyboardInterrupt, SaltSystemExit) as exc:
log.warning('Stopping the Salt Proxy Minion')
self.action_log_info('Stopping')
if isinstance(exc, KeyboardInterrupt):
log.warning('Exiting on Ctrl-c')
self.shutdown()
@ -492,21 +518,21 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
def shutdown(self, exitcode=0, exitmsg=None):
'''
If sub-classed, run any shutdown operations on this method.
:param exitcode
:param exitmsg
'''
if hasattr(self, 'minion') and 'proxymodule' in self.minion.opts:
proxy_fn = self.minion.opts['proxymodule'].loaded_base_name + '.shutdown'
self.minion.opts['proxymodule'][proxy_fn](self.minion.opts)
log.info('The proxy minion is shutting down..')
msg = 'The proxy minion is shutdown. '
if exitmsg is not None:
exitmsg = msg + exitmsg
else:
exitmsg = msg.strip()
super(ProxyMinion, self).shutdown(exitcode, exitmsg)
self.action_log_info('Shutting down')
super(ProxyMinion, self).shutdown(
exitcode, ('The Salt {0} is shutdown. {1}'.format(
self.__class__.__name__, (exitmsg or '')).strip()))
# pylint: enable=no-member
class Syndic(parsers.SyndicOptionParser):
class Syndic(parsers.SyndicOptionParser, DaemonsControlMixIn):
'''
Create a syndic server
'''
@ -534,24 +560,17 @@ class Syndic(parsers.SyndicOptionParser):
pki_dir=self.config['pki_dir'],
)
logfile = self.config['log_file']
if logfile is not None and not logfile.startswith(('tcp://',
'udp://',
'file://')):
if logfile is not None and not logfile.startswith(('tcp://', 'udp://', 'file://')):
# Logfile is not using Syslog, verify
current_umask = os.umask(0o027)
verify_files([logfile], self.config['user'])
os.umask(current_umask)
except OSError as err:
log.exception('Failed to prepare salt environment')
self.shutdown(err.errno)
except OSError as error:
self.environment_failure(error)
self.setup_logfile_logger()
verify_log(self.config)
log.info(
'Setting up the Salt Syndic Minion "{0}"'.format(
self.config['id']
)
)
self.action_log_info('Setting up "{0}"'.format(self.config['id']))
# Late import so logging works correctly
import salt.minion
@ -575,21 +594,22 @@ class Syndic(parsers.SyndicOptionParser):
'''
super(Syndic, self).start()
if check_user(self.config['user']):
log.info('The salt syndic is starting up')
self.action_log_info('Starting up')
self.verify_hash_type()
try:
self.syndic.tune_in()
except KeyboardInterrupt:
log.warning('Stopping the Salt Syndic Minion')
self.action_log_info('Stopping')
self.shutdown()
def shutdown(self, exitcode=0, exitmsg=None):
'''
If sub-classed, run any shutdown operations on this method.
:param exitcode
:param exitmsg
'''
log.info('The salt syndic is shutting down..')
msg = 'The salt syndic is shutdown. '
if exitmsg is not None:
exitmsg = msg + exitmsg
else:
exitmsg = msg.strip()
super(Syndic, self).shutdown(exitcode, exitmsg)
self.action_log_info('Shutting down')
super(Syndic, self).shutdown(
exitcode, ('The Salt {0} is shutdown. {1}'.format(
self.__class__.__name__, (exitmsg or '')).strip()))

4
salt/crypt.py
View File

@ -585,11 +585,11 @@ class AsyncAuth(object):
if self.opts.get('syndic_master', False): # Is syndic
syndic_finger = self.opts.get('syndic_finger', self.opts.get('master_finger', False))
if syndic_finger:
if salt.utils.pem_finger(m_pub_fn) != syndic_finger:
if salt.utils.pem_finger(m_pub_fn, sum_type=self.opts['hash_type']) != syndic_finger:
self._finger_fail(syndic_finger, m_pub_fn)
else:
if self.opts.get('master_finger', False):
if salt.utils.pem_finger(m_pub_fn) != self.opts['master_finger']:
if salt.utils.pem_finger(m_pub_fn, sum_type=self.opts['hash_type']) != self.opts['master_finger']:
self._finger_fail(self.opts['master_finger'], m_pub_fn)
auth['publish_port'] = payload['publish_port']
raise tornado.gen.Return(auth)

14
salt/modules/key.py
View File

@ -21,10 +21,9 @@ def finger():
salt '*' key.finger
'''
return salt.utils.pem_finger(
os.path.join(__opts__['pki_dir'], 'minion.pub'),
sum_type=__opts__['hash_type']
)
# MD5 here is temporary. Change to SHA256 when retired.
return salt.utils.pem_finger(os.path.join(__opts__['pki_dir'], 'minion.pub'),
sum_type=__opts__.get('hash_type', 'md5'))
def finger_master():
@ -37,7 +36,6 @@ def finger_master():
salt '*' key.finger_master
'''
return salt.utils.pem_finger(
os.path.join(__opts__['pki_dir'], 'minion_master.pub'),
sum_type=__opts__['hash_type']
)
# MD5 here is temporary. Change to SHA256 when retired.
return salt.utils.pem_finger(os.path.join(__opts__['pki_dir'], 'minion_master.pub'),
sum_type=__opts__.get('hash_type', 'md5'))

4
salt/utils/__init__.py
View File

@ -858,10 +858,10 @@ def path_join(*parts):
))
def pem_finger(path=None, key=None, sum_type='md5'):
def pem_finger(path=None, key=None, sum_type='sha256'):
'''
Pass in either a raw pem string, or the path on disk to the location of a
pem file, and the type of cryptographic hash to use. The default is md5.
pem file, and the type of cryptographic hash to use. The default is SHA256.
The fingerprint of the pem will be returned.
If neither a key nor a path are passed in, a blank string will be returned.

4
salt/utils/cloud.py
View File

@ -2510,8 +2510,10 @@ def init_cachedir(base=None):
return base
# FIXME: This function seems used nowhere. Dead code?
def request_minion_cachedir(
minion_id,
opts,
fingerprint='',
pubkey=None,
provider=None,
@ -2530,7 +2532,7 @@ def request_minion_cachedir(
base = os.path.join(syspaths.CACHE_DIR, 'cloud')
if not fingerprint and pubkey is not None:
fingerprint = salt.utils.pem_finger(key=pubkey)
fingerprint = salt.utils.pem_finger(key=pubkey, sum_type=opts.get('hash_type', 'sha256'))
init_cachedir(base)

221
tests/unit/daemons_test.py Normal file
View File

@ -0,0 +1,221 @@
# -*- coding: utf-8 -*-
'''
:codeauthor: :email:`Bo Maryniuk <bo@suse.de>`
'''
# Import python libs
from __future__ import absolute_import
# Import Salt Testing libs
from salttesting import TestCase, skipIf
from salttesting.helpers import ensure_in_syspath
from salttesting.mock import patch, MagicMock, NO_MOCK, NO_MOCK_REASON
ensure_in_syspath('../')
# Import Salt libs
import integration
from salt.cli import daemons
class LoggerMock(object):
'''
Logger data collector
'''
def __init__(self):
'''
init
:return:
'''
self.reset()
def reset(self):
'''
Reset values
:return:
'''
self.messages = list()
def info(self, data):
'''
Collects the data from the logger of info type.
:param data:
:return:
'''
self.messages.append({'message': data, 'type': 'info'})
def warning(self, data):
'''
Collects the data from the logger of warning type.
:param data:
:return:
'''
self.messages.append({'message': data, 'type': 'warning'})
def has_message(self, msg, log_type=None):
'''
Check if log has message.
:param data:
:return:
'''
for data in self.messages:
if (data['type'] == log_type or not log_type) and data['message'].find(msg) > -1:
return True
return False
@skipIf(NO_MOCK, NO_MOCK_REASON)
class DaemonsStarterTestCase(TestCase, integration.SaltClientTestCaseMixIn):
'''
Unit test for the daemons starter classes.
'''
def test_master_daemon_hash_type_verified(self):
'''
Verify if Master is verifying hash_type config option.
:return:
'''
def _create_master():
'''
Create master instance
:return:
'''
master = daemons.Master()
master.config = {'user': 'dummy', 'hash_type': alg}
for attr in ['master', 'start_log_info', 'prepare']:
setattr(master, attr, MagicMock())
return master
_logger = LoggerMock()
with patch('salt.cli.daemons.check_user', MagicMock(return_value=True)):
with patch('salt.cli.daemons.log', _logger):
for alg in ['md5', 'sha1']:
_create_master().start()
self.assertTrue(_logger.messages)
self.assertTrue(_logger.has_message('Do not use {alg}'.format(alg=alg),
log_type='warning'))
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_master().start()
self.assertTrue(_logger.messages)
self.assertFalse(_logger.has_message('Do not use '))
def test_minion_daemon_hash_type_verified(self):
'''
Verify if Minion is verifying hash_type config option.
:return:
'''
def _create_minion():
'''
Create minion instance
:return:
'''
obj = daemons.Minion()
obj.config = {'user': 'dummy', 'hash_type': alg}
for attr in ['minion', 'start_log_info', 'prepare', 'shutdown']:
setattr(obj, attr, MagicMock())
return obj
_logger = LoggerMock()
with patch('salt.cli.daemons.check_user', MagicMock(return_value=True)):
with patch('salt.cli.daemons.log', _logger):
for alg in ['md5', 'sha1']:
_create_minion().start()
self.assertTrue(_logger.messages)
self.assertTrue(_logger.has_message('Do not use {alg}'.format(alg=alg),
log_type='warning'))
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_minion().start()
self.assertTrue(_logger.messages)
self.assertFalse(_logger.has_message('Do not use '))
def test_proxy_minion_daemon_hash_type_verified(self):
'''
Verify if ProxyMinion is verifying hash_type config option.
:return:
'''
def _create_proxy_minion():
'''
Create proxy minion instance
:return:
'''
obj = daemons.ProxyMinion()
obj.config = {'user': 'dummy', 'hash_type': alg}
for attr in ['minion', 'start_log_info', 'prepare', 'shutdown']:
setattr(obj, attr, MagicMock())
return obj
_logger = LoggerMock()
with patch('salt.cli.daemons.check_user', MagicMock(return_value=True)):
with patch('salt.cli.daemons.log', _logger):
for alg in ['md5', 'sha1']:
_create_proxy_minion().start()
self.assertTrue(_logger.messages)
self.assertTrue(_logger.has_message('Do not use {alg}'.format(alg=alg),
log_type='warning'))
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_proxy_minion().start()
self.assertTrue(_logger.messages)
self.assertFalse(_logger.has_message('Do not use '))
def test_syndic_daemon_hash_type_verified(self):
'''
Verify if Syndic is verifying hash_type config option.
:return:
'''
def _create_syndic():
'''
Create syndic instance
:return:
'''
obj = daemons.Syndic()
obj.config = {'user': 'dummy', 'hash_type': alg}
for attr in ['syndic', 'start_log_info', 'prepare', 'shutdown']:
setattr(obj, attr, MagicMock())
return obj
_logger = LoggerMock()
with patch('salt.cli.daemons.check_user', MagicMock(return_value=True)):
with patch('salt.cli.daemons.log', _logger):
for alg in ['md5', 'sha1']:
_create_syndic().start()
self.assertTrue(_logger.messages)
self.assertTrue(_logger.has_message('Do not use {alg}'.format(alg=alg),
log_type='warning'))
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_syndic().start()
self.assertTrue(_logger.messages)
self.assertFalse(_logger.has_message('Do not use '))
if __name__ == '__main__':
from integration import run_tests
run_tests(DaemonsStarterTestCase, needs_daemon=False)