Re: [PATCH 09/10] qemu: Always set labels for TPM state

On Wed, Mar 20, 2024 at 10:18:39AM -0400, Stefan Berger wrote: > On 3/20/24 08:23, Peter Krempa wrote: > > On Wed, Mar 20, 2024 at 10:19:14 +0100, Andrea Bolognani wrote: > > > Consider the case in which one host (mig-one) exports its > > > local filesystem /srv/nfs/libvirt/swtpm via NFS, and at the > > > same time bind-mounts it to /var/lib/libvirt/swtpm; another > > > host (mig-two) mounts the same filesystem to the same > > > location, this time via NFS. Additionally, in order to > > > allow migration in both directions, on mig-one the > > > /var/lib/libvirt/swtpm directory is listed in the > > > shared_filesystems qemu.conf option. > > > > > > When migrating from mig-one to mig-two, things work just fine; > > > going in the opposite direction, however, results in an error: > > > > > > # virsh migrate cirros qemu+ssh://mig-one/system > > > error: internal error: QEMU unexpectedly closed the monitor (vm='cirros'): > > > qemu-system-x86_64: tpm-emulator: Setting the stateblob (type 1) failed with a TPM error 0x1f > > > qemu-system-x86_64: error while loading state for instance 0x0 of device 'tpm-emulator' > > > qemu-system-x86_64: load of migration failed: Input/output error > > > > > > This is because the directory on mig-one is considered a > > > shared filesystem and thus labeling is skipped, resulting in > > > a SELinux denial. > > > > > > The solution is quite simple: remove the check and always > > > relabel. We know that it's okay to do so not just because it > > > makes the error seen above go away, but also because no such > > > check currently exists for disks and other types of persistent > > > storage such as NVRAM files, which always get relabeled. > > > > Did you consider the case when the migration fails and the VM will be > > restored to run on the source host again? In such case doin the > > relabelling might break the source host. > > Right. I seem to remember testing such scenarios. I had to put an exit() (or > something like it) into swtpm on the destination side to trigger the > fallback to the source side. The swtpm on the source side had closed file > access and wants to open them (lockfile) again and so the files needed to be > labeled correctly if the storage on the source side is > on the disk and exported via NFS from there (iirc). If the storage is > NFS-exported from a 3rd host it probably would not require the labels. I didn't really consider the failure scenario, so thank you for bringing that up. I think it would be still fine. If the source has NFS storage, then access will keep working regardless of what relabeling the destination has been up to in the meantime. And if the source has local storage, then the relabeling on the destination (via NFS) will not actually have touched the SELinux labels. The only concern I have is that, when going from local to NFS, labels might have been restored on the source side. But I assume that restoring only happens once the migration has been confirmed as successful? I'll check. Once again, as far as I can tell (please let me know if I'm wrong!) there is no special casing when it comes to disks and other types of persistent storage, so if this approach was problematic I would have expected many issues to have been reported by now.