From 2399a296e3ff21e47e9b86b517b6ae9f8f525c01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Tue, 22 Jul 2025 12:26:32 +0200 Subject: [PATCH] Preserve vTPM state between power off and power on Without this patch, due to power_on calling _hard_reboot, which in turn undefines the VM to ensure a clean domain XML, the TPM data is erased by libvirt. This is very surprising to users who store persistent data in the TPM, such as keys required to decrypt the main disk of the VM. Closes-Bug: #2118888 Signed-Off-By: jonas.schaefer@cloudandheat.com Change-Id: Iefb879428681003d6db604b70353a91913c92461 --- nova/tests/fixtures/libvirt.py | 1 + nova/tests/unit/virt/libvirt/test_driver.py | 108 +++++++++++++++++++- nova/tests/unit/virt/libvirt/test_guest.py | 7 ++ nova/virt/libvirt/driver.py | 44 +++++++- nova/virt/libvirt/guest.py | 19 +++- 5 files changed, 170 insertions(+), 9 deletions(-) diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 08e9496f5064..9e5fd1595de2 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -100,6 +100,7 @@ VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY = 7 VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1 VIR_DOMAIN_UNDEFINE_NVRAM = 4 +VIR_DOMAIN_UNDEFINE_KEEP_TPM = 64 VIR_DOMAIN_AFFECT_CURRENT = 0 VIR_DOMAIN_AFFECT_LIVE = 1 diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index ff15bafb0e69..b8f311e6c2b7 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1762,6 +1762,39 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_which.assert_not_called() + @mock.patch.object(libvirt_driver.LibvirtDriver, + '_register_all_undefined_instance_details', + new=mock.Mock()) + @mock.patch.object(host.Host, 'has_min_version', return_value=True) + def test_keep_tpm_supported(self, mock_version): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr.init_host('dummyhost') + self.assertTrue( + drvr._may_keep_vtpm, + "LibvirtDriver did not correctly detect libvirt version " + "supporting KEEP_TPM" + ) + + @mock.patch.object(libvirt_driver.LibvirtDriver, + '_register_all_undefined_instance_details', + new=mock.Mock()) + @mock.patch.object(host.Host, 'has_min_version') + def test_keep_tpm_unsupported(self, mock_version): + def version_check(lv_ver=None, **kwargs): + if lv_ver == libvirt_driver.MIN_VERSION_INT_FOR_KEEP_TPM: + return False + return True + + mock_version.side_effect = version_check + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr.init_host('dummyhost') + self.assertFalse( + drvr._may_keep_vtpm, + "LibvirtDriver did not correctly detect libvirt version which " + "does not support KEEP_TPM" + ) + def test__check_multipath_misconfiguration(self): self.flags(volume_use_multipath=False, volume_enforce_multipath=True, group='libvirt') @@ -19263,6 +19296,51 @@ class LibvirtConnTestCase(test.NoDBTestCase, # ensure no raise for no such domain drvr._undefine_domain(instance) + @mock.patch.object(host.Host, "get_guest") + def test_undefine_domain_disarms_keep_vtpm_if_not_supported( + self, mock_get): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr._may_keep_vtpm = False # normally set by init_host + instance = objects.Instance(**self.test_instance) + fake_guest = mock.Mock() + mock_get.return_value = fake_guest + + drvr._undefine_domain(instance, keep_vtpm=True) + + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=False, + ) + + # Check that it truly forces it to False and doesn't do a `not` or + # something weird :-). + fake_guest.reset_mock() + drvr._undefine_domain(instance, keep_vtpm=False) + + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=False, + ) + + @mock.patch.object(host.Host, "get_guest") + def test_undefine_domain_passes_keep_vtpm_if_supported(self, mock_get): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr._may_keep_vtpm = True # normally set by init_host + instance = objects.Instance(**self.test_instance) + fake_guest = mock.Mock() + mock_get.return_value = fake_guest + + drvr._undefine_domain(instance, keep_vtpm=True) + + fake_guest.delete_configuration.assert_called_once_with(keep_vtpm=True) + + # Check that it does not force keep_vtpm to true, just because it is + # supported. + fake_guest.reset_mock() + drvr._undefine_domain(instance, keep_vtpm=False) + + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=False, + ) + @mock.patch.object(host.Host, "list_instance_domains") @mock.patch.object(objects.BlockDeviceMappingList, "bdms_by_instance_uuid") @mock.patch.object(objects.InstanceList, "get_by_filters") @@ -21834,7 +21912,33 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_delete_files.assert_called_once_with(fake_inst) # vTPM secret should not be deleted until instance is deleted. mock_delete_vtpm.assert_not_called() - mock_undefine.assert_called_once_with(fake_inst) + mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False) + + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain') + @mock.patch('nova.crypto.delete_vtpm_secret') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.delete_instance_files') + @mock.patch('nova.virt.driver.block_device_info_get_mapping') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._unplug_vifs') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vpmems', + new=mock.Mock(return_value=None)) + def test_cleanup_preserves_tpm_if_not_destroying_disks( + self, mock_unplug, mock_get_mapping, mock_delete_files, + mock_delete_vtpm, mock_undefine, + ): + """Test with default parameters.""" + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) + fake_inst = objects.Instance(**self.test_instance) + mock_get_mapping.return_value = [] + mock_delete_files.return_value = True + + with mock.patch.object(fake_inst, 'save'): + drvr.cleanup('ctxt', fake_inst, 'netinfo', destroy_disks=False) + + mock_unplug.assert_called_once_with(fake_inst, 'netinfo', True) + mock_get_mapping.assert_called_once_with(None) + mock_delete_files.assert_not_called() + mock_delete_vtpm.assert_not_called() + mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=True) @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain') @mock.patch('nova.crypto.delete_vtpm_secret') @@ -21859,7 +21963,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr.cleanup('ctxt', fake_inst, 'netinfo') # vTPM secret should not be deleted until instance is deleted. mock_delete_vtpm.assert_not_called() - mock_undefine.assert_called_once_with(fake_inst) + mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False) @mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files', return_value=True) diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py index 6d9eb6ede505..359013c54ea6 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/nova/tests/unit/virt/libvirt/test_guest.py @@ -138,6 +138,13 @@ class GuestTestCase(test.NoDBTestCase): fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM) + def test_delete_configuration_with_keep_vtpm_true(self): + self.guest.delete_configuration(keep_vtpm=True) + self.domain.undefineFlags.assert_called_once_with( + fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM | + fakelibvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM) + def test_delete_configuration_exception(self): self.domain.undefineFlags.side_effect = fakelibvirt.libvirtError( 'oops') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 7ebbb7a72092..b95f2791abe3 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -265,6 +265,9 @@ MIN_VIRTIO_SOUND_QEMU_VERSION = (8, 2, 0) # Minimum version of Qemu that supports multifd migration with post-copy MIN_MULTIFD_WITH_POSTCOPY_QEMU_VERSION = (10, 1, 0) +# Minimum version to preserve vTPM data +MIN_VERSION_INT_FOR_KEEP_TPM = (8, 9, 0) + REGISTER_IMAGE_PROPERTY_DEFAULTS = [ 'hw_machine_type', 'hw_cdrom_bus', @@ -578,6 +581,10 @@ class LibvirtDriver(driver.ComputeDriver): # See also nova.virt.libvirt.cpu.api.API.core(). self.cpu_api = libvirt_cpu.API() + # Cache the availability of the VIR_DOMAIN_UNDEFINE_KEEP_TPM flag in + # this libvirt version. This is set in init_host. + self._may_keep_vtpm = False + def _discover_vpmems(self, vpmem_conf=None): """Discover vpmems on host and configuration. @@ -899,6 +906,12 @@ class LibvirtDriver(driver.ComputeDriver): self._check_vtpm_support() + # Cache the availability of the VIR_DOMAIN_UNDEFINE_KEEP_TPM flag in + # this libvirt version. + self._may_keep_vtpm = self._host.has_min_version( + MIN_VERSION_INT_FOR_KEEP_TPM, + ) + self._check_multipath() # Even if we already checked the whitelist at startup, this driver @@ -1635,11 +1648,32 @@ class LibvirtDriver(driver.ComputeDriver): self.cleanup(context, instance, network_info, block_device_info, destroy_disks, destroy_secrets=destroy_secrets) - def _undefine_domain(self, instance): + def _delete_guest_configuration(self, guest, keep_vtpm): + """Wrapper around guest.delete_configuration which incorporates version + checks for the additional arguments. + + :param guest: The domain to undefine. + :param keep_vtpm: If set, the vTPM data (if any) is not deleted during + undefine. + + This flag may be ignored if libvirt is too old to support + preserving vTPM data (see bug #2118888). + """ + if keep_vtpm and not self._may_keep_vtpm: + LOG.warning( + "Temporary undefine operation is deleting vTPM contents. " + "Please upgrade libvirt to >= 8.9.0 to avoid this.", + instance=guest.uuid, + ) + keep_vtpm = False + + guest.delete_configuration(keep_vtpm=keep_vtpm) + + def _undefine_domain(self, instance, keep_vtpm=False): try: guest = self._host.get_guest(instance) try: - guest.delete_configuration() + self._delete_guest_configuration(guest, keep_vtpm=keep_vtpm) except libvirt.libvirtError as e: with excutils.save_and_reraise_exception() as ctxt: errcode = e.get_error_code() @@ -1802,7 +1836,7 @@ class LibvirtDriver(driver.ComputeDriver): self._cleanup_ephemeral_encryption_secrets( context, instance, block_device_info) - self._undefine_domain(instance) + self._undefine_domain(instance, keep_vtpm=not cleanup_instance_disks) def _cleanup_ephemeral_encryption_secrets( self, context, instance, block_device_info @@ -2377,7 +2411,7 @@ class LibvirtDriver(driver.ComputeDriver): # undefine it. If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - guest.delete_configuration() + self._delete_guest_configuration(guest, keep_vtpm=True) try: dev.copy(conf.to_xml(), reuse_ext=True) @@ -3506,7 +3540,7 @@ class LibvirtDriver(driver.ComputeDriver): # If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - guest.delete_configuration() + self._delete_guest_configuration(guest, keep_vtpm=True) # NOTE (rmk): Establish a temporary mirror of our root disk and # issue an abort once we have a complete copy. diff --git a/nova/virt/libvirt/guest.py b/nova/virt/libvirt/guest.py index e6c60d29d1fb..55cd70b3f9a9 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -295,11 +295,26 @@ class Guest(object): yield VCPUInfo( id=vcpu[0], cpu=vcpu[3], state=vcpu[1], time=vcpu[2]) - def delete_configuration(self): - """Undefines a domain from hypervisor.""" + def delete_configuration(self, keep_vtpm=False): + """Undefines a domain from hypervisor. + + :param keep_vtpm: If true, the vTPM data will be preserved. Otherwise, + it will be deleted. Defaults to false (that is, deleting the vTPM + data). + + Calling this with `keep_vtpm` set to True should, eventually, be + followed up with a call where it is set to False (after re-defining + the VM in libvirt with the same UUID), to prevent orphaning the vTPM + data in libvirt's data directory. + + It is the caller's responsibility to ensure that keep_vtpm is only set + to true on libvirt versions which support it, that is >= 8.9.0. + """ try: flags = libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM + if keep_vtpm: + flags |= libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM self._domain.undefineFlags(flags) except libvirt.libvirtError: LOG.debug("Error from libvirt during undefineFlags for guest "