From a5486e57f5b13454d6a1334675ba98075f179da7 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Tue, 9 Jan 2018 16:04:03 +0100 Subject: [PATCH] security: full path option for DomainSetPathLabel virSecurityManagerDomainSetPathLabel is used to make a path known to the security modules, but today is used interchangably for - paths to files/dirs to be accessed directly - paths to a dir, but the access will actually be to files therein Depending on the security module it is important to know which of these types it will be. The argument allowSubtree augments the call to the implementations of DomainSetPathLabel that can - per security module - decide if extra actions shall be taken. For now dac/selinux handle this as before, but apparmor will make use of it to add a wildcard to the path that was passed. Signed-off-by: Christian Ehrhardt Signed-off-by: Michal Privoznik --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_process.c | 4 ++-- src/security/security_apparmor.c | 17 +++++++++++++++-- src/security/security_dac.c | 3 ++- src/security/security_driver.h | 3 ++- src/security/security_manager.c | 19 +++++++++++++++++-- src/security/security_manager.h | 4 ++-- src/security/security_selinux.c | 3 ++- src/security/security_stack.c | 5 +++-- 9 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0f4c422836..5c171e4cbd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -692,7 +692,7 @@ qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver, } if (qemuSecurityDomainSetPathLabel(driver->securityManager, - vm->def, path) < 0) + vm->def, path, false) < 0) goto cleanup; ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a0f430f89f..1a0923af36 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3401,7 +3401,7 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver, } if (qemuSecurityDomainSetPathLabel(driver->securityManager, - def, path) < 0) { + def, path, true) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to label %s"), path); return -1; @@ -4514,7 +4514,7 @@ qemuProcessMakeDir(virQEMUDriverPtr driver, } if (qemuSecurityDomainSetPathLabel(driver->securityManager, - vm->def, path) < 0) + vm->def, path, true) < 0) goto cleanup; ret = 0; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index dcd6f52c16..432fab5260 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -956,9 +956,22 @@ AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr, static int AppArmorSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, - const char *path) + const char *path, + bool allowSubtree) { - return reload_profile(mgr, def, path, true); + int rc = -1; + char *full_path = NULL; + + if (allowSubtree) { + if (virAsprintf(&full_path, "%s/{,**}", path) < 0) + return -1; + rc = reload_profile(mgr, def, full_path, true); + VIR_FREE(full_path); + } else { + rc = reload_profile(mgr, def, path, true); + } + + return rc; } static int diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 609d2595b2..74446d6644 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -2081,7 +2081,8 @@ virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr, static int virSecurityDACDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, - const char *path) + const char *path, + bool allowSubtree ATTRIBUTE_UNUSED) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDefPtr seclabel; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 47dad8ba20..95e7c4de07 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -139,7 +139,8 @@ typedef int (*virSecurityDomainRestoreInputLabel) (virSecurityManagerPtr mgr, virDomainInputDefPtr input); typedef int (*virSecurityDomainSetPathLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, - const char *path); + const char *path, + bool allowSubtree); typedef int (*virSecurityDomainSetChardevLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainChrSourceDefPtr dev_source, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 9249aba1fa..fdeea4d533 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1045,15 +1045,30 @@ virSecurityManagerGetNested(virSecurityManagerPtr mgr) } +/** + * virSecurityManagerDomainSetPathLabel: + * @mgr: security manager object + * @vm: domain definition object + * @path: path to label + * @allowSubtree: whether to allow just @path or its subtree too + * + * This function relabels given @path so that @vm can access it. + * If @allowSubtree is set to true the manager will grant access + * to @path and its subdirectories at any level (currently + * implemented only by AppArmor). + * + * Returns: 0 on success, -1 on error. + */ int virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, - const char *path) + const char *path, + bool allowSubtree) { if (mgr->drv->domainSetPathLabel) { int ret; virObjectLock(mgr); - ret = mgr->drv->domainSetPathLabel(mgr, vm, path); + ret = mgr->drv->domainSetPathLabel(mgr, vm, path, allowSubtree); virObjectUnlock(mgr); return ret; } diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 013e3b9b18..c36a8b488f 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -179,10 +179,10 @@ int virSecurityManagerRestoreInputLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virDomainInputDefPtr input); - int virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, - const char *path); + const char *path, + bool allowSubtree); int virSecurityManagerSetChardevLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 0815a02d18..c26cdacd9f 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3028,7 +3028,8 @@ virSecuritySELinuxGetSecurityMountOptions(virSecurityManagerPtr mgr, static int virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, - const char *path) + const char *path, + bool allowSubtree ATTRIBUTE_UNUSED) { virSecurityLabelDefPtr seclabel; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 0375e7d89d..9615f9f972 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -704,7 +704,8 @@ virSecurityStackRestoreInputLabel(virSecurityManagerPtr mgr, static int virSecurityStackDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, - const char *path) + const char *path, + bool allowSubtree) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityStackItemPtr item = priv->itemsHead; @@ -712,7 +713,7 @@ virSecurityStackDomainSetPathLabel(virSecurityManagerPtr mgr, for (; item; item = item->next) { if (virSecurityManagerDomainSetPathLabel(item->securityManager, - vm, path) < 0) + vm, path, allowSubtree) < 0) rc = -1; }