security: Introduce VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP flag

Our decision whether to remember seclabel for a disk image
depends on a few factors. If the image is readonly or shared or
not the chain top the remembering is suppressed for the image.
However, the virSecurityManagerSetImageLabel() is too low level
to determine whether passed @src is chain top or not. Even though
the function has the @parent argument it does not necessarily
reflect the chain top - it only points to the top level image in
the chain we want to relabel and not to the topmost image of the
whole chain. And this can't be derived from the passed domain
definition reliably neither - in some cases (like snapshots or
block copy) the @src is added to the definition only after the
operation succeeded. Therefore, introduce a flag which callers
can use to help us with the decision.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index f412054d0eed8d473834c00c9e1273188bc3ad78..9046b510049af76359ebeb82d15fe77fd165f556 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -889,14 +889,14 @@ static int
 virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
                                     virDomainDefPtr def,
                                     virStorageSourcePtr src,
-                                    virStorageSourcePtr parent)
+                                    virStorageSourcePtr parent,
+                                    bool isChainTop)
 {
     virSecurityLabelDefPtr secdef;
     virSecurityDeviceLabelDefPtr disk_seclabel;
     virSecurityDeviceLabelDefPtr parent_seclabel = NULL;
     virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
     bool remember;
-    bool is_toplevel = parent == src || parent->externalDataStore == src;
     uid_t user;
     gid_t group;
 
@@ -954,7 +954,7 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
      * but the top layer, or read only image, or disk explicitly
      * marked as shared.
      */
-    remember = is_toplevel && !src->readonly && !src->shared;
+    remember = isChainTop && !src->readonly && !src->shared;
 
     return virSecurityDACSetOwnership(mgr, src, NULL, user, group, remember);
 }
@@ -970,7 +970,9 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr,
     virStorageSourcePtr n;
 
     for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
-        if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent) < 0)
+        const bool isChainTop = flags & VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP;
+
+        if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent, isChainTop) < 0)
             return -1;
 
         if (n->externalDataStore &&
@@ -983,6 +985,8 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr,
 
         if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
             break;
+
+        flags &= ~VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP;
     }
 
     return 0;
@@ -2114,7 +2118,8 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
         if (virDomainDiskGetType(def->disks[i]) == VIR_STORAGE_TYPE_DIR)
             continue;
         if (virSecurityDACSetImageLabel(mgr, def, def->disks[i]->src,
-                                        VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0)
+                                        VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN |
+                                        VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0)
             return -1;
     }
 

diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index b92ea5dc8787ea5592d362a7126ec66d7e736ed3..7699bcbc6fb0765978210dd4303539741ddd75ef 100644 (file)
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -151,6 +151,10 @@ virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr);
 
 typedef enum {
     VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN = 1 << 0,
+    /* The VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP should be set if the
+     * image passed to virSecurityManagerSetImageLabel() is the top parent of
+     * the whole backing chain. */
+    VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP = 1 << 1,
 } virSecurityDomainImageLabelFlags;
 
 int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 2241a35e6e8342da17537d33df5fd08cce61defc..e9aa8af87748f76465e3f7d65f4abd2350aabbb2 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1824,7 +1824,8 @@ static int
 virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
                                         virDomainDefPtr def,
                                         virStorageSourcePtr src,
-                                        virStorageSourcePtr parent)
+                                        virStorageSourcePtr parent,
+                                        bool isChainTop)
 {
     virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
     virSecurityLabelDefPtr secdef;
@@ -1832,7 +1833,6 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
     virSecurityDeviceLabelDefPtr parent_seclabel = NULL;
     char *use_label = NULL;
     bool remember;
-    bool is_toplevel = parent == src || parent->externalDataStore == src;
     g_autofree char *vfioGroupDev = NULL;
     const char *path = src->path;
     int ret;
@@ -1856,7 +1856,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
      * but the top layer, or read only image, or disk explicitly
      * marked as shared.
      */
-    remember = is_toplevel && !src->readonly && !src->shared;
+    remember = isChainTop && !src->readonly && !src->shared;
 
     disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
                                                         SECURITY_SELINUX_NAME);
@@ -1873,7 +1873,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
             return 0;
 
         use_label = parent_seclabel->label;
-    } else if (is_toplevel) {
+    } else if (parent == src || parent->externalDataStore == src) {
         if (src->shared) {
             use_label = data->file_context;
         } else if (src->readonly) {
@@ -1930,7 +1930,9 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr,
     virStorageSourcePtr n;
 
     for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
-        if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent) < 0)
+        const bool isChainTop = flags & VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP;
+
+        if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent, isChainTop) < 0)
             return -1;
 
         if (n->externalDataStore &&
@@ -1943,6 +1945,8 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr,
 
         if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
             break;
+
+        flags &= ~VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP;
     }
 
     return 0;
@@ -3146,7 +3150,8 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
             continue;
         }
         if (virSecuritySELinuxSetImageLabel(mgr, def, def->disks[i]->src,
-                                            VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0)
+                                            VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN |
+                                            VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0)
             return -1;
     }
     /* XXX fixme process  def->fss if relabel == true */