diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index baf5a5579f..7d137f9c4d 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -972,6 +972,40 @@ class ManagementInterface(BaseInterface): raise exception.UnsupportedDriverExtension( driver=task.node.driver, extension='get_boot_mode') + def get_secure_boot_state(self, task): + """Get the current secure boot state for the node. + + NOTE: Not all drivers support this method. Older hardware + may not implement that. + + :param task: A task from TaskManager. + :raises: MissingParameterValue if a required parameter is missing + :raises: DriverOperationError or its derivative in case + of driver runtime error. + :raises: UnsupportedDriverExtension if secure boot is + not supported by the driver or the hardware + :returns: Boolean + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='get_secure_boot_state') + + def set_secure_boot_state(self, task, state): + """Set the current secure boot state for the node. + + NOTE: Not all drivers support this method. Older hardware + may not implement that. + + :param task: A task from TaskManager. + :param state: A new state as a boolean. + :raises: MissingParameterValue if a required parameter is missing + :raises: DriverOperationError or its derivative in case + of driver runtime error. + :raises: UnsupportedDriverExtension if secure boot is + not supported by the driver or the hardware + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='set_secure_boot_state') + @abc.abstractmethod def get_sensors_data(self, task): """Get sensors data method. diff --git a/ironic/drivers/modules/boot_mode_utils.py b/ironic/drivers/modules/boot_mode_utils.py index eea09d1c08..6eebe50aab 100644 --- a/ironic/drivers/modules/boot_mode_utils.py +++ b/ironic/drivers/modules/boot_mode_utils.py @@ -14,11 +14,13 @@ # under the License. from oslo_log import log as logging +from oslo_utils import excutils from ironic.common import boot_modes from ironic.common import exception from ironic.common.i18n import _ from ironic.common import utils as common_utils +from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.conf import CONF from ironic.drivers import utils as driver_utils @@ -296,3 +298,53 @@ def get_boot_mode(node): 'bios': boot_modes.LEGACY_BIOS, 'uefi': boot_modes.UEFI}) return CONF.deploy.default_boot_mode + + +@task_manager.require_exclusive_lock +def configure_secure_boot_if_needed(task): + """Configures secure boot if it has been requested for the node.""" + if not is_secure_boot_requested(task.node): + return + + try: + task.driver.management.set_secure_boot_state(task, True) + except exception.UnsupportedDriverExtension: + # TODO(dtantsur): make a failure in Xena + LOG.warning('Secure boot was requested for node %(node)s but its ' + 'management interface %(driver)s does not support it. ' + 'This warning will become an error in a future release.', + {'node': task.node.uuid, + 'driver': task.node.management_interface}) + except Exception as exc: + with excutils.save_and_reraise_exception(): + LOG.error('Failed to configure secure boot for node %(node)s: ' + '%(error)s', + {'node': task.node.uuid, 'error': exc}, + exc_info=not isinstance(exc, exception.IronicException)) + else: + LOG.info('Secure boot has been enabled for node %s', task.node.uuid) + + +@task_manager.require_exclusive_lock +def deconfigure_secure_boot_if_needed(task): + """Deconfigures secure boot if it has been requested for the node.""" + if not is_secure_boot_requested(task.node): + return + + try: + task.driver.management.set_secure_boot_state(task, False) + except exception.UnsupportedDriverExtension: + # NOTE(dtantsur): don't make it a hard failure to allow tearing down + # misconfigured nodes. + LOG.debug('Secure boot was requested for node %(node)s but its ' + 'management interface %(driver)s does not support it.', + {'node': task.node.uuid, + 'driver': task.node.management_interface}) + except Exception as exc: + with excutils.save_and_reraise_exception(): + LOG.error('Failed to deconfigure secure boot for node %(node)s: ' + '%(error)s', + {'node': task.node.uuid, 'error': exc}, + exc_info=not isinstance(exc, exception.IronicException)) + else: + LOG.info('Secure boot has been disabled for node %s', task.node.uuid) diff --git a/ironic/drivers/modules/pxe_base.py b/ironic/drivers/modules/pxe_base.py index 5d9ac49bae..0944e7d4e7 100644 --- a/ironic/drivers/modules/pxe_base.py +++ b/ironic/drivers/modules/pxe_base.py @@ -133,6 +133,8 @@ class PXEBaseMixin(object): pxe_utils.clean_up_pxe_env(task, images_info, ipxe_enabled=self.ipxe_enabled) + boot_mode_utils.deconfigure_secure_boot_if_needed(task) + @METRICS.timer('PXEBaseMixin.prepare_ramdisk') def prepare_ramdisk(self, task, ramdisk_params): """Prepares the boot of Ironic ramdisk using PXE. @@ -240,6 +242,7 @@ class PXEBaseMixin(object): :returns: None """ boot_mode_utils.sync_boot_mode(task) + boot_mode_utils.configure_secure_boot_if_needed(task) node = task.node boot_option = deploy_utils.get_boot_option(node) diff --git a/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py b/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py index 0298ff90cd..6b52f6c76f 100644 --- a/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py +++ b/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py @@ -16,8 +16,12 @@ from unittest import mock from ironic.common import boot_modes +from ironic.common import exception +from ironic.conductor import task_manager from ironic.drivers.modules import boot_mode_utils +from ironic.drivers.modules import fake from ironic.tests import base as tests_base +from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils @@ -64,3 +68,67 @@ class GetBootModeTestCase(tests_base.TestCase): boot_mode = boot_mode_utils.get_boot_mode(self.node) self.assertEqual(boot_modes.UEFI, boot_mode) self.assertEqual(0, mock_log.warning.call_count) + + +@mock.patch.object(fake.FakeManagement, 'set_secure_boot_state', autospec=True) +class SecureBootTestCase(db_base.DbTestCase): + + def setUp(self): + super(SecureBootTestCase, self).setUp() + self.node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + instance_info={'capabilities': {'secure_boot': 'true'}}) + self.task = task_manager.TaskManager(self.context, self.node.id) + + def test_configure_none_requested(self, mock_set_state): + self.task.node.instance_info = {} + boot_mode_utils.configure_secure_boot_if_needed(self.task) + self.assertFalse(mock_set_state.called) + + @mock.patch.object(boot_mode_utils.LOG, 'warning', autospec=True) + def test_configure_unsupported(self, mock_warn, mock_set_state): + mock_set_state.side_effect = exception.UnsupportedDriverExtension + # Will become a failure in Xena + boot_mode_utils.configure_secure_boot_if_needed(self.task) + mock_set_state.assert_called_once_with(self.task.driver.management, + self.task, True) + self.assertTrue(mock_warn.called) + + def test_configure_exception(self, mock_set_state): + mock_set_state.side_effect = RuntimeError('boom') + self.assertRaises(RuntimeError, + boot_mode_utils.configure_secure_boot_if_needed, + self.task) + mock_set_state.assert_called_once_with(self.task.driver.management, + self.task, True) + + def test_configure(self, mock_set_state): + boot_mode_utils.configure_secure_boot_if_needed(self.task) + mock_set_state.assert_called_once_with(self.task.driver.management, + self.task, True) + + def test_deconfigure_none_requested(self, mock_set_state): + self.task.node.instance_info = {} + boot_mode_utils.deconfigure_secure_boot_if_needed(self.task) + self.assertFalse(mock_set_state.called) + + @mock.patch.object(boot_mode_utils.LOG, 'warning', autospec=True) + def test_deconfigure_unsupported(self, mock_warn, mock_set_state): + mock_set_state.side_effect = exception.UnsupportedDriverExtension + boot_mode_utils.deconfigure_secure_boot_if_needed(self.task) + mock_set_state.assert_called_once_with(self.task.driver.management, + self.task, False) + self.assertFalse(mock_warn.called) + + def test_deconfigure(self, mock_set_state): + boot_mode_utils.deconfigure_secure_boot_if_needed(self.task) + mock_set_state.assert_called_once_with(self.task.driver.management, + self.task, False) + + def test_deconfigure_exception(self, mock_set_state): + mock_set_state.side_effect = RuntimeError('boom') + self.assertRaises(RuntimeError, + boot_mode_utils.deconfigure_secure_boot_if_needed, + self.task) + mock_set_state.assert_called_once_with(self.task.driver.management, + self.task, False) diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py index d0bb625ff9..4c0536fc51 100644 --- a/ironic/tests/unit/drivers/modules/test_ipxe.py +++ b/ironic/tests/unit/drivers/modules/test_ipxe.py @@ -34,6 +34,7 @@ from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers import base as drivers_base from ironic.drivers.modules import agent_base +from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import ipxe from ironic.drivers.modules import pxe_base @@ -890,10 +891,13 @@ class iPXEBootTestCase(db_base.DbTestCase): boot_devices.PXE, persistent=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + autospec=True) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True) def test_prepare_instance_localboot(self, clean_up_pxe_config_mock, - set_boot_device_mock): + set_boot_device_mock, + secure_boot_mock): with task_manager.acquire(self.context, self.node.uuid) as task: instance_info = task.node.instance_info instance_info['capabilities'] = {'boot_option': 'local'} @@ -905,6 +909,7 @@ class iPXEBootTestCase(db_base.DbTestCase): set_boot_device_mock.assert_called_once_with(task, boot_devices.DISK, persistent=True) + secure_boot_mock.assert_called_once_with(task) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True) @@ -957,10 +962,13 @@ class iPXEBootTestCase(db_base.DbTestCase): self.assertFalse(cache_mock.called) self.assertFalse(dhcp_factory_mock.return_value.update_dhcp.called) + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', + autospec=True) @mock.patch.object(pxe_utils, 'clean_up_pxe_env', autospec=True) @mock.patch.object(pxe_utils, 'get_instance_image_info', autospec=True) def test_clean_up_instance(self, get_image_info_mock, - clean_up_pxe_env_mock): + clean_up_pxe_env_mock, + secure_boot_mock): with task_manager.acquire(self.context, self.node.uuid) as task: image_info = {'kernel': ['', '/path/to/kernel'], 'ramdisk': ['', '/path/to/ramdisk']} @@ -970,6 +978,7 @@ class iPXEBootTestCase(db_base.DbTestCase): task, image_info, ipxe_enabled=True) get_image_info_mock.assert_called_once_with( task, ipxe_enabled=True) + secure_boot_mock.assert_called_once_with(task) @mock.patch.object(ipxe.iPXEBoot, '__init__', lambda self: None) diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 1cd12feb00..03c4b0d01f 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -1238,7 +1238,7 @@ class CleanUpFullFlowTestCase(db_base.DbTestCase): mock_get_deploy_image_info.return_value = {} with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: + shared=False) as task: task.driver.deploy.clean_up(task) mock_get_instance_image_info.assert_called_with(task, ipxe_enabled=False) diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 5c75424026..1ccc336993 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -35,6 +35,7 @@ from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers import base as drivers_base from ironic.drivers.modules import agent_base +from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import fake from ironic.drivers.modules import ipxe @@ -694,10 +695,13 @@ class PXEBootTestCase(db_base.DbTestCase): set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + autospec=True) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True) def test_prepare_instance_localboot(self, clean_up_pxe_config_mock, - set_boot_device_mock): + set_boot_device_mock, + secure_boot_mock): with task_manager.acquire(self.context, self.node.uuid) as task: instance_info = task.node.instance_info instance_info['capabilities'] = {'boot_option': 'local'} @@ -709,6 +713,7 @@ class PXEBootTestCase(db_base.DbTestCase): set_boot_device_mock.assert_called_once_with(task, boot_devices.DISK, persistent=True) + secure_boot_mock.assert_called_once_with(task) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True) @@ -784,10 +789,13 @@ class PXEBootTestCase(db_base.DbTestCase): def test_prepare_instance_ramdisk_pxe_conf_exists(self): self._test_prepare_instance_ramdisk(config_file_exits=False) + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', + autospec=True) @mock.patch.object(pxe_utils, 'clean_up_pxe_env', autospec=True) @mock.patch.object(pxe_utils, 'get_instance_image_info', autospec=True) def test_clean_up_instance(self, get_image_info_mock, - clean_up_pxe_env_mock): + clean_up_pxe_env_mock, + secure_boot_mock): with task_manager.acquire(self.context, self.node.uuid) as task: image_info = {'kernel': ['', '/path/to/kernel'], 'ramdisk': ['', '/path/to/ramdisk']} @@ -797,6 +805,7 @@ class PXEBootTestCase(db_base.DbTestCase): ipxe_enabled=False) get_image_info_mock.assert_called_once_with(task, ipxe_enabled=False) + secure_boot_mock.assert_called_once_with(task) class PXERamdiskDeployTestCase(db_base.DbTestCase): diff --git a/releasenotes/notes/secure-boot-cf1c134bfb75768d.yaml b/releasenotes/notes/secure-boot-cf1c134bfb75768d.yaml new file mode 100644 index 0000000000..1bb28c56f4 --- /dev/null +++ b/releasenotes/notes/secure-boot-cf1c134bfb75768d.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + The ``pxe`` and ``ipxe`` boot interfaces now automatically configure + secure boot if the management interface supports it. +deprecations: + - | + Currently the bare metal API permits setting the ``secure_boot`` capability + for nodes, which driver does not support setting secure boot. This is + deprecated and will become a failure in the Xena cycle. +other: + - | + Extends ``ManagementInterface`` with two new calls: + ``get_secure_boot_state`` and ``set_secure_boot_state``. They are + optional and may be implemented for hardware that supports dynamically + enabling/disabling secure boot.