183 lines
8.2 KiB
Diff
183 lines
8.2 KiB
Diff
From 04b7241a14015f67b68d9779be71f9e8f91791c3 Mon Sep 17 00:00:00 2001
|
|
Message-Id: <04b7241a14015f67b68d9779be71f9e8f91791c3@dist-git>
|
|
From: Michal Privoznik <mprivozn@redhat.com>
|
|
Date: Mon, 9 Mar 2020 14:58:56 +0100
|
|
Subject: [PATCH] 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>
|
|
(cherry picked from commit 62f3d8adbc0381223499ff2bef45b23e7dca401d)
|
|
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1803551
|
|
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
|
Message-Id: <bd4b1cca6feabd0c6d421603abe38397090c5394.1583760062.git.mprivozn@redhat.com>
|
|
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
|
|
---
|
|
src/security/security_dac.c | 15 ++++++++++-----
|
|
src/security/security_manager.h | 4 ++++
|
|
src/security/security_selinux.c | 17 +++++++++++------
|
|
3 files changed, 25 insertions(+), 11 deletions(-)
|
|
|
|
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
|
|
index 0cfe3626d4..66906245eb 100644
|
|
--- a/src/security/security_dac.c
|
|
+++ b/src/security/security_dac.c
|
|
@@ -885,14 +885,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;
|
|
|
|
@@ -950,7 +950,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);
|
|
}
|
|
@@ -966,7 +966,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 &&
|
|
@@ -979,6 +981,8 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr,
|
|
|
|
if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
|
|
break;
|
|
+
|
|
+ flags &= ~VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP;
|
|
}
|
|
|
|
return 0;
|
|
@@ -2106,7 +2110,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 b92ea5dc87..7699bcbc6f 100644
|
|
--- 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 d7362327e6..985c7eda1a 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 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;
|
|
@@ -3142,7 +3146,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 */
|
|
--
|
|
2.25.1
|
|
|