On 04/01/2009 12:50 PM, Daniel P. Berrange wrote:
On Wed, Apr 01, 2009 at 10:33:56AM -0400, Daniel J Walsh wrote:
Main goal of this patch it to verify data being written to xml and
inform the user when he makes a mistake.
Only able to verify static/dynamic in domain_conf.
Added verify code to qemu_driver.c for model and potentially label.
diff --git a/src/Makefile.am b/src/Makefile.am
index d5aac11..8d53740 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -170,9 +170,9 @@ STORAGE_HELPER_DISK_SOURCES = \
SECURITY_DRIVER_SOURCES = \
security.h security.c
-SECURITY_DRIVER_SELINUX_SOURCES = \
- security_selinux.h security_selinux.c
-
+if WITH_SECDRIVER_SELINUX
+SECURITY_DRIVER_SOURCES += security_selinux.h security_selinux.c
+endif
NODE_DEVICE_DRIVER_SOURCES = \
node_device.c node_device.h
@@ -390,9 +390,6 @@ endif
libvirt_driver_security_la_SOURCES = $(SECURITY_DRIVER_SOURCES)
noinst_LTLIBRARIES += libvirt_driver_security.la
libvirt_la_LIBADD += libvirt_driver_security.la
-if WITH_SECDRIVER_SELINUX
-libvirt_driver_security_la_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES)
-endif
What's the purpose of this change ?
The reason for keeping an explicit variable name SECURITY_DRIVER_SELINUX_SOURCES
was that those sounds need to be added to EXTRA_DIST, even when the compilation
of the selinux bits is turned off - so that 'make dist' always gets all files.
diff --git a/src/domain_conf.c b/src/domain_conf.c
index 2696449..c9259db 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;
}
ACK, this looks fine
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a5f9f92..5c88009 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 b2974bb..cd48193 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..72aadf4 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);
+ }
+ }
A few whitespace problems here - HACKING file has some helpful
emacs rules to avoid this
+ 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..396f47f 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 1708d55..7b8e107 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@xxxxxxxxx>
+ * Dan Walsh<dwalsh@xxxxxxxxxx>
*
* SELinux security driver.
*/
@@ -361,6 +364,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;
+}
Some whitespace problem there too
+
+static int
SELinuxSetSecurityLabel(virConnectPtr conn,
virSecurityDriverPtr drv,
virDomainObjPtr vm)
@@ -406,6 +423,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..f068a00
--- /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 (strcmp (doi, "1") != 0)
Can you switch that one to STREQ
Basically looks good aside from the whitespace bug/makefile change
Regards,
Daniel
Try again...
Sorry forgot to do the make syntax-check,
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@xxxxxxxxx>
+ * Dan Walsh <dwalsh@xxxxxxxxxx>
*
* 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;
+}
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list