Merge pull request #31830 from Unity-Technologies/hotfix/keychain-cert-changed

Hotfix/keychain cert changed

salt/modules/mac_keychain.py

@@ -7,6 +7,14 @@ from __future__ import absolute_import
 
 # Import python libs
 import logging
+import re
+
+import shlex
+try:
+    import pipes
+    HAS_DEPS = True
+except ImportError:
+    HAS_DEPS = False
 
 # Import salt libs
 import salt.utils
@@ -14,12 +22,19 @@ import salt.utils
 log = logging.getLogger(__name__)
 __virtualname__ = 'keychain'
 
+if hasattr(shlex, 'quote'):
+    _quote = shlex.quote
+elif HAS_DEPS and hasattr(pipes, 'quote'):
+    _quote = pipes.quote
+else:
+    _quote = None
+
 
 def __virtual__():
     '''
     Only work on Mac OS
     '''
-    if salt.utils.is_darwin():
+    if salt.utils.is_darwin() and _quote is not None:
         return __virtualname__
     return False
 
@@ -118,7 +133,8 @@ def list_certs(keychain="/Library/Keychains/System.keychain"):
 
 
     '''
-    cmd = 'security find-certificate -a {0} | grep -o "alis".*\\" | grep -o \'\\"[-A-Za-z0-9.:() ]*\\"\''.format(keychain)
+    cmd = 'security find-certificate -a {0} | grep -o "alis".*\\" | ' \
+          'grep -o \'\\"[-A-Za-z0-9.:() ]*\\"\''.format(_quote(keychain))
     out = __salt__['cmd.run'](cmd, python_shell=True)
     return out.replace('"', '').split('\n')
 
@@ -144,8 +160,8 @@ def get_friendly_name(cert, password):
         info.
 
     '''
-    cmd = 'openssl pkcs12 -in {0} -passin pass:{1} -info -nodes -nokeys 2> /dev/null | grep friendlyName:'.format(cert,
-                                                                                                                  password)
+    cmd = 'openssl pkcs12 -in {0} -passin pass:{1} -info -nodes -nokeys 2> /dev/null | ' \
+          'grep friendlyName:'.format(_quote(cert), _quote(password))
     out = __salt__['cmd.run'](cmd, python_shell=True)
     return out.replace("friendlyName: ", "").strip()
 
