[libvirt] [PATCH 0/4] Fix for virIdentityGetSystem when SELinux is disabled
If SELinux is compiled into libvirt but it is disabled on the host, libvirtd logs: error : virIdentityGetSystem:173 : Unable to lookup SELinux process context: Invalid argument on each and every client connection. This patch series adds a runtime check for SELinux to this function. I've added security_disable() to securityselinuxhelper so virIdentityGetSystem can be tested twice, once with SELinux enabled and once with it disabled. A few other libselinux functions have also been added, so now securityselinuxlabeltest and securityselinuxtest do not need to be skipped even when SELinux isn't enabled on the test system. Michael Chapman (4): tests: Flesh out securityselinuxhelper tests: SELinux tests do not need to be skipped virIdentityGetSystem: don't fail if SELinux is disabled tests: Test virIdentityGetSystem src/util/viridentity.c | 18 ++- tests/Makefile.am | 4 + tests/securityselinuxhelper.c | 162 ++++++++++++++++++++- tests/securityselinuxhelperdata/lxc_contexts | 5 + .../virtual_domain_context | 2 + .../virtual_image_context | 2 + tests/securityselinuxlabeltest.c | 3 - tests/securityselinuxtest.c | 3 - tests/viridentitytest.c | 75 +++++++++- 9 files changed, 254 insertions(+), 20 deletions(-) create mode 100644 tests/securityselinuxhelperdata/lxc_contexts create mode 100644 tests/securityselinuxhelperdata/virtual_domain_context create mode 100644 tests/securityselinuxhelperdata/virtual_image_context -- 1.8.5.3
Add fake implementations of: - is_selinux_enabled - security_disable - selinux_virtual_domain_context_path - selinux_virtual_image_context_path - selinux_lxc_contexts_path - selabel_open - selabel_close - selabel_lookup_raw The selabel_* functions back onto the real implementations if SELinux is enabled on the test system, otherwise we just implement a fake selabel handle which errors out on all labelling lookups. With these changes in place, securityselinuxtest and securityselinuxlabeltest don't need to skip all tests if SELinux isn't available; they can exercise much of the security manager code. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- tests/securityselinuxhelper.c | 162 ++++++++++++++++++++- tests/securityselinuxhelperdata/lxc_contexts | 5 + .../virtual_domain_context | 2 + .../virtual_image_context | 2 + 4 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 tests/securityselinuxhelperdata/lxc_contexts create mode 100644 tests/securityselinuxhelperdata/virtual_domain_context create mode 100644 tests/securityselinuxhelperdata/virtual_image_context diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c index d996825..703381c 100644 --- a/tests/securityselinuxhelper.c +++ b/tests/securityselinuxhelper.c @@ -28,6 +28,9 @@ # include <linux/magic.h> #endif #include <selinux/selinux.h> +#if HAVE_SELINUX_LABEL_H +# include <selinux/label.h> +#endif #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -39,10 +42,32 @@ # define NFS_SUPER_MAGIC 0x6969 #endif +#define VIR_FROM_THIS VIR_FROM_NONE + +#include "viralloc.h" #include "virstring.h" static int (*realstatfs)(const char *path, struct statfs *buf); static int (*realsecurity_get_boolean_active)(const char *name); +static int (*realis_selinux_enabled)(void); + +static const char *(*realselinux_virtual_domain_context_path)(void); +static const char *(*realselinux_virtual_image_context_path)(void); + +#ifdef HAVE_SELINUX_LXC_CONTEXTS_PATH +static const char *(*realselinux_lxc_contexts_path)(void); +#endif + +#if HAVE_SELINUX_LABEL_H +static struct selabel_handle *(*realselabel_open)(unsigned int backend, + struct selinux_opt *opts, + unsigned nopts); +static void (*realselabel_close)(struct selabel_handle *handle); +static int (*realselabel_lookup_raw)(struct selabel_handle *handle, + security_context_t *con, + const char *key, + int type); +#endif static void init_syms(void) { @@ -59,6 +84,20 @@ static void init_syms(void) LOAD_SYM(statfs); LOAD_SYM(security_get_boolean_active); + LOAD_SYM(is_selinux_enabled); + + LOAD_SYM(selinux_virtual_domain_context_path); + LOAD_SYM(selinux_virtual_image_context_path); + +#ifdef HAVE_SELINUX_LXC_CONTEXTS_PATH + LOAD_SYM(selinux_lxc_contexts_path); +#endif + +#if HAVE_SELINUX_LABEL_H + LOAD_SYM(selabel_open); + LOAD_SYM(selabel_close); + LOAD_SYM(selabel_lookup_raw); +#endif #undef LOAD_SYM } @@ -76,12 +115,16 @@ static void init_syms(void) int getcon_raw(security_context_t *context) { - if (getenv("FAKE_CONTEXT") == NULL) { + if (!is_selinux_enabled()) { + errno = EINVAL; + return -1; + } + if (getenv("FAKE_SELINUX_CONTEXT") == NULL) { *context = NULL; errno = EINVAL; return -1; } - return VIR_STRDUP_QUIET(*context, getenv("FAKE_CONTEXT")); + return VIR_STRDUP_QUIET(*context, getenv("FAKE_SELINUX_CONTEXT")); } int getcon(security_context_t *context) @@ -91,17 +134,21 @@ int getcon(security_context_t *context) int getpidcon_raw(pid_t pid, security_context_t *context) { + if (!is_selinux_enabled()) { + errno = EINVAL; + return -1; + } if (pid != getpid()) { *context = NULL; errno = ESRCH; return -1; } - if (getenv("FAKE_CONTEXT") == NULL) { + if (getenv("FAKE_SELINUX_CONTEXT") == NULL) { *context = NULL; errno = EINVAL; return -1; } - return VIR_STRDUP_QUIET(*context, getenv("FAKE_CONTEXT")); + return VIR_STRDUP_QUIET(*context, getenv("FAKE_SELINUX_CONTEXT")); } int getpidcon(pid_t pid, security_context_t *context) @@ -111,7 +158,11 @@ int getpidcon(pid_t pid, security_context_t *context) int setcon_raw(security_context_t context) { - return setenv("FAKE_CONTEXT", context, 1); + if (!is_selinux_enabled()) { + errno = EINVAL; + return -1; + } + return setenv("FAKE_SELINUX_CONTEXT", context, 1); } int setcon(security_context_t context) @@ -178,9 +229,28 @@ int statfs(const char *path, struct statfs *buf) return ret; } +int is_selinux_enabled(void) +{ + return getenv("FAKE_SELINUX_DISABLED") == NULL; +} + +int security_disable(void) +{ + if (!is_selinux_enabled()) { + errno = ENOENT; + return -1; + } + + return setenv("FAKE_SELINUX_DISABLED", "1", 1); +} int security_getenforce(void) { + if (!is_selinux_enabled()) { + errno = ENOENT; + return -1; + } + /* For the purpose of our test, we are enforcing. */ return 1; } @@ -188,6 +258,11 @@ int security_getenforce(void) int security_get_boolean_active(const char *name) { + if (!is_selinux_enabled()) { + errno = ENOENT; + return -1; + } + /* For the purpose of our test, nfs is not permitted. */ if (STREQ(name, "virt_use_nfs")) return 0; @@ -195,3 +270,80 @@ int security_get_boolean_active(const char *name) init_syms(); return realsecurity_get_boolean_active(name); } + +const char *selinux_virtual_domain_context_path(void) +{ + init_syms(); + + if (realis_selinux_enabled()) + return realselinux_virtual_domain_context_path(); + + return abs_builddir "/securityselinuxhelperdata/virtual_domain_context"; +} + +const char *selinux_virtual_image_context_path(void) +{ + init_syms(); + + if (realis_selinux_enabled()) + return realselinux_virtual_image_context_path(); + + return abs_builddir "/securityselinuxhelperdata/virtual_image_context"; +} + +#ifdef HAVE_SELINUX_LXC_CONTEXTS_PATH +const char *selinux_lxc_contexts_path(void) +{ + init_syms(); + + if (realis_selinux_enabled()) + return realselinux_lxc_contexts_path(); + + return abs_builddir "/securityselinuxhelperdata/lxc_contexts"; +} +#endif + +#if HAVE_SELINUX_LABEL_H +struct selabel_handle *selabel_open(unsigned int backend, + struct selinux_opt *opts, + unsigned nopts) +{ + char *fake_handle; + + init_syms(); + + if (realis_selinux_enabled()) + return realselabel_open(backend, opts, nopts); + + /* struct selabel_handle is opaque; fake it */ + if (VIR_ALLOC(fake_handle) < 0) + return NULL; + return (struct selabel_handle *)fake_handle; +} + +void selabel_close(struct selabel_handle *handle) +{ + init_syms(); + + if (realis_selinux_enabled()) + return realselabel_close(handle); + + VIR_FREE(handle); +} + +int selabel_lookup_raw(struct selabel_handle *handle, + security_context_t *con, + const char *key, + int type) +{ + init_syms(); + + if (realis_selinux_enabled()) + return realselabel_lookup_raw(handle, con, key, type); + + /* Unimplemented */ + errno = ENOENT; + return -1; +} + +#endif diff --git a/tests/securityselinuxhelperdata/lxc_contexts b/tests/securityselinuxhelperdata/lxc_contexts new file mode 100644 index 0000000..c07b347 --- /dev/null +++ b/tests/securityselinuxhelperdata/lxc_contexts @@ -0,0 +1,5 @@ +process = "system_u:system_r:svirt_lxc_net_t:s0" +content = "system_u:object_r:virt_var_lib_t:s0" +file = "system_u:object_r:svirt_sandbox_file_t:s0" +sandbox_kvm_process = "system_u:system_r:svirt_qemu_net_t:s0" +sandbox_lxc_process = "system_u:system_r:svirt_lxc_net_t:s0" diff --git a/tests/securityselinuxhelperdata/virtual_domain_context b/tests/securityselinuxhelperdata/virtual_domain_context new file mode 100644 index 0000000..150f281 --- /dev/null +++ b/tests/securityselinuxhelperdata/virtual_domain_context @@ -0,0 +1,2 @@ +system_u:system_r:svirt_t:s0 +system_u:system_r:svirt_tcg_t:s0 diff --git a/tests/securityselinuxhelperdata/virtual_image_context b/tests/securityselinuxhelperdata/virtual_image_context new file mode 100644 index 0000000..8ab1e27 --- /dev/null +++ b/tests/securityselinuxhelperdata/virtual_image_context @@ -0,0 +1,2 @@ +system_u:object_r:svirt_image_t:s0 +system_u:object_r:virt_content_t:s0 -- 1.8.5.3
With the previous commit's securityselinuxhelper enhancements, the SELinux security manager can be tested even without SELinux enabled on the test system. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- tests/securityselinuxlabeltest.c | 3 --- tests/securityselinuxtest.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index f98033d..3505f8c 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -322,9 +322,6 @@ mymain(void) if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false))) { virErrorPtr err = virGetLastError(); - if (err->code == VIR_ERR_CONFIG_UNSUPPORTED) - return EXIT_AM_SKIP; - fprintf(stderr, "Unable to initialize security driver: %s\n", err->message); return EXIT_FAILURE; diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index 99b9b24..feb5366 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -272,9 +272,6 @@ mymain(void) if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false))) { virErrorPtr err = virGetLastError(); - if (err->code == VIR_ERR_CONFIG_UNSUPPORTED) - return EXIT_AM_SKIP; - fprintf(stderr, "Unable to initialize security driver: %s\n", err->message); return EXIT_FAILURE; -- 1.8.5.3
If SELinux is compiled into libvirt but it is disabled on the host, libvirtd logs: error : virIdentityGetSystem:173 : Unable to lookup SELinux process context: Invalid argument on each and every client connection. Use is_selinux_enabled() to skip retrieval of the process's SELinux context if SELinux is disabled. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/util/viridentity.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/util/viridentity.c b/src/util/viridentity.c index 4f5127c..bd6adcf 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -168,16 +168,18 @@ virIdentityPtr virIdentityGetSystem(void) goto cleanup; #if WITH_SELINUX - if (getcon(&con) < 0) { - virReportSystemError(errno, "%s", - _("Unable to lookup SELinux process context")); - goto cleanup; - } - if (VIR_STRDUP(seccontext, con) < 0) { + if (is_selinux_enabled()) { + if (getcon(&con) < 0) { + virReportSystemError(errno, "%s", + _("Unable to lookup SELinux process context")); + goto cleanup; + } + if (VIR_STRDUP(seccontext, con) < 0) { + freecon(con); + goto cleanup; + } freecon(con); - goto cleanup; } - freecon(con); #endif if (!(ret = virIdentityNew())) -- 1.8.5.3
Test it once with SELinux enabled and once with it disabled. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- tests/Makefile.am | 4 +++ tests/viridentitytest.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index cd91734..acc9e26 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -779,6 +779,10 @@ virstoragetest_LDADD = $(LDADDS) viridentitytest_SOURCES = \ viridentitytest.c testutils.h testutils.c viridentitytest_LDADD = $(LDADDS) +if WITH_SELINUX +viridentitytest_DEPENDENCIES = libsecurityselinuxhelper.la \ + ../src/libvirt.la +endif WITH_SELINUX virkeycodetest_SOURCES = \ virkeycodetest.c testutils.h testutils.c diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c index 814db4f..9fcdcd3 100644 --- a/tests/viridentitytest.c +++ b/tests/viridentitytest.c @@ -22,6 +22,10 @@ #include <stdlib.h> +#if WITH_SELINUX +# include <selinux/selinux.h> +#endif + #include "testutils.h" #include "viridentity.h" @@ -148,7 +152,7 @@ static int testIdentityEqual(const void *data ATTRIBUTE_UNUSED) goto cleanup; if (virIdentityIsEqual(identa, identb)) { - VIR_DEBUG("Mis-atched identities should not be equal"); + VIR_DEBUG("Mis-matched identities should not be equal"); goto cleanup; } @@ -159,17 +163,86 @@ cleanup: return ret; } +static int testIdentityGetSystem(const void *data) +{ + const char *context = data; + int ret = -1; + virIdentityPtr ident = NULL; + const char *val; + +#if !WITH_SELINUX + if (context) { + VIR_DEBUG("libvirt not compiled with SELinux, skipping this test"); + ret = EXIT_AM_SKIP; + goto cleanup; + } +#endif + + if (!(ident = virIdentityGetSystem())) { + VIR_DEBUG("Unable to get system identity"); + goto cleanup; + } + + if (virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_SELINUX_CONTEXT, + &val) < 0) + goto cleanup; + + if (STRNEQ_NULLABLE(val, context)) { + VIR_DEBUG("Unexpected SELinux context attribute"); + goto cleanup; + } + + ret = 0; +cleanup: + virObjectUnref(ident); + return ret; +} + +static int testSetFakeSELinuxContext(const void *data ATTRIBUTE_UNUSED) +{ +#if WITH_SELINUX + return setcon_raw((security_context_t)data); +#else + VIR_DEBUG("libvirt not compiled with SELinux, skipping this test"); + return EXIT_AM_SKIP; +#endif +} + +static int testDisableFakeSELinux(const void *data ATTRIBUTE_UNUSED) +{ +#if WITH_SELINUX + return security_disable(); +#else + VIR_DEBUG("libvirt not compiled with SELinux, skipping this test"); + return EXIT_AM_SKIP; +#endif +} + static int mymain(void) { + const char *context = "unconfined_u:unconfined_r:unconfined_t:s0"; int ret = 0; if (virtTestRun("Identity attributes ", testIdentityAttrs, NULL) < 0) ret = -1; if (virtTestRun("Identity equality ", testIdentityEqual, NULL) < 0) ret = -1; + if (virtTestRun("Setting fake SELinux context ", testSetFakeSELinuxContext, context) < 0) + ret = -1; + if (virtTestRun("System identity (fake SELinux enabled) ", testIdentityGetSystem, context) < 0) + ret = -1; + if (virtTestRun("Disabling fake SELinux ", testDisableFakeSELinux, NULL) < 0) + ret = -1; + if (virtTestRun("System identity (fake SELinux disabled) ", testIdentityGetSystem, NULL) < 0) + ret = -1; return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } +#if WITH_SELINUX +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libsecurityselinuxhelper.so") +#else VIRT_TEST_MAIN(mymain) +#endif -- 1.8.5.3
On 06.03.2014 07:02, Michael Chapman wrote:
If SELinux is compiled into libvirt but it is disabled on the host, libvirtd logs:
error : virIdentityGetSystem:173 : Unable to lookup SELinux process context: Invalid argument
on each and every client connection.
This patch series adds a runtime check for SELinux to this function.
I've added security_disable() to securityselinuxhelper so virIdentityGetSystem can be tested twice, once with SELinux enabled and once with it disabled. A few other libselinux functions have also been added, so now securityselinuxlabeltest and securityselinuxtest do not need to be skipped even when SELinux isn't enabled on the test system.
Michael Chapman (4): tests: Flesh out securityselinuxhelper tests: SELinux tests do not need to be skipped virIdentityGetSystem: don't fail if SELinux is disabled tests: Test virIdentityGetSystem
src/util/viridentity.c | 18 ++- tests/Makefile.am | 4 + tests/securityselinuxhelper.c | 162 ++++++++++++++++++++- tests/securityselinuxhelperdata/lxc_contexts | 5 + .../virtual_domain_context | 2 + .../virtual_image_context | 2 + tests/securityselinuxlabeltest.c | 3 - tests/securityselinuxtest.c | 3 - tests/viridentitytest.c | 75 +++++++++- 9 files changed, 254 insertions(+), 20 deletions(-) create mode 100644 tests/securityselinuxhelperdata/lxc_contexts create mode 100644 tests/securityselinuxhelperdata/virtual_domain_context create mode 100644 tests/securityselinuxhelperdata/virtual_image_context
ACKed and pushed. Michal
participants (2)
-
Michael Chapman -
Michal Privoznik