Skip to content
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

Introduce VTPM for guests #4780

Merged
merged 75 commits into from
Sep 9, 2022
Merged

Conversation

psafont
Copy link
Member

@psafont psafont commented Aug 26, 2022

The feature is experimental. Currently the contents of the TPMs are stored in xapi's database, this means they are not always up-to-date which makes the feature unable to work with HA and with VM snapshots. Cloning and checkpoints work, this means that migrations work as well.

vtpms are always associated with a VM and it's not possible to change the VM once set. The VTPM can only be associated to a VM if the VM is halted.
vtpms have the fields unique and secure. Unique can be set at creation time, is false by default, and will avoid creating copies of the contents of the vTPMs. secure is not configurable and defaults to false

To enable the experimental feature xe-enable-experimental-feature vtpm has to be run on the pool coordinator. In case the coordinator changes it's better if this is run on all hosts.

The last commit introduces swtpm-wrapper into the repo, this has been moved from the .spec repo, and the former will need a change to remove its own copy when including these changes.

psafont and others added 30 commits February 24, 2022 10:06
For now the {open,close}_handle and a with_mapping are added to the
module, this allows users to map and unmap memory from domains to dom0
memory with correct permissions.

The map and unmap functions are not exposed to ensure the use of the
same handle for both.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
This binary is not installed when building the package

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
This is restricted to local access only, through the UNIX socket.

This allows the software TPM daemon to retrieve and push the contents of
the TPM from and to the xapi database.

Any remote accesses to the contents are forbidden,

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Bring vtpm branch up-to-date with master
This will be used for creating sandboxes for swtpm

While we're at it, create an interface file for sandboxes to hide
implementation details, add useful methods for Path and tweak methods

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Xenopsd manages several services that sustain the lifecycle of each
domain. To launch a service, xenopsd usually starts up a daemon and it
waits for this daemon to write its pid on xenstore to signal the service
is ready to attend requests.

While this method is convenient in order to be able to cancel the wait
using xenstore watches, it's unfortunate as it makes the daemons depend
on xenstore even if it isn't needed to supply the service.

This change implements an alternative way to set up a cancellable wait
on the service readiness using inotify and epoll by setting up
filesystem watches instead of xenstore ones.

The polling loop wakes up every second to have a direct method to deal
with timeouts as well as unrelated events that happened in the
filesystem.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
For every domain now xenopsd creates a chroot (which starts up a guard
daemon instance), launches an SWTPM instance, then waits on its pidfile
to be created.
On domain destruction is shuts down both instances: first SWTPM, then
the guard.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
This fits the documentation of chroots better

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
…managed

CP-38554: Start up and stop SWTPM and guard for every domain
Now swtpm is only started for domains that have vtpm=true added to their
platform's metada. This can be done using xe e.g.
xe vm-param-add uuid=VM-XXX param-name=platform vtpm=true

Shutting down domains without the swtpm and the xapi-guard services
running works as normal and without any errors

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
This was missed when parametrizing the code, varstored was hardcoded

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Now VTPM objects are created and attached to a VM when it is started for
the first and they are destroyed when the VM they are attached to are
destroyed.

These objects are empty and do not reflect the state of the actual vTPM
attached to the domains.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
…aram

CP-39414: start swtpm depending on the VM's platform metadata
Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Needs to be moved to a separate module to avoid cycles in the build
system, about to use this type from another module.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Xenopsd needs to know not just whether a VM has a vTPM or not, but also
its UUID (in case a XAPI DB storage backend is used).

For now only 1 vTPM/VM is supported, as before.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
psafont and others added 12 commits August 3, 2022 11:04
Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
The current persistence backend needs to add copying to secrets, this
should be removed as soon as the backend is removed, we want to avoid
other code to depend on it.

The is_unique flag prevents copying the vtpm contents when cloning,
instead a new vtpm is recreated, this breaks Guest features like
bitlocker on the newly created VM.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
This fails because xapi's DB may not have the contents of the VTPM at
the time the snapshot is taken. This means that taking a snapshot could
lead to data loss, instead block it.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
This also blocks creation of VMs from templates with vtpm in the platform data.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
The state of VTPMs and xapi's database are usually out-of-sync. Using
them while HA is enabled can lead to data loss. Block the two features
from being used at the same time.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Relevant messages from its changelog:

CP-35104: Add swtpm-wrapper for proper logging

CP-35054: Start SWTPM daemon deprivilideged

CP-40195: Only call swtpm_setup for manufacture
swtpm_setup is run as root and handles guest data on subsequent boots
after TPM manufacture. Avoid running it completely after the initial
manufacture to avoid any possible security issues.
The '.lock' will only be present after running swtpm_setup so run chown
conditionally. At the same time, handle errors from chown rather than
silently ignoring them.

CA-369317: Don't ignore errors from swtpm_setup

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
@psafont psafont requested review from robhoes and lindig August 26, 2022 13:21
@lindig
Copy link
Contributor

lindig commented Aug 26, 2022

A lot of code that has been reviewed previously. I mostly glanced over it and looked more closely at some error handling.

@@ -12,9 +12,7 @@
* GNU Lesser General Public License for more details.
*)

module D = Debug.Make (struct
let name = "xapi" (* this is set to 'xapi' deliberately! :) *)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And now it has been changed away from 'xapi' deliberately? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spelunked and found no justification for it in the commit that introduced the module. It's more helpful to have the actual module that prints the loglines, same reasoning I used to change all the unjustified ones 3 years back, it just helps whith triaging

Although the module has existed since the Rio release, it doesn't make
sense to state so. This is because until now it was impossible to create
vtpm objects in the database, trying to create one of them resulted in
an exception. This has the added benefit to mark all the parts of the
class to be prototypes and this will make them easier to change before
the feature becomes stable.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Copy link
Member

@robhoes robhoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming everything was previously reviewed on the feature branch, this is good. We won't merge it immediately though.

This allows hvmloader to load the correct acpi table for guests using
tpm, while retaining the previous acpi table for existing guests

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
@robhoes robhoes merged commit de99204 into xapi-project:master Sep 9, 2022
@psafont psafont deleted the private/paus/vtpm branch September 9, 2022 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants