[PATCH 1/2] security: Introduce VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT flag

Michal Privoznik mprivozn at redhat.com
Thu Feb 27 12:07:35 UTC 2020


Our decision whether to remember seclabel for a disk image
depends on a few factors. If the image is readonly or shared or
not top parent of a backing chain the remembering is suppressed
for the image. However, the virSecurityManagerSetImageLabel() is
too low level to determine whether passed @src is top parent or
not. Even though it has domain definition available, 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 at redhat.com>
---
 src/security/security_dac.c     | 16 +++++++++++-----
 src/security/security_manager.h |  1 +
 src/security/security_selinux.c | 18 ++++++++++++------
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index f412054d0e..3f8b04b307 100644
--- 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 is_topparent)
 {
     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 = is_topparent && !src->readonly && !src->shared;
 
     return virSecurityDACSetOwnership(mgr, src, NULL, user, group, remember);
 }
@@ -967,10 +967,13 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr,
                                     virStorageSourcePtr parent,
                                     virSecurityDomainImageLabelFlags flags)
 {
+    bool is_topparent = flags & VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT;
     virStorageSourcePtr n;
 
+    flags &= ~VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT;
+
     for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
-        if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent) < 0)
+        if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent, is_topparent) < 0)
             return -1;
 
         if (n->externalDataStore &&
@@ -983,6 +986,8 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr,
 
         if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
             break;
+
+        is_topparent = false;
     }
 
     return 0;
@@ -2114,7 +2119,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_TOP_PARENT) < 0)
             return -1;
     }
 
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index b92ea5dc87..11904fda89 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -151,6 +151,7 @@ virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr);
 
 typedef enum {
     VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN = 1 << 0,
+    VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT = 1 << 1,
 } virSecurityDomainImageLabelFlags;
 
 int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 2241a35e6e..0aa0c2bb71 100644
--- 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 is_topparent)
 {
     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 = is_topparent && !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) {
@@ -1927,10 +1927,13 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr,
                                         virStorageSourcePtr parent,
                                         virSecurityDomainImageLabelFlags flags)
 {
+    bool is_topparent = flags & VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT;
     virStorageSourcePtr n;
 
+    flags &= ~VIR_SECURITY_DOMAIN_IMAGE_TOP_PARENT;
+
     for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
-        if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent) < 0)
+        if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent, is_topparent) < 0)
             return -1;
 
         if (n->externalDataStore &&
@@ -1943,6 +1946,8 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr,
 
         if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
             break;
+
+        is_topparent = false;
     }
 
     return 0;
@@ -3146,7 +3151,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_TOP_PARENT) < 0)
             return -1;
     }
     /* XXX fixme process  def->fss if relabel == true */
-- 
2.24.1




More information about the libvir-list mailing list