On Fri, Aug 10, 2012 at 03:50:25PM -0600, Eric Blake wrote:
> On 08/10/2012 07:48 AM, Daniel P. Berrange wrote:
> > +++ 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)
Ok, adding these
> > +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?
Obsolete code, removing it.
>
> > +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 ','?
No. Subtle distinction in semantics. 'cN.cM' is used to denote a
category range, while 'cM,cM' is used to denate a category pair
>
> > + 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?
Added a check for this.
>
> > +
> > + 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?
Good point, I'm changing this to enforce gotSens == sensMin
> > +
> > + /* We can only run these tests if we're unconfined */
>
> Does the test gracefully skip when run in a different context?
This is an obsolete comment now - it predates my use of the
LD_PRELOAD hack
> > + 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?
I'll change one of the 's0-s0' case to just 's0'
> > 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.
Ah yes, good point
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
