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

Re: [libvirt] [PATCH 8/8] Add test case for SELinux label generation



On 08/10/2012 07:48 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> This test case validates the correct generation of SELinux labels
> for VMs, wrt the current process label. Since we can't actually
> change the label of the test program process, we create a shared
> library libsecurityselinuxhelper.so which overrides the getcon()
> and setcon() libselinux.so functions. When started the test case
> will check to see if LD_PRELOAD is set, and if not, it will
> re-exec() itself setting LD_PRELOAD=libsecurityselinuxhelper.so

Since the test is already Linux only (hence the _name_ selinux), I'm
okay with taking advantage of backdoors like this.

> +++ b/tests/securityselinuxhelper.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (C) 2011-2012 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
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * License along with this library;  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include <config.h>
> +
> +#include <selinux/selinux.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <errno.h>
> +/*
> + * 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
> + */
> +
> +int getcon(security_context_t *context)
> +{
> +    if (getenv("FAKE_CONTEXT") == NULL) {
> +        *context = NULL;
> +        errno = EINVAL;
> +        return -1;
> +    }
> +    *context = strdup(getenv("FAKE_CONTEXT"));

No check for allocation failure?  (not that it's likely, but it never
hurts to be thorough)

> +    return 0;
> +}
> +
> +int getpidcon(pid_t pid, security_context_t *context)
> +{
> +    if (pid != getpid()) {
> +        *context = NULL;
> +        errno = ESRCH;
> +        return -1;
> +    }
> +    if (getenv("FAKE_CONTEXT") == NULL) {
> +        *context = NULL;
> +        errno = EINVAL;
> +        return -1;
> +    }
> +    *context = strdup(getenv("FAKE_CONTEXT"));

And again...

> +++ b/tests/securityselinuxtest.c

> +
> +static virDomainDefPtr
> +testBuildDomainDef(bool dynamic,
> +                   const char *label,
> +                   const char *baselabel)
> +{
> +    virDomainDefPtr def;
> +
> +    if (VIR_ALLOC(def) < 0)
> +        goto no_memory;
> +
> +    def->seclabel.type = dynamic ? VIR_DOMAIN_SECLABEL_DYNAMIC : VIR_DOMAIN_SECLABEL_STATIC;
> +//    if (!(def->seclabel.model = strdup("selinux")))
> +//        goto no_memory;

Why the comments?

> +static bool
> +testSELinuxCheckCon(context_t con,
> +                    const char *user,
> +                    const char *role,
> +                    const char *type,
> +                    int sensMin,
> +                    int sensMax,
> +                    int catMin,
> +                    int catMax)
> +{

> +    tmp++;
> +    if (virStrToLong_i(tmp, &tmp, 10, &gotCatOne) < 0) {
> +        fprintf(stderr, "Malformed range %s, cannot parse category one\n",
> +                tmp);
> +        return false;
> +    }
> +    if (tmp && *tmp == ',')

Did you mean '.' instead of ','?

> +        tmp++;
> +    if (tmp && *tmp == 'c') {
> +        tmp++;
> +        if (virStrToLong_i(tmp, &tmp, 10, &gotCatTwo) < 0) {
> +            fprintf(stderr, "Malformed range %s, cannot parse category two\n",
> +                    tmp);
> +            return false;
> +        }
> +    } else {
> +        gotCatTwo = gotCatOne;
> +    }

Where do you check that theres no junk after the last category?

> +
> +    if (gotSens < sensMin ||
> +        gotSens > sensMax) {
> +        fprintf(stderr, "Sensitivity %d is out of range %d-%d\n",
> +                gotSens, sensMin, sensMax);
> +        return false;
> +    }

Are you meaning to do a range check here (where input of s2-s3 could let
libvirt choose s3), or actually enforcing that libvirt always picks the
lowest end of the range?

> +    if (gotCatOne < catMin ||
> +        gotCatOne > catMax) {
> +        fprintf(stderr, "Category one %d is out of range %d-%d\n",
> +                gotCatTwo, catMin, catMax);
> +        return false;
> +    }
> +    if (gotCatTwo < catMin ||
> +        gotCatTwo > catMax) {
> +        fprintf(stderr, "Category two %d is out of range %d-%d\n",
> +                gotCatTwo, catMin, catMax);
> +        return false;
> +    }

No check that gotCatOne <= gotCatTwo?

> +static int
> +mymain(void)
> +{

> +
> +    /* We can only run these tests if we're unconfined */

Does the test gracefully skip when run in a different context?

> +    DO_TEST_GEN_LABEL("dynamic unconfined, s0, c0.c1023",
> +                      "unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023",
> +                      true, NULL, NULL,
> +                      "unconfined_u", "unconfined_r", "svirt_t", "svirt_image_t",
> +                      0, 0, 0, 1023);
> +    DO_TEST_GEN_LABEL("dynamic virtd, s0, c0.c1023",
> +                      "system_u:system_r:virtd_t:s0-s0:c0.c1023",
> +                      true, NULL, NULL,
> +                      "system_u", "system_r", "svirt_t", "svirt_image_t",
> +                      0, 0, 0, 1023);
> +    DO_TEST_GEN_LABEL("dynamic virtd, s0, c0.c10",
> +                      "system_u:system_r:virtd_t:s0-s0:c0.c10",
> +                      true, NULL, NULL,
> +                      "system_u", "system_r", "svirt_t", "svirt_image_t",
> +                      0, 0, 0, 10);
> +    DO_TEST_GEN_LABEL("dynamic virtd, s2-s3, c0.c1023",
> +                      "system_u:system_r:virtd_t:s2-s3:c0.c1023",
> +                      true, NULL, NULL,
> +                      "system_u", "system_r", "svirt_t", "svirt_image_t",
> +                      2, 3, 0, 1023);

No test of a degenerate single-range category?

> +
> +    return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libsecurityselinuxhelper.so")
> diff --git a/tests/testutils.h b/tests/testutils.h
> index f372c23..16414d9 100644
> --- a/tests/testutils.h
> +++ b/tests/testutils.h
> @@ -70,4 +70,13 @@ int virtTestMain(int argc,
>          return virtTestMain(argc, argv, func);  \
>      }
>  
> +# define VIRT_TEST_MAIN_PRELOAD(func, lib)          \
> +    int main(int argc, char **argv) {               \
> +        if (getenv("LD_PRELOAD") == NULL) {         \
> +            setenv("LD_PRELOAD", lib, 1);           \
> +            execv(argv[0], argv);                   \

This is insufficient - if LD_PRELOAD is already set for other reasons
(such as valgrind), then you still want to modify it and re-exec.  I
think you need to strstr() the existing contents for the new lib before
deciding whether to re-exec.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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