From 36928c1613a88a3c0c679abc2345f1996214aca1 Mon Sep 17 00:00:00 2001 From: Adrian Vladu Date: Sun, 12 Oct 2014 19:54:02 -0700 Subject: [PATCH] Fixes winrm access denied bug for Windows versions 6.0 and 6.1 On Windows Vista, 2008, 2008 R2 and 7, changing the configuration of the winrm service will fail with an "Access is denied" error if the User Account Control remote restrictions are enabled. The solution to this issue is to temporarily disable the User Account Control remote restrictions. https://support.microsoft.com/kb/951016 Co-Authored-By: Robert Tingirica Closes-Bug: #1379854 Change-Id: I07b61b94900d39ad0aee544e26ee37bbc94bf7b9 --- .../plugins/windows/winrmcertificateauth.py | 64 +++++++---- .../plugins/windows/winrmlistener.py | 54 +++++++--- .../windows/test_winrmcertificateauth.py | 36 ++++++- .../plugins/windows/test_winrmlistener.py | 23 +++- .../tests/utils/windows/test_security.py | 101 ++++++++++++++++++ cloudbaseinit/utils/windows/security.py | 41 +++++++ 6 files changed, 279 insertions(+), 40 deletions(-) create mode 100644 cloudbaseinit/tests/utils/windows/test_security.py create mode 100644 cloudbaseinit/utils/windows/security.py diff --git a/cloudbaseinit/plugins/windows/winrmcertificateauth.py b/cloudbaseinit/plugins/windows/winrmcertificateauth.py index 727a129a..937ef79b 100644 --- a/cloudbaseinit/plugins/windows/winrmcertificateauth.py +++ b/cloudbaseinit/plugins/windows/winrmcertificateauth.py @@ -16,8 +16,10 @@ from cloudbaseinit import exception from cloudbaseinit.openstack.common import log as logging +from cloudbaseinit.osutils import factory as osutils_factory from cloudbaseinit.plugins import base from cloudbaseinit.plugins import constants +from cloudbaseinit.utils.windows import security from cloudbaseinit.utils.windows import winrmconfig from cloudbaseinit.utils.windows import x509 @@ -53,28 +55,52 @@ class ConfigWinRMCertificateAuthPlugin(base.BasePlugin): "as a certificate has not been provided in the metadata") return (base.PLUGIN_EXECUTION_DONE, False) - winrm_config = winrmconfig.WinRMConfig() - winrm_config.set_auth_config(certificate=True) + osutils = osutils_factory.get_os_utils() + security_utils = security.WindowsSecurityUtils() - for cert_data in certs_data: - cert_manager = x509.CryptoAPICertManager() - cert_thumprint, cert_upn = cert_manager.import_cert( - cert_data, store_name=x509.STORE_NAME_ROOT) + # On Windows Vista, 2008, 2008 R2 and 7, changing the configuration of + # the winrm service will fail with an "Access is denied" error if the + # User Account Control remote restrictions are enabled. + # The solution to this issue is to temporarily disable the User Account + # Control remote restrictions. + # https://support.microsoft.com/kb/951016 + disable_uac_remote_restrictions = (osutils.check_os_version(6, 0) and + not osutils.check_os_version(6, 2) + and security_utils + .get_uac_remote_restrictions()) - if not cert_upn: - LOG.error("WinRM certificate authentication cannot be " - "configured as the provided certificate lacks a " - "subject alt name containing an UPN (OID " - "1.3.6.1.4.1.311.20.2.3)") - continue + try: + if disable_uac_remote_restrictions: + LOG.debug("Disabling UAC remote restrictions") + security_utils.set_uac_remote_restrictions(enable=False) - if winrm_config.get_cert_mapping(cert_thumprint, cert_upn): - winrm_config.delete_cert_mapping(cert_thumprint, cert_upn) + winrm_config = winrmconfig.WinRMConfig() + winrm_config.set_auth_config(certificate=True) - LOG.info("Creating WinRM certificate mapping for user " - "%(user_name)s with UPN %(cert_upn)s", - {'user_name': user_name, 'cert_upn': cert_upn}) - winrm_config.create_cert_mapping(cert_thumprint, cert_upn, - user_name, password) + for cert_data in certs_data: + cert_manager = x509.CryptoAPICertManager() + cert_thumprint, cert_upn = cert_manager.import_cert( + cert_data, store_name=x509.STORE_NAME_ROOT) + + if not cert_upn: + LOG.error("WinRM certificate authentication cannot be " + "configured as the provided certificate lacks a " + "subject alt name containing an UPN (OID " + "1.3.6.1.4.1.311.20.2.3)") + continue + + if winrm_config.get_cert_mapping(cert_thumprint, cert_upn): + winrm_config.delete_cert_mapping(cert_thumprint, cert_upn) + + LOG.info("Creating WinRM certificate mapping for user " + "%(user_name)s with UPN %(cert_upn)s", + {'user_name': user_name, 'cert_upn': cert_upn}) + winrm_config.create_cert_mapping(cert_thumprint, cert_upn, + user_name, password) + + finally: + if disable_uac_remote_restrictions: + LOG.debug("Enabling UAC remote restrictions") + security_utils.set_uac_remote_restrictions(enable=True) return (base.PLUGIN_EXECUTION_DONE, False) diff --git a/cloudbaseinit/plugins/windows/winrmlistener.py b/cloudbaseinit/plugins/windows/winrmlistener.py index 76770f23..3e0689e8 100644 --- a/cloudbaseinit/plugins/windows/winrmlistener.py +++ b/cloudbaseinit/plugins/windows/winrmlistener.py @@ -19,6 +19,7 @@ from oslo.config import cfg from cloudbaseinit.openstack.common import log as logging from cloudbaseinit.osutils import factory as osutils_factory from cloudbaseinit.plugins import base +from cloudbaseinit.utils.windows import security from cloudbaseinit.utils.windows import winrmconfig from cloudbaseinit.utils.windows import x509 @@ -62,31 +63,52 @@ class ConfigWinRMListenerPlugin(base.BasePlugin): def execute(self, service, shared_data): osutils = osutils_factory.get_os_utils() + security_utils = security.WindowsSecurityUtils() if not self._check_winrm_service(osutils): return (base.PLUGIN_EXECUTE_ON_NEXT_BOOT, False) - winrm_config = winrmconfig.WinRMConfig() - winrm_config.set_auth_config(basic=CONF.winrm_enable_basic_auth) + # On Windows Vista, 2008, 2008 R2 and 7, changing the configuration of + # the winrm service will fail with an "Access is denied" error if the + # User Account Control remote restrictions are enabled. + # The solution to this issue is to temporarily disable the User Account + # Control remote restrictions. + # https://support.microsoft.com/kb/951016 + disable_uac_remote_restrictions = (osutils.check_os_version(6, 0) and + not osutils.check_os_version(6, 2) + and security_utils + .get_uac_remote_restrictions()) - cert_manager = x509.CryptoAPICertManager() - cert_thumbprint = cert_manager.create_self_signed_cert( - self._cert_subject) + try: + if disable_uac_remote_restrictions: + LOG.debug("Disabling UAC remote restrictions") + security_utils.set_uac_remote_restrictions(enable=False) - protocol = winrmconfig.LISTENER_PROTOCOL_HTTPS + winrm_config = winrmconfig.WinRMConfig() + winrm_config.set_auth_config(basic=CONF.winrm_enable_basic_auth) - if winrm_config.get_listener(protocol=protocol): - winrm_config.delete_listener(protocol=protocol) + cert_manager = x509.CryptoAPICertManager() + cert_thumbprint = cert_manager.create_self_signed_cert( + self._cert_subject) - winrm_config.create_listener( - cert_thumbprint=cert_thumbprint, - protocol=protocol) + protocol = winrmconfig.LISTENER_PROTOCOL_HTTPS - listener_config = winrm_config.get_listener(protocol=protocol) - listener_port = listener_config.get("Port") + if winrm_config.get_listener(protocol=protocol): + winrm_config.delete_listener(protocol=protocol) - rule_name = "WinRM %s" % protocol - osutils.firewall_create_rule(rule_name, listener_port, - osutils.PROTOCOL_TCP) + winrm_config.create_listener(cert_thumbprint=cert_thumbprint, + protocol=protocol) + + listener_config = winrm_config.get_listener(protocol=protocol) + listener_port = listener_config.get("Port") + + rule_name = "WinRM %s" % protocol + osutils.firewall_create_rule(rule_name, listener_port, + osutils.PROTOCOL_TCP) + + finally: + if disable_uac_remote_restrictions: + LOG.debug("Enabling UAC remote restrictions") + security_utils.set_uac_remote_restrictions(enable=True) return (base.PLUGIN_EXECUTION_DONE, False) diff --git a/cloudbaseinit/tests/plugins/windows/test_winrmcertificateauth.py b/cloudbaseinit/tests/plugins/windows/test_winrmcertificateauth.py index 33c048cd..1eed1b72 100644 --- a/cloudbaseinit/tests/plugins/windows/test_winrmcertificateauth.py +++ b/cloudbaseinit/tests/plugins/windows/test_winrmcertificateauth.py @@ -32,14 +32,19 @@ class ConfigWinRMCertificateAuthPluginTests(unittest.TestCase): self._ctypes_mock = mock.MagicMock() self._win32com_mock = mock.MagicMock() self._pywintypes_mock = mock.MagicMock() + self._moves_mock = mock.MagicMock() self._module_patcher = mock.patch.dict( 'sys.modules', {'ctypes': self._ctypes_mock, 'win32com': self._win32com_mock, - 'pywintypes': self._pywintypes_mock}) + 'pywintypes': self._pywintypes_mock, + 'six.moves': self._moves_mock}) + self._module_patcher.start() + self._winreg_mock = self._moves_mock.winreg + self.winrmcert = importlib.import_module( 'cloudbaseinit.plugins.windows.winrmcertificateauth') self._certif_auth = self.winrmcert.ConfigWinRMCertificateAuthPlugin() @@ -82,22 +87,47 @@ class ConfigWinRMCertificateAuthPluginTests(unittest.TestCase): @mock.patch('cloudbaseinit.utils.windows.winrmconfig.WinRMConfig') @mock.patch('cloudbaseinit.utils.windows.x509.CryptoAPICertManager.' 'import_cert') - def _test_execute(self, mock_import_cert, mock_WinRMConfig, + @mock.patch('cloudbaseinit.osutils.factory.get_os_utils') + @mock.patch('cloudbaseinit.utils.windows.security.WindowsSecurityUtils' + '.set_uac_remote_restrictions') + @mock.patch('cloudbaseinit.utils.windows.security.WindowsSecurityUtils' + '.get_uac_remote_restrictions') + def _test_execute(self, get_uac_rs, set_uac_rs, mock_get_os_utils, + mock_import_cert, mock_WinRMConfig, mock_get_credentials, cert_data, cert_upn): + mock_osutils = mock.MagicMock() mock_service = mock.MagicMock() mock_cert_thumprint = mock.MagicMock() fake_credentials = ('fake user', 'fake password') mock_get_credentials.return_value = fake_credentials + mock_import_cert.return_value = (mock_cert_thumprint, cert_upn) mock_WinRMConfig.get_cert_mapping.return_value = True mock_service.get_client_auth_certs.return_value = [cert_data] + mock_get_os_utils.return_value = mock_osutils + + expected_set_token_calls = [mock.call(enable=False), + mock.call(enable=True)] + + mock_osutils.check_os_version.side_effect = [True, False] + get_uac_rs.return_value = True + + expected_check_version_calls = [mock.call(6, 0), mock.call(6, 2)] + response = self._certif_auth.execute(mock_service, shared_data='fake data') - mock_service.get_client_auth_certs.assert_called_once_with() + if not cert_data: self.assertEqual((base.PLUGIN_EXECUTION_DONE, False), response) else: + mock_service.get_client_auth_certs.assert_called_once_with() + self.assertEqual(expected_check_version_calls, + mock_osutils.check_os_version.call_args_list) + mock_get_os_utils.assert_called_once_with() + self.assertEqual(expected_set_token_calls, + set_uac_rs.call_args_list) + mock_get_credentials.assert_called_once_with('fake data') mock_import_cert.assert_called_once_with( cert_data, store_name=self.winrmcert.x509.STORE_NAME_ROOT) diff --git a/cloudbaseinit/tests/plugins/windows/test_winrmlistener.py b/cloudbaseinit/tests/plugins/windows/test_winrmlistener.py index 7eb736e3..0bcf20e7 100644 --- a/cloudbaseinit/tests/plugins/windows/test_winrmlistener.py +++ b/cloudbaseinit/tests/plugins/windows/test_winrmlistener.py @@ -29,14 +29,17 @@ class ConfigWinRMListenerPluginTests(unittest.TestCase): self._mock_wintypes = mock.MagicMock() self._mock_pywintypes = mock.MagicMock() self._mock_win32 = mock.MagicMock() + self._moves_mock = mock.MagicMock() self._module_patcher = mock.patch.dict( 'sys.modules', {'ctypes': self._mock_wintypes, 'ctypes.wintypes': self._mock_wintypes, 'pywintypes': self._mock_pywintypes, - 'win32com': self._mock_win32}) + 'win32com': self._mock_win32, + 'six.moves': self._moves_mock}) self._module_patcher.start() + self._winreg_mock = self._moves_mock.winreg winrmlistener = importlib.import_module('cloudbaseinit.plugins.' 'windows.winrmlistener') @@ -84,7 +87,12 @@ class ConfigWinRMListenerPluginTests(unittest.TestCase): @mock.patch('cloudbaseinit.utils.windows.winrmconfig.WinRMConfig') @mock.patch('cloudbaseinit.utils.windows.x509.CryptoAPICertManager' '.create_self_signed_cert') - def _test_execute(self, mock_create_cert, mock_WinRMConfig, + @mock.patch('cloudbaseinit.utils.windows.security.WindowsSecurityUtils' + '.set_uac_remote_restrictions') + @mock.patch('cloudbaseinit.utils.windows.security.WindowsSecurityUtils' + '.get_uac_remote_restrictions') + def _test_execute(self, get_uac_rs, set_uac_rs, mock_create_cert, + mock_WinRMConfig, mock_check_winrm_service, mock_get_os_utils, service_status): mock_service = mock.MagicMock() @@ -98,6 +106,13 @@ class ConfigWinRMListenerPluginTests(unittest.TestCase): mock_WinRMConfig().get_listener.return_value = mock_listener_config mock_listener_config.get.return_value = 9999 + mock_osutils.check_os_version.side_effect = [True, False] + get_uac_rs.return_value = True + + expected_check_version_calls = [mock.call(6, 0), mock.call(6, 2)] + expected_set_token_calls = [mock.call(enable=False), + mock.call(enable=True)] + response = self._winrmlistener.execute(mock_service, shared_data) mock_get_os_utils.assert_called_once_with() @@ -107,6 +122,10 @@ class ConfigWinRMListenerPluginTests(unittest.TestCase): self.assertEqual((base.PLUGIN_EXECUTE_ON_NEXT_BOOT, service_status), response) else: + self.assertEqual(expected_check_version_calls, + mock_osutils.check_os_version.call_args_list) + self.assertEqual(expected_set_token_calls, + set_uac_rs.call_args_list) mock_WinRMConfig().set_auth_config.assert_called_once_with( basic=CONF.winrm_enable_basic_auth) mock_create_cert.assert_called_once_with( diff --git a/cloudbaseinit/tests/utils/windows/test_security.py b/cloudbaseinit/tests/utils/windows/test_security.py new file mode 100644 index 00000000..bb85c1ee --- /dev/null +++ b/cloudbaseinit/tests/utils/windows/test_security.py @@ -0,0 +1,101 @@ +# Copyright 2014 Cloudbase Solutions Srl +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import importlib +import mock +import unittest + + +class FakeWindowsError(Exception): + """WindowsError is available on Windows only.""" + def __init__(self, errno): + self.errno = errno + + +class WindowsSecurityUtilsTests(unittest.TestCase): + + def setUp(self): + self._moves_mock = mock.MagicMock() + + self._module_patcher = mock.patch.dict( + 'sys.modules', + {'six.moves': self._moves_mock}) + + self._module_patcher.start() + self._winreg_mock = self._moves_mock.winreg + + self.security = importlib.import_module( + "cloudbaseinit.utils.windows.security") + self.security.WindowsError = FakeWindowsError + + self._security_utils = self.security.WindowsSecurityUtils() + + def tearDown(self): + self._module_patcher.stop() + + def test_set_uac_remote_restrictions(self): + fake_value = False + self._security_utils.set_uac_remote_restrictions( + enable=fake_value) + + self._winreg_mock.SetValueEx.assert_called_once_with( + self._winreg_mock.CreateKey.return_value.__enter__(), + self._security_utils._LATFP_VALUE_NAME, 0, + self._winreg_mock.REG_DWORD, not fake_value) + + self._winreg_mock.CreateKey.assert_called_once_with( + self._winreg_mock.HKEY_LOCAL_MACHINE, + self._security_utils._SYSTEM_POLICIES_KEY) + + def _test_get_uac_remote_restrictions_win_error(self, ret_error=False): + fake_errno = 2 + if ret_error: + fake_errno = 0 + + self._winreg_mock.OpenKey.side_effect = [ + self.security.WindowsError(fake_errno)] + if ret_error: + self.assertRaises(self.security.WindowsError, + self._security_utils.get_uac_remote_restrictions) + else: + response = self._security_utils.get_uac_remote_restrictions() + self.assertTrue(response) + + def test_get_uac_remote_restrictions_win_error_ret_error(self): + self._test_get_uac_remote_restrictions_win_error(ret_error=True) + + def test_get_uac_remote_restrictions_win_error(self): + self._test_get_uac_remote_restrictions_win_error(ret_error=False) + + def test_get_uac_remote_restrictions_no_error(self): + key = mock.MagicMock() + fake_key_value = 0 + key.__enter__.return_value = fake_key_value + fake_reg_type = mock.sentinel.fake_reg_type + + self._winreg_mock.OpenKey.return_value = key + self._winreg_mock.QueryValueEx.return_value = (fake_key_value, + fake_reg_type) + + response = self._security_utils.get_uac_remote_restrictions() + + self._winreg_mock.QueryValueEx.assert_called_once_with( + fake_key_value, + self._security_utils._LATFP_VALUE_NAME) + + self._winreg_mock.OpenKey.assert_called_once_with( + self._winreg_mock.HKEY_LOCAL_MACHINE, + self._security_utils._SYSTEM_POLICIES_KEY) + + self.assertTrue(bool(response)) diff --git a/cloudbaseinit/utils/windows/security.py b/cloudbaseinit/utils/windows/security.py new file mode 100644 index 00000000..aa950a52 --- /dev/null +++ b/cloudbaseinit/utils/windows/security.py @@ -0,0 +1,41 @@ +# Copyright 2014 Cloudbase Solutions Srl +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from six.moves import winreg + + +class WindowsSecurityUtils(object): + _SYSTEM_POLICIES_KEY = ("SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\" + "Policies\\System") + _LATFP_VALUE_NAME = "LocalAccountTokenFilterPolicy" + + # https://support.microsoft.com/kb/951016 + def set_uac_remote_restrictions(self, enable=True): + with winreg.CreateKey(winreg.HKEY_LOCAL_MACHINE, + self._SYSTEM_POLICIES_KEY) as key_name: + winreg.SetValueEx(key_name, self._LATFP_VALUE_NAME, 0, + winreg.REG_DWORD, int(not enable)) + + def get_uac_remote_restrictions(self): + try: + with winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, + self._SYSTEM_POLICIES_KEY) as key: + (value, regtype) = winreg.QueryValueEx(key, + self._LATFP_VALUE_NAME) + return not bool(value) + except WindowsError as e: + if e.errno == 0x2: + return True + else: + raise