security_dac: Allow caller to suppress owner remembering

One caller in particular (virSecurityDACSetImageLabelInternal)
will want to have the feature turned on only in some cases.
Introduce @remember member to _virSecurityDACChownItem to track
whether caller wants to do owner remembering or not.
The actual remembering is then enabled if both caller wanted it
and the feature is turned on in the config file.

Technically, we could skip over paths that don't have remember
enabled when creating a list of paths to lock. We won't touch
their XATTRs after all. Well, I rather play it safe and keep them
on the locking list for now.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index c19421fa8f9f1eddd8b66af3394d0155fad0db70..a39dae52269e6bb15ee93c86d6e999fdc865f6f4 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -79,6 +79,7 @@ struct _virSecurityDACChownItem {
     const virStorageSource *src;
     uid_t uid;
     gid_t gid;
+    bool remember; /* Whether owner remembering should be done for @path/@src */
     bool restore; /* Whether current operation is 'set' or 'restore' */
 };
 
@@ -100,6 +101,7 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr list,
                               const virStorageSource *src,
                               uid_t uid,
                               gid_t gid,
+                              bool remember,
                               bool restore)
 {
     int ret = -1;
@@ -116,6 +118,7 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr list,
     item->src = src;
     item->uid = uid;
     item->gid = gid;
+    item->remember = remember;
     item->restore = restore;
 
     if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0)
@@ -155,9 +158,12 @@ virSecurityDACChownListFree(void *opaque)
  * @src: disk source to chown
  * @uid: user ID
  * @gid: group ID
+ * @remember: if the original owner should be recorded/recalled
  * @restore: if current operation is set or restore
  *
  * Appends an entry onto transaction list.
+ * The @remember should be true if caller wishes to record/recall
+ * the original owner of @path/@src.
  * The @restore should be true if the operation is restoring
  * seclabel and false otherwise.
  *
@@ -170,13 +176,15 @@ virSecurityDACTransactionAppend(const char *path,
                                 const virStorageSource *src,
                                 uid_t uid,
                                 gid_t gid,
+                                bool remember,
                                 bool restore)
 {
     virSecurityDACChownListPtr list = virThreadLocalGet(&chownList);
     if (!list)
         return 0;
 
-    if (virSecurityDACChownListAppend(list, path, src, uid, gid, restore) < 0)
+    if (virSecurityDACChownListAppend(list, path, src,
+                                      uid, gid, remember, restore) < 0)
         return -1;
 
     return 1;
@@ -235,6 +243,7 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
 
     for (i = 0; i < list->nItems; i++) {
         virSecurityDACChownItemPtr item = list->items[i];
+        const bool remember = item->remember && list->lock;
 
         if (!item->restore) {
             rv = virSecurityDACSetOwnership(list->manager,
@@ -242,12 +251,12 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
                                             item->path,
                                             item->uid,
                                             item->gid,
-                                            list->lock);
+                                            remember);
         } else {
             rv = virSecurityDACRestoreFileLabelInternal(list->manager,
                                                         item->src,
                                                         item->path,
-                                                        list->lock);
+                                                        remember);
         }
 
         if (rv < 0)
@@ -256,12 +265,13 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED,
 
     for (; rv < 0 && i > 0; i--) {
         virSecurityDACChownItemPtr item = list->items[i - 1];
+        const bool remember = item->remember && list->lock;
 
         if (!item->restore) {
             virSecurityDACRestoreFileLabelInternal(list->manager,
                                                    item->src,
                                                    item->path,
-                                                   list->lock);
+                                                   remember);
         } else {
             VIR_WARN("Ignoring failed restore attempt on %s",
                      NULLSTR(item->src ? item->src->path : item->path));
@@ -752,7 +762,8 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
     /* Be aware that this function might run in a separate process.
      * Therefore, any driver state changes would be thrown away. */
 
-    if ((rc = virSecurityDACTransactionAppend(path, src, uid, gid, false)) < 0)
+    if ((rc = virSecurityDACTransactionAppend(path, src,
+                                              uid, gid, remember, false)) < 0)
         return -1;
     else if (rc > 0)
         return 0;
@@ -826,7 +837,7 @@ virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
     /* Be aware that this function might run in a separate process.
      * Therefore, any driver state changes would be thrown away. */
 
-    if ((rv = virSecurityDACTransactionAppend(path, src, uid, gid, true)) < 0)
+    if ((rv = virSecurityDACTransactionAppend(path, src, uid, gid, recall, true)) < 0)
         return -1;
     else if (rv > 0)
         return 0;
@@ -853,7 +864,7 @@ static int
 virSecurityDACRestoreFileLabel(virSecurityManagerPtr mgr,
                                const char *path)
 {
-    return virSecurityDACRestoreFileLabelInternal(mgr, NULL, path, false);
+    return virSecurityDACRestoreFileLabelInternal(mgr, NULL, path, true);
 }
 
 
@@ -900,7 +911,7 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
             return -1;
     }
 
-    return virSecurityDACSetOwnership(mgr, src, NULL, user, group, false);
+    return virSecurityDACSetOwnership(mgr, src, NULL, user, group, true);
 }
 
 
@@ -967,7 +978,7 @@ virSecurityDACRestoreImageLabelInt(virSecurityManagerPtr mgr,
         }
     }
 
