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

RFC: improved UEFI Secure Boot certificates management in XAPI #4647

Closed
stormi opened this issue Mar 10, 2022 · 7 comments
Closed

RFC: improved UEFI Secure Boot certificates management in XAPI #4647

stormi opened this issue Mar 10, 2022 · 7 comments

Comments

@stormi
Copy link
Contributor

stormi commented Mar 10, 2022

tl;dr

We would like to contribute a change that makes XAPI keep the local UEFI Secure Boot certificates that are stored on host disks in sync with the ones stored in XAPI's Pool object.

Context

UEFI Secureboot certificates (PK, KEK, db and dbx) are currently stored in several places in a Citrix Hypervisor or XCP-ng pool.

  • In XAPI, at the pool level: pool.uefi_certificates. This is a tarball containing one to four .auth files.
    • In Citrix Hypervisor, they are populated from the Host firmware's UEFI variables (except PK.auth which comes from the varstored RPM). BIOS hosts don't support Guest Secure Boot in CH.
    • In XCP-ng, we provide users with a secureboot-certs script that is able to either install the certificates from local files or download the default certs from the Internet (XCP-ng docs).
  • In XAPI, at the host level: host.uefi_certificates. To my knowledge, it's always in sync with the pool, and I don't know why it exists in the first place. Insight welcome on this!
  • On each host's filesystem, at /usr/share/varstored on Citrix Hypervisor, at /var/lib/uefistored on XCP-ng to comply with the FHS as those are volatile files. I'm not entirely sure why those files exist in the first place but I assume this might be in an attempt to make varstored not too dependent on XAPI.
  • In each UEFI VM's NVRAM store (stored in XAPI's db AFAIK), populated at first boot by varstored(CH)/uefistored(XCP-ng) and then never touched again in normal use. Some variables such as the dbx may be modified later by the guest OS itself, but not by the hypervisor (outside user intervention).

To a VM, the only source of truth for UEFI variables is its NVRAM store. All the other levels of certificate storage are just a convenience to populate the NVRAM store of new VMs at first boot with usable certificates.

All the layers described above inherit from each other in some way:

  • XAPI Host level is synced with XAPI Pool level
  • Each time a UEFI VM is started, XAPI may or may not update the certificates on disk before the VM starts. This is what we will discuss here.
  • When an UEFI VM starts, for each certificate database (PK, KEK, db, dbx) that is missing from its NVRAM store, varstored/uefistored reads it from disk and writes it to the NVRAM store. The VM's certs are never touched again by varstored/uefistored afterwards, unless the user deletes them, in which case the missing certs will be inherited again at next VM boot.

In theory, in current implementation of XAPI, certs are not meant to be inherited from Pool (XAPI) to host disk after they have been inherited once.

In practice, there is a bug in Citrix Hypervisor 8.2 that causes the certificates to be rewritten to disk every time a UEFI VM starts. @benjamreis contributed a fix for that recently: #4624

The issue

Despite it was a bug, the fact that the certificates were written to disk each time a UEFI VM starts was interesting to us: it would keep the certs on disk in sync with the Pool certs in XAPI 🎉.

On Citrix Hypervisor, this might not be really important, as certs are read from the master host's firmware at first boot and not modified afterwards (I'm not sure... Maybe they're updated if the firmware is updated. I haven't checked that part.).

But on XCP-ng, it is important because we support modifying the pool's UEFI certificates at will. For example, users may update the dbx (revocation database) to the latest provided by Microsoft to enhance the overall security on their pool for new VMs (for new VMs only... As we said earlier, VMs that already have certificates live their own "certificate life").

So @benjamreis' fix is actually a functional regression for us :D

Proposal #1

The current "XAPI to host disk sync" algorigthm is as follows:

  • When a UEFI VM starts:
    • if KEK.auth doesn't exist, extract the tarball from XAPI and write the files on disk
    • else do nothing

The current buggy behaviour in CH 8.2, fixed on master, is:

  • When a UEFI VM starts:
    • extract the tarball from XAPI and write the files on disk, always

Note: this does not remove existing files that are absent from XAPI, though. Files are just overwritten if they exist.

The new behaviour we would like to contribute is:

  • When a UEFI VM starts:
  • Compare the checksums of files on disk with those in XAPI
  • If a file is present on disk but missing from XAPI, remove it from disk
  • If a file is present on disk but differs from XAPI, sync it from XAPI

If we agree on this solution, we will contribute a PR to implement it.

Proposal #2

In my view, a better but heavier solution would be: get rid of the files on disk, completely. Let varstored/uefistored get them from XAPI directly when needed (that is, when booting up a UEFI VM that doesn't have certificates yet).

This would remove a layer that seems to be causing more issues than it solves (I may be wrong on this statement).

I'm not sure this would make varstored more dependent on XAPI, as varstored supports several back-ends. Only the XAPI back-end would be dependent on XAPI but it already is as far as I can tell.

Maybe we could also remove the certificates that are stored at host level in XAPI. Here again I can be wrong, but I doubt they add a real value. After all, the certs in Pool/Host levels are only meant to bootstrap new VMs. If users need to use specific certificates for specific VMs, they still have complete control over the certs in VMs anyway (using the varstored-setup and varstore-set commands).

I can't promise our planning would allow us to contribute such changes but I would like to discuss them at least.

@edwintorok
Copy link
Contributor

Instead of VM start the synchronization could also be performed exactly once on toolstack startup. That way you avoid race conditions where a VM rebooting might see a disappearing file (if your renames aren't atomic).
And Xapi_host.set_uefi_certificates could also immediately write out the new file, which would take care of updates without toolstack restarts.
If you want to update it at the pool level then Xapi_pool.set_uefi_certificates would need to be implemented that loops over all hosts and calls Xapi_host.set_uefi_certificates (this will need to avoid loops though where setting host level cert gets propagated up to pool level).

@stormi
Copy link
Contributor Author

stormi commented Mar 10, 2022

If there's no reason to keep doing the synchronization on VM start, this looks good to me. Would it be needed also when a host joins a pool so that it inherits the pool certs?

@psafont
Copy link
Member

psafont commented Mar 10, 2022

A merge of the certificates between the joiner and the pool would be needed, I think

@stormi
Copy link
Contributor Author

stormi commented Mar 11, 2022

I don't see how we could do a "merge", so I think the pool certs should replace the joining host certs.

benjamreis added a commit to xcp-ng/xen-api that referenced this issue Mar 24, 2022
- `Pool.set_uefi_certificates` is implemented and calls
`Host.set_uefi_certificates` on all hosts

- `Host.set_uefi_certificates` writes certificates on disks

- On XAPI startup certificates stored in XAPI's `Host.uefi_certificates`
are written on disks

- When a host joins the pool `Host.set_uefi_certificates` is called
with the certificates stored in XAPI's `Pool.uefi_certificates`

This means:
	- At every XAPI startup the certificates in host disks are synced with
XAPI's `Host.uefi_certificates`

	- When `Pool.set_uefi_certificates` is called all hosts are synced in XAPi and disks
with XAPI's `Pool.uefi_certificates`

	- When `Host.set_uefi_certificates` is called the host certificates on disk
are synced with XAPI's `Host.uefi_certificates`

Also: `Host.set_uefi_certificates` no longer calls `Pool.set_uefi_certificates`, this requires changes
in external libs (varstored, uefistored, etc) to set the pool's certificates: call `Pool.set_uefi_certificates`.

See: xapi-project#4647

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
@stormi
Copy link
Contributor Author

stormi commented Mar 28, 2022

So, unless there are arguments against, we would like to completely bypass host.set_uefi_certificates and deprecate host.uefi_certificates.

The reasons:

  • Unless we've missed something, host.uefi_certificates is never read anywhere. When certs are written to disk on any given host, they're actually read from the pool!
  • The main use of host.set_uefi_certificates is actually to populate the pool certificates. As we'll provide a pool.set_uefi_certificates entry point, it becomes obsolete.

And this would also remove the need to do anything when a host joins a pool, as the certs will be written to disk when the toolstack restarts, directly read from the pool object.

benjamreis added a commit to xcp-ng/xen-api that referenced this issue Apr 7, 2022
- `Pool.set_uefi_certificates` is implemented and writes the certificates
on all its hosts' disks

- `Host.set_uefi_certificates` is now deprecated and transmit the call to the pool method
- `Host.uefi_certificates` is deprecated as well as it's getter, the value is not updated.

- On XAPI startup certificates stored in XAPI's `Pool.uefi_certificates`
are written on disks

- When a host joins the pool's certificates are written on its disk.

This means:
	- At every XAPI startup the certificates in host disks are synced with
XAPI's `Pool.uefi_certificates`

	- When `Pool.set_uefi_certificates` is called all hosts are synced on their disks
with XAPI's `Pool.uefi_certificates`.

Also: `Host.set_uefi_certificates` calls should be replaced by `Pool.set_uefi_certificates`, this requires changes
in external libs (varstored, uefistored, etc) to set the pool's certificates: call `Pool.set_uefi_certificates`.

See: xapi-project#4647

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
@benjamreis
Copy link
Contributor

Solved by merged of #4659

@benjamreis
Copy link
Contributor

@stormi this can be closed. :)

@psafont psafont closed this as completed Apr 14, 2022
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

No branches or pull requests

4 participants