On 01/03/2018 06:00 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 fullpath 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 | 3 ++-
> src/security/security_selinux.c | 3 ++-
> src/security/security_stack.c | 5 +++--
> 9 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 70fb406..ac3e182 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..60a8e08 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 fullpath)
@fullpath seems misleading to me. At first I though that this is
absolute vs. relative path. Maybe allowSubtree is a better name?
Also, I know we don't do it everywhere, but given how ambiguous this
argument's name is can we have a comment describing the function and its
arguments please?
> {
> - return reload_profile(mgr, def, path, true);
> + int rc = -1;
> + char *full_path = NULL;
> +
> + if (fullpath) {
> + 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);
Almost. Curly braces and else should be at one line. But then you get a
syntax-check error because there's another rule saying that if one
branch has curly braces the other one has to have them too.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
