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 #31162 from isbm/isbm-md5-to-sha1

Browse Source 
Remove MD5 digest from everywhere and default to SHA256
This commit is contained in:
Mike Place 2016-03-07 12:11:36 -07:00
commit 826fea6582
11 changed files with 312 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
conf/master
View File

@ -465,10 +465,13 @@
#default_top: base
# The hash_type is the hash to use when discovering the hash of a file on
# the master server. The default is md5, but sha1, sha224, sha256, sha384
# 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 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

8
conf/minion
View File

@ -444,12 +444,14 @@
#fileserver_limit_traversal: False
# The hash_type is the hash to use when discovering the hash of a file in
# the local fileserver. The default is md5, but sha1, sha224, sha256, sha384
# and sha512 are also supported.
# the local fileserver. The default is sha256, sha224, sha384 and sha512 are also supported.
#
# WARNING: While md5 and sha1 are 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

9
conf/proxy
View File

@ -419,12 +419,15 @@
#fileserver_limit_traversal: False
# The hash_type is the hash to use when discovering the hash of a file in
# the local fileserver. The default is md5, but sha1, sha224, sha256, sha384
# and sha512 are also supported.
# the local fileserver. The default is sha256 but sha224, sha384 and sha512
# are also supported.
#
# WARNING: While md5 and sha1 are 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

83
salt/cli/daemons.py
View File

@ -58,7 +58,50 @@ from salt.exceptions import SaltSystemExit
logger = salt.log.setup.logging.getLogger(__name__)
class Master(parsers.MasterOptionParser):
class DaemonsMixin(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']:
logger.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 start_log_info(self):
'''
Say daemon starting.
:return:
'''
logger.info('The Salt {d_name} is starting up'.format(d_name=self.__class__.__name__))
def shutdown_log_info(self):
'''
Say daemon shutting down.
:return:
'''
logger.info('The Salt {d_name} is shut down'.format(d_name=self.__class__.__name__))
def environment_failure(self, error):
'''
Log environment failure for the daemon and exit with the error code.
:param error:
:return:
'''
logger.exception('Failed to create environment for {d_name}: {reason}'.format(
d_name=self.__class__.__name__, reason=error.message))
sys.exit(error.errno)
class Master(parsers.MasterOptionParser, DaemonsMixin): # pylint: disable=no-init
'''
Creates a master server
'''
@ -114,8 +157,7 @@ class Master(parsers.MasterOptionParser):
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:
logger.exception('Failed to prepare salt environment')
sys.exit(err.errno)
self.environment_failure(err)
self.setup_logfile_logger()
verify_log(self.config)
@ -153,17 +195,18 @@ class Master(parsers.MasterOptionParser):
'''
self.prepare()
if check_user(self.config['user']):
logger.info('The salt master is starting up')
self.verify_hash_type()
self.start_log_info()
self.master.start()
def shutdown(self):
'''
If sub-classed, run any shutdown operations on this method.
'''
logger.info('The salt master is shut down')
self.shutdown_log_info()
class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
class Minion(parsers.MinionOptionParser, DaemonsMixin): # pylint: disable=no-init
'''
Create a minion server
'''
@ -226,8 +269,7 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
verify_files([logfile], self.config['user'])
os.umask(current_umask)
except OSError as err:
logger.exception('Failed to prepare salt environment')
sys.exit(err.errno)
self.environment_failure(err)
self.setup_logfile_logger()
verify_log(self.config)
@ -273,7 +315,8 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
try:
self.prepare()
if check_user(self.config['user']):
logger.info('The salt minion is starting up')
self.verify_hash_type()
self.start_log_info()
self.minion.tune_in()
finally:
self.shutdown()
@ -310,10 +353,10 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init
'''
If sub-classed, run any shutdown operations on this method.
'''
logger.info('The salt minion is shut down')
self.shutdown_log_info()
class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
class ProxyMinion(parsers.ProxyMinionOptionParser, DaemonsMixin): # pylint: disable=no-init
'''
Create a proxy minion server
'''
@ -388,8 +431,7 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
os.umask(current_umask)
except OSError as err:
logger.exception('Failed to prepare salt environment')
sys.exit(err.errno)
self.environment_failure(err)
self.setup_logfile_logger()
verify_log(self.config)
@ -431,7 +473,8 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
try:
self.prepare()
if check_user(self.config['user']):
logger.info('The proxy minion is starting up')
self.verify_hash_type()
self.start_log_info()
self.minion.tune_in()
except (KeyboardInterrupt, SaltSystemExit) as exc:
logger.warn('Stopping the Salt Proxy Minion')
@ -449,10 +492,10 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init
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)
logger.info('The proxy minion is shut down')
self.shutdown_log_info()
class Syndic(parsers.SyndicOptionParser):
class Syndic(parsers.SyndicOptionParser, DaemonsMixin): # pylint: disable=no-init
'''
Create a syndic server
'''
@ -488,8 +531,7 @@ class Syndic(parsers.SyndicOptionParser):
verify_files([logfile], self.config['user'])
os.umask(current_umask)
except OSError as err:
logger.exception('Failed to prepare salt environment')
sys.exit(err.errno)
self.environment_failure(err)
self.setup_logfile_logger()
verify_log(self.config)
@ -521,7 +563,8 @@ class Syndic(parsers.SyndicOptionParser):
'''
self.prepare()
if check_user(self.config['user']):
logger.info('The salt syndic is starting up')
self.verify_hash_type()
self.start_log_info()
try:
self.syndic.tune_in()
except KeyboardInterrupt:
@ -532,4 +575,4 @@ class Syndic(parsers.SyndicOptionParser):
'''
If sub-classed, run any shutdown operations on this method.
'''
logger.info('The salt syndic is shut down')
self.shutdown_log_info()

4
salt/crypt.py
View File

@ -559,11 +559,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)