@@ -205,3 +221,29 @@ def unlock_keychain(keychain, password):
     '''
     cmd = 'security unlock-keychain -p {0} {1}'.format(password, keychain)
     __salt__['cmd.run'](cmd)
+
+
+def get_hash(name, password=None):
+    '''
+    Returns the hash of a certificate in the keychain.
+
+    name
+        The name of the certificate (which you can get from keychain.get_friendly_name) or the
+        location of a p12 file.
+
+    password
+        The password that is used in the certificate. Only required if your passing a p12 file.
+        Note: This will be outputted to logs
+    '''
+
+    if '.p12' in name[-4:]:
+        cmd = 'openssl pkcs12 -in {0} -passin pass:{1} -passout pass:{1}'.format(name, password)
+    else:
+        cmd = 'security find-certificate -c "{0}" -m -p'.format(name)
+
+    out = __salt__['cmd.run'](cmd)
+    matches = re.search('-----BEGIN CERTIFICATE-----(.*)-----END CERTIFICATE-----', out, re.DOTALL | re.MULTILINE)
+    if matches:
+        return matches.group(1)
+    else:
+        return False

salt/states/mac_keychain.py

@@ -67,6 +67,24 @@ def installed(name, password, keychain="/Library/Keychains/System.keychain", **k
     certs = __salt__['keychain.list_certs'](keychain)
     friendly_name = __salt__['keychain.get_friendly_name'](name, password)
 
+    if friendly_name in certs:
+        file_hash = __salt__['keychain.get_hash'](name, password)
+        keychain_hash = __salt__['keychain.get_hash'](friendly_name)
+
+        if file_hash != keychain_hash:
+            out = __salt__['keychain.uninstall'](friendly_name, keychain,
+                                                 keychain_password=kwargs.get('keychain_password'))
+            if "unable" not in out:
+                ret['comment'] += "Found a certificate with the same name but different hash, removing it.\n"
+                ret['changes']['uninstalled'] = friendly_name
+
+                # Reset the certs found
+                certs = __salt__['keychain.list_certs'](keychain)
+            else:
+                ret['result'] = False
+                ret['comment'] += "Found an incorrect cert but was unable to uninstall it: {0}".format(friendly_name)
+                return ret
+
     if friendly_name not in certs:
         out = __salt__['keychain.install'](name, password, keychain, **kwargs)
         if "imported" in out:

tests/unit/states/mac_keychain_test.py

@@ -11,7 +11,8 @@ from salttesting import TestCase
 from salttesting.helpers import ensure_in_syspath
 from salttesting.mock import (
     MagicMock,
-    patch
+    patch,
+    call
 )
 
 ensure_in_syspath('../../')
@@ -58,9 +59,11 @@ class KeychainTestCase(TestCase):
         list_mock = MagicMock(return_value=['Friendly Name'])
         friendly_mock = MagicMock(return_value='Friendly Name')
         install_mock = MagicMock(return_value='1 identity imported.')
+        hash_mock = MagicMock(return_value='ABCD')
         with patch.dict(keychain.__salt__, {'keychain.list_certs': list_mock,
                                             'keychain.get_friendly_name': friendly_mock,
-                                            'keychain.install': install_mock}):
+                                            'keychain.install': install_mock,
+                                            'keychain.get_hash': hash_mock}):
             out = keychain.installed('/path/to/cert.p12', 'passw0rd')
             list_mock.assert_called_once_with('/Library/Keychains/System.keychain')
             friendly_mock.assert_called_once_with('/path/to/cert.p12', 'passw0rd')
@@ -198,6 +201,37 @@ class KeychainTestCase(TestCase):
             install_mock.assert_called_once_with('/tmp/path/to/cert.p12', 'passw0rd', '/Library/Keychains/System.keychain')
             self.assertEqual(out, expected)
 
+    def test_installed_cert_hash_different(self):
+        '''
+            Test installing a certificate into the OSX keychain when it's already installed but
+            the certificate has changed
+        '''
+        expected = {
+            'changes': {'installed': 'Friendly Name', 'uninstalled': 'Friendly Name'},
+            'comment': 'Found a certificate with the same name but different hash, removing it.\n',
+            'name': '/path/to/cert.p12',
+            'result': True
+        }
+
+        list_mock = MagicMock(side_effect=[['Friendly Name'], []])
+        friendly_mock = MagicMock(return_value='Friendly Name')
+        install_mock = MagicMock(return_value='1 identity imported.')
+        uninstall_mock = MagicMock(return_value='removed.')
+        hash_mock = MagicMock(side_effect=['ABCD', 'XYZ'])
+        with patch.dict(keychain.__salt__, {'keychain.list_certs': list_mock,
+                                            'keychain.get_friendly_name': friendly_mock,
+                                            'keychain.install': install_mock,
+                                            'keychain.uninstall': uninstall_mock,
+                                            'keychain.get_hash': hash_mock}):
+            out = keychain.installed('/path/to/cert.p12', 'passw0rd')
+            list_mock.assert_has_calls(calls=[call('/Library/Keychains/System.keychain'),
+                                              call('/Library/Keychains/System.keychain')])
+            friendly_mock.assert_called_once_with('/path/to/cert.p12', 'passw0rd')
+            install_mock.assert_called_once_with('/path/to/cert.p12', 'passw0rd', '/Library/Keychains/System.keychain')
+            uninstall_mock.assert_called_once_with('Friendly Name', '/Library/Keychains/System.keychain',
+                                                   keychain_password=None)
+            self.assertEqual(out, expected)
+
 
 if __name__ == '__main__':
     from integration import run_tests