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

8 files changed, 196 insertions, 5 deletions

diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py
index baf5a5579..7d137f9c4 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 eea09d1c0..6eebe50aa 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 5d9ac49ba..0944e7d4e 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 0298ff90c..6b52f6c76 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 d0bb625ff..4c0536fc5 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 1cd12feb0..03c4b0d01 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 5c7542402..1ccc33699 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 000000000..1bb28c56f
--- /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.