[libvirt] [PATCH 3/4] security, apparmor: add (Set|Restore)ChardevLabel

[Michal Privoznik]

On 01/03/2018 06:00 PM, Christian Ehrhardt wrote:
> Since 1b4f66e "security: introduce virSecurityManager
> (Set|Restore)ChardevLabel" this is a public API of security manager.
> 
> Implementing this in apparmor avoids miss any rules that should be
> added for devices labeled via these calls.

I believe s/miss/missing/, s/labeled/labelled/

> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> ---
>  src/security/security_apparmor.c | 74 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
> index 60a8e08..e91f157 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -946,6 +946,77 @@ AppArmorRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
>  }
>  
>  static int
> +AppArmorSetChardevLabel(virSecurityManagerPtr mgr,
> +                        virDomainDefPtr def,
> +                        virDomainChrSourceDefPtr dev_source,
> +                        bool chardevStdioLogd ATTRIBUTE_UNUSED)
> +{
> +    char *in = NULL, *out = NULL;
> +    int ret = -1;
> +
> +    virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def,
> +                                                SECURITY_APPARMOR_NAME);
> +    if (!secdef)
> +        return 0;

There shouldn't be empty line in variable declaration. However, there
should be one after the block.

BTW: here and in the function below - if we need to call a function with
a long name to initialize a variable we usually do that on a separate line.

  virSecurityLabelDefPtr secdef;

  secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
  if (!secdef)
    return 0;

> +
> +    switch ((virDomainChrType) dev_source->type) {
> +    case VIR_DOMAIN_CHR_TYPE_DEV:
> +    case VIR_DOMAIN_CHR_TYPE_FILE:
> +    case VIR_DOMAIN_CHR_TYPE_UNIX:
> +    case VIR_DOMAIN_CHR_TYPE_PTY:
> +        ret = reload_profile(mgr, def, dev_source->data.file.path, true);
> +        break;
> +
> +    case VIR_DOMAIN_CHR_TYPE_PIPE:
> +        if (virAsprintf(&in, "%s.in", dev_source->data.file.path) < 0 ||
> +            virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0)
> +            goto done;
> +        if (virFileExists(in)) {
> +            if (reload_profile(mgr, def, in, true) < 0)
> +                goto done;
> +        }
> +        if (virFileExists(out)) {
> +            if (reload_profile(mgr, def, out, true) < 0)
> +                goto done;
> +        }
> +        ret = reload_profile(mgr, def, dev_source->data.file.path, true);
> +        break;
> +
> +    case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
> +    case VIR_DOMAIN_CHR_TYPE_NULL:
> +    case VIR_DOMAIN_CHR_TYPE_VC:
> +    case VIR_DOMAIN_CHR_TYPE_STDIO:
> +    case VIR_DOMAIN_CHR_TYPE_UDP:
> +    case VIR_DOMAIN_CHR_TYPE_TCP:
> +    case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
> +    case VIR_DOMAIN_CHR_TYPE_NMDM:
> +    case VIR_DOMAIN_CHR_TYPE_LAST:
> +        ret = 0;
> +        break;
> +    }
> +
> + done:
> +    VIR_FREE(in);
> +    VIR_FREE(out);
> +    return ret;
> +}
> +
> +static int
> +AppArmorRestoreChardevLabel(virSecurityManagerPtr mgr,
> +                            virDomainDefPtr def,
> +                            virDomainChrSourceDefPtr dev_source ATTRIBUTE_UNUSED,
> +                            bool chardevStdioLogd ATTRIBUTE_UNUSED)
> +{
> +    virSecurityLabelDefPtr secdef =
> +        virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> +
> +    if (!secdef)
> +        return 0;
> +
> +    return reload_profile(mgr, def, NULL, false);
> +}
> +
> +static int
>  AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr,
>                             virDomainDefPtr def,
>                             const char *savefile)
> @@ -1067,6 +1138,9 @@ virSecurityDriver virAppArmorSecurityDriver = {
>  
>      .domainSetPathLabel                 = AppArmorSetPathLabel,
>  
> +    .domainSetSecurityChardevLabel      = AppArmorSetChardevLabel,
> +    .domainRestoreSecurityChardevLabel  = AppArmorRestoreChardevLabel,
> +
>      .domainSetSecurityImageFDLabel      = AppArmorSetFDLabel,
>      .domainSetSecurityTapFDLabel        = AppArmorSetFDLabel,
>  
> 

Michal