qemu: Use qemuSecuritySetSavedStateLabel() to label restore path

Currently, when restoring from a domain the path that the domain
restores from is labelled under qemuSecuritySetAllLabel() (and after
v6.3.0-rc1~108 even outside transactions). While this grants QEMU
the access, it has a flaw, because once the domain is restored, up
and running then qemuSecurityDomainRestorePathLabel() is called,
which is not real counterpart. In case of DAC driver the
SetAllLabel() does nothing with the restore path but
RestorePathLabel() does - it chown()-s the file back and since there
is no original label remembered, the file is chown()-ed to
root:root. While the apparent solution is to have DAC driver set the
label (and thus remember the original one) in SetAllLabel(), we can
do better.

Turns out, we are opening the file ourselves (because it may live on
a root squashed NFS) and then are just passing the FD to QEMU. But
this means, that we don't have to chown() the file at all, we need
to set SELinux labels and/or add the path to AppArmor profile.

And since we want to restore labels right after QEMU is done loading
the migration stream (we don't want to wait until
qemuSecurityRestoreAllLabel()), the best way to approach this is to
have separate APIs for labelling and restoring label on the restore
file.

I will investigate whether AppArmor can use the SavedStateLabel()
API instead of passing the restore path to SetAllLabel().

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1851016

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ee5b9ffe603d6def14c46a4f6106a5700c6c056..8ea4197d000129b91cfff2b346d2b833bc931a13 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6952,8 +6952,6 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
         qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
                         asyncJob, VIR_QEMU_PROCESS_STOP_MIGRATED);
     }
-    if (qemuSecurityDomainRestorePathLabel(driver, vm, path, true) < 0)
-        VIR_WARN("failed to restore save state label on %s", path);
     return ret;
 }
 

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d36088ba9854f26c930a031f7f2b4f2feca989f2..70fc24b993189920d2b87881e582d50ec774c2b0 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7073,6 +7073,7 @@ qemuProcessStart(virConnectPtr conn,
     qemuProcessIncomingDefPtr incoming = NULL;
     unsigned int stopFlags;
     bool relabel = false;
+    bool relabelSavedState = false;
     int ret = -1;
     int rv;
 
@@ -7109,6 +7110,13 @@ qemuProcessStart(virConnectPtr conn,
     if (qemuProcessPrepareHost(driver, vm, flags) < 0)
         goto stop;
 
+    if (migratePath) {
+        if (qemuSecuritySetSavedStateLabel(driver->securityManager,
+                                           vm->def, migratePath) < 0)
+            goto cleanup;
+        relabelSavedState = true;
+    }
+
     if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming,
                                 snapshot, vmop, flags)) < 0) {
         if (rv == -2)
@@ -7145,6 +7153,10 @@ qemuProcessStart(virConnectPtr conn,
     ret = 0;
 
  cleanup:
+    if (relabelSavedState &&
+        qemuSecurityRestoreSavedStateLabel(driver->securityManager,
+                                           vm->def, migratePath) < 0)
+        VIR_WARN("failed to restore save state label on %s", migratePath);
     qemuProcessIncomingDefFree(incoming);
     return ret;
 

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index f93d189df976c2d88728b9e32232c0b1f4e2f463..88925be2acbef19288aa6d9552c559b20288d2ec 100644 (file)
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -39,13 +39,6 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
     qemuDomainObjPrivatePtr priv = vm->privateData;
     pid_t pid = -1;
 
-    /* Explicitly run this outside of transaction. We really want to relabel
-     * the file in the host and not in the domain's namespace. */
-    if (virSecurityManagerDomainSetPathLabelRO(driver->securityManager,
-                                               vm->def,
-                                               stdin_path) < 0)
-        goto cleanup;
-
     if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
         pid = vm->pid;