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 :|