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

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))) { + 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; +} + static gboolean gvir_sandbox_builder_clean_post_start_default(GVirSandboxBuilder *builder G_GNUC_UNUSED, GVirSandboxConfig *config G_GNUC_UNUSED, -- 1.8.4.5

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.
+ 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. Patch is fine otherwise, I can squash these changes in before pushing if you don't want to send yet another iteration ;) Christophe

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

On Wed, Jun 18, 2014 at 01:22:12PM +0200, Cedric Bosdonnat wrote:
On Wed, 2014-06-18 at 11:11 +0200, Christophe Fergeau wrote:
+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.
I've pushed this patch with this missing _unref squashed in. Christophe
participants (3)
-
Cedric Bosdonnat
-
Christophe Fergeau
-
Cédric Bosdonnat