[libvirt] [PATCH 04/11] security: Remove security driver internals for disk labelling
Peter Krempa
pkrempa at redhat.com
Wed Jan 23 16:10:59 UTC 2019
Security labelling of disks consists of labelling of the disk image
itself and it's backing chain. Modify
virSecurityManager[Set|Restore]ImageLabel to take a boolean flag that
will label the full chain rather than the top image itself.
This allows to delete/unify some parts of the code and will also
simplify callers in some cases.
Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
src/qemu/qemu_security.c | 6 ++--
src/security/security_apparmor.c | 24 +++------------
src/security/security_dac.c | 40 +++++++------------------
src/security/security_driver.h | 15 +++-------
src/security/security_manager.c | 20 ++++++++-----
src/security/security_manager.h | 6 ++--
src/security/security_nop.c | 25 +++-------------
src/security/security_selinux.c | 42 ++++++++-------------------
src/security/security_stack.c | 50 +++++---------------------------
9 files changed, 60 insertions(+), 168 deletions(-)
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 5faa34a4fd..4940195216 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -170,8 +170,7 @@ qemuSecuritySetImageLabel(virQEMUDriverPtr driver,
goto cleanup;
if (virSecurityManagerSetImageLabel(driver->securityManager,
- vm->def,
- src) < 0)
+ vm->def, src, false) < 0)
goto cleanup;
if (virSecurityManagerTransactionCommit(driver->securityManager,
@@ -201,8 +200,7 @@ qemuSecurityRestoreImageLabel(virQEMUDriverPtr driver,
goto cleanup;
if (virSecurityManagerRestoreImageLabel(driver->securityManager,
- vm->def,
- src) < 0)
+ vm->def, src, false) < 0)
goto cleanup;
if (virSecurityManagerTransactionCommit(driver->securityManager,
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 43310361ba..a61105cbb7 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -691,7 +691,8 @@ AppArmorClearSecuritySocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
static int
AppArmorRestoreSecurityImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingStore ATTRIBUTE_UNUSED)
{
if (!virStorageSourceIsLocalStorage(src))
return 0;
@@ -699,13 +700,6 @@ AppArmorRestoreSecurityImageLabel(virSecurityManagerPtr mgr,
return reload_profile(mgr, def, NULL, false);
}
-static int
-AppArmorRestoreSecurityDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk)
-{
- return AppArmorRestoreSecurityImageLabel(mgr, def, disk->src);
-}
/* Called when hotplugging */
static int
@@ -799,7 +793,8 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr mgr,
static int
AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingStore ATTRIBUTE_UNUSED)
{
int rc = -1;
char *profile_name = NULL;
@@ -844,14 +839,6 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
return rc;
}
-static int
-AppArmorSetSecurityDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk)
-{
- return AppArmorSetSecurityImageLabel(mgr, def, disk->src);
-}
-
static int
AppArmorSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def)
@@ -1188,9 +1175,6 @@ virSecurityDriver virAppArmorSecurityDriver = {
.domainSecurityVerify = AppArmorSecurityVerify,
- .domainSetSecurityDiskLabel = AppArmorSetSecurityDiskLabel,
- .domainRestoreSecurityDiskLabel = AppArmorRestoreSecurityDiskLabel,
-
.domainSetSecurityImageLabel = AppArmorSetSecurityImageLabel,
.domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel,
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 533d990de1..08ff0d89c0 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -897,22 +897,17 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
static int
virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingChain)
{
- return virSecurityDACSetImageLabelInternal(mgr, def, src, NULL);
-}
-
-static int
-virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk)
+ virStorageSourcePtr n;
-{
- virStorageSourcePtr next;
-
- for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
- if (virSecurityDACSetImageLabelInternal(mgr, def, next, disk->src) < 0)
+ for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
+ if (virSecurityDACSetImageLabelInternal(mgr, def, n, src) < 0)
return -1;
+
+ if (!backingChain)
+ break;
}
return 0;
@@ -969,21 +964,13 @@ virSecurityDACRestoreImageLabelInt(virSecurityManagerPtr mgr,
static int
virSecurityDACRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingChain ATTRIBUTE_UNUSED)
{
return virSecurityDACRestoreImageLabelInt(mgr, def, src, false);
}
-static int
-virSecurityDACRestoreDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk)
-{
- return virSecurityDACRestoreImageLabelInt(mgr, def, disk->src, false);
-}
-
-
static int
virSecurityDACSetHostdevLabelHelper(const char *file,
void *opaque)
@@ -1853,9 +1840,7 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
/* XXX fixme - we need to recursively label the entire tree :-( */
if (virDomainDiskGetType(def->disks[i]) == VIR_STORAGE_TYPE_DIR)
continue;
- if (virSecurityDACSetDiskLabel(mgr,
- def,
- def->disks[i]) < 0)
+ if (virSecurityDACSetImageLabel(mgr, def, def->disks[i]->src, true) < 0)
return -1;
}
@@ -2295,9 +2280,6 @@ virSecurityDriver virSecurityDriverDAC = {
.domainSecurityVerify = virSecurityDACVerify,
- .domainSetSecurityDiskLabel = virSecurityDACSetDiskLabel,
- .domainRestoreSecurityDiskLabel = virSecurityDACRestoreDiskLabel,
-
.domainSetSecurityImageLabel = virSecurityDACSetImageLabel,
.domainRestoreSecurityImageLabel = virSecurityDACRestoreImageLabel,
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 70c8cde50b..df270cdc02 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -54,18 +54,12 @@ typedef int (*virSecurityDriverTransactionCommit) (virSecurityManagerPtr mgr,
bool lock);
typedef void (*virSecurityDriverTransactionAbort) (virSecurityManagerPtr mgr);
-typedef int (*virSecurityDomainRestoreDiskLabel) (virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk);
typedef int (*virSecurityDomainSetDaemonSocketLabel)(virSecurityManagerPtr mgr,
virDomainDefPtr vm);
typedef int (*virSecurityDomainSetSocketLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def);
typedef int (*virSecurityDomainClearSocketLabel)(virSecurityManagerPtr mgr,
virDomainDefPtr def);
-typedef int (*virSecurityDomainSetDiskLabel) (virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk);
typedef int (*virSecurityDomainRestoreHostdevLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainHostdevDefPtr dev,
@@ -119,10 +113,12 @@ typedef int (*virSecurityDomainSetHugepages) (virSecurityManagerPtr mgr,
const char *path);
typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src);
+ virStorageSourcePtr src,
+ bool backingChain);
typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src);
+ virStorageSourcePtr src,
+ bool backingChain);
typedef int (*virSecurityDomainSetMemoryLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainMemoryDefPtr mem);
@@ -171,9 +167,6 @@ struct _virSecurityDriver {
virSecurityDomainSecurityVerify domainSecurityVerify;
- virSecurityDomainSetDiskLabel domainSetSecurityDiskLabel;
- virSecurityDomainRestoreDiskLabel domainRestoreSecurityDiskLabel;
-
virSecurityDomainSetImageLabel domainSetSecurityImageLabel;
virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index f6b4c2d5d5..5493f0f66b 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -418,10 +418,10 @@ virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virDomainDiskDefPtr disk)
{
- if (mgr->drv->domainRestoreSecurityDiskLabel) {
+ if (mgr->drv->domainRestoreSecurityImageLabel) {
int ret;
virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityDiskLabel(mgr, vm, disk);
+ ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk->src, true);
virObjectUnlock(mgr);
return ret;
}
@@ -436,6 +436,7 @@ virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr,
* @mgr: security manager object
* @vm: domain definition object
* @src: disk source definition to operate on
+ * @backingChain: Restore labels also on backingChains of @src
*
* Removes security label from a single storage image.
*
@@ -444,12 +445,13 @@ virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr,
int
virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingChain)
{
if (mgr->drv->domainRestoreSecurityImageLabel) {
int ret;
virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src);
+ ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src, backingChain);
virObjectUnlock(mgr);
return ret;
}
@@ -526,10 +528,10 @@ virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virDomainDiskDefPtr disk)
{
- if (mgr->drv->domainSetSecurityDiskLabel) {
+ if (mgr->drv->domainSetSecurityImageLabel) {
int ret;
virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityDiskLabel(mgr, vm, disk);
+ ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk->src, true);
virObjectUnlock(mgr);
return ret;
}
@@ -544,6 +546,7 @@ virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr,
* @mgr: security manager object
* @vm: domain definition object
* @src: disk source definition to operate on
+ * @backingChain: set labels also on backing chain of @src
*
* Labels a single storage image with the configured security label.
*
@@ -552,12 +555,13 @@ virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr,
int
virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingChain)
{
if (mgr->drv->domainSetSecurityImageLabel) {
int ret;
virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, src);
+ ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, src, backingChain);
virObjectUnlock(mgr);
return ret;
}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index f7beb29f86..0207113b14 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -156,10 +156,12 @@ virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr);
int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
- virStorageSourcePtr src);
+ virStorageSourcePtr src,
+ bool backingChain);
int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
- virStorageSourcePtr src);
+ virStorageSourcePtr src,
+ bool backingChain);
int virSecurityManagerSetMemoryLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
diff --git a/src/security/security_nop.c b/src/security/security_nop.c
index ff739f8199..21e668c169 100644
--- a/src/security/security_nop.c
+++ b/src/security/security_nop.c
@@ -55,14 +55,6 @@ virSecurityDriverGetDOINop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
return "0";
}
-static int
-virSecurityDomainRestoreDiskLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
- virDomainDefPtr vm ATTRIBUTE_UNUSED,
- virDomainDiskDefPtr disk ATTRIBUTE_UNUSED)
-{
- return 0;
-}
-
static int
virSecurityDomainSetDaemonSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr vm ATTRIBUTE_UNUSED)
@@ -84,14 +76,6 @@ virSecurityDomainClearSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
return 0;
}
-static int
-virSecurityDomainSetDiskLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
- virDomainDefPtr vm ATTRIBUTE_UNUSED,
- virDomainDiskDefPtr disk ATTRIBUTE_UNUSED)
-{
- return 0;
-}
-
static int
virSecurityDomainRestoreHostdevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr vm ATTRIBUTE_UNUSED,
@@ -225,7 +209,8 @@ virSecurityGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
static int
virSecurityDomainRestoreImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def ATTRIBUTE_UNUSED,
- virStorageSourcePtr src ATTRIBUTE_UNUSED)
+ virStorageSourcePtr src ATTRIBUTE_UNUSED,
+ bool backingChain ATTRIBUTE_UNUSED)
{
return 0;
}
@@ -233,7 +218,8 @@ virSecurityDomainRestoreImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED
static int
virSecurityDomainSetImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def ATTRIBUTE_UNUSED,
- virStorageSourcePtr src ATTRIBUTE_UNUSED)
+ virStorageSourcePtr src ATTRIBUTE_UNUSED,
+ bool backingChain ATTRIBUTE_UNUSED)
{
return 0;
}
@@ -292,9 +278,6 @@ virSecurityDriver virSecurityDriverNop = {
.domainSecurityVerify = virSecurityDomainVerifyNop,
- .domainSetSecurityDiskLabel = virSecurityDomainSetDiskLabelNop,
- .domainRestoreSecurityDiskLabel = virSecurityDomainRestoreDiskLabelNop,
-
.domainSetSecurityImageLabel = virSecurityDomainSetImageLabelNop,
.domainRestoreSecurityImageLabel = virSecurityDomainRestoreImageLabelNop,
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 5cdb839c13..106494ff3a 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1771,20 +1771,11 @@ virSecuritySELinuxRestoreImageLabelInt(virSecurityManagerPtr mgr,
}
-static int
-virSecuritySELinuxRestoreDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk)
-{
- return virSecuritySELinuxRestoreImageLabelInt(mgr, def, disk->src,
- false);
-}
-
-
static int
virSecuritySELinuxRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingChain ATTRIBUTE_UNUSED)
{
return virSecuritySELinuxRestoreImageLabelInt(mgr, def, src, false);
}
@@ -1869,28 +1860,23 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
static int
virSecuritySELinuxSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virStorageSourcePtr src)
-{
- return virSecuritySELinuxSetImageLabelInternal(mgr, def, src, NULL);
-}
-
-
-static int
-virSecuritySELinuxSetDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virDomainDiskDefPtr disk)
-
+ virStorageSourcePtr src,
+ bool backingChain)
{
- virStorageSourcePtr next;
+ virStorageSourcePtr n;
- for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
- if (virSecuritySELinuxSetImageLabelInternal(mgr, def, next, disk->src) < 0)
+ for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
+ if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, src) < 0)
return -1;
+
+ if (!backingChain)
+ break;
}
return 0;
}
+
static int
virSecuritySELinuxSetHostdevLabelHelper(const char *file, void *opaque)
{
@@ -3026,8 +3012,7 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
def->disks[i]->dst);
continue;
}
- if (virSecuritySELinuxSetDiskLabel(mgr,
- def, def->disks[i]) < 0)
+ if (virSecuritySELinuxSetImageLabel(mgr, def, def->disks[i]->src, true) < 0)
return -1;
}
/* XXX fixme process def->fss if relabel == true */
@@ -3441,9 +3426,6 @@ virSecurityDriver virSecurityDriverSELinux = {
.domainSecurityVerify = virSecuritySELinuxVerify,
- .domainSetSecurityDiskLabel = virSecuritySELinuxSetDiskLabel,
- .domainRestoreSecurityDiskLabel = virSecuritySELinuxRestoreDiskLabel,
-
.domainSetSecurityImageLabel = virSecuritySELinuxSetImageLabel,
.domainRestoreSecurityImageLabel = virSecuritySELinuxRestoreImageLabel,
diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 3e60d5d2b7..e1c98a75e3 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -267,42 +267,6 @@ virSecurityStackReserveLabel(virSecurityManagerPtr mgr,
}
-static int
-virSecurityStackSetDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr vm,
- virDomainDiskDefPtr disk)
-{
- virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
- virSecurityStackItemPtr item = priv->itemsHead;
- int rc = 0;
-
- for (; item; item = item->next) {
- if (virSecurityManagerSetDiskLabel(item->securityManager, vm, disk) < 0)
- rc = -1;
- }
-
- return rc;
-}
-
-
-static int
-virSecurityStackRestoreDiskLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr vm,
- virDomainDiskDefPtr disk)
-{
- virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
- virSecurityStackItemPtr item = priv->itemsHead;
- int rc = 0;
-
- for (; item; item = item->next) {
- if (virSecurityManagerRestoreDiskLabel(item->securityManager, vm, disk) < 0)
- rc = -1;
- }
-
- return rc;
-}
-
-
static int
virSecurityStackSetHostdevLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
@@ -600,14 +564,16 @@ virSecurityStackGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
static int
virSecurityStackSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingChain)
{
virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityStackItemPtr item = priv->itemsHead;
int rc = 0;
for (; item; item = item->next) {
- if (virSecurityManagerSetImageLabel(item->securityManager, vm, src) < 0)
+ if (virSecurityManagerSetImageLabel(item->securityManager, vm, src,
+ backingChain) < 0)
rc = -1;
}
@@ -617,7 +583,8 @@ virSecurityStackSetImageLabel(virSecurityManagerPtr mgr,
static int
virSecurityStackRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ bool backingChain)
{
virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityStackItemPtr item = priv->itemsHead;
@@ -625,7 +592,7 @@ virSecurityStackRestoreImageLabel(virSecurityManagerPtr mgr,
for (; item; item = item->next) {
if (virSecurityManagerRestoreImageLabel(item->securityManager,
- vm, src) < 0)
+ vm, src, backingChain) < 0)
rc = -1;
}
@@ -816,9 +783,6 @@ virSecurityDriver virSecurityDriverStack = {
.domainSecurityVerify = virSecurityStackVerify,
- .domainSetSecurityDiskLabel = virSecurityStackSetDiskLabel,
- .domainRestoreSecurityDiskLabel = virSecurityStackRestoreDiskLabel,
-
.domainSetSecurityImageLabel = virSecurityStackSetImageLabel,
.domainRestoreSecurityImageLabel = virSecurityStackRestoreImageLabel,
--
2.20.1
More information about the libvir-list
mailing list