Common framework for configuring secure boot

Two drivers already support turning secore boot on and off,
Redfish will follow soon. This patch adds ManagementInterface
calls to get and set the secure boot state.

Story: #2008270
Task: #41561
Change-Id: I96b2697163def52618b4c051a5c85adf7d1818a5

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.

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)

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)

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)

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)

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)

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):

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.