Skip to content

Commit

Permalink
Add TPM when switching to EFI
Browse files Browse the repository at this point in the history
This is useful for OSes which default to BIOS. It matches what
`virt-install --boot uefi` does. This fixes e.g. secure boot on
Fedora VMs.

https://issues.redhat.com/browse/RHEL-24522
  • Loading branch information
martinpitt committed Feb 19, 2024
1 parent 56ff41d commit 22fec23
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/components/vm/overview/firmware.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { Tooltip } from "@patternfly/react-core/dist/esm/components/Tooltip";
import { useDialogs, DialogsContext } from 'dialogs.jsx';

import { ModalError } from 'cockpit-components-inline-notification.jsx';
import { domainSetOSFirmware, domainCanInstall } from "../../../libvirtApi/domain.js";
import { domainAddTPM, domainCanInstall, domainSetOSFirmware } from "../../../libvirtApi/domain.js";
import { supportsUefiXml, labelForFirmwarePath } from './helpers.jsx';

const _ = cockpit.gettext;
Expand Down Expand Up @@ -61,6 +61,10 @@ class FirmwareModal extends React.Component {
objPath: vm.id,
loaderType: stateToXml(this.state.firmware)
});

if (this.state.firmware == 'efi' && !vm.hasTPM && vm.capabilities.supportsTPM)
await domainAddTPM({ connectionName: vm.connectionName, vmName: vm.name });

Dialogs.close();
} catch (exc) {
this.dialogErrorSet(_("Failed to change firmware"), exc.message);
Expand Down
14 changes: 14 additions & 0 deletions src/libvirt-xml-parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ export function getDomainCapSupportsSpice(capsXML) {
return hasSpiceGraphics || hasSpiceChannel;
}

export function getDomainCapSupportsTPM(capsXML) {
const domainCapsElem = getElem(capsXML);
const tpmCapsElems = domainCapsElem.getElementsByTagName("tpm")?.[0]
?.getElementsByTagName("enum")?.[0]
?.getElementsByTagName("value");
return tpmCapsElems?.length > 0;
}

export function getSingleOptionalElem(parent, name) {
const subElems = parent.getElementsByTagName(name);
return subElems.length > 0 ? subElems[0] : undefined; // optional
Expand Down Expand Up @@ -262,6 +270,7 @@ export function parseDomainDumpxml(connectionName, domXml, objPath) {
const watchdog = parseDumpxmlForWatchdog(devicesElem);
const vsock = parseDumpxmlForVsock(devicesElem);
const hasSpice = parseDumpxmlForSpice(devicesElem);
const hasTPM = parseDumpxmlForTPM(devicesElem);

const hasInstallPhase = parseDumpxmlMachinesMetadataElement(metadataElem, 'has_install_phase') === 'true';
const installSourceType = parseDumpxmlMachinesMetadataElement(metadataElem, 'install_source_type');
Expand Down Expand Up @@ -306,6 +315,7 @@ export function parseDomainDumpxml(connectionName, domXml, objPath) {
vsock,
metadata,
hasSpice,
hasTPM,
};
}

Expand Down Expand Up @@ -549,6 +559,10 @@ function parseDumpxmlForSpice(devicesElem) {
return false;
}

function parseDumpxmlForTPM(devicesElem) {
return devicesElem.getElementsByTagName('tpm').length > 0;
}

export function parseDumpxmlForFilesystems(devicesElem) {
const filesystems = [];
const filesystemElems = devicesElem.getElementsByTagName('filesystem');
Expand Down
15 changes: 15 additions & 0 deletions src/libvirtApi/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import {
getDomainCapCPUHostModel,
getDomainCapDiskBusTypes,
getDomainCapSupportsSpice,
getDomainCapSupportsTPM,
getSingleOptionalElem,
parseDomainDumpxml,
getHostDevElemBySource,
Expand Down Expand Up @@ -661,6 +662,7 @@ export async function domainGet({
cpuHostModel: getDomainCapCPUHostModel(domCaps),
supportedDiskBusTypes: getDomainCapDiskBusTypes(domCaps),
supportsSpice: getDomainCapSupportsSpice(domCaps),
supportsTPM: getDomainCapSupportsTPM(domCaps),
};

const [state] = await call(connectionName, objPath, 'org.libvirt.Domain', 'GetState', [0], { timeout, type: 'u' });
Expand Down Expand Up @@ -1054,3 +1056,16 @@ export async function domainReplaceSpice({ connectionName, id: objPath }) {
throw ex; // not-covered: see above
}
}

export async function domainAddTPM({ connectionName, vmName }) {
const args = ["virt-xml", "-c", `qemu:///${connectionName}`, "--add-device", "--tpm", "default", vmName];
return cockpit.spawn(args, { err: "message", superuser: connectionName === "system" ? "try" : null })
// RHEL 8 does not support --tpm default; arch (and likely other system setups) fails with
// unsupported configuration: TPM version '2.0' is not supported
.catch(ex => {
if (ex.message?.includes("Unknown TPM backend type") || ex.message?.includes("unsupported configuration"))
console.log("Failed to add TPM:", ex);
else
return Promise.reject(ex);
});
}
1 change: 1 addition & 0 deletions test/browser/run-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ fi
EXCLUDES="$EXCLUDES
TestMachinesCreate.testConfigureBeforeInstall
TestMachinesCreate.testConfigureBeforeInstallBios
TestMachinesCreate.testConfigureBeforeInstallBiosTPM
TestMachinesCreate.testCreateBasicValidation
TestMachinesCreate.testCreateNameGeneration
TestMachinesCreate.testCreateDownloadRhel
Expand Down
60 changes: 60 additions & 0 deletions test/check-machines-create
Original file line number Diff line number Diff line change
Expand Up @@ -2126,6 +2126,9 @@ vnc_password= "{vnc_passwd}"
# Attach some interface
m.execute("virsh attach-interface --persistent VmNotInstalled bridge virbr0")

# BIOS machine doesn't have TPM by default
self.assertNotIn("tpm", m.execute("virsh dumpxml VmNotInstalled"))

# Change the os boot firmware configuration
b.wait_in_text("#vm-VmNotInstalled-firmware", "BIOS")
b.click("#vm-VmNotInstalled-firmware")
Expand All @@ -2134,6 +2137,13 @@ vnc_password= "{vnc_passwd}"
b.click("#firmware-dialog-apply")
b.wait_not_present(".pf-v5-c-modal-box__body")
b.wait_in_text("#vm-VmNotInstalled-firmware", "UEFI")
self.assertIn("<os firmware='efi'>", m.execute("virsh dumpxml VmNotInstalled"))

# auto-enabled software TPM (doesn't work on RHEL 8 or arch)
if not m.image.startswith("rhel-8") and not m.image.startswith("centos-8") and m.image != 'arch':
tpm_xml = m.execute("virsh dumpxml VmNotInstalled | xmllint --xpath /domain/devices/tpm -")
self.assertIn('model="tpm-crb"', tpm_xml)
self.assertIn('type="emulator"', tpm_xml)

# Temporarily delete the OVMF binary and check the firmware options again
if "fedora" in m.image or "rhel" in m.image or "centos" in m.image:
Expand Down Expand Up @@ -2244,6 +2254,56 @@ vnc_password= "{vnc_passwd}"
self.assertIn("<os>", domainXML)
self.assertNotIn("efi", domainXML)

@testlib.skipImage("does not support virt-xml --tpm", "rhel-8*", "centos-8*")
@testlib.skipImage("does not support TPM 2.0", "arch")
def testConfigureBeforeInstallBiosTPM(self):
m = self.machine
b = self.browser
TestMachinesCreate.CreateVmRunner(self)

# create VM with BIOS
vmName = "subVmTest1"
self.login_and_go("/machines")
self.waitPageInit()
dialog = TestMachinesCreate.VmDialog(self, sourceType='file',
name=vmName,
location=TestMachinesCreate.TestCreateConfig.NOVELL_MOCKUP_ISO_PATH,
memory_size=128, memory_size_unit='MiB',
storage_size=256, storage_size_unit='MiB',
create_and_run=False)
dialog.open().fill()
b.click(".pf-v5-c-modal-box__footer #create-and-edit")
b.wait_not_present("#create-vm-dialog")
b.wait_in_text(f"#vm-{vmName}-firmware", "BIOS")

# manually add TPM to BIOS
m.execute(f"virt-xml --add-device --tpm default {vmName}")
self.assertIn("tpm", m.execute(f"virsh dumpxml {vmName}"))
# this doesn't change the UI, so we have to reload to ensure that the UI picks up the change
# otherwise this is a race condition
b.reload()
b.enter_page('/machines')

# Change the os boot firmware to EFI
b.click(f"#vm-{vmName}-firmware")
b.wait_visible(".pf-v5-c-modal-box__body")
b.select_from_dropdown(".pf-v5-c-modal-box__body select", "efi")
b.click("#firmware-dialog-apply")
b.wait_not_present(".pf-v5-c-modal-box__body")
b.wait_in_text(f"#vm-{vmName}-firmware", "UEFI")

# did not add a second TPM; that would fail in the dialog anyway, but let's double-check the backend
out = m.execute(f"virsh dumpxml {vmName} | xmllint --xpath /domain/devices/tpm - | grep --count '<tpm'")
self.assertEqual(out.strip(), "1")

# Install the VM
b.click(f"#vm-{vmName}-system-install")
b.wait_in_text(f"#vm-{vmName}-system-state", "Running")

# AppArmor policy fixed in later versions, don't bother with reporting
if m.image == 'debian-stable':
self.allow_journal_messages('.*apparmor="DENIED".*name="/etc/ssl/openssl.cnf".*comm="swtpm".*')

def testCreateDownloadRhel(self):
runner = TestMachinesCreate.CreateVmRunner(self, rhel_download=True)
config = TestMachinesCreate.TestCreateConfig
Expand Down

0 comments on commit 22fec23

Please sign in to comment.