Eric Blake wrote: > 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 > FYI, this is causing a build failure on a host without libattr-devel make[2]: Entering directory `/home/jfehlig/virt/upstream/libvirt/tests' CCLD libshunload.la CC securityselinuxhelper.lo securityselinuxhelper.c:34:24: fatal error: attr/xattr.h: No such file or directory compilation terminated. make[2]: *** [securityselinuxhelper.lo] Error 1 I need to go now but should have time to take a look later. Regards, Jim > 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; > } > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list