[PATCHv1.5 5/8] security: DAC: Plumb usage of chown callback

[Peter Krempa <pkrempa@xxxxxxxxxx>]

Use the callback to set disk and storage image labels by modifying the
existing functions and adding wrappers to avoid refactoring a lot of the
code.
---
 src/security/security_dac.c | 89 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 69 insertions(+), 20 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 1fb0c86..989329f 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -230,23 +230,54 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr)
 }

 static int
-virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
+virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
+                                   virStorageSourcePtr src,
+                                   const char *path,
+                                   uid_t uid,
+                                   gid_t gid)
 {
+    int rc;
+    int chown_errno;
+
     VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
-             path, (long) uid, (long) gid);
+             NULLSTR(src ? src->path : path), (long) uid, (long) gid);
+
+    if (priv && src && priv->chownCallback) {
+        rc = priv->chownCallback(src, uid, gid);
+
+        /* on -2 returned an error was already reported */
+        if (rc == -2)
+            return -1;

-    if (chown(path, uid, gid) < 0) {
+        /* on -1 only errno was set */
+        chown_errno = errno;
+    } else {
         struct stat sb;
-        int chown_errno = errno;

-        if (stat(path, &sb) >= 0) {
+        if (!path) {
+            if (!src || !src->path)
+                return 0;
+
+            if (!virStorageSourceIsLocalStorage(src))
+                return 0;
+
+            path = src->path;
+        }
+
+        rc = chown(path, uid, gid);
+        chown_errno = errno;
+
+        if (rc < 0 &&
+            stat(path, &sb) >= 0) {
             if (sb.st_uid == uid &&
                 sb.st_gid == gid) {
                 /* It's alright, there's nothing to change anyway. */
                 return 0;
             }
         }
+    }

+    if (rc < 0) {
         if (chown_errno == EOPNOTSUPP || chown_errno == EINVAL) {
             VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not "
                      "supported by filesystem",
@@ -270,13 +301,31 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
     return 0;
 }

+
 static int
-virSecurityDACRestoreSecurityFileLabel(const char *path)
+virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
+{
+    return virSecurityDACSetOwnershipInternal(NULL, NULL, path, uid, gid);
+}
+
+
+static int
+virSecurityDACRestoreSecurityFileLabelInternal(virSecurityDACDataPtr priv,
+                                               virStorageSourcePtr src,
+                                               const char *path)
 {
-    VIR_INFO("Restoring DAC user and group on '%s'", path);
+    VIR_INFO("Restoring DAC user and group on '%s'",
+             NULLSTR(src ? src->path : path));

     /* XXX record previous ownership */
-    return virSecurityDACSetOwnership(path, 0, 0);
+    return virSecurityDACSetOwnershipInternal(priv, src, path, 0, 0);
+}
+
+
+static int
+virSecurityDACRestoreSecurityFileLabel(const char *path)
+{
+    return virSecurityDACRestoreSecurityFileLabelInternal(NULL, NULL, path);
 }


@@ -294,10 +343,6 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr,
     if (!priv->dynamicOwnership)
         return 0;

-    /* XXX: Add support for gluster DAC permissions */
-    if (!src->path || !virStorageSourceIsLocalStorage(src))
-        return 0;
-
     secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
     if (secdef && !secdef->relabel)
         return 0;
@@ -315,7 +360,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr,
             return -1;
     }

-    return virSecurityDACSetOwnership(src->path, user, group);
+    return virSecurityDACSetOwnershipInternal(priv, src, NULL, user, group);
 }


@@ -349,9 +394,6 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
     if (!priv->dynamicOwnership)
         return 0;

-    if (!src->path || !virStorageSourceIsLocalStorage(src))
-        return 0;
-
     /* Don't restore labels on readoly/shared disks, because other VMs may
      * still be accessing these. Alternatively we could iterate over all
      * running domains and try to figure out if it is in use, but this would
@@ -373,9 +415,16 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
      * ownership, because that kills access on the destination host which is
      * sub-optimal for the guest VM's I/O attempts :-) */
     if (migrated) {
-        int rc = virFileIsSharedFS(src->path);
-        if (rc < 0)
-            return -1;
+        int rc = 1;
+
+        if (virStorageSourceIsLocalStorage(src)) {
+            if (!src->path)
+                return 0;
+
+            if ((rc = virFileIsSharedFS(src->path)) < 0)
+                return -1;
+        }
+
         if (rc == 1) {
             VIR_DEBUG("Skipping image label restore on %s because FS is shared",
                       src->path);
@@ -383,7 +432,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
         }
     }

-    return virSecurityDACRestoreSecurityFileLabel(src->path);
+    return virSecurityDACRestoreSecurityFileLabelInternal(priv, src, NULL);
 }


-- 
2.0.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list