CP-39744, CA-370858: Block actions depending on unimplemented VTPM functionality #4796
Conversation
| ) | ||
| ] | ||
|
|
||
| (* The options can be set with the variable xapiflags in /etc/sysconfig/xapi. |
There was a problem hiding this comment.
I think they can also be set in /etc/xapi.conf, that is what I usually change.
There was a problem hiding this comment.
That sounds like an easier mechanism to use, although I have to say it would be much better if xapi.conf was autogenerated from xapi_globs
psafont
force-pushed
the
private/paus/nobios-tpm
branch
from
ffe4438 to
627c3b9
Compare
psafont
changed the title
psafont
force-pushed
the
private/paus/nobios-tpm
branch
from
627c3b9 to
9cf0f87
Compare
| let vm_record = vm.record () in | ||
| (* disallow exports and cross-pool migrations of VMs with VTPMs *) | ||
| ( if vm_record.API.vM_VTPMs <> [] then | ||
| let message = "Exporting VM metadata with VTPMs attached" in |
There was a problem hiding this comment.
We intend to implement this, so as a temporary measure this is probably fine (we limit the libraries that the CLI server can access on purpose to avoid it accidentally bypassing RBAC checks)
There was a problem hiding this comment.
But why do you do this here in the CLI, while the export handle below already has the check?
There was a problem hiding this comment.
Because the export ,etadata check happens at the end of the operation, after 8 minutes passed, which I think is bad UX
ocaml/xapi/xapi_xenops.ml
Outdated
| ( match (firmware, vm.API.vM_VTPMs) with | ||
| | Xenops_types.Vm.Bios, _ :: _ -> | ||
| let message = "Booting BIOS VM with VTPMs attached" in | ||
| Helpers.maybe_raise_vtpm_uninmplemented __FUNCTION__ message |
There was a problem hiding this comment.
Should this include a reference to the VM?
There was a problem hiding this comment.
I don't think it's needed, the whole action to start a VM fails, this already signals the VM that fails to the user, both in CLI and GUI
The optional parameter for firmware served no purpose, and forced the addition of a unit parameter, remove both. This forces the unit-tests to use a particular firmware to run all tests, use Bios as the default. The firmware value is extracted in preparation of boot blocking depending on the firmware and the presence of vTPM Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
The Physical Presence Interface part of the vTPM specification is only implemented for UEFI guests, other functionality may be missing. Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
This makes it easy to track and link the removal of the function in the quality gate to promotion of the feature to a stable one. Also make raising the exception configurable so developers can easily disable the check and test changes around the functionality without having to recompile xapi. This is done by writing xapiflags=-ignore-vtpm-unimplemented in /etc/sysconfig/xapi and running xe-toolstack-restart Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
We're unable to serialize the data because the field for the contents is not exposed in the API and it's based on a secret, which can be dangerous once it's been implemented. Exports are exposed using an HTTP endpoint, this means it's an indirect operation and that other operations that use the feature will fail in extraneous ways in a non-instantaneous way. To avoid this the two methods that use it in xapi are changed as well (vm-export and VM cross-pool migrations). This makes the failure immediate and clear. Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
psafont
force-pushed
the
private/paus/nobios-tpm
branch
from
9cf0f87 to
2e3b83b
Compare
4 participants
CP-39744: Block BIOS VMs with vTPM attached from booting
The Physical Presence Interface part of the vTPM specification is only
implemented for UEFI guests, other functionality may be missing.
Also makes the check that raises exceptions centralised and makes it togglable using /etc/sysconfic/xapi
CA-370858: disallow VM exports with VTPMs attached
We're unable to serialize the data because the field for the contents is not exposed in the API and it's based on a secret, which can be dangerous once it's been implemented.
Exports are exposed using an HTTP endpoint, this means it's an indirect operation and that other operations that use the feature will fail in extraneous ways in a non-instantaneous way.
To avoid this the two methods that use it in xapi are changed as well (vm-export and VM cross-pool migrations). This makes the failure immediate and clear.