External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-06 09:55:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

security: Remove security driver internals for disk labeling

Browse Source 
Security labeling of disks consists of labeling 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@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
Peter Krempa 2019-01-23 11:50:33 +01:00
parent e7d14bf965
commit 43479005ee
10 changed files with 72 additions and 170 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cfg.mk
View File

@ -309,6 +309,7 @@ sc_flags_usage:
{ echo '$(ME): new API should use "unsigned int flags"' 1>&2; \
exit 1; } || :
@prohibit=' flags ATTRIBUTE_UNUSED' \
exclude='virSecurityDomainImageLabelFlags' \
halt='flags should be checked with virCheckFlags' \
$(_sc_search_regexp)
@prohibit='^[^@]*([^d] (int|long long)|[^dg] long) flags[;,)]' \

6
src/qemu/qemu_security.c
View File

@ -170,8 +170,7 @@ qemuSecuritySetImageLabel(virQEMUDriverPtr driver,
goto cleanup;
if (virSecurityManagerSetImageLabel(driver->securityManager,
vm->def,
src) < 0)
vm->def, src, 0) < 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, 0) < 0)
goto cleanup;
if (virSecurityManagerTransactionCommit(driver->securityManager,

24
src/security/security_apparmor.c
View File

@ -691,7 +691,8 @@ AppArmorClearSecuritySocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
static int
AppArmorRestoreSecurityImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src)
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags 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,
virSecurityDomainImageLabelFlags flags 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,

41
src/security/security_dac.c
View File

@ -897,22 +897,17 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
static int
virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src)
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags)
{
return virSecurityDACSetImageLabelInternal(mgr, def, src, NULL);
}
virStorageSourcePtr n;
static int
virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainDiskDefPtr disk)
{
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 (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
break;
}
return 0;
@ -969,21 +964,13 @@ virSecurityDACRestoreImageLabelInt(virSecurityManagerPtr mgr,
static int
virSecurityDACRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src)
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags 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,8 @@ 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,
VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0)
return -1;
}
@ -2295,9 +2281,6 @@ virSecurityDriver virSecurityDriverDAC = {
.domainSecurityVerify = virSecurityDACVerify,
.domainSetSecurityDiskLabel = virSecurityDACSetDiskLabel,
.domainRestoreSecurityDiskLabel = virSecurityDACRestoreDiskLabel,
.domainSetSecurityImageLabel = virSecurityDACSetImageLabel,
.domainRestoreSecurityImageLabel = virSecurityDACRestoreImageLabel,

16
src/security/security_driver.h
View File

@ -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,
@ -117,12 +111,15 @@ typedef char *(*virSecurityDomainGetMountOptions) (virSecurityManagerPtr mgr,
typedef int (*virSecurityDomainSetHugepages) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *path);
typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src);
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags);
typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src);
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags);
typedef int (*virSecurityDomainSetMemoryLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainMemoryDefPtr mem);
@ -171,9 +168,6 @@ struct _virSecurityDriver {
virSecurityDomainSecurityVerify domainSecurityVerify;
virSecurityDomainSetDiskLabel domainSetSecurityDiskLabel;
virSecurityDomainRestoreDiskLabel domainRestoreSecurityDiskLabel;
virSecurityDomainSetImageLabel domainSetSecurityImageLabel;
virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel;

26
src/security/security_manager.c
View File

@ -418,10 +418,11 @@ 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,
VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN);
virObjectUnlock(mgr);
return ret;
}
@ -436,20 +437,22 @@ virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr,
* @mgr: security manager object
* @vm: domain definition object
* @src: disk source definition to operate on
* @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
*
* Removes security label from a single storage image.
* Removes security label from @src according to @flags.
*
* Returns: 0 on success, -1 on error.
*/
int
virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virStorageSourcePtr src)
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags)
{
if (mgr->drv->domainRestoreSecurityImageLabel) {
int ret;
virObjectLock(mgr);
ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src);
ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src, flags);
virObjectUnlock(mgr);
return ret;
}
@ -526,10 +529,11 @@ 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,
VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN);
virObjectUnlock(mgr);
return ret;
}
@ -544,20 +548,22 @@ virSecurityManagerSetDiskLabel(virSecurityManagerPtr mgr,
* @mgr: security manager object
* @vm: domain definition object
* @src: disk source definition to operate on
* @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
*
* Labels a single storage image with the configured security label.
* Labels a storage image with the configured security label according to @flags.
*
* Returns: 0 on success, -1 on error.
*/
int
virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virStorageSourcePtr src)
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags)
{
if (mgr->drv->domainSetSecurityImageLabel) {
int ret;
virObjectLock(mgr);
ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, src);
ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, src, flags);
virObjectUnlock(mgr);
return ret;
}

10
src/security/security_manager.h
View File

@ -154,12 +154,18 @@ char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr,
virDomainDefPtr vm);
virSecurityManagerPtr* virSecurityManagerGetNested(virSecurityManagerPtr mgr);
typedef enum {
VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN = 1 << 0,
} virSecurityDomainImageLabelFlags;
int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virStorageSourcePtr src);
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags);
int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virStorageSourcePtr src);
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags);
int virSecurityManagerSetMemoryLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,

25
src/security/security_nop.c
View File

@ -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,
virSecurityDomainImageLabelFlags flags 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,
virSecurityDomainImageLabelFlags flags ATTRIBUTE_UNUSED)
{
return 0;
}
@ -292,9 +278,6 @@ virSecurityDriver virSecurityDriverNop = {
.domainSecurityVerify = virSecurityDomainVerifyNop,
.domainSetSecurityDiskLabel = virSecurityDomainSetDiskLabelNop,
.domainRestoreSecurityDiskLabel = virSecurityDomainRestoreDiskLabelNop,
.domainSetSecurityImageLabel = virSecurityDomainSetImageLabelNop,
.domainRestoreSecurityImageLabel = virSecurityDomainRestoreImageLabelNop,

43
src/security/security_selinux.c
View File

@ -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,
virSecurityDomainImageLabelFlags flags ATTRIBUTE_UNUSED)
{
return virSecuritySELinuxRestoreImageLabelInt(mgr, def, src, false);
}
@ -1869,28 +1860,23 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
static int
virSecuritySELinuxSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src)
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags)
{
return virSecuritySELinuxSetImageLabelInternal(mgr, def, src, NULL);
}
virStorageSourcePtr n;
static int
virSecuritySELinuxSetDiskLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainDiskDefPtr disk)
{
virStorageSourcePtr next;
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 (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
break;
}
return 0;
}
static int
virSecuritySELinuxSetHostdevLabelHelper(const char *file, void *opaque)
{
@ -3026,8 +3012,8 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
def->disks[i]->dst);
continue;
}
if (virSecuritySELinuxSetDiskLabel(mgr,
def, def->disks[i]) < 0)
if (virSecuritySELinuxSetImageLabel(mgr, def, def->disks[i]->src,
VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN) < 0)
return -1;
}
/* XXX fixme process def->fss if relabel == true */
@ -3441,9 +3427,6 @@ virSecurityDriver virSecurityDriverSELinux = {
.domainSecurityVerify = virSecuritySELinuxVerify,
.domainSetSecurityDiskLabel = virSecuritySELinuxSetDiskLabel,
.domainRestoreSecurityDiskLabel = virSecuritySELinuxRestoreDiskLabel,
.domainSetSecurityImageLabel = virSecuritySELinuxSetImageLabel,
.domainRestoreSecurityImageLabel = virSecuritySELinuxRestoreImageLabel,

50
src/security/security_stack.c
View File

@ -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,
virSecurityDomainImageLabelFlags flags)
{
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,
flags) < 0)
rc = -1;
}
@ -617,7 +583,8 @@ virSecurityStackSetImageLabel(virSecurityManagerPtr mgr,
static int
virSecurityStackRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virStorageSourcePtr src)
virStorageSourcePtr src,
virSecurityDomainImageLabelFlags flags)
{
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, flags) < 0)
rc = -1;
}
@ -816,9 +783,6 @@ virSecurityDriver virSecurityDriverStack = {
.domainSecurityVerify = virSecurityStackVerify,
.domainSetSecurityDiskLabel = virSecurityStackSetDiskLabel,
.domainRestoreSecurityDiskLabel = virSecurityStackRestoreDiskLabel,
.domainSetSecurityImageLabel = virSecurityStackSetImageLabel,
.domainRestoreSecurityImageLabel = virSecurityStackRestoreImageLabel,