[libvirt] [PATCH 04/11] security: Remove security driver internals for disk labelling

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(a)redhat.com&gt; --- 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