Re: [libvirt] [PATCH 2/1] selinux: enhance test to cover nfs label failure

Daniel Berrange (correctly) pointed out that we should do a better job of testing selinux labeling fallbacks on NFS disks that lack labeling support. * tests/securityselinuxhelper.c (includes): Makefile already guaranteed xattr support. Add additional headers. (init_syms): New function, borrowing from vircgroupmock.c. (setfilecon_raw, getfilecon_raw): Fake NFS failure. (statfs): Fake an NFS mount point. (security_getenforce, security_get_boolean_active): Don't let host environment affect test. * tests/securityselinuxlabeldata/nfs.data: New file. * tests/securityselinuxlabeldata/nfs.xml: New file. * tests/securityselinuxlabeltest.c (testSELinuxCreateDisks) (testSELinuxDeleteDisks): Setup and cleanup for fake NFS mount. (testSELinuxCheckLabels): Test handling of SELinux NFS denial. Fix memory leak. (testSELinuxLabeling): Avoid infinite loop on dirty tree. (mymain): Add new test. --- tests/securityselinuxhelper.c | 84 ++++++++++++++++++++++++++++++---- tests/securityselinuxlabeldata/nfs.txt | 1 + tests/securityselinuxlabeldata/nfs.xml | 24 ++++++++++ tests/securityselinuxlabeltest.c | 17 +++++-- 4 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 tests/securityselinuxlabeldata/nfs.txt create mode 100644 tests/securityselinuxlabeldata/nfs.xml

diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c index a82ca6d..d7aae26 100644 --- a/tests/securityselinuxhelper.c +++ b/tests/securityselinuxhelper.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2012 Red Hat, Inc. + * Copyright (C) 2011-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -19,22 +19,51 @@ #include <config.h> +/* This file is only compiled on Linux, and only if xattr support was + * detected. */ + +#include <dlfcn.h> +#include <errno.h> +#include <linux/magic.h> #include <selinux/selinux.h> +#include <stdio.h> #include <stdlib.h> #include <string.h> +#include <sys/vfs.h> #include <unistd.h> -#include <errno.h> -#if WITH_ATTR -# include <attr/xattr.h> -#endif +#include <attr/xattr.h> #include "virstring.h" +static int (*realstatfs)(const char *path, struct statfs *buf); +static int (*realsecurity_get_boolean_active)(const char *name); + +static void init_syms(void) +{ + if (realstatfs) + return; + +# define LOAD_SYM(name) \ + do { \ + if (!(real ## name = dlsym(RTLD_NEXT, #name))) { \ + fprintf(stderr, "Cannot find real '%s' symbol\n", #name); \ + abort(); \ + } \ + } while (0) + + LOAD_SYM(statfs); + LOAD_SYM(security_get_boolean_active); +} + + /* * The kernel policy will not allow us to arbitrarily change * test process context. This helper is used as an LD_PRELOAD * so that the libvirt code /thinks/ it is changing/reading - * the process context, where as in fact we're faking it all + * the process context, where as in fact we're faking it all. + * Furthermore, we fake out that we are using an nfs subdirectory, + * where we control whether selinux is enforcing and whether + * the virt_use_nfs bool is set. */ int getcon_raw(security_context_t *context) @@ -83,10 +112,13 @@ int setcon(security_context_t context) } -#if WITH_ATTR int setfilecon_raw(const char *path, security_context_t con) { const char *constr = con; + if (STRPREFIX(path, abs_builddir "/securityselinuxlabeldata/nfs/")) { + errno = EOPNOTSUPP; + return -1; + } return setxattr(path, "user.libvirt.selinux", constr, strlen(constr), 0); } @@ -101,6 +133,10 @@ int getfilecon_raw(const char *path, security_context_t *con) char *constr = NULL; ssize_t len = getxattr(path, "user.libvirt.selinux", NULL, 0); + if (STRPREFIX(path, abs_builddir "/securityselinuxlabeldata/nfs/")) { + errno = EOPNOTSUPP; + return -1; + } if (len < 0) return -1; if (!(constr = malloc(len+1))) @@ -114,8 +150,40 @@ int getfilecon_raw(const char *path, security_context_t *con) constr[len] = '\0'; return 0; } + + int getfilecon(const char *path, security_context_t *con) { return getfilecon_raw(path, con); } -#endif + + +int statfs(const char *path, struct statfs *buf) +{ + int ret; + + init_syms(); + + ret = realstatfs(path, buf); + if (!ret && STREQ(path, abs_builddir "/securityselinuxlabeldata/nfs")) + buf->f_type = NFS_SUPER_MAGIC; + return ret; +} + + +int security_getenforce(void) +{ + /* For the purpose of our test, we are enforcing. */ + return 1; +} + + +int security_get_boolean_active(const char *name) +{ + /* For the purpose of our test, nfs is not permitted. */ + if (STREQ(name, "virt_use_nfs")) + return 0; + + init_syms(); + return realsecurity_get_boolean_active(name); +} diff --git a/tests/securityselinuxlabeldata/nfs.txt b/tests/securityselinuxlabeldata/nfs.txt new file mode 100644 index 0000000..4c1698e --- /dev/null +++ b/tests/securityselinuxlabeldata/nfs.txt @@ -0,0 +1 @@ +/nfs/plain.raw;EOPNOTSUPP diff --git a/tests/securityselinuxlabeldata/nfs.xml b/tests/securityselinuxlabeldata/nfs.xml new file mode 100644 index 0000000..46a1440 --- /dev/null +++ b/tests/securityselinuxlabeldata/nfs.xml @@ -0,0 +1,24 @@ +<domain type='kvm'> + <name>vm1</name> + <uuid>c7b3edbd-edaf-9455-926a-d65c16db1800</uuid> + <memory unit='KiB'>219200</memory> + <os> + <type arch='i686' machine='pc-1.0'>hvm</type> + <boot dev='cdrom'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/nfs/plain.raw'/> + <target dev='vda' bus='virtio'/> + </disk> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> + <listen type='address' address='0.0.0.0'/> + </graphics> + </devices> + <seclabel model="selinux" type="dynamic" relabel="yes"> + <label>system_u:system_r:svirt_t:s0:c41,c264</label> + <imagelabel>system_u:object_r:svirt_image_t:s0:c41,c264</imagelabel> + </seclabel> +</domain> diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index efe825a..fa99f99 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -209,7 +209,7 @@ testSELinuxCreateDisks(testSELinuxFile *files, size_t nfiles) { size_t i; - if (virFileMakePath(abs_builddir "/securityselinuxlabeldata") < 0) + if (virFileMakePath(abs_builddir "/securityselinuxlabeldata/nfs") < 0) return -1; for (i = 0; i < nfiles; i++) { @@ -228,6 +228,10 @@ testSELinuxDeleteDisks(testSELinuxFile *files, size_t nfiles) if (unlink(files[i].file) < 0) return -1; } + if (rmdir(abs_builddir "/securityselinuxlabeldata/nfs") < 0) + return -1; + /* Ignore failure to remove non-empty directory with in-tree build */ + rmdir(abs_builddir "/securityselinuxlabeldata"); return 0; } @@ -238,9 +242,13 @@ testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles) security_context_t ctx; for (i = 0; i < nfiles; i++) { + ctx = NULL; if (getfilecon(files[i].file, &ctx) < 0) { if (errno == ENODATA) { - ctx = NULL; + /* nothing to do */ + } else if (errno == EOPNOTSUPP) { + if (VIR_STRDUP(ctx, "EOPNOTSUPP") < 0) + return -1; } else { virReportSystemError(errno, "Cannot read label on %s", @@ -252,8 +260,10 @@ testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles) virReportError(VIR_ERR_INTERNAL_ERROR, "File %s context '%s' did not match epected '%s'", files[i].file, ctx, files[i].context); + VIR_FREE(ctx); return -1; } + VIR_FREE(ctx); } return 0; } @@ -287,7 +297,7 @@ testSELinuxLabeling(const void *opaque) cleanup: if (testSELinuxDeleteDisks(files, nfiles) < 0) - goto cleanup; + VIR_WARN("unable to fully clean up"); virDomainDefFree(def); for (i = 0; i < nfiles; i++) { @@ -334,6 +344,7 @@ mymain(void) DO_TEST_LABELING("disks"); DO_TEST_LABELING("kernel"); DO_TEST_LABELING("chardev"); + DO_TEST_LABELING("nfs"); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; }