12
salt/modules/key.py
View File

@ -21,10 +21,8 @@ def finger():
salt '*' key.finger
'''
return salt.utils.pem_finger(
os.path.join(__opts__['pki_dir'], 'minion.pub'),
sum_type=__opts__['hash_type']
)
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 +35,5 @@ 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']
)
return salt.utils.pem_finger(os.path.join(__opts__['pki_dir'], 'minion_master.pub'),
sum_type=__opts__.get('hash_type', 'md5'))

26
salt/modules/tomcat.py
View File

@ -631,7 +631,7 @@ def deploy_war(war,
def passwd(passwd,
user='',
alg='md5',
alg='sha1',
realm=None):
'''
This function replaces the $CATALINA_HOME/bin/digest.sh script
@ -646,23 +646,15 @@ def passwd(passwd,
salt '*' tomcat.passwd secret tomcat sha1
salt '*' tomcat.passwd secret tomcat sha1 'Protected Realm'
'''
if alg == 'md5':
m = hashlib.md5()
elif alg == 'sha1':
m = hashlib.sha1()
else:
return False
# Shouldn't it be SHA265 instead of SHA1?
digest = hasattr(hashlib, alg) and getattr(hashlib, alg) or None
if digest:
if realm:
digest.update('{0}:{1}:{2}'.format(user, realm, passwd,))
else:
digest.update(passwd)
if realm:
m.update('{0}:{1}:{2}'.format(
user,
realm,
passwd,
))
else:
m.update(passwd)
return m.hexdigest()
return digest and digest.hexdigest() or False
# Non-Manager functions

2
salt/modules/win_file.py
View File

@ -842,7 +842,7 @@ def chgrp(path, group):
return None
def stats(path, hash_type='md5', follow_symlinks=True):
def stats(path, hash_type='sha256', follow_symlinks=True):
'''
Return a dict containing the stats for a given file

12
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.
@ -1979,7 +1979,7 @@ def safe_walk(top, topdown=True, onerror=None, followlinks=True, _seen=None):
yield top, dirs, nondirs
def get_hash(path, form='md5', chunk_size=65536):
def get_hash(path, form='sha256', chunk_size=65536):
'''
Get the hash sum of a file
@ -1989,10 +1989,10 @@ def get_hash(path, form='md5', chunk_size=65536):
``get_sum`` cannot really be trusted since it is vulnerable to
collisions: ``get_sum(..., 'xyz') == 'Hash xyz not supported'``
'''
try:
hash_type = getattr(hashlib, form)
except (AttributeError, TypeError):
hash_type = hasattr(hashlib, form) and getattr(hashlib, form) or None
if hash_type is None:
raise ValueError('Invalid hash type: {0}'.format(form))
with salt.utils.fopen(path, 'rb') as ifile:
hash_obj = hash_type()
# read the file in in chunks, not the entire file

3
salt/utils/cloud.py
View File

@ -2465,6 +2465,7 @@ def init_cachedir(base=None):
def request_minion_cachedir(
minion_id,
opts=None,
fingerprint='',
pubkey=None,
provider=None,
@ -2484,7 +2485,7 @@ def request_minion_cachedir(
if not fingerprint:
if pubkey is not None:
fingerprint = salt.utils.pem_finger(key=pubkey)
fingerprint = salt.utils.pem_finger(key=pubkey, sum_type=(opts and opts.get('hash_type') or 'sha256'))
init_cachedir(base)

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

@ -0,0 +1,209 @@
# -*- 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.last_message = self.last_type = None
def info(self, data):
'''
Collects the data from the logger of info type.
:param data:
:return:
'''
self.last_message = data
self.last_type = 'info'
def warning(self, data):
'''
Collects the data from the logger of warning type.
:param data:
:return:
'''
self.last_message = data
self.last_type = 'warning'
@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.logger', _logger):
for alg in ['md5', 'sha1']:
_create_master().start()
self.assertEqual(_logger.last_type, 'warning')
self.assertTrue(_logger.last_message)
self.assertTrue(_logger.last_message.find('Do not use {alg}'.format(alg=alg)) > -1)
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_master().start()
self.assertEqual(_logger.last_type, None)
self.assertFalse(_logger.last_message)
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.logger', _logger):
for alg in ['md5', 'sha1']:
_create_minion().start()
self.assertEqual(_logger.last_type, 'warning')
self.assertTrue(_logger.last_message)
self.assertTrue(_logger.last_message.find('Do not use {alg}'.format(alg=alg)) > -1)
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_minion().start()
self.assertEqual(_logger.last_type, None)
self.assertFalse(_logger.last_message)
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.logger', _logger):
for alg in ['md5', 'sha1']:
_create_proxy_minion().start()
self.assertEqual(_logger.last_type, 'warning')
self.assertTrue(_logger.last_message)
self.assertTrue(_logger.last_message.find('Do not use {alg}'.format(alg=alg)) > -1)
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_proxy_minion().start()
self.assertEqual(_logger.last_type, None)
self.assertFalse(_logger.last_message)
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.logger', _logger):
for alg in ['md5', 'sha1']:
_create_syndic().start()
self.assertEqual(_logger.last_type, 'warning')
self.assertTrue(_logger.last_message)
self.assertTrue(_logger.last_message.find('Do not use {alg}'.format(alg=alg)) > -1)
_logger.reset()
for alg in ['sha224', 'sha256', 'sha384', 'sha512']:
_create_syndic().start()
self.assertEqual(_logger.last_type, None)
self.assertFalse(_logger.last_message)
if __name__ == '__main__':
from integration import run_tests
run_tests(DaemonsStarterTestCase, needs_daemon=False)