From: Daniel P. Berrange Date: Fri, 3 Apr 2009 10:55:51 +0000 (+0000) Subject: Improve security label error reporting & verification (Dan Walsh) X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=11b0ed46c57dd8cd5f64e6acd70f402feb162d66;p=libvirt.git Improve security label error reporting & verification (Dan Walsh) --- diff --git a/ChangeLog b/ChangeLog index 4bcaf13eb1..3bf68fc4c9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +Thu Apr 3 11:55:00 BST 2009 Daniel P. Berrange + + Improve error reporting/ verification of security labels + (Dan Walsh) + * src/domain_conf.c: Improve error reporting for parsing of + seclabel XML + * src/libvirt_private.syms: Export virSecurityDriverVerify + * src/qemu_driver.c: Verify seclabel when creating or + defining a new domain + * src/security.c, src/security.h, src/security_linux.c: Add + functions for verifying security labels + * tests/.gitignore: Ignore seclabeltest + * tests/Makefile.am, tests/seclabeltest.c: Add test for + security driver + Thu Apr 2 19:41:00 BST 2009 Daniel P. Berrange Mingw portability fixes diff --git a/src/Makefile.am b/src/Makefile.am index 60b2d466fc..f176b46d6a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -419,7 +419,8 @@ EXTRA_DIST += \ $(STORAGE_DRIVER_DISK_SOURCES) \ $(NODE_DEVICE_DRIVER_SOURCES) \ $(NODE_DEVICE_DRIVER_HAL_SOURCES) \ - $(NODE_DEVICE_DRIVER_DEVKIT_SOURCES) + $(NODE_DEVICE_DRIVER_DEVKIT_SOURCES) \ + $(SECURITY_DRIVER_SELINUX_SOURCES) # # Build our version script. This is composed of three parts: diff --git a/src/domain_conf.c b/src/domain_conf.c index 3d734148d4..0efa50feec 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 78f093c196..350a931eb3 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 fb857ee839..8e0231b976 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 e2bd20aeda..573895e8d4 100644 --- a/src/security.c +++ b/src/security.c @@ -27,6 +27,25 @@ static virSecurityDriverPtr security_drivers[] = { NULL }; +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 8cc2c17fe9..05bf88c5df 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 091fe6ee5a..73d6aeaba0 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 + * Dan Walsh * * SELinux security driver. */ @@ -356,6 +357,20 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn, return rc; } +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, @@ -402,6 +417,7 @@ virSecurityDriver virSELinuxSecurityDriver = { .name = SECURITY_SELINUX_NAME, .probe = SELinuxSecurityDriverProbe, .open = SELinuxSecurityDriverOpen, + .domainSecurityVerify = SELinuxSecurityVerify, .domainSetSecurityImageLabel = SELinuxSetSecurityImageLabel, .domainRestoreSecurityImageLabel = SELinuxRestoreSecurityImageLabel, .domainGenSecurityLabel = SELinuxGenSecurityLabel, diff --git a/tests/.cvsignore b/tests/.cvsignore index 9d809c958b..4f33d0bd81 100644 --- a/tests/.cvsignore +++ b/tests/.cvsignore @@ -15,6 +15,7 @@ nodedevxml2xmltest nodeinfotest statstest qparamtest +seclabeltest *.gcda *.gcno *.exe diff --git a/tests/.gitignore b/tests/.gitignore index 9d809c958b..4f33d0bd81 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 28b273702c..52d5c398e4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -64,6 +64,10 @@ if WITH_QEMU noinst_PROGRAMS += qemuxml2argvtest qemuxml2xmltest endif +if WITH_SECDRIVER_SELINUX +noinst_PROGRAMS += seclabeltest +endif + noinst_PROGRAMS += nodedevxml2xmltest test_scripts = \ @@ -114,6 +118,10 @@ if WITH_QEMU TESTS += qemuxml2argvtest qemuxml2xmltest endif +if WITH_SECDRIVER_SELINUX +TESTS += seclabeltest +endif + TESTS += nodedevxml2xmltest path_add = $$abs_top_builddir/src$(PATH_SEPARATOR)$$abs_top_builddir/qemud @@ -203,6 +211,14 @@ statstest_SOURCES = \ statstest.c testutils.h testutils.c statstest_LDADD = $(LDADDS) +if WITH_SECDRIVER_SELINUX +seclabeltest_SOURCES = \ + seclabeltest.c +seclabeltest_LDADD = ../src/libvirt_driver_security.la $(LDADDS) +else +EXTRA_DIST += seclabeltest.c +endif + 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 0000000000..fe83db856a --- /dev/null +++ b/tests/seclabeltest.c @@ -0,0 +1,45 @@ +#include + +#include +#include +#include +#include +#include +#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); + } + + return 0; +}