[libvirt] [PATCH v3 2/2] capabilities: add baselabel per sec driver/virt type to secmodel

[Daniel P. Berrange]

On Fri, Sep 06, 2013 at 06:29:56PM +0200, Giuseppe Scrivano wrote:
> Expand the "secmodel" XML fragment of "host" with a sequence of
> baselabel's which describe the default security context used by
> libvirt with a specific security model and virtualization type:
> 
> <secmodel>
>   <model>selinux</model>
>   <doi>0</doi>
>   <baselabel type='kvm'>system_u:system_r:svirt_t:s0</baselabel>
>   <baselabel type='qemu'>system_u:system_r:svirt_t:s0</baselabel>

s/svirt_t/svirt_tcg_t/ for the qemu example just to illustrate
that it is sometimes diferent.

> </secmodel>
> <secmodel>
>   <model>dac</model>
>   <doi>0</doi>
>   <baselabel type='kvm'>0:0</baselabel>
>   <baselabel type='qemu'>0:0</baselabel>

I'd suggest  '107:107' for these examples since that's the usual
ID for Fedora 'qemu' user.

> </secmodel>
> 
> "baselabel" is driver-specific information, e.g. in the DAC security
> model, it indicates USER_ID:GROUP_ID.
> 
> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
>  docs/schemas/capability.rng                  |  8 ++++
>  src/conf/capabilities.c                      | 60 +++++++++++++++++++++++++++-
>  src/conf/capabilities.h                      | 14 +++++++
>  src/libvirt_private.syms                     |  1 +
>  src/lxc/lxc_conf.c                           | 10 ++++-
>  src/qemu/qemu_conf.c                         | 21 ++++++++--
>  tests/capabilityschemadata/caps-qemu-kvm.xml |  2 +
>  tests/capabilityschemadata/caps-test3.xml    |  2 +
>  8 files changed, 111 insertions(+), 7 deletions(-)
> 

> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 1acc936..b0e2ff9 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -184,6 +184,20 @@ virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
>  }
>  
>  static void
> +virCapabilitiesFreeSecModel(virCapsHostSecModelPtr secmodel)
> +{
> +    size_t i;
> +    for (i = 0; i < secmodel->nlabels; i++) {
> +        VIR_FREE(secmodel->labels[i].type);
> +        VIR_FREE(secmodel->labels[i].label);
> +    }
> +
> +    VIR_FREE(secmodel->labels);
> +    VIR_FREE(secmodel->model);
> +    VIR_FREE(secmodel->doi);
> +}

For functions which don't actually free the passed-in pointer
itself, we prefer to use 'Clear' instead of 'Free' in the name,
to make it more obvious to people what the semantics are.

> +
> +static void
>  virCapabilitiesDispose(void *object)
>  {
>      virCapsPtr caps = object;
> @@ -204,8 +218,7 @@ virCapabilitiesDispose(void *object)
>      VIR_FREE(caps->host.migrateTrans);
>  
>      for (i = 0; i < caps->host.nsecModels; i++) {
> -        VIR_FREE(caps->host.secModels[i].model);
> -        VIR_FREE(caps->host.secModels[i].doi);
> +        virCapabilitiesFreeSecModel(&caps->host.secModels[i]);
>      }
>      VIR_FREE(caps->host.secModels);
>  
> @@ -507,6 +520,44 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,

> diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
> index c1cee3f..557191a 100644
> --- a/src/lxc/lxc_conf.c
> +++ b/src/lxc/lxc_conf.c
> @@ -126,10 +126,13 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver)
>  
>      if (driver) {
>          /* Security driver data */
> -        const char *doi, *model;
> +        const char *doi, *model, *label, *type;
>  
>          doi = virSecurityManagerGetDOI(driver->securityManager);
>          model = virSecurityManagerGetModel(driver->securityManager);
> +        label = virSecurityManagerGetBaseLabel(driver->securityManager,
> +                                               VIR_DOMAIN_VIRT_LXC);

Hmm, the virSecurityManagerGetBaseLabel method can raise a VIR_ERR_NO_SUPPORT
message if unsupported, which would be ignored here. It is none the less
valid for this method to be not-implemented by a driver. Since I don't believe
we have a need to report errors in this method, I think we should remove the
call to virReportError in its impl.


> +        type = virDomainVirtTypeToString(VIR_DOMAIN_VIRT_LXC);
>          /* Allocate the primary security driver for LXC. */
>          if (VIR_ALLOC(caps->host.secModels) < 0)
>              goto error;
> @@ -138,6 +141,11 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver)
>              goto error;
>          if (VIR_STRDUP(caps->host.secModels[0].doi, doi) < 0)
>              goto error;
> +        if (label &&
> +            virCapabilitiesHostSecModelAddBaseLabel(&caps->host.secModels[0],
> +                                                    type,
> +                                                    label) < 0)
> +            goto error;
>  
>          VIR_DEBUG("Initialized caps for security driver \"%s\" with "
>                    "DOI \"%s\"", model, doi);
> diff --git a/tests/capabilityschemadata/caps-qemu-kvm.xml b/tests/capabilityschemadata/caps-qemu-kvm.xml
> index 1fbc22b..066ec71 100644
> --- a/tests/capabilityschemadata/caps-qemu-kvm.xml
> +++ b/tests/capabilityschemadata/caps-qemu-kvm.xml
> @@ -25,6 +25,8 @@
>      <secmodel>
>        <model>selinux</model>
>        <doi>0</doi>
> +      <baselabel type='kvm'>system_u:system_r:svirt_t:s0</baselabel>
> +      <baselabel type='qemu'>system_u:system_r:svirt_t:s0</baselabel>

s/svirt_t/svirt_tcg_t/ in this example

>      </secmodel>
>    </host>
>  
> diff --git a/tests/capabilityschemadata/caps-test3.xml b/tests/capabilityschemadata/caps-test3.xml
> index e6c56c5..d359f25 100644
> --- a/tests/capabilityschemadata/caps-test3.xml
> +++ b/tests/capabilityschemadata/caps-test3.xml
> @@ -82,6 +82,8 @@
>      <secmodel>
>        <model>dac</model>
>        <doi>0</doi>
> +      <baselabel type='kvm'>0:0</baselabel>
> +      <baselabel type='qemu'>0:0</baselabel>
>      </secmodel>
>    </host>

s/0/107/


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 :|