libvirt: Don't reuse a stale secure boot VARS template - #378
Conversation
|
|
||
| /// Files bcvk created for a domain outside of its libvirt-managed storage: | ||
| /// the persistent Ignition config and the secure boot VARS template. | ||
| /// |
There was a problem hiding this comment.
Hmm is there no way we can convince libvirt to "take ownership" of this stuff?
But ok as is
There was a problem hiding this comment.
libvirt removes a domain's NVRAM file on undefine --nvram (since 1.2.9, so on every RHEL 9/10 and Fedora version), but it can't enroll custom keys itself: firmware autoselection's enrolled-keys only picks the distro's vars. So bcvk now writes the keyed vars as the domain's <nvram> path, where it used to pass them as a template, and rm and rm-all's existing --nvram removes the file. The rm changes are dropped. The file is still regenerated on every run, since libvirt leaves a transient domain's NVRAM behind. Your sign-off is kept on 4128e8a and d22d3bc.
Tested on a 16-core RHEL 10.2 devspace (libvirt 11.10): I created a VM with key set A, ran rm, then re-created it with key set B. B's PK was enrolled and no file was left behind. The new integration test passes, and it fails against main.
Generated-by: https://github.com/cgwalters/#llms
| }; | ||
|
|
||
| let ignition = dom | ||
| .find("bootc:ignition-persistent-path") |
There was a problem hiding this comment.
Ah I see. We should perhaps have a const for this
With --secure-boot-keys, bcvk wrote <name>_OVMF_VARS.fd into the storage pool and passed it to libvirt as the NVRAM template. libvirt copied it into its own NVRAM file, which `virsh undefine --nvram` removes, but the template was bcvk's and nothing removed it. It was also only created if it didn't exist yet, so a VM re-created under the same name (after `libvirt rm`, or with --replace) silently got the previous VM's enrolled keys instead of the ones passed. A UKI signed with the new keys then fails with "Access Denied", which looks like a signing problem rather than a stale file. Point <nvram> at the file itself instead of using it as a template, so it is the domain's NVRAM and libvirt removes it on `undefine --nvram`, which rm and rm-all already pass. It is also always regenerated, since libvirt uses an existing NVRAM file as is and a transient VM (which is never undefined) leaves its file behind. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
Enroll one key set, remove the VM and check its NVRAM file is gone, then put the old file back (as an older bcvk or a transient VM leaves it) and re-create the VM under the same name with another key set: the new PK must be the one enrolled. This is the case where a UKI signed with the new keys failed with "Access Denied". Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
061e73b to
d22d3bc
Compare
2 participants
bcvk libvirt run --secure-boot-keyswrote a per-VM<name>_OVMF_VARS.fd, with the keys enrolled, into the storage pool and passed it to libvirt as the NVRAMtemplate. libvirt removes its own copy onundefine --nvram, but nothing removed bcvk's file, and bcvk only wrote it if it didn't exist yet. So re-creating a VM under the same name (afterlibvirt rm, or with--replace) got the old VM's keys, and a UKI signed with the new keys failed with "Access Denied".The first commit makes that file the domain's NVRAM itself, with no template, so libvirt owns it and the
--nvramthatrmandrm-allalready pass removes it. bcvk regenerates it on everylibvirt run, because libvirt uses an existing NVRAM file as is and a transient VM leaves its file behind.libvirt startdoesn't touch it, so variables the guest writes survive restarts. The second commit adds an integration test.Testing, on a 16-core RHEL 10.2 devspace (libvirt 11.10, qemu:///session):
test_libvirt_secure_boot_vars_not_reusedenrolls key set A, runsrmand checks the NVRAM file is gone, then puts the old file back, re-creates the VM with key set B and checks withvirt-fw-varsthat B's PK is enrolled. It fails on main and passes with this branch.test_libvirt_port_forward_xmlbecause port 9090 was taken on the devspace, andtest_libvirt_run_transient_vmwith a permission error on the shared base disk during the parallel run; it passed when rerun alone.The
Signed-off-by: Colin Walters <walters@verbum.org>on these commits was added on cgwalters' approval of the review draft (cgwalters-forge#9 (review)), and kept through the rework that followed his review comment here.Generated-by: https://github.com/cgwalters/#llms