[libvirt] [PATCH 2/4] security: full path option for DomainSetPathLabel

[Michal Privoznik]

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 at canonical.com>
> ---
>  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