[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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;
>  }
>   


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]