[libvirt] [PATCH v3 1/2] security: add new internal function "virSecurityManagerGetBaseLabel"

[Daniel P. Berrange]

On Fri, Sep 06, 2013 at 06:29:55PM +0200, Giuseppe Scrivano wrote:
> virSecurityManagerGetBaseLabel queries the default settings used by
> a security model.
> 
> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
>  src/libvirt_private.syms         |  1 +
>  src/security/security_apparmor.c |  8 ++++++++
>  src/security/security_dac.c      | 34 ++++++++++++++++++++++++----------
>  src/security/security_dac.h      |  7 +++----
>  src/security/security_driver.h   |  4 ++++
>  src/security/security_manager.c  | 22 ++++++++++++++++++++--
>  src/security/security_manager.h  |  2 ++
>  src/security/security_nop.c      | 10 ++++++++++
>  src/security/security_selinux.c  | 12 ++++++++++++
>  src/security/security_stack.c    |  9 +++++++++
>  10 files changed, 93 insertions(+), 16 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 35f0f1b..aea7e94 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1033,6 +1033,7 @@ virSecurityDriverLookup;
>  # security/security_manager.h
>  virSecurityManagerClearSocketLabel;
>  virSecurityManagerGenLabel;
> +virSecurityManagerGetBaseLabel;
>  virSecurityManagerGetDOI;
>  virSecurityManagerGetModel;
>  virSecurityManagerGetMountOptions;
> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
> index adc9918..2d74cdd 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -931,6 +931,12 @@ AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>      return opts;
>  }
>  
> +static const char *
> +AppArmorGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
> +                     int virtType ATTRIBUTE_UNUSED)
> +{
> +    return NULL;
> +}
>  
>  virSecurityDriver virAppArmorSecurityDriver = {
>      .privateDataLen                     = 0,
> @@ -972,4 +978,6 @@ virSecurityDriver virAppArmorSecurityDriver = {
>      .domainSetSecurityTapFDLabel        = AppArmorSetFDLabel,
>  
>      .domainGetSecurityMountOptions      = AppArmorGetMountOptions,
> +
> +    .getBaseLabel                       = AppArmoryGetBaseLabel,
>  };
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 6876bd5..019c789 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -47,22 +47,25 @@ struct _virSecurityDACData {
>      gid_t *groups;
>      int ngroups;
>      bool dynamicOwnership;
> +    char *baselabel;
>  };
>  
> -void
> -virSecurityDACSetUser(virSecurityManagerPtr mgr,
> -                      uid_t user)
> +/* returns -1 on error, 0 on success */
> +int
> +virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
> +                              uid_t user,
> +                              gid_t group)
>  {
>      virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
>      priv->user = user;
> -}
> -
> -void
> -virSecurityDACSetGroup(virSecurityManagerPtr mgr,
> -                       gid_t group)
> -{
> -    virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
>      priv->group = group;
> +
> +    if (virAsprintf(&priv->baselabel, "%u:%u",
> +                    (unsigned int) user,
> +                    (unsigned int) group) < 0)
> +        return -1;
> +
> +    return 0;
>  }
>  
>  void
> @@ -217,6 +220,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr)
>  {
>      virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
>      VIR_FREE(priv->groups);
> +    VIR_FREE(priv->baselabel);
>      return 0;
>  }
>  
> @@ -1170,6 +1174,14 @@ virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>      return NULL;
>  }
>  
> +static const char *
> +virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr,
> +                           int virt ATTRIBUTE_UNUSED)
> +{
> +    virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> +    return priv->baselabel;
> +}
> +
>  virSecurityDriver virSecurityDriverDAC = {
>      .privateDataLen                     = sizeof(virSecurityDACData),
>      .name                               = SECURITY_DAC_NAME,
> @@ -1212,4 +1224,6 @@ virSecurityDriver virSecurityDriverDAC = {
>      .domainSetSecurityTapFDLabel        = virSecurityDACSetTapFDLabel,
>  
>      .domainGetSecurityMountOptions      = virSecurityDACGetMountOptions,
> +
> +    .getBaseLabel                       = virSecurityDACGetBaseLabel,
>  };
> diff --git a/src/security/security_dac.h b/src/security/security_dac.h
> index 02432a5..dbcf56f 100644
> --- a/src/security/security_dac.h
> +++ b/src/security/security_dac.h
> @@ -25,10 +25,9 @@
>  
>  extern virSecurityDriver virSecurityDriverDAC;
>  
> -void virSecurityDACSetUser(virSecurityManagerPtr mgr,
> -                           uid_t user);
> -void virSecurityDACSetGroup(virSecurityManagerPtr mgr,
> -                            gid_t group);
> +int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
> +                                  uid_t user,
> +                                  gid_t group);

It would be desirable to have this re-factoring done in a separate,
prior, patch from that which adds the GetBaseLabel hook.

> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 92fb504..c4b8f10 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -146,8 +146,10 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver,
>      if (!mgr)
>          return NULL;
>  
> -    virSecurityDACSetUser(mgr, user);
> -    virSecurityDACSetGroup(mgr, group);
> +    if (virSecurityDACSetUserAndGroup(mgr, user, group) < 0) {
> +        virSecurityManagerDispose(mgr);
> +        return NULL;
> +    }

Likewise this block


> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 38de060..1c2ea64 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1827,6 +1827,17 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def,
>  }
>  
>  
> +static const char *
> +virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
> +{
> +    virSecuritySELinuxDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> +    if (virtType == VIR_DOMAIN_VIRT_QEMU)
> +        return priv->alt_domain_context;

alt_domain_context is not guaranteed to be present, so you need to have

   if (virtType == VIR_DOMAIN_VIRT_QEMU && priv->alt_domain_context)
       ....


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|