On 01/09/2018 04:04 PM, Christian Ehrhardt wrote:
> 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 <christian.ehrhardt@xxxxxxxxxxxxx>
> ---
> 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 | 5 +++--
> src/security/security_manager.h | 16 ++++++++++++++--
> src/security/security_selinux.c | 3 ++-
> src/security/security_stack.c | 5 +++--
> 9 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 0f4c422..5c171e4 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 a0f430f..1a0923a 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 dcd6f52..432fab5 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 609d259..74446d6 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 47dad8b..95e7c4d 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 9249aba..4e80409 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -1048,12 +1048,13 @@ virSecurityManagerGetNested(virSecurityManagerPtr mgr)
> 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 013e3b9..e1475b6 100644
> --- a/src/security/security_manager.h
> +++ b/src/security/security_manager.h
> @@ -179,10 +179,22 @@ int virSecurityManagerRestoreInputLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr vm,
> virDomainInputDefPtr input);
>
> -
> +/**
> + * virSecurityManagerDomainSetPathLabel
> + * @mgr: Storage file to chown
> + * @vm: target uid
> + * @path: string describing the path
> + * @allowSubtree:
> + *
> + * set @allowSubtree to true if the call is not only meant for the actual path
> + * in @path, but instead to also allow access to all potential subtress.
> + * Example on @path = "/path":
> + * False => /path
> + * True => /path but also /path/... (including all deeper levels) */
Usually we put the description into .c rather than .h. Also, the
description of the arguments looks a bit off. @mgr is not a 'storage
file to chown' ;-). I'm fixing this and moving it to security_manager.c
just above the function.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