-    return virSecurityDACRestoreFileLabelInternal(mgr, src, NULL, false);
+    return virSecurityDACRestoreFileLabelInternal(mgr, src, NULL, true);
 }
 
 
@@ -995,7 +1006,7 @@ virSecurityDACSetHostdevLabelHelper(const char *file,
     if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL) < 0)
         return -1;
 
-    return virSecurityDACSetOwnership(mgr, NULL, file, user, group, false);
+    return virSecurityDACSetOwnership(mgr, NULL, file, user, group, true);
 }
 
 
@@ -1371,7 +1382,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
     case VIR_DOMAIN_CHR_TYPE_FILE:
         ret = virSecurityDACSetOwnership(mgr, NULL,
                                          dev_source->data.file.path,
-                                         user, group, false);
+                                         user, group, true);
         break;
 
     case VIR_DOMAIN_CHR_TYPE_PIPE:
@@ -1379,12 +1390,12 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
             virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0)
             goto done;
         if (virFileExists(in) && virFileExists(out)) {
-            if (virSecurityDACSetOwnership(mgr, NULL, in, user, group, false) < 0 ||
-                virSecurityDACSetOwnership(mgr, NULL, out, user, group, false) < 0)
+            if (virSecurityDACSetOwnership(mgr, NULL, in, user, group, true) < 0 ||
+                virSecurityDACSetOwnership(mgr, NULL, out, user, group, true) < 0)
                 goto done;
         } else if (virSecurityDACSetOwnership(mgr, NULL,
                                               dev_source->data.file.path,
-                                              user, group, false) < 0) {
+                                              user, group, true) < 0) {
             goto done;
         }
         ret = 0;
@@ -1399,7 +1410,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
              * and passed via FD */
             if (virSecurityDACSetOwnership(mgr, NULL,
                                            dev_source->data.nix.path,
-                                           user, group, false) < 0)
+                                           user, group, true) < 0)
                 goto done;
         }
         ret = 0;
@@ -1589,7 +1600,7 @@ virSecurityDACSetGraphicsLabel(virSecurityManagerPtr mgr,
     if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0)
         return -1;
 
-    if (virSecurityDACSetOwnership(mgr, NULL, rendernode, user, group, false) < 0)
+    if (virSecurityDACSetOwnership(mgr, NULL, rendernode, user, group, true) < 0)
         return -1;
 
     return 0;
@@ -1632,7 +1643,7 @@ virSecurityDACSetInputLabel(virSecurityManagerPtr mgr,
 
         ret = virSecurityDACSetOwnership(mgr, NULL,
                                          input->source.evdev,
-                                         user, group, false);
+                                         user, group, true);
         break;
 
     case VIR_DOMAIN_INPUT_TYPE_MOUSE:
@@ -1837,7 +1848,7 @@ virSecurityDACSetMemoryLabel(virSecurityManagerPtr mgr,
 
         ret = virSecurityDACSetOwnership(mgr, NULL,
                                          mem->nvdimmPath,
-                                         user, group, false);
+                                         user, group, true);
         break;
 
     case VIR_DOMAIN_MEMORY_MODEL_DIMM:
@@ -1874,7 +1885,7 @@ virSecurityDACSetSEVLabel(virSecurityManagerPtr mgr,
         return -1;
 
     if (virSecurityDACSetOwnership(mgr, NULL, DEV_SEV,
-                                   user, group, false) < 0)
+                                   user, group, true) < 0)
         return -1;
 
     return 0;
@@ -1961,31 +1972,31 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
     if (def->os.loader && def->os.loader->nvram &&
         virSecurityDACSetOwnership(mgr, NULL,
                                    def->os.loader->nvram,
-                                   user, group, false) < 0)
+                                   user, group, true) < 0)
         return -1;
 
     if (def->os.kernel &&
         virSecurityDACSetOwnership(mgr, NULL,
                                    def->os.kernel,
-                                   user, group, false) < 0)
+                                   user, group, true) < 0)
         return -1;
 
     if (def->os.initrd &&
         virSecurityDACSetOwnership(mgr, NULL,
                                    def->os.initrd,
-                                   user, group, false) < 0)
+                                   user, group, true) < 0)
         return -1;
 
     if (def->os.dtb &&
         virSecurityDACSetOwnership(mgr, NULL,
                                    def->os.dtb,
-                                   user, group, false) < 0)
+                                   user, group, true) < 0)
         return -1;
 
     if (def->os.slic_table &&
         virSecurityDACSetOwnership(mgr, NULL,
                                    def->os.slic_table,
-                                   user, group, false) < 0)
+                                   user, group, true) < 0)
         return -1;
 
     return 0;
@@ -2007,7 +2018,7 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr,
     if (virSecurityDACGetImageIds(secdef, priv, &user, &group) < 0)
         return -1;
 
-    return virSecurityDACSetOwnership(mgr, NULL, savefile, user, group, false);
+    return virSecurityDACSetOwnership(mgr, NULL, savefile, user, group, true);
 }
 
 
@@ -2327,7 +2338,7 @@ virSecurityDACDomainSetPathLabel(virSecurityManagerPtr mgr,
     if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0)
         return -1;
 
-    return virSecurityDACSetOwnership(mgr, NULL, path, user, group, false);
+    return virSecurityDACSetOwnership(mgr, NULL, path, user, group, true);
 }
 
 virSecurityDriver virSecurityDriverDAC = {