From b5a7fc38aa6b8f1542e9f7837d93a7295632c703 Mon Sep 17 00:00:00 2001 From: Alessandro Pilotti Date: Thu, 13 Oct 2016 00:15:38 +0300 Subject: [PATCH] Resets service user password at each execution In a cloud environment instance images are typically cloned. This implies that the credentials used by the Cloudbase-Init service, even if randomly generated, are identical across instances of the same image, unless replaced during boot, e.g. by the post-sysprep specialize actions. Since this cannot be controlled in cases in which sysprep or similar mechanisms are not used (e.g. a Nova image snapshot), this patch adds a mechanism to reset the Cloudbase-Init service password at each execution. This avoids potential "pass the hash" type of attacks executed from user-data across instances booted from the same image. Change-Id: Ib778acc4c01f476c600e15aa77ed777523a77538 Closes-Bug: #1631567 Co-Authored-By: Adrian Vladu Co-Authored-By: Alexandru Coman --- cloudbaseinit/conf/default.py | 8 ++ cloudbaseinit/init.py | 3 + cloudbaseinit/osutils/base.py | 11 ++ cloudbaseinit/osutils/windows.py | 76 +++++++++- cloudbaseinit/tests/fake.py | 7 + cloudbaseinit/tests/osutils/test_windows.py | 152 +++++++++++++++++++- cloudbaseinit/tests/test_init.py | 2 + 7 files changed, 254 insertions(+), 5 deletions(-) diff --git a/cloudbaseinit/conf/default.py b/cloudbaseinit/conf/default.py index 0e99afd8..65de112b 100644 --- a/cloudbaseinit/conf/default.py +++ b/cloudbaseinit/conf/default.py @@ -121,6 +121,14 @@ class GlobalOptions(conf_base.Options): 'the password is a clear text password, coming from the ' 'metadata. The last option is `no`, when the user is ' 'never forced to change the password.'), + cfg.BoolOpt( + 'reset_service_password', default=True, + help='If set to True, the service user password will be ' + 'reset at each execution with a new random value of ' + 'appropriate length and complexity, unless the user is ' + 'a built-in or domain account.' + 'This is needed to avoid "pass the hash" attacks on ' + 'Windows cloned instances.'), cfg.ListOpt( 'metadata_services', default=[ diff --git a/cloudbaseinit/init.py b/cloudbaseinit/init.py index 230d2e7c..7fbbbcad 100644 --- a/cloudbaseinit/init.py +++ b/cloudbaseinit/init.py @@ -117,6 +117,9 @@ class InitManager(object): LOG.info('Cloudbase-Init version: %s', version.get_version()) osutils = osutils_factory.get_os_utils() + if CONF.reset_service_password: + # Avoid pass the hash attacks from cloned instances + osutils.reset_service_password() osutils.wait_for_boot_completion() reboot_required = self._handle_plugins_stage( diff --git a/cloudbaseinit/osutils/base.py b/cloudbaseinit/osutils/base.py index 192591ec..e3d0364f 100644 --- a/cloudbaseinit/osutils/base.py +++ b/cloudbaseinit/osutils/base.py @@ -82,6 +82,9 @@ class BaseOSUtils(object): def wait_for_boot_completion(self): pass + def reset_service_password(self): + return False + def terminate(self): pass @@ -118,3 +121,11 @@ class BaseOSUtils(object): def change_password_next_logon(self, username): """Force the given user to change his password at the next login.""" raise NotImplementedError() + + def set_service_credentials(self, service_name, username, password): + """Set the username and password for a given service.""" + raise NotImplementedError() + + def get_service_username(self, service_name): + """Retrieve the username under which a service runs.""" + raise NotImplementedError() diff --git a/cloudbaseinit/osutils/windows.py b/cloudbaseinit/osutils/windows.py index f909fe7f..3e6be26e 100644 --- a/cloudbaseinit/osutils/windows.py +++ b/cloudbaseinit/osutils/windows.py @@ -21,6 +21,7 @@ import struct import time from oslo_log import log as oslo_logging +import pywintypes import six from six.moves import winreg from tzlocal import windows_tz @@ -29,6 +30,8 @@ import win32net import win32netcon import win32process import win32security +import win32service +import winerror import wmi from cloudbaseinit import exception @@ -681,7 +684,14 @@ class WindowsUtils(base.BaseOSUtils): return service_list[0] def check_service_exists(self, service_name): - return self._get_service(service_name) is not None + try: + with self._get_service_handle(service_name): + return True + except pywintypes.error as ex: + print(ex) + if ex.winerror == winerror.ERROR_SERVICE_DOES_NOT_EXIST: + return False + raise def get_service_status(self, service_name): service = self._get_service(service_name) @@ -721,6 +731,70 @@ class WindowsUtils(base.BaseOSUtils): ' %(ret_val)d' % {'service_name': service_name, 'ret_val': ret_val}) + @staticmethod + @contextlib.contextmanager + def _get_service_handle(service_name, + service_access=win32service.SERVICE_QUERY_CONFIG, + scm_access=win32service.SC_MANAGER_CONNECT): + hscm = win32service.OpenSCManager(None, None, scm_access) + hs = None + try: + hs = win32service.OpenService(hscm, service_name, service_access) + yield hs + finally: + if hs: + win32service.CloseServiceHandle(hs) + win32service.CloseServiceHandle(hscm) + + def set_service_credentials(self, service_name, username, password): + LOG.debug('Setting service credentials: %s', service_name) + with self._get_service_handle( + service_name, win32service.SERVICE_CHANGE_CONFIG) as hs: + win32service.ChangeServiceConfig( + hs, + win32service.SERVICE_NO_CHANGE, + win32service.SERVICE_NO_CHANGE, + win32service.SERVICE_NO_CHANGE, + None, + None, + False, + None, + username, + password, + None) + + def get_service_username(self, service_name): + LOG.debug('Getting service username: %s', service_name) + with self._get_service_handle(service_name) as hs: + cfg = win32service.QueryServiceConfig(hs) + return cfg[7] + + def reset_service_password(self): + """This is needed to avoid pass the hash attacks.""" + if not self.check_service_exists(self._service_name): + LOG.info("Service does not exist: %s", self._service_name) + return False + + service_username = self.get_service_username(self._service_name) + # Ignore builtin accounts + if "\\" not in service_username: + LOG.info("Skipping password reset, service running as a built-in " + "account: %s", service_username) + return False + domain, username = service_username.split('\\') + if domain != ".": + LOG.info("Skipping password reset, service running as a domain " + "account: %s", service_username) + return False + + LOG.debug('Resetting password for service user: %s', service_username) + maximum_length = self.get_maximum_password_length() + password = self.generate_random_password(maximum_length) + self.set_user_password(username, password) + self.set_service_credentials( + self._service_name, service_username, password) + return True + def terminate(self): # Wait for the service to start. Polling the service "Started" property # is not enough diff --git a/cloudbaseinit/tests/fake.py b/cloudbaseinit/tests/fake.py index 33dc5106..ae0eb066 100644 --- a/cloudbaseinit/tests/fake.py +++ b/cloudbaseinit/tests/fake.py @@ -18,3 +18,10 @@ class FakeComError(Exception): def __init__(self): super(FakeComError, self).__init__() self.excepinfo = [None, None, None, None, None, -2144108544] + + +class FakeError(Exception): + + def __init__(self, msg="Fake error."): + super(FakeError, self).__init__(msg) + self.winerror = None diff --git a/cloudbaseinit/tests/osutils/test_windows.py b/cloudbaseinit/tests/osutils/test_windows.py index a77f9982..8b17ca41 100644 --- a/cloudbaseinit/tests/osutils/test_windows.py +++ b/cloudbaseinit/tests/osutils/test_windows.py @@ -50,12 +50,16 @@ class TestWindowsUtils(testutils.CloudbaseInitTestBase): def setUp(self): self._pywintypes_mock = mock.MagicMock() + self._pywintypes_mock.error = fake.FakeError self._pywintypes_mock.com_error = fake.FakeComError self._win32com_mock = mock.MagicMock() self._win32process_mock = mock.MagicMock() self._win32security_mock = mock.MagicMock() self._win32net_mock = mock.MagicMock() self._win32netcon_mock = mock.MagicMock() + self._win32service_mock = mock.MagicMock() + self._winerror_mock = mock.MagicMock() + self._winerror_mock.ERROR_SERVICE_DOES_NOT_EXIST = 0x424 self._wmi_mock = mock.MagicMock() self._wmi_mock.x_wmi = WMIError self._moves_mock = mock.MagicMock() @@ -73,6 +77,8 @@ class TestWindowsUtils(testutils.CloudbaseInitTestBase): 'win32security': self._win32security_mock, 'win32net': self._win32net_mock, 'win32netcon': self._win32netcon_mock, + 'win32service': self._win32service_mock, + 'winerror': self._winerror_mock, 'wmi': self._wmi_mock, 'six.moves': self._moves_mock, 'six.moves.xmlrpc_client': self._xmlrpc_client_mock, @@ -884,14 +890,152 @@ class TestWindowsUtils(testutils.CloudbaseInitTestBase): self.assertEqual('fake name', response) @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' - '._get_service') - def test_check_service_exists(self, mock_get_service): - mock_get_service.return_value = 'not None' + '._get_service_handle') + def test_check_service(self, mock_get_service_handle): + mock_context_manager = mock.MagicMock() + mock_context_manager.__enter__.return_value = "fake name" + mock_get_service_handle.return_value = mock_context_manager - response = self._winutils.check_service_exists('fake name') + self.assertTrue(self._winutils.check_service_exists("fake_name")) + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '._get_service_handle') + def test_check_service_fail(self, mock_get_service_handle): + exc = self._pywintypes_mock.error("ERROR_SERVICE_DOES_NOT_EXIST") + exc.winerror = self._winerror_mock.ERROR_SERVICE_DOES_NOT_EXIST + + exc2 = self._pywintypes_mock.error("NOT ERROR_SERVICE_DOES_NOT_EXIST") + exc2.winerror = None + + mock_context_manager = mock.MagicMock() + mock_context_manager.__enter__.side_effect = [exc, exc2] + mock_get_service_handle.return_value = mock_context_manager + + self.assertFalse(self._winutils.check_service_exists("fake_name")) + self.assertRaises(self._pywintypes_mock.error, + self._winutils.check_service_exists, + "fake_name") + + def test_get_service_handle(self): + open_scm = self._win32service_mock.OpenSCManager + open_scm.return_value = mock.sentinel.hscm + open_service = self._win32service_mock.OpenService + open_service.return_value = mock.sentinel.hs + close_service = self._win32service_mock.CloseServiceHandle + args = ("fake_name", mock.sentinel.service_access, + mock.sentinel.scm_access) + + with self._winutils._get_service_handle(*args) as hs: + self.assertIs(hs, mock.sentinel.hs) + + open_scm.assert_called_with(None, None, mock.sentinel.scm_access) + open_service.assert_called_with(mock.sentinel.hscm, "fake_name", + mock.sentinel.service_access) + close_service.assert_has_calls([mock.call(mock.sentinel.hs), + mock.call(mock.sentinel.hscm)]) + + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '._get_service_handle') + def test_set_service_credentials(self, mock_get_service): + self._win32service_mock.SERVICE_CHANGE_CONFIG = mock.sentinel.change + self._win32service_mock.SERVICE_NO_CHANGE = mock.sentinel.no_change + mock_change_service = self._win32service_mock.ChangeServiceConfig + mock_context_manager = mock.MagicMock() + mock_context_manager.__enter__.return_value = mock.sentinel.hs + mock_get_service.return_value = mock_context_manager + + self._winutils.set_service_credentials( + mock.sentinel.service, mock.sentinel.user, mock.sentinel.password) + + mock_get_service.assert_called_with(mock.sentinel.service, + mock.sentinel.change) + mock_change_service.assert_called_with( + mock.sentinel.hs, mock.sentinel.no_change, mock.sentinel.no_change, + mock.sentinel.no_change, None, None, False, None, + mock.sentinel.user, mock.sentinel.password, None) + + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '._get_service_handle') + def test_get_service_username(self, mock_get_service): + mock_context_manager = mock.MagicMock() + mock_context_manager.__enter__.return_value = mock.sentinel.hs + mock_get_service.return_value = mock_context_manager + mock_query_service = self._win32service_mock.QueryServiceConfig + mock_query_service.return_value = [mock.sentinel.value] * 8 + + response = self._winutils.get_service_username(mock.sentinel.service) + + mock_get_service.assert_called_with(mock.sentinel.service) + mock_query_service.assert_called_with(mock.sentinel.hs) + self.assertIs(response, mock.sentinel.value) + + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '.set_service_credentials') + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '.set_user_password') + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '.generate_random_password') + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '.get_service_username') + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' + '.check_service_exists') + def _test_reset_service_password(self, mock_service_exists, + mock_get_username, mock_generate_password, + mock_set_password, mock_set_credentials, + service_exists, service_username): + mock_service_exists.return_value = service_exists + mock_get_username.return_value = service_username + mock_generate_password.return_value = mock.sentinel.password + + with self.snatcher: + response = self._winutils.reset_service_password() + + if not service_exists: + self.assertEqual( + ["Service does not exist: %s" % self._winutils._service_name], + self.snatcher.output) + self.assertFalse(response) + return + + if "\\" not in service_username: + self.assertEqual( + ["Skipping password reset, service running as a built-in " + "account: %s" % service_username], self.snatcher.output) + self.assertFalse(response) + return + + domain, username = service_username.split('\\') + if domain != ".": + self.assertEqual( + ["Skipping password reset, service running as a domain " + "account: %s" % service_username], self.snatcher.output) + self.assertFalse(response) + return + + mock_set_password.assert_called_once_with(username, + mock.sentinel.password) + mock_set_credentials.assert_called_once_with( + self._winutils._service_name, service_username, + mock.sentinel.password) + self.assertEqual(mock_generate_password.call_count, 1) self.assertTrue(response) + def test_reset_service_password(self): + self._test_reset_service_password( + service_exists=True, service_username="EXAMPLE.COM\\username") + + def test_reset_service_password_no_service(self): + self._test_reset_service_password(service_exists=False, + service_username=None) + + def test_reset_service_password_built_in_account(self): + self._test_reset_service_password(service_exists=True, + service_username="username") + + def test_reset_service_password_domain_account(self): + self._test_reset_service_password(service_exists=True, + service_username=".\\username") + @mock.patch('cloudbaseinit.osutils.windows.WindowsUtils' '._get_service') def test_get_service_status(self, mock_get_service): diff --git a/cloudbaseinit/tests/test_init.py b/cloudbaseinit/tests/test_init.py index 831114ae..0096f21f 100644 --- a/cloudbaseinit/tests/test_init.py +++ b/cloudbaseinit/tests/test_init.py @@ -217,6 +217,8 @@ class TestInitManager(unittest.TestCase): self._init.configure_host() self.assertEqual(expected_logging, snatcher.output) mock_check_latest_version.assert_called_once_with() + if CONF.reset_service_password: + self.osutils.reset_service_password.assert_called_once_with() self.osutils.wait_for_boot_completion.assert_called_once_with() mock_get_metadata_service.assert_called_once_with() fake_service.get_name.assert_called_once_with()