Re: [libvirt] Reworked the XML verification patch for svirt

Fixed Whitespace. Removed Makefile patch. Use STREQ in seclabeltest.

diff --git a/src/domain_conf.c b/src/domain_conf.c index 3d73414..0efa50f 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -1859,29 +1859,44 @@ virSecurityLabelDefParseXML(virConnectPtr conn, if (virXPathNode(conn, "./seclabel", ctxt) == NULL) return 0; + p = virXPathStringLimit(conn, "string(./seclabel/@model)", + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); + if (p == NULL) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("missing security model")); + goto error; + } + def->seclabel.model = p; + p = virXPathStringLimit(conn, "string(./seclabel/@type)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - if (p == NULL) - goto error; - if ((def->seclabel.type = virDomainSeclabelTypeFromString(p)) < 0) + if (p == NULL) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("missing security type")); goto error; + } + def->seclabel.type = virDomainSeclabelTypeFromString(p); VIR_FREE(p); + if (def->seclabel.type < 0) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, + _("invalid security type")); + goto error; + } /* Only parse details, if using static labels, or * if the 'live' VM XML is requested */ if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC || !(flags & VIR_DOMAIN_XML_INACTIVE)) { - p = virXPathStringLimit(conn, "string(./seclabel/@model)", - VIR_SECURITY_MODEL_BUFLEN-1, ctxt); - if (p == NULL) - goto error; - def->seclabel.model = p; p = virXPathStringLimit(conn, "string(./seclabel/label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - if (p == NULL) + if (p == NULL) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, + _("security label is missing")); goto error; + } + def->seclabel.label = p; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 78f093c..350a931 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -248,6 +248,7 @@ free_qparam_set; # security.h +virSecurityDriverVerify; virSecurityDriverStartup; virSecurityDriverInit; virSecurityDriverSetDOI; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index fb857ee..8e0231b 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2115,6 +2115,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (virSecurityDriverVerify(conn, def) < 0) + goto cleanup; + vm = virDomainFindByName(&driver->domains, def->name); if (vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, @@ -3398,6 +3401,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (virSecurityDriverVerify(conn, def) < 0) + goto cleanup; + vm = virDomainFindByName(&driver->domains, def->name); if (vm) { virDomainObjUnlock(vm); diff --git a/src/security.c b/src/security.c index e2bd20a..573895e 100644 --- a/src/security.c +++ b/src/security.c @@ -28,6 +28,25 @@ static virSecurityDriverPtr security_drivers[] = { }; int +virSecurityDriverVerify(virConnectPtr conn, virDomainDefPtr def) +{ + unsigned int i; + const virSecurityLabelDefPtr secdef = &def->seclabel; + + if (STREQ(secdef->model, "none")) + return 0; + + for (i = 0; security_drivers[i] != NULL ; i++) { + if (STREQ(security_drivers[i]->name, secdef->model)) { + return security_drivers[i]->domainSecurityVerify(conn, def); + } + } + virSecurityReportError(conn, VIR_ERR_XML_ERROR, + _("invalid security model")); + return -1; +} + +int virSecurityDriverStartup(virSecurityDriverPtr *drv, const char *name) { diff --git a/src/security.h b/src/security.h index 8cc2c17..05bf88c 100644 --- a/src/security.h +++ b/src/security.h @@ -46,11 +46,14 @@ typedef int (*virSecurityDomainRestoreLabel) (virConnectPtr conn, typedef int (*virSecurityDomainSetLabel) (virConnectPtr conn, virSecurityDriverPtr drv, virDomainObjPtr vm); +typedef int (*virSecurityDomainSecurityVerify) (virConnectPtr conn, + virDomainDefPtr def); struct _virSecurityDriver { const char *name; virSecurityDriverProbe probe; virSecurityDriverOpen open; + virSecurityDomainSecurityVerify domainSecurityVerify; virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel; virSecurityDomainSetImageLabel domainSetSecurityImageLabel; virSecurityDomainGenLabel domainGenSecurityLabel; @@ -71,6 +74,9 @@ struct _virSecurityDriver { int virSecurityDriverStartup(virSecurityDriverPtr *drv, const char *name); +int +virSecurityDriverVerify(virConnectPtr conn, virDomainDefPtr def); + void virSecurityReportError(virConnectPtr conn, int code, const char *fmt, ...) ATTRIBUTE_FORMAT(printf, 3, 4); diff --git a/src/security_selinux.c b/src/security_selinux.c index 091fe6e..abf975b 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008 Red Hat, Inc. + * Copyright (C) 2008,2009 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 @@ -8,6 +8,7 @@ * * Authors: * James Morris <jmorris(a)namei.org&gt; + * Dan Walsh <dwalsh(a)redhat.com&gt; * * SELinux security driver. */ @@ -357,6 +360,20 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn, } static int +SELinuxSecurityVerify(virConnectPtr conn, virDomainDefPtr def) +{ + const virSecurityLabelDefPtr secdef = &def->seclabel; + if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) { + if (security_check_context(secdef->label) != 0) { + virSecurityReportError(conn, VIR_ERR_XML_ERROR, + _("Invalid security label %s"), secdef->label); + return -1; + } + } + return 0; +} + +static int SELinuxSetSecurityLabel(virConnectPtr conn, virSecurityDriverPtr drv, virDomainObjPtr vm) @@ -402,6 +419,7 @@ virSecurityDriver virSELinuxSecurityDriver = { .name = SECURITY_SELINUX_NAME, .probe = SELinuxSecurityDriverProbe, .open = SELinuxSecurityDriverOpen, + .domainSecurityVerify = SELinuxSecurityVerify, .domainSetSecurityImageLabel = SELinuxSetSecurityImageLabel, .domainRestoreSecurityImageLabel = SELinuxRestoreSecurityImageLabel, .domainGenSecurityLabel = SELinuxGenSecurityLabel, diff --git a/tests/.gitignore b/tests/.gitignore index 9d809c9..4f33d0b 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -15,6 +15,7 @@ nodedevxml2xmltest nodeinfotest statstest qparamtest +seclabeltest *.gcda *.gcno *.exe diff --git a/tests/Makefile.am b/tests/Makefile.am index 28b2737..48db913 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -54,7 +54,7 @@ EXTRA_DIST = \ nodedevschemadata noinst_PROGRAMS = virshtest conftest \ - nodeinfotest statstest qparamtest + nodeinfotest statstest qparamtest seclabeltest if WITH_XEN noinst_PROGRAMS += xml2sexprtest sexpr2xmltest \ @@ -97,6 +97,7 @@ EXTRA_DIST += $(test_scripts) TESTS = virshtest \ nodeinfotest \ statstest \ + seclabeltest \ qparamtest \ $(test_scripts) @@ -203,6 +204,10 @@ statstest_SOURCES = \ statstest.c testutils.h testutils.c statstest_LDADD = $(LDADDS) +seclabeltest_SOURCES = \ + seclabeltest.c +seclabeltest_LDADD = ../src/libvirt_driver_security.la $(LDADDS) + qparamtest_SOURCES = \ qparamtest.c testutils.h testutils.c qparamtest_LDADD = $(LDADDS) diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c new file mode 100644 index 0000000..8805333 --- /dev/null +++ b/tests/seclabeltest.c @@ -0,0 +1,60 @@ +#include <config.h> + +#include <unistd.h> +#include <stdlib.h> +#include <stdio.h> +#include <string.h> +#include <errno.h> +#include "security.h" + +int +main (int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) +{ + int ret; + + const char *doi, *model; + virSecurityDriverPtr security_drv; + + ret = virSecurityDriverStartup (&security_drv, "selinux"); + if (ret == -1) + { + fprintf (stderr, "Failed to start security driver"); + exit (-1); + } + /* No security driver wanted to be enabled: just return */ + if (ret == -2) + return 0; + + model = virSecurityDriverGetModel (security_drv); + if (!model) + { + fprintf (stderr, "Failed to copy secModel model: %s", + strerror (errno)); + exit (-1); + } + + doi = virSecurityDriverGetDOI (security_drv); + if (!doi) + { + fprintf (stderr, "Failed to copy secModel DOI: %s", + strerror (errno)); + exit (-1); + } + + virConnectPtr conn; + conn = virConnectOpen (NULL); + if (conn == NULL) + { + fprintf (stderr, "First virConnectOpen() failed\n"); + exit (1); + } + virSecurityDriverSetDOI (conn, security_drv, "1"); + doi = virSecurityDriverGetDOI (security_drv); + if (STREQ(doi, "1")) + { + fprintf (stderr, "Failed to set secModel DOI: %s", strerror (errno)); + exit (1); + } + virConnectClose (conn); + return 0; +}