Merge "Common framework for configuring secure boot"

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.