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(a)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