Re: [PATCH v4 6/7] qemu: tpm: Avoid security labels on incoming migration with shared storage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/29/22 09:04, Michal Prívozník wrote:
On 10/24/22 12:28, Stefan Berger wrote:
When using shared storage there is no need to apply security labels on the
storage since the files have to have been labeled already on the source
side and we must assume that the source and destination side have been
setup to use the same uid and gid for running swtpm as well as share the
same security labels. Whether the security labels can be used at all
depends on the shared storage and whether and how it supports them.

Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
---
  src/qemu/qemu_tpm.c | 17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index cffa77cfa3..5a0d298052 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -932,10 +932,19 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
      virCommandSetPidFile(cmd, pidfile);
      virCommandSetErrorFD(cmd, &errfd);
- if (qsudo tpm2_nvdefine -C o -a "nt=extend|ownerread|ownerwrite|policywrite|writedefine|orderly" 2(driver, vm, cmd,
-                                     cfg->swtpm_user, cfg->swtpm_group,
-                                     NULL, &cmdret) < 0)
-        return -1;
+    if (incomingMigration &&
+        virFileIsSharedFS(tpm->data.emulator.storagepath) == 1) {
+        /* security labels must have been set up on source already */
+        if (qemuSecurityCommandRun(driver, vm, cmd,
+                                   cfg->swtpm_user, cfg->swtpm_group,
+                                   NULL, &cmdret) < 0) {
+            goto error;
+        }
+    } else if (qemuSecurityStartTPMEmulator(driver, vm, cmd,
+                                            cfg->swtpm_user, cfg->swtpm_group,
+                                            NULL, &cmdret) < 0) {
+        goto error;
+    }
if (cmdret < 0) {
          /* virCommandRun() hidden in qemuSecurityStartTPMEmulator()

Turns out, this is patch is problematic with SELinux [1]. Couple of
problems here:

1) When a domain is being started up, libvirt creates an unique SELinux
label. This is not migrated onto the destination - here another unique
label is created on incoming migration. This means that the state might
be not available to the destination swtpm binary.

2) the log file should be relabelled regardless - it's not on the shared
volume. And since its label is set in qemuSecurityStartTPMEmulator(), it
won't be set on the destination.


While I could deal with 2), I am not sure what to do with 1). Because
when I tried to set the label (basically by reverting this patch), the
destination was unhappy as it could not lock the .lock file.

Stefan, do you have any bright idea how to fix this?

Sorry, no, unfortunately I don't.
SELinux and NFS had some issues and due to other reasons as well I had to run my machine in permissive mode but I don't remember the exact details. What I at least was not able to do was to run qemuSecurityStartTPMEmulator on the destination side - this didn't work just to begin with. Also, NFS is one example of a shared filesystem and I am not sure what issues other shared filesystems may or may not have when SELinux is used -- could a shared filesystem entertain local SELinux labels and not have the issue that NFS has with SELinux labels? In the worst case we may have to put a blocker into the code if SELinux(+NFS) is being used. Migration across shared storage shouldn't be a problem with AppArmor at least.

    Stefan


1: https://bugzilla.redhat.com/show_bug.cgi?id=2130192

Michal






[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux