-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIndows 11/2022 VMs for cluster level >= 4.6 - failed to create/edit such VMs due to the TPM device #1596
Comments
Once again we see a case in which we need to compensate the lack of functionality (in this case of defining TPM) in the vm portal but this time I don't think the right place is the backend side because, while it's ok to add TPM when changing the OS to one that requires TPM, it's not ok to drop the TPM data without some kind of warning when changing the OS to one that doesn't support TPM - so for "ordinary" clients I think we should still reject such input and require them to explicitly specify whether TPM should be enabled or disabled. When it comes to the vm portal, I wouldn't necessarily do it exactly as the webadmin does, but set TPM=disabled when the OS doesn't support TPM and TPM=enabled when it requires it (and unlike the webadmin, always set TPM=disabled when the OS supports TPM but doesn't require it) - if you want to handle both cases above (in my opinion the second flow is more theoretical than a practical one but I don't want to debate over that again :) ) @mz-pdm what do you think? |
It sounds reasonable to enable TPM here only when required by the guest OS, since TPM presence may imply further limitations (e.g. no snapshots with memory, if applicable here) or expectations (e.g. security). But what to do about TPM data deletion in the more theoretical scenario of the guest OS change that disables TPM? |
Sounds ok to silently enable TPM only when required by the guest OS, i.,e. currently for Win 2022 & Win 11. Our basic guidelines are 1) if a VM created via webadmin with defaults settings is identical to the one created via the VM portal for TPM - and the answer is yes.
We can warn the user and block it in that case but I prefer allowing it by silently disabling TPM for non required OS. We are already doing that for other use cases as well, i.e. changing cluster for an existing VM might reset assigned hosts and disable cpu pass-through without warning the user. If a user changes his VM's OS or cluster, he should be aware that it might have consequences... we can add a tololtip with a general warning. |
It's important to understand that if the user changes the TPM-enabled OS by mistake to another OS and then returns it back then the consequence may be that the guest OS won't boot anymore anywhere and must be reinstalled or some recovery procedure applied. Which is probably more serious than the other use cases with changed configuration. I'm not sure whether Windows stores any boot-critical data into TPM by default -- there is some info at https://docs.microsoft.com/en-us/windows/security/information-protection/tpm/how-windows-uses-the-tpm and around; probably not but users may use any of the functionalities mentioned there, e.g. BitLocker, and then it would be a real problem. I'd say that displaying a confirmation dialog with a warning before committing such a guest OS change, similarly to what we do in webadmin when TPM is being disabled, would be a good and good enough protection. |
which warning do you refer to on webadmin? So the logic on VM portal should include the following: For editing of existing vms:
Did I miss a use case? |
The warning is not displayed if there is no stored TPM data yet. After enabling TPM, run the VM for a couple of minutes (no guest OS needed) then stop it and try to change the OS. You should get a dialog saying "TPM was disabled and the current TPM data will be irrecoverably deleted. If you want to keep the data, cancel this dialog and enable TPM again before confirming your changes.".
Yes.
Yes. Technically, it's not needed if there is no TPM data stored yet, but it's OK to always show the dialog in such a case.
Yes.
Yes.
I can't think of any. |
@ahadas @mz-pdm
The cause in the case above seems to be UEFI. Web Admin forces UEFI even if it doesn't match the cluster defaults or if user explicitly has picked different option (both during create and edit). Interestingly, the chipset is changed silently also if user changes OS to one that requires TPM. This seems risky to me and not something that a regular user should be able to do in VM Portal. |
@rszwajko 'uefi' is the default firmware in cluster level 4.7 but that default should only apply to new VMs that are created from scratch (from the blank template), it shouldn't affect VMs that are created from other templates or VMs that are being edited. |
@ahadas New VM scenario in VM Portal:
REST:
Response (400 - Bad Request):
|
Ah ok, yeah, the default changed to UEFI in cluster level 4.7 and you do it with 4.6 cluster level |
That's fine for admin user. Can we treat such advanced TPM-related errors as non-recoverable for a regular user? |
I wouldn't limit it only to admin users - I think the VM portal should specify UEFI for VMs that are set with Win 11/2022 also when they are created by non-admin users |
@ahadas The corner cases (i.e. discussed above) are not easy to handle automatically. UI would need to follow backend logic defined here. In order to repeat the same tests more information need to be exposed via REST i.e. |
@rszwajko you're right but we don't really need the solution to be that generic. The VM portal can always set UEFI for Windows 11/2022 - otherwise it wouldn't work. That logic on the backend is there to covers all flows, like adding a RHEL VM with TPM on different cluster levels (which is not possible using the VM portal). I'd rather check with OsRepository if the OS requires TPM and ask for UEFI in that case, regardless of the cluster level and other considerations |
yep. This is PR #1610 :) |
sorry, not TPM.. let me rephrase the last part: |
For new VMs and for edit VMs while vm.tpm_enabled = false is changed to TPM-required=true OS: So I agree with Arik that this should be the same behavior on VM portal. @ahadas @mz-pdm But instead of setting it via rest/vm portal as well, why can't it be the default behavior on backend? The user can't anyway change it or override it successfully so what's the point of setting it via rest? For edit VMs:
Supporting |
in general when creating a vm using rest-api and specifying some settings, the current expectation is that the vm will inherit the other settings from the template (and in the past from instance types). it was easier to cope with that standard and instead of changing the chipset and firmware according to the specified operating system, do the same and validate that we end up with a valid combination specifically in this case, it also didn't make sense to change the firmware to UEFI "automatically" behind the scenes because UEFI was tech preview before 4.7 and in 4.7 it is the default firmware
I wouldn't do that at this point - the VM portal lacks so many features for the sake of simplicity (like high availability) that it wouldn't make much sense to add chipset and firmware in my opinion. users should really use the default one (from the template or from the cluster when creating vms from scrach) unless they must switch, and the only case would be those new windows versions |
As long as the user can create a VM based on a Blank template or based on a template while overriding/updating few settings like OS, then we need to cope with those use cases on VM portal as well. On webadmin/frontend you do enforce setting firmware to UEFI for cluster level > 4.5 here https://github.com/oVirt/ovirt-engine/blob/af4ac851f145175eacb8b6d12fe9231381bcb810/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java#L4039-L4042 for required OS, so we'll need to do the same on VM portal for those specific cases when UEFI should be set and it's not (CL 4.6 or editing an existing VM without UEFI set).
+1, agree |
right, the text above was in response to |
Following important changes on api-metamodel has been done since 1.3.7: * Change asciidoc link-generation oVirt/ovirt-engine-api-metamodel#28 * Add RPM packaging * Switch from log4j backend to java.util.logging backend Following important changes on api-model has been done since 4.5.10: * Image I/O Proxy is replaced by ovirt-imageio https://bugzilla.redhat.com/2042401 * Deprecate VmPool.display attribute https://bugzilla.redhat.com/2106349 * Updated documentation to SystemOptionService https://bugzilla.redhat.com/1974974 * Add TpmSupport to OperatingSystemInfo oVirt/ovirt-web-ui#1596 * Change slashes to hyphens in xrefs oVirt/ovirt-engine-api-model#80 Signed-off-by: Martin Perina <mperina@redhat.com>
Following important changes on api-metamodel has been done since 1.3.7: * Change asciidoc link-generation oVirt/ovirt-engine-api-metamodel#28 * Add RPM packaging * Switch from log4j backend to java.util.logging backend Following important changes on api-model has been done since 4.5.10: * Image I/O Proxy is replaced by ovirt-imageio https://bugzilla.redhat.com/2042401 * Deprecate VmPool.display attribute https://bugzilla.redhat.com/2106349 * Updated documentation to SystemOptionService https://bugzilla.redhat.com/1974974 * Add TpmSupport to OperatingSystemInfo oVirt/ovirt-web-ui#1596 * Change slashes to hyphens in xrefs oVirt/ovirt-engine-api-model#80 Signed-off-by: Martin Perina <mperina@redhat.com>
The fix for basic flow (CL is 4.7 and firmware is set to UEFI) is merged: #1610 Now need to handle left cases when vm.cluster level >= 4.6 and vm.TPM-required OS (win 11/2022) and vm.Chipset type <> UEFI. In those cases the Chipset type should be changed automatically without notifying the user to UEFI. same as supported on webadmin. |
In order to enable TPM, the VM needs to use UEFI firmware. Currently oVirt supports 2 types of UEFI firmwares: 1. standard - 'q35_ovmf' 2. secure - 'q35_secure_boot' As requested in [1], we should force 'q35_ovmf' firmware type if the current firmware prevents enabling TPM: 1. in 'create VM' scenario - override the cluster defaults 2. in 'edit VM' scenario - override previous value Known issues: 1. if firmware is different than cluster defaults then a warning icon is displayed in the Web Admin UI -> VM details -> General tab 2. in VM Portal it's not possible to revert firmware to previous value which prevents some OS configurations i.e. Windows XP [1] oVirt#1596 (comment) Reference-Url: oVirt#1596
In order to enable TPM, the VM needs to use UEFI firmware. Currently oVirt supports 2 types of UEFI firmwares: 1. standard - 'q35_ovmf' 2. secure - 'q35_secure_boot' As requested in [1], we should force 'q35_ovmf' firmware type if the current firmware prevents enabling TPM: 1. in 'create VM' scenario - override the cluster defaults 2. in 'edit VM' scenario - override previous value Known issues: 1. if firmware is different than cluster defaults then a warning icon is displayed in the Web Admin UI -> VM details -> General tab 2. in VM Portal it's not possible to revert firmware to previous value which prevents some OS configurations i.e. Windows XP [1] oVirt#1596 (comment) Reference-Url: oVirt#1596
In order to enable TPM, the VM needs to use UEFI firmware. Currently oVirt supports 2 types of UEFI firmwares: 1. standard - 'q35_ovmf' 2. secure - 'q35_secure_boot' As requested in [1], we should force 'q35_ovmf' firmware type if the current firmware prevents enabling TPM: 1. in 'create VM' scenario - override the cluster defaults 2. in 'edit VM' scenario - override previous value Known issues: 1. if firmware is different than cluster defaults then a warning icon is displayed in the Web Admin UI -> VM details -> General tab 2. in VM Portal it's not possible to revert firmware to previous value which prevents some OS configurations i.e. Windows XP [1] #1596 (comment) Reference-Url: #1596
Since TPM is not set on backend automatically based on OS type and Cluster level version, the following 2 scenarios occur:
for
cluster level
>= 4.6: when trying to create a new VM with OS set toWindows 11
or2022
or when editing an existing VM and set OS toWindows 11
or2022
, the creation/edit failed in rest backend with the following error:TPM device is required by the guest OS
E.g.
When trying to edit a VM with OS
Windows 11
or2022
andcluster level
>= 4.6 by changing the OS to non supported TPM one, e.g. Linux, the following error appears:Details:
On webadmin the TPM is enabled/disabled on frontend based on OS and cluster level: https://github.com/oVirt/ovirt-engine/blob/91bc0b8f9e4d0cbfa8799f860a008948f3e6ed0a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java#L4047
So for fixing we should either enable/disbale the TPM on backend based on OS or the same logic as on webadmin frontend should be used by web-ui as well.
The text was updated successfully, but these errors were encountered: