[libvirt] Reworked the XML verification patch for svirt
Daniel P. Berrange
berrange at redhat.com
Thu Apr 2 10:23:35 UTC 2009
On Wed, Apr 01, 2009 at 02:39:36PM -0400, Daniel J Walsh wrote:
>
> 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 at namei.org>
> + * Dan Walsh <dwalsh at redhat.com>
> *
> * 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;
> +}
ACK, this looks good now.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list