[libvirt] [PATCH v4] Only set SELinux seclabel if supported by the host.

[Cedric Bosdonnat]

On Wed, 2014-06-18 at 11:11 +0200, Christophe Fergeau wrote:
> Hi,
> 
> On Tue, Jun 17, 2014 at 04:01:53PM +0200, Cédric Bosdonnat wrote:
> > This code depends on new API in libvirt-gconfig to extract the
> > secmodels handled by the host.
> > ---
> >  Diff to v3:
> >   * Added yet another missing g_object_unref.
> >   * Fixed the logic for supportsSelinux
> >  libvirt-sandbox/libvirt-sandbox-builder.c | 49 +++++++++++++++++++++++++++----
> >  1 file changed, 43 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c
> > index 48b3acc..d6b5735 100644
> > --- a/libvirt-sandbox/libvirt-sandbox-builder.c
> > +++ b/libvirt-sandbox/libvirt-sandbox-builder.c
> > @@ -322,12 +322,10 @@ static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *build
> >      return TRUE;
> >  }
> >  
> > -
> > -static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder G_GNUC_UNUSED,
> > -                                                        GVirSandboxConfig *config G_GNUC_UNUSED,
> > -                                                        const gchar *statedir G_GNUC_UNUSED,
> > -                                                        GVirConfigDomain *domain,
> > -                                                        GError **error G_GNUC_UNUSED)
> > +static gboolean gvir_sandbox_builder_construct_security_selinux (GVirSandboxBuilder *builder,
> > +                                                                 GVirSandboxConfig *config,
> > +                                                                 GVirConfigDomain *domain,
> > +                                                                 GError **error)
> >  {
> >      GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new();
> >      const char *label = gvir_sandbox_config_get_security_label(config);
> > @@ -360,6 +358,45 @@ static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
> >      return TRUE;
> 
> 
> >  }
> >  
> > +static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder,
> > +                                                        GVirSandboxConfig *config,
> > +                                                        const gchar *statedir G_GNUC_UNUSED,
> > +                                                        GVirConfigDomain *domain,
> > +                                                        GError **error)
> > +{
> > +    GVirConnection *connection = gvir_sandbox_builder_get_connection(builder);
> > +    GVirConfigCapabilities *configCapabilities;
> > +    GVirConfigCapabilitiesHost *hostCapabilities;
> > +    GList *secmodels, *iter;
> > +    gboolean supportsSelinux = FALSE;
> > +
> > +    /* What security models are available on the host? */
> > +    if (!(configCapabilities = gvir_connection_get_capabilities(connection, error))) {
> 
> Missing g_object_unref(connection); here too.

Oops, I forgot that one case indeed.

> > +        return FALSE;
> > +    }
> > +
> > +    hostCapabilities = gvir_config_capabilities_get_host(configCapabilities);
> > +
> > +    secmodels = gvir_config_capabilities_host_get_secmodels(hostCapabilities);
> > +    for (iter = secmodels; iter != NULL; iter = iter->next) {
> > +        if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model(
> > +                GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter->data)), "selinux"))
> > +            supportsSelinux = TRUE;
> > +        g_object_unref(iter->data);
> > +    }
> > +
> > +    g_list_free(secmodels);
> > +    g_object_unref(hostCapabilities);
> > +    g_object_unref(configCapabilities);
> > +    g_object_unref(connection);
> > +
> > +    if (supportsSelinux)
> > +        return gvir_sandbox_builder_construct_security_selinux(builder, config,
> > +                                                               domain, error);
> > +
> > +    return TRUE;
> 
> Wondering whether this we should return FALSE when we did nothing
> because we only support SELinux.

No idea what the original intent was... but we shouldn't fail if we just
not using any security label: that may be a valid use case.

> Patch is fine otherwise, I can squash these changes in before pushing if
> you don't want to send yet another iteration ;)

Well, that would save a few mails to the mailing list ;)

--
Cedric