From 60ef23f49d1897b779e965e7f161c4fd2fec9010 Mon Sep 17 00:00:00 2001 From: "Sigmon, Jeffrey" Date: Fri, 23 Jun 2017 18:09:56 -0500 Subject: [PATCH 1/5] Passphrase protect master private key --- conf/master | 16 +++++++++++++ salt/config/__init__.py | 8 +++++++ salt/crypt.py | 42 +++++++++++++++++++++++------------ salt/key.py | 11 ++++++--- salt/transport/mixins/auth.py | 6 ++++- tests/unit/crypt_test.py | 21 ++++++++++++++++++ 6 files changed, 86 insertions(+), 18 deletions(-) diff --git a/conf/master b/conf/master index fc31c801f8..9c87017260 100644 --- a/conf/master +++ b/conf/master @@ -285,6 +285,22 @@ ##### Security settings ##### ########################################## +# Enable passphrase protection of Master private key. Although a string value +# is acceptable; passwords should be stored in an external vaulting mechanism +# and retrieved via sdb. See https://docs.saltstack.com/en/latest/topics/sdb/. +# Passphrase protection is off by default but an example of an sdb profile and +# query is as follows. +# masterkeyring: +# driver: keyring +# service: system +# +# key_pass: sdb://masterkeyring/key_pass + +# Enable passphrase protection of the Master signing_key. This only applies if +# master_sign_pubkey is set to True. This is disabled by default. +# master_sign_pubkey: True +# signing_key_pass: sdb://masterkeyring/signing_pass + # Enable "open mode", this mode still maintains encryption, but turns off # authentication, this is only intended for highly secure environments or for # the situation where your keys end up in a bad state. If you run in open mode diff --git a/salt/config/__init__.py b/salt/config/__init__.py index e286c223f7..969e221a4b 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -421,6 +421,12 @@ VALID_OPTS = { # Allow a daemon to function even if the key directories are not secured 'permissive_pki_access': bool, + # The passphrase of the master's private key + 'key_pass': str, + + # The passphrase of the master's private signing key + 'signing_key_pass': str, + # The path to a directory to pull in configuration file includes 'default_include': str, @@ -1395,6 +1401,8 @@ DEFAULT_MASTER_OPTS = { 'key_logfile': os.path.join(salt.syspaths.LOGS_DIR, 'key'), 'verify_env': True, 'permissive_pki_access': False, + 'key_pass': None, + 'signing_key_pass': None, 'default_include': 'master.d/*.conf', 'winrepo_dir': os.path.join(salt.syspaths.BASE_FILE_ROOTS_DIR, 'win', 'repo'), 'winrepo_dir_ng': os.path.join(salt.syspaths.BASE_FILE_ROOTS_DIR, 'win', 'repo-ng'), diff --git a/salt/crypt.py b/salt/crypt.py index 6658e46919..4889eb3e60 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -51,10 +51,11 @@ import salt.payload import salt.transport.client import salt.transport.frame import salt.utils.rsax931 +import salt.utils.sdb import salt.utils.verify import salt.version from salt.exceptions import ( - AuthenticationError, SaltClientError, SaltReqTimeoutError + AuthenticationError, SaltClientError, SaltReqTimeoutError, MasterExit ) import tornado.gen @@ -91,7 +92,7 @@ def dropfile(cachedir, user=None): os.umask(mask) # restore original umask -def gen_keys(keydir, keyname, keysize, user=None): +def gen_keys(keydir, keyname, keysize, user=None, passphrase=None): ''' Generate a RSA public keypair for use with salt @@ -99,6 +100,7 @@ def gen_keys(keydir, keyname, keysize, user=None): :param str keyname: The type of salt server for whom this key should be written. (i.e. 'master' or 'minion') :param int keysize: The number of bits in the key :param str user: The user on the system who should own this keypair + :param str passphrase: The passphrase which should be used to encrypt the private key :rtype: str :return: Path on the filesystem to the RSA private key @@ -120,7 +122,7 @@ def gen_keys(keydir, keyname, keysize, user=None): cumask = os.umask(191) with salt.utils.fopen(priv, 'wb+') as f: - f.write(gen.exportKey('PEM')) + f.write(gen.exportKey('PEM', passphrase)) os.umask(cumask) with salt.utils.fopen(pub, 'wb+') as f: f.write(gen.publickey().exportKey('PEM')) @@ -138,13 +140,13 @@ def gen_keys(keydir, keyname, keysize, user=None): return priv -def sign_message(privkey_path, message): +def sign_message(privkey_path, message, passphrase=None): ''' Use Crypto.Signature.PKCS1_v1_5 to sign a message. Returns the signature. ''' log.debug('salt.crypt.sign_message: Loading private key') with salt.utils.fopen(privkey_path) as f: - key = RSA.importKey(f.read()) + key = RSA.importKey(f.read(), passphrase) log.debug('salt.crypt.sign_message: Signing message.') signer = PKCS1_v1_5.new(key) return signer.sign(SHA.new(message)) @@ -163,7 +165,7 @@ def verify_signature(pubkey_path, message, signature): return verifier.verify(SHA.new(message), signature) -def gen_signature(priv_path, pub_path, sign_path): +def gen_signature(priv_path, pub_path, sign_path, passphrase=None): ''' creates a signature for the given public-key with the given private key and writes it to sign_path @@ -172,7 +174,7 @@ def gen_signature(priv_path, pub_path, sign_path): with salt.utils.fopen(pub_path) as fp_: mpub_64 = fp_.read() - mpub_sig = sign_message(priv_path, mpub_64) + mpub_sig = sign_message(priv_path, mpub_64, passphrase) mpub_sig_64 = binascii.b2a_base64(mpub_sig) if os.path.isfile(sign_path): return False @@ -230,7 +232,9 @@ class MasterKeys(dict): self.pub_path = os.path.join(self.opts['pki_dir'], 'master.pub') self.rsa_path = os.path.join(self.opts['pki_dir'], 'master.pem') - self.key = self.__get_keys() + key_pass = salt.utils.sdb.sdb_get(self.opts['key_pass'], self.opts) + self.key = self.__get_keys(passphrase=key_pass) + self.pub_signature = None # set names for the signing key-pairs @@ -257,11 +261,15 @@ class MasterKeys(dict): # create a new signing key-pair to sign the masters # auth-replies when a minion tries to connect else: + + key_pass = salt.utils.sdb.sdb_get(self.opts['signing_key_pass'], self.opts) + self.pub_sign_path = os.path.join(self.opts['pki_dir'], opts['master_sign_key_name'] + '.pub') self.rsa_sign_path = os.path.join(self.opts['pki_dir'], opts['master_sign_key_name'] + '.pem') - self.sign_key = self.__get_keys(name=opts['master_sign_key_name']) + self.sign_key = self.__get_keys(name=opts['master_sign_key_name'], + passphrase=key_pass) # We need __setstate__ and __getstate__ to avoid pickling errors since # some of the member variables correspond to Cython objects which are @@ -274,7 +282,7 @@ class MasterKeys(dict): def __getstate__(self): return {'opts': self.opts} - def __get_keys(self, name='master'): + def __get_keys(self, name='master', passphrase=None): ''' Returns a key object for a key in the pki-dir ''' @@ -282,16 +290,22 @@ class MasterKeys(dict): name + '.pem') if os.path.exists(path): with salt.utils.fopen(path) as f: - key = RSA.importKey(f.read()) + try: + key = RSA.importKey(f.read(), passphrase) + except ValueError as e: + message = 'Unable to read key: {0}; passphrase may be incorrect'.format(path) + log.error(message) + raise MasterExit(message) log.debug('Loaded {0} key: {1}'.format(name, path)) else: log.info('Generating {0} keys: {1}'.format(name, self.opts['pki_dir'])) gen_keys(self.opts['pki_dir'], name, self.opts['keysize'], - self.opts.get('user')) - with salt.utils.fopen(self.rsa_path) as f: - key = RSA.importKey(f.read()) + self.opts.get('user'), + passphrase) + with salt.utils.fopen(path) as f: + key = RSA.importKey(f.read(), passphrase) return key def get_pub_str(self, name='master'): diff --git a/salt/key.py b/salt/key.py index 51d7be6a89..688f19dbcd 100644 --- a/salt/key.py +++ b/salt/key.py @@ -25,6 +25,7 @@ import salt.minion import salt.utils import salt.utils.event import salt.utils.kinds +import salt.utils.sdb # pylint: disable=import-error,no-name-in-module,redefined-builtin import salt.ext.six as six @@ -374,6 +375,8 @@ class Key(object): io_loop=io_loop ) + self.passphrase = salt.utils.sdb.sdb_get(self.opts['signing_key_pass'], self.opts) + def _check_minions_directories(self): ''' Return the minion keys directory paths @@ -409,7 +412,7 @@ class Key(object): ''' keydir, keyname, keysize, user = self._get_key_attrs(keydir, keyname, keysize, user) - salt.crypt.gen_keys(keydir, keyname, keysize, user) + salt.crypt.gen_keys(keydir, keyname, keysize, user, self.passphrase) return salt.utils.pem_finger(os.path.join(keydir, keyname + '.pub')) def gen_signature(self, privkey, pubkey, sig_path): @@ -418,7 +421,8 @@ class Key(object): ''' return salt.crypt.gen_signature(privkey, pubkey, - sig_path) + sig_path, + self.passphrase) def gen_keys_signature(self, priv, pub, signature_path, auto_create=False, keysize=None): ''' @@ -452,7 +456,8 @@ class Key(object): salt.crypt.gen_keys(self.opts['pki_dir'], self.opts['master_sign_key_name'], keysize or self.opts['keysize'], - self.opts.get('user')) + self.opts.get('user'), + self.passphrase) priv = self.opts['pki_dir'] + '/' + self.opts['master_sign_key_name'] + '.pem' else: diff --git a/salt/transport/mixins/auth.py b/salt/transport/mixins/auth.py index fa9476c477..59109201b9 100644 --- a/salt/transport/mixins/auth.py +++ b/salt/transport/mixins/auth.py @@ -438,9 +438,13 @@ class AESReqServerMixin(object): else: # the master has its own signing-keypair, compute the master.pub's # signature and append that to the auth-reply + + # get the key_pass for the signing key + key_pass = salt.utils.sdb.sdb_get(self.opts['signing_key_pass'], self.opts) + log.debug("Signing master public key before sending") pub_sign = salt.crypt.sign_message(self.master_key.get_sign_paths()[1], - ret['pub_key']) + ret['pub_key'], key_pass) ret.update({'pub_sig': binascii.b2a_base64(pub_sign)}) mcipher = PKCS1_OAEP.new(self.master_key.key) diff --git a/tests/unit/crypt_test.py b/tests/unit/crypt_test.py index f548820397..83cfd8f6e3 100644 --- a/tests/unit/crypt_test.py +++ b/tests/unit/crypt_test.py @@ -100,10 +100,31 @@ class CryptTestCase(TestCase): crypt.gen_keys('/keydir', 'keyname', 2048) salt.utils.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + @patch('os.umask', MagicMock()) + @patch('os.chmod', MagicMock()) + @patch('os.chown', MagicMock()) + @patch('os.access', MagicMock(return_value=True)) + def test_gen_keys_with_passphrase(self): + with patch('salt.utils.fopen', mock_open()): + open_priv_wb = call('/keydir/keyname.pem', 'wb+') + open_pub_wb = call('/keydir/keyname.pub', 'wb+') + with patch('os.path.isfile', return_value=True): + self.assertEqual(crypt.gen_keys('/keydir', 'keyname', 2048, passphrase='password'), '/keydir/keyname.pem') + self.assertNotIn(open_priv_wb, salt.utils.fopen.mock_calls) + self.assertNotIn(open_pub_wb, salt.utils.fopen.mock_calls) + with patch('os.path.isfile', return_value=False): + with patch('salt.utils.fopen', mock_open()): + crypt.gen_keys('/keydir', 'keyname', 2048) + salt.utils.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + def test_sign_message(self): with patch('salt.utils.fopen', mock_open(read_data=PRIVKEY_DATA)): self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG)) + def test_sign_message_with_passphrase(self): + with patch('salt.utils.fopen', mock_open(read_data=PRIVKEY_DATA)): + self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) + def test_verify_signature(self): with patch('salt.utils.fopen', mock_open(read_data=PUBKEY_DATA)): self.assertTrue(crypt.verify_signature('/keydir/keyname.pub', MSG, SIG)) From bb29afd0223bcad9b82736da0c4f49624c696596 Mon Sep 17 00:00:00 2001 From: "Sigmon, Jeffrey" Date: Fri, 23 Jun 2017 18:09:56 -0500 Subject: [PATCH 2/5] Passphrase protect master private key --- conf/master | 16 +++++++++++ salt/config/__init__.py | 8 ++++++ salt/crypt.py | 50 ++++++++++++++++++++++------------- salt/key.py | 11 +++++--- salt/transport/mixins/auth.py | 6 ++++- tests/unit/test_crypt.py | 21 +++++++++++++++ 6 files changed, 90 insertions(+), 22 deletions(-) diff --git a/conf/master b/conf/master index f999409e9e..3aa9742651 100644 --- a/conf/master +++ b/conf/master @@ -301,6 +301,22 @@ ##### Security settings ##### ########################################## +# Enable passphrase protection of Master private key. Although a string value +# is acceptable; passwords should be stored in an external vaulting mechanism +# and retrieved via sdb. See https://docs.saltstack.com/en/latest/topics/sdb/. +# Passphrase protection is off by default but an example of an sdb profile and +# query is as follows. +# masterkeyring: +# driver: keyring +# service: system +# +# key_pass: sdb://masterkeyring/key_pass + +# Enable passphrase protection of the Master signing_key. This only applies if +# master_sign_pubkey is set to True. This is disabled by default. +# master_sign_pubkey: True +# signing_key_pass: sdb://masterkeyring/signing_pass + # Enable "open mode", this mode still maintains encryption, but turns off # authentication, this is only intended for highly secure environments or for # the situation where your keys end up in a bad state. If you run in open mode diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 8d94d6e3e8..e94e387885 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -458,6 +458,12 @@ VALID_OPTS = { # Allow a daemon to function even if the key directories are not secured 'permissive_pki_access': bool, + # The passphrase of the master's private key + 'key_pass': str, + + # The passphrase of the master's private signing key + 'signing_key_pass': str, + # The path to a directory to pull in configuration file includes 'default_include': str, @@ -1557,6 +1563,8 @@ DEFAULT_MASTER_OPTS = { 'key_logfile': os.path.join(salt.syspaths.LOGS_DIR, 'key'), 'verify_env': True, 'permissive_pki_access': False, + 'key_pass': None, + 'signing_key_pass': None, 'default_include': 'master.d/*.conf', 'winrepo_dir': os.path.join(salt.syspaths.BASE_FILE_ROOTS_DIR, 'win', 'repo'), 'winrepo_dir_ng': os.path.join(salt.syspaths.BASE_FILE_ROOTS_DIR, 'win', 'repo-ng'), diff --git a/salt/crypt.py b/salt/crypt.py index 8df48f1ccf..f151f61709 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -54,10 +54,11 @@ import salt.payload import salt.transport.client import salt.transport.frame import salt.utils.rsax931 +import salt.utils.sdb import salt.utils.verify import salt.version from salt.exceptions import ( - AuthenticationError, SaltClientError, SaltReqTimeoutError + AuthenticationError, SaltClientError, SaltReqTimeoutError, MasterExit ) import tornado.gen @@ -94,7 +95,7 @@ def dropfile(cachedir, user=None): os.umask(mask) # restore original umask -def gen_keys(keydir, keyname, keysize, user=None): +def gen_keys(keydir, keyname, keysize, user=None, passphrase=None): ''' Generate a RSA public keypair for use with salt @@ -102,6 +103,7 @@ def gen_keys(keydir, keyname, keysize, user=None): :param str keyname: The type of salt server for whom this key should be written. (i.e. 'master' or 'minion') :param int keysize: The number of bits in the key :param str user: The user on the system who should own this keypair + :param str passphrase: The passphrase which should be used to encrypt the private key :rtype: str :return: Path on the filesystem to the RSA private key @@ -123,7 +125,7 @@ def gen_keys(keydir, keyname, keysize, user=None): cumask = os.umask(191) with salt.utils.files.fopen(priv, 'wb+') as f: - f.write(gen.exportKey('PEM')) + f.write(gen.exportKey('PEM', passphrase)) os.umask(cumask) with salt.utils.files.fopen(pub, 'wb+') as f: f.write(gen.publickey().exportKey('PEM')) @@ -142,7 +144,7 @@ def gen_keys(keydir, keyname, keysize, user=None): @salt.utils.decorators.memoize -def _get_key_with_evict(path, timestamp): +def _get_key_with_evict(path, timestamp, passphrase): ''' Load a key from disk. `timestamp` above is intended to be the timestamp of the file's last modification. This fn is memoized so if it is called with the @@ -152,11 +154,11 @@ def _get_key_with_evict(path, timestamp): ''' log.debug('salt.crypt._get_key_with_evict: Loading private key') with salt.utils.files.fopen(path) as f: - key = RSA.importKey(f.read()) + key = RSA.importKey(f.read(), passphrase) return key -def _get_rsa_key(path): +def _get_rsa_key(path, passphrase): ''' Read a key off the disk. Poor man's simple cache in effect here, we memoize the result of calling _get_rsa_with_evict. This means @@ -168,14 +170,14 @@ def _get_rsa_key(path): retrieve the key from disk. ''' log.debug('salt.crypt._get_rsa_key: Loading private key') - return _get_key_with_evict(path, str(os.path.getmtime(path))) + return _get_key_with_evict(path, str(os.path.getmtime(path)), passphrase) -def sign_message(privkey_path, message): +def sign_message(privkey_path, message, passphrase=None): ''' Use Crypto.Signature.PKCS1_v1_5 to sign a message. Returns the signature. ''' - key = _get_rsa_key(privkey_path) + key = _get_rsa_key(privkey_path, passphrase) log.debug('salt.crypt.sign_message: Signing message.') signer = PKCS1_v1_5.new(key) return signer.sign(SHA.new(message)) @@ -194,7 +196,7 @@ def verify_signature(pubkey_path, message, signature): return verifier.verify(SHA.new(message), signature) -def gen_signature(priv_path, pub_path, sign_path): +def gen_signature(priv_path, pub_path, sign_path, passphrase=None): ''' creates a signature for the given public-key with the given private key and writes it to sign_path @@ -203,7 +205,7 @@ def gen_signature(priv_path, pub_path, sign_path): with salt.utils.files.fopen(pub_path) as fp_: mpub_64 = fp_.read() - mpub_sig = sign_message(priv_path, mpub_64) + mpub_sig = sign_message(priv_path, mpub_64, passphrase) mpub_sig_64 = binascii.b2a_base64(mpub_sig) if os.path.isfile(sign_path): return False @@ -261,7 +263,9 @@ class MasterKeys(dict): self.pub_path = os.path.join(self.opts['pki_dir'], 'master.pub') self.rsa_path = os.path.join(self.opts['pki_dir'], 'master.pem') - self.key = self.__get_keys() + key_pass = salt.utils.sdb.sdb_get(self.opts['key_pass'], self.opts) + self.key = self.__get_keys(passphrase=key_pass) + self.pub_signature = None # set names for the signing key-pairs @@ -288,11 +292,15 @@ class MasterKeys(dict): # create a new signing key-pair to sign the masters # auth-replies when a minion tries to connect else: + + key_pass = salt.utils.sdb.sdb_get(self.opts['signing_key_pass'], self.opts) + self.pub_sign_path = os.path.join(self.opts['pki_dir'], opts['master_sign_key_name'] + '.pub') self.rsa_sign_path = os.path.join(self.opts['pki_dir'], opts['master_sign_key_name'] + '.pem') - self.sign_key = self.__get_keys(name=opts['master_sign_key_name']) + self.sign_key = self.__get_keys(name=opts['master_sign_key_name'], + passphrase=key_pass) # We need __setstate__ and __getstate__ to avoid pickling errors since # some of the member variables correspond to Cython objects which are @@ -305,7 +313,7 @@ class MasterKeys(dict): def __getstate__(self): return {'opts': self.opts} - def __get_keys(self, name='master'): + def __get_keys(self, name='master', passphrase=None): ''' Returns a key object for a key in the pki-dir ''' @@ -313,16 +321,22 @@ class MasterKeys(dict): name + '.pem') if os.path.exists(path): with salt.utils.files.fopen(path) as f: - key = RSA.importKey(f.read()) + try: + key = RSA.importKey(f.read(), passphrase) + except ValueError as e: + message = 'Unable to read key: {0}; passphrase may be incorrect'.format(path) + log.error(message) + raise MasterExit(message) log.debug('Loaded {0} key: {1}'.format(name, path)) else: log.info('Generating {0} keys: {1}'.format(name, self.opts['pki_dir'])) gen_keys(self.opts['pki_dir'], name, self.opts['keysize'], - self.opts.get('user')) - with salt.utils.files.fopen(self.rsa_path) as f: - key = RSA.importKey(f.read()) + self.opts.get('user'), + passphrase) + with salt.utils.files.fopen(path) as f: + key = RSA.importKey(f.read(), passphrase) return key def get_pub_str(self, name='master'): diff --git a/salt/key.py b/salt/key.py index d463fcc13e..9546ffd3cd 100644 --- a/salt/key.py +++ b/salt/key.py @@ -27,6 +27,7 @@ import salt.utils.args import salt.utils.event import salt.utils.files import salt.utils.kinds +import salt.utils.sdb # pylint: disable=import-error,no-name-in-module,redefined-builtin import salt.ext.six as six @@ -376,6 +377,8 @@ class Key(object): io_loop=io_loop ) + self.passphrase = salt.utils.sdb.sdb_get(self.opts['signing_key_pass'], self.opts) + def _check_minions_directories(self): ''' Return the minion keys directory paths @@ -411,7 +414,7 @@ class Key(object): ''' keydir, keyname, keysize, user = self._get_key_attrs(keydir, keyname, keysize, user) - salt.crypt.gen_keys(keydir, keyname, keysize, user) + salt.crypt.gen_keys(keydir, keyname, keysize, user, self.passphrase) return salt.utils.pem_finger(os.path.join(keydir, keyname + '.pub')) def gen_signature(self, privkey, pubkey, sig_path): @@ -420,7 +423,8 @@ class Key(object): ''' return salt.crypt.gen_signature(privkey, pubkey, - sig_path) + sig_path, + self.passphrase) def gen_keys_signature(self, priv, pub, signature_path, auto_create=False, keysize=None): ''' @@ -454,7 +458,8 @@ class Key(object): salt.crypt.gen_keys(self.opts['pki_dir'], self.opts['master_sign_key_name'], keysize or self.opts['keysize'], - self.opts.get('user')) + self.opts.get('user'), + self.passphrase) priv = self.opts['pki_dir'] + '/' + self.opts['master_sign_key_name'] + '.pem' else: diff --git a/salt/transport/mixins/auth.py b/salt/transport/mixins/auth.py index 5388506356..5e4b8da3ae 100644 --- a/salt/transport/mixins/auth.py +++ b/salt/transport/mixins/auth.py @@ -441,9 +441,13 @@ class AESReqServerMixin(object): else: # the master has its own signing-keypair, compute the master.pub's # signature and append that to the auth-reply + + # get the key_pass for the signing key + key_pass = salt.utils.sdb.sdb_get(self.opts['signing_key_pass'], self.opts) + log.debug("Signing master public key before sending") pub_sign = salt.crypt.sign_message(self.master_key.get_sign_paths()[1], - ret['pub_key']) + ret['pub_key'], key_pass) ret.update({'pub_sig': binascii.b2a_base64(pub_sign)}) mcipher = PKCS1_OAEP.new(self.master_key.key) diff --git a/tests/unit/test_crypt.py b/tests/unit/test_crypt.py index 7d53f22522..cf519d0634 100644 --- a/tests/unit/test_crypt.py +++ b/tests/unit/test_crypt.py @@ -103,11 +103,32 @@ class CryptTestCase(TestCase): crypt.gen_keys('/keydir', 'keyname', 2048) salt.utils.files.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + @patch('os.umask', MagicMock()) + @patch('os.chmod', MagicMock()) + @patch('os.chown', MagicMock()) + @patch('os.access', MagicMock(return_value=True)) + def test_gen_keys_with_passphrase(self): + with patch('salt.utils.fopen', mock_open()): + open_priv_wb = call('/keydir/keyname.pem', 'wb+') + open_pub_wb = call('/keydir/keyname.pub', 'wb+') + with patch('os.path.isfile', return_value=True): + self.assertEqual(crypt.gen_keys('/keydir', 'keyname', 2048, passphrase='password'), '/keydir/keyname.pem') + self.assertNotIn(open_priv_wb, salt.utils.fopen.mock_calls) + self.assertNotIn(open_pub_wb, salt.utils.fopen.mock_calls) + with patch('os.path.isfile', return_value=False): + with patch('salt.utils.fopen', mock_open()): + crypt.gen_keys('/keydir', 'keyname', 2048) + salt.utils.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + def test_sign_message(self): key = RSA.importKey(PRIVKEY_DATA) with patch('salt.crypt._get_rsa_key', return_value=key): self.assertEqual(SIG, salt.crypt.sign_message('/keydir/keyname.pem', MSG)) + def test_sign_message_with_passphrase(self): + with patch('salt.utils.fopen', mock_open(read_data=PRIVKEY_DATA)): + self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) + def test_verify_signature(self): with patch('salt.utils.files.fopen', mock_open(read_data=PUBKEY_DATA)): self.assertTrue(crypt.verify_signature('/keydir/keyname.pub', MSG, SIG)) From 7875ee9d3416eaa3d53a8b030c5d88797196761b Mon Sep 17 00:00:00 2001 From: Anil Kabra Date: Fri, 28 Jul 2017 07:44:50 -0500 Subject: [PATCH 3/5] Fixed error due to new files module in utils --- tests/unit/test_crypt.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/unit/test_crypt.py b/tests/unit/test_crypt.py index cb9555ae6d..822fc07c12 100644 --- a/tests/unit/test_crypt.py +++ b/tests/unit/test_crypt.py @@ -108,34 +108,34 @@ class CryptTestCase(TestCase): @patch('os.chown', MagicMock()) @patch('os.access', MagicMock(return_value=True)) def test_gen_keys_with_passphrase(self): - with patch('salt.utils.fopen', mock_open()): + with patch('salt.utils.files.open', mock_open()): open_priv_wb = call('/keydir/keyname.pem', 'wb+') open_pub_wb = call('/keydir/keyname.pub', 'wb+') with patch('os.path.isfile', return_value=True): self.assertEqual(crypt.gen_keys('/keydir', 'keyname', 2048, passphrase='password'), '/keydir/keyname.pem') - self.assertNotIn(open_priv_wb, salt.utils.fopen.mock_calls) - self.assertNotIn(open_pub_wb, salt.utils.fopen.mock_calls) + self.assertNotIn(open_priv_wb, salt.utils.files.open.mock_calls) + self.assertNotIn(open_pub_wb, salt.utils.files.open.mock_calls) with patch('os.path.isfile', return_value=False): - with patch('salt.utils.fopen', mock_open()): + with patch('salt.utils.files.open', mock_open()): crypt.gen_keys('/keydir', 'keyname', 2048) - salt.utils.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + salt.utils.files.open.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) @patch('os.umask', MagicMock()) @patch('os.chmod', MagicMock()) @patch('os.chown', MagicMock()) @patch('os.access', MagicMock(return_value=True)) def test_gen_keys_with_passphrase(self): - with patch('salt.utils.fopen', mock_open()): + with patch('salt.utils.files.open', mock_open()): open_priv_wb = call('/keydir/keyname.pem', 'wb+') open_pub_wb = call('/keydir/keyname.pub', 'wb+') with patch('os.path.isfile', return_value=True): self.assertEqual(crypt.gen_keys('/keydir', 'keyname', 2048, passphrase='password'), '/keydir/keyname.pem') - self.assertNotIn(open_priv_wb, salt.utils.fopen.mock_calls) - self.assertNotIn(open_pub_wb, salt.utils.fopen.mock_calls) + self.assertNotIn(open_priv_wb, salt.utils.files.open.mock_calls) + self.assertNotIn(open_pub_wb, salt.utils.files.open.mock_calls) with patch('os.path.isfile', return_value=False): - with patch('salt.utils.fopen', mock_open()): + with patch('salt.utils.files.open', mock_open()): crypt.gen_keys('/keydir', 'keyname', 2048) - salt.utils.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + salt.utils.files.open.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) def test_sign_message(self): key = RSA.importKey(PRIVKEY_DATA) @@ -143,11 +143,11 @@ class CryptTestCase(TestCase): self.assertEqual(SIG, salt.crypt.sign_message('/keydir/keyname.pem', MSG)) def test_sign_message_with_passphrase(self): - with patch('salt.utils.fopen', mock_open(read_data=PRIVKEY_DATA)): + with patch('salt.utils.files.open', mock_open(read_data=PRIVKEY_DATA)): self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) def test_sign_message_with_passphrase(self): - with patch('salt.utils.fopen', mock_open(read_data=PRIVKEY_DATA)): + with patch('salt.utils.files.open', mock_open(read_data=PRIVKEY_DATA)): self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) def test_verify_signature(self): From 2f3c50206b763677b3e0df40ab2cc66e7fd78201 Mon Sep 17 00:00:00 2001 From: Anil Kabra Date: Fri, 28 Jul 2017 09:40:57 -0500 Subject: [PATCH 4/5] Fix calls to files utility module method from open from fopen --- tests/unit/test_crypt.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/unit/test_crypt.py b/tests/unit/test_crypt.py index 822fc07c12..6394ad774d 100644 --- a/tests/unit/test_crypt.py +++ b/tests/unit/test_crypt.py @@ -108,34 +108,34 @@ class CryptTestCase(TestCase): @patch('os.chown', MagicMock()) @patch('os.access', MagicMock(return_value=True)) def test_gen_keys_with_passphrase(self): - with patch('salt.utils.files.open', mock_open()): + with patch('salt.utils.files.fopen', mock_open()): open_priv_wb = call('/keydir/keyname.pem', 'wb+') open_pub_wb = call('/keydir/keyname.pub', 'wb+') with patch('os.path.isfile', return_value=True): self.assertEqual(crypt.gen_keys('/keydir', 'keyname', 2048, passphrase='password'), '/keydir/keyname.pem') - self.assertNotIn(open_priv_wb, salt.utils.files.open.mock_calls) - self.assertNotIn(open_pub_wb, salt.utils.files.open.mock_calls) + self.assertNotIn(open_priv_wb, salt.utils.files.fopen.mock_calls) + self.assertNotIn(open_pub_wb, salt.utils.files.fopen.mock_calls) with patch('os.path.isfile', return_value=False): - with patch('salt.utils.files.open', mock_open()): + with patch('salt.utils.files.fopen', mock_open()): crypt.gen_keys('/keydir', 'keyname', 2048) - salt.utils.files.open.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + salt.utils.files.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) @patch('os.umask', MagicMock()) @patch('os.chmod', MagicMock()) @patch('os.chown', MagicMock()) @patch('os.access', MagicMock(return_value=True)) def test_gen_keys_with_passphrase(self): - with patch('salt.utils.files.open', mock_open()): + with patch('salt.utils.files.fopen', mock_open()): open_priv_wb = call('/keydir/keyname.pem', 'wb+') open_pub_wb = call('/keydir/keyname.pub', 'wb+') with patch('os.path.isfile', return_value=True): self.assertEqual(crypt.gen_keys('/keydir', 'keyname', 2048, passphrase='password'), '/keydir/keyname.pem') - self.assertNotIn(open_priv_wb, salt.utils.files.open.mock_calls) - self.assertNotIn(open_pub_wb, salt.utils.files.open.mock_calls) + self.assertNotIn(open_priv_wb, salt.utils.files.fopen.mock_calls) + self.assertNotIn(open_pub_wb, salt.utils.files.fopen.mock_calls) with patch('os.path.isfile', return_value=False): - with patch('salt.utils.files.open', mock_open()): + with patch('salt.utils.files.fopen', mock_open()): crypt.gen_keys('/keydir', 'keyname', 2048) - salt.utils.files.open.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) + salt.utils.files.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) def test_sign_message(self): key = RSA.importKey(PRIVKEY_DATA) @@ -143,11 +143,11 @@ class CryptTestCase(TestCase): self.assertEqual(SIG, salt.crypt.sign_message('/keydir/keyname.pem', MSG)) def test_sign_message_with_passphrase(self): - with patch('salt.utils.files.open', mock_open(read_data=PRIVKEY_DATA)): + with patch('salt.utils.files.fopen', mock_open(read_data=PRIVKEY_DATA)): self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) def test_sign_message_with_passphrase(self): - with patch('salt.utils.files.open', mock_open(read_data=PRIVKEY_DATA)): + with patch('salt.utils.files.fopen', mock_open(read_data=PRIVKEY_DATA)): self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) def test_verify_signature(self): From b77b2e2a056b1a4056a3a5c0310a1dc11c2240d0 Mon Sep 17 00:00:00 2001 From: Anil Kabra Date: Fri, 28 Jul 2017 14:02:31 -0500 Subject: [PATCH 5/5] Fix unit test signing with passphrase --- tests/unit/test_crypt.py | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/tests/unit/test_crypt.py b/tests/unit/test_crypt.py index 6394ad774d..4f9a860936 100644 --- a/tests/unit/test_crypt.py +++ b/tests/unit/test_crypt.py @@ -120,34 +120,14 @@ class CryptTestCase(TestCase): crypt.gen_keys('/keydir', 'keyname', 2048) salt.utils.files.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) - @patch('os.umask', MagicMock()) - @patch('os.chmod', MagicMock()) - @patch('os.chown', MagicMock()) - @patch('os.access', MagicMock(return_value=True)) - def test_gen_keys_with_passphrase(self): - with patch('salt.utils.files.fopen', mock_open()): - open_priv_wb = call('/keydir/keyname.pem', 'wb+') - open_pub_wb = call('/keydir/keyname.pub', 'wb+') - with patch('os.path.isfile', return_value=True): - self.assertEqual(crypt.gen_keys('/keydir', 'keyname', 2048, passphrase='password'), '/keydir/keyname.pem') - self.assertNotIn(open_priv_wb, salt.utils.files.fopen.mock_calls) - self.assertNotIn(open_pub_wb, salt.utils.files.fopen.mock_calls) - with patch('os.path.isfile', return_value=False): - with patch('salt.utils.files.fopen', mock_open()): - crypt.gen_keys('/keydir', 'keyname', 2048) - salt.utils.files.fopen.assert_has_calls([open_priv_wb, open_pub_wb], any_order=True) - def test_sign_message(self): key = RSA.importKey(PRIVKEY_DATA) with patch('salt.crypt._get_rsa_key', return_value=key): self.assertEqual(SIG, salt.crypt.sign_message('/keydir/keyname.pem', MSG)) def test_sign_message_with_passphrase(self): - with patch('salt.utils.files.fopen', mock_open(read_data=PRIVKEY_DATA)): - self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) - - def test_sign_message_with_passphrase(self): - with patch('salt.utils.files.fopen', mock_open(read_data=PRIVKEY_DATA)): + key = RSA.importKey(PRIVKEY_DATA) + with patch('salt.crypt._get_rsa_key', return_value=key): self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password')) def test_verify_signature(self):