The internal representation of security labels contains some bools (e.g. @relabel, @implicit). But the stdbool.h file is not included anywhere in the header file, therefore if somebody just includes "virseclabel.h" they also need to include <stdbool.h>. That's not right as it should be virseclabel.h who drags in all the dependencies. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/util/virseclabel.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h index 94c4dfc..558d4eb 100644 --- a/src/util/virseclabel.h +++ b/src/util/virseclabel.h @@ -22,6 +22,8 @@ #ifndef __SECLABEL_H # define __SECLABEL_H +# include <stdbool.h> + typedef enum { VIR_DOMAIN_SECLABEL_DEFAULT, VIR_DOMAIN_SECLABEL_NONE, -- 2.8.4

This change alone is not needed, but it prepares environment for subsequent patches where we will need virSecurityManager much deeper in the code. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/security/security_dac.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index fd74e8b..97be862 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -349,12 +349,13 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv, static int -virSecurityDACSetOwnership(virSecurityDACDataPtr priv, +virSecurityDACSetOwnership(virSecurityManagerPtr mgr, virStorageSourcePtr src, const char *path, uid_t uid, gid_t gid) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); struct stat sb; if (!path && src && src->path && @@ -442,7 +443,7 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr, return -1; } - return virSecurityDACSetOwnership(priv, src, NULL, user, group); + return virSecurityDACSetOwnership(mgr, src, NULL, user, group); } @@ -550,7 +551,7 @@ virSecurityDACSetHostdevLabelHelper(const char *file, if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL) < 0) return -1; - return virSecurityDACSetOwnership(priv, NULL, file, user, group); + return virSecurityDACSetOwnership(mgr, NULL, file, user, group); } @@ -850,7 +851,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, switch ((virDomainChrType) dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACSetOwnership(priv, NULL, + ret = virSecurityDACSetOwnership(mgr, NULL, dev_source->data.file.path, user, group); break; @@ -860,10 +861,10 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0) goto done; if (virFileExists(in) && virFileExists(out)) { - if (virSecurityDACSetOwnership(priv, NULL, in, user, group) < 0 || - virSecurityDACSetOwnership(priv, NULL, out, user, group) < 0) + if (virSecurityDACSetOwnership(mgr, NULL, in, user, group) < 0 || + virSecurityDACSetOwnership(mgr, NULL, out, user, group) < 0) goto done; - } else if (virSecurityDACSetOwnership(priv, NULL, + } else if (virSecurityDACSetOwnership(mgr, NULL, dev_source->data.file.path, user, group) < 0) { goto done; @@ -873,7 +874,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_CHR_TYPE_UNIX: if (!dev_source->data.nix.listen) { - if (virSecurityDACSetOwnership(priv, NULL, + if (virSecurityDACSetOwnership(mgr, NULL, dev_source->data.nix.path, user, group) < 0) goto done; @@ -1033,7 +1034,7 @@ virSecurityDACSetInputLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) return -1; - ret = virSecurityDACSetOwnership(priv, NULL, input->source.evdev, user, group); + ret = virSecurityDACSetOwnership(mgr, NULL, input->source.evdev, user, group); break; case VIR_DOMAIN_INPUT_TYPE_MOUSE: @@ -1199,27 +1200,27 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, return -1; if (def->os.loader && def->os.loader->nvram && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.loader->nvram, user, group) < 0) return -1; if (def->os.kernel && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.kernel, user, group) < 0) return -1; if (def->os.initrd && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.initrd, user, group) < 0) return -1; if (def->os.dtb && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.dtb, user, group) < 0) return -1; if (def->os.slic_table && - virSecurityDACSetOwnership(priv, NULL, + virSecurityDACSetOwnership(mgr, NULL, def->os.slic_table, user, group) < 0) return -1; @@ -1242,7 +1243,7 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetImageIds(secdef, priv, &user, &group) < 0) return -1; - return virSecurityDACSetOwnership(priv, NULL, savefile, user, group); + return virSecurityDACSetOwnership(mgr, NULL, savefile, user, group); } @@ -1561,7 +1562,7 @@ virSecurityDACDomainSetPathLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) return -1; - return virSecurityDACSetOwnership(priv, NULL, path, user, group); + return virSecurityDACSetOwnership(mgr, NULL, path, user, group); } virSecurityDriver virSecurityDriverDAC = { -- 2.8.4

This is new internal class that is going to remember <device,[array of seclabels]> pairs. Moreover, it is going to be able to flush the pairs into a file so that a helper (which is introduced later in the series) can look into the file and answer question: "Is this path in use by libvirt and if so what security labels should it have?" You can say that we already have security drivers for that. And you would be right. But unfortunately on a Linux system, some processes running in it reset security labels sometimes, possibly cutting of a running domain. For instance udev. There has been a problem (race you can say), where libvirt set seclabels on a disk device, and wanted to start a domain but meanwhile udev came and restored the seclabels. With this module we can have a small helper that could be used by udev to find out what seclabels should a device have. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/Makefile.am | 1 + src/libvirt_private.syms | 4 +++ src/util/virudev.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virudev.h | 31 ++++++++++++++++++++++ 4 files changed, 104 insertions(+) create mode 100644 src/util/virudev.c create mode 100644 src/util/virudev.h diff --git a/src/Makefile.am b/src/Makefile.am index 8ee5567..2ea6f2b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -179,6 +179,7 @@ UTIL_SOURCES = \ util/virtime.h util/virtime.c \ util/virtpm.h util/virtpm.c \ util/virtypedparam.c util/virtypedparam.h \ + util/virudev.c util/virudev.h \ util/virusb.c util/virusb.h \ util/viruri.h util/viruri.c \ util/virutil.c util/virutil.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bec2628..5ae8037 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2573,6 +2573,10 @@ virTypedParamsSerialize; virTypedParamsValidate; +# util/virudev.h +virUdevMgrNew; + + # util/viruri.h virURIFormat; virURIFormatParams; diff --git a/src/util/virudev.c b/src/util/virudev.c new file mode 100644 index 0000000..66b5a58 --- /dev/null +++ b/src/util/virudev.c @@ -0,0 +1,68 @@ +/* + * virudev.c: udev rules engine + * + * Copyright (C) 2016 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, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "virudev.h" +#include "virobject.h" + +struct _virUdevMgr { + virObjectLockable parent; +}; + +static virClassPtr virUdevMgrClass; + + +static void +virUdevMgrDispose(void *obj ATTRIBUTE_UNUSED) +{ + /* nada */ +} + + +static int virUdevMgrOnceInit(void) +{ + if (!(virUdevMgrClass = virClassNew(virClassForObjectLockable(), + "virUdevMgr", + sizeof(virUdevMgr), + virUdevMgrDispose))) + return -1; + + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(virUdevMgr) + + +virUdevMgrPtr virUdevMgrNew(void) +{ + virUdevMgrPtr mgr; + + if (virUdevMgrInitialize() < 0) + return NULL; + + if (!(mgr = virObjectLockableNew(virUdevMgrClass))) + return NULL; + + return mgr; +} diff --git a/src/util/virudev.h b/src/util/virudev.h new file mode 100644 index 0000000..28e336f --- /dev/null +++ b/src/util/virudev.h @@ -0,0 +1,31 @@ +/* + * virudev.h: udev rules engine + * + * Copyright (C) 2016 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, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#ifndef __VIR_UDEV_H__ +# define __VIR_UDEV_H__ + +typedef struct _virUdevMgr virUdevMgr; +typedef virUdevMgr *virUdevMgrPtr; + +virUdevMgrPtr virUdevMgrNew(void); + +#endif -- 2.8.4

Now that we are able to store security labels for devices, next step is to flush them into a file. For more convenience I've chosen JSON format (as we have all the APIs needed for processing the format). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- po/POTFILES.in | 1 + src/libvirt_private.syms | 2 + src/util/virudev.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virudev.h | 5 ++ 4 files changed, 165 insertions(+) diff --git a/po/POTFILES.in b/po/POTFILES.in index 1469240..dabc612 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -248,6 +248,7 @@ src/util/virthreadpool.c src/util/virtime.c src/util/virtpm.c src/util/virtypedparam.c +src/util/virudev.c src/util/viruri.c src/util/virusb.c src/util/virutil.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 184317a..b465a0d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2575,6 +2575,8 @@ virTypedParamsValidate; # util/virudev.h virUdevMgrAddLabel; +virUdevMgrDumpFile; +virUdevMgrDumpStr; virUdevMgrNew; virUdevMgrRemoveAllLabels; diff --git a/src/util/virudev.c b/src/util/virudev.c index f4799e7..7f52149 100644 --- a/src/util/virudev.c +++ b/src/util/virudev.c @@ -24,7 +24,9 @@ #include "internal.h" #include "viralloc.h" +#include "virfile.h" #include "virhash.h" +#include "virjson.h" #include "virobject.h" #include "virudev.h" @@ -112,6 +114,68 @@ udevSeclabelUpdate(udevSeclabelPtr list, } +static virJSONValuePtr +udevSeclabelDump(const virSecurityDeviceLabelDef *seclabel) +{ + virJSONValuePtr object; + + if (!(object = virJSONValueNewObject()) || + virJSONValueObjectAppendString(object, "model", seclabel->model) < 0 || + virJSONValueObjectAppendString(object, "label", seclabel->label) < 0) + goto error; + + return object; + + error: + virJSONValueFree(object); + return NULL; +} + + +static int +udevSeclabelsDump(void *payload, + const void *name, + void *opaque) +{ + udevSeclabelPtr list = payload; + const char *device = name; + virJSONValuePtr seclabels = opaque; + virJSONValuePtr deviceLabels = NULL; + virJSONValuePtr deviceJSON = NULL; + size_t i; + int ret = -1; + + if (!(deviceLabels = virJSONValueNewArray())) + return ret; + + for (i = 0; i < list->nseclabels; i++) { + virJSONValuePtr seclabel = udevSeclabelDump(list->seclabels[i]); + + if (!seclabel || + virJSONValueArrayAppend(deviceLabels, seclabel) < 0) { + virJSONValueFree(seclabel); + goto cleanup; + } + } + + if (!(deviceJSON = virJSONValueNewObject()) || + virJSONValueObjectAppendString(deviceJSON, "device", device) < 0 || + virJSONValueObjectAppend(deviceJSON, "labels", deviceLabels) < 0) + goto cleanup; + deviceLabels = NULL; + + if (virJSONValueArrayAppend(seclabels, deviceJSON) < 0) + goto cleanup; + deviceJSON = NULL; + + ret = 0; + cleanup: + virJSONValueFree(deviceJSON); + virJSONValueFree(deviceLabels); + return ret; +} + + static void virUdevMgrDispose(void *obj) { @@ -202,3 +266,96 @@ virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, virObjectUnlock(mgr); return ret; } + + +static virJSONValuePtr +virUdevSeclabelDump(virUdevMgrPtr mgr) +{ + virJSONValuePtr seclabels; + + if (!(seclabels = virJSONValueNewArray())) + return NULL; + + if (virHashForEach(mgr->labels, udevSeclabelsDump, seclabels) < 0) { + virJSONValueFree(seclabels); + return NULL; + } + + return seclabels; +} + + +static char * +virUdevMgrDumpInternal(virUdevMgrPtr mgr) +{ + virJSONValuePtr object = NULL; + virJSONValuePtr child = NULL; + char *ret = NULL; + + if (!(object = virJSONValueNewObject())) + goto cleanup; + + if (!(child = virUdevSeclabelDump(mgr))) + goto cleanup; + + if (virJSONValueObjectAppend(object, "labels", child) < 0) { + virJSONValueFree(child); + goto cleanup; + } + + ret = virJSONValueToString(object, true); + cleanup: + virJSONValueFree(object); + return ret; +} + + +char * +virUdevMgrDumpStr(virUdevMgrPtr mgr) +{ + char *ret; + + virObjectLock(mgr); + ret = virUdevMgrDumpInternal(mgr); + virObjectUnlock(mgr); + return ret; +} + + +static int +virUdevMgrRewriter(int fd, void *opaque) +{ + const char *str = opaque; + + return safewrite(fd, str, strlen(str)); +} + + +int +virUdevMgrDumpFile(virUdevMgrPtr mgr, + const char *filename) +{ + int ret = -1; + char *state; + + virObjectLock(mgr); + + if (!(state = virUdevMgrDumpInternal(mgr))) + goto cleanup; + + /* Here we shouldn't use pure virFileWriteStr() as that one is not atomic. + * We can be interrupted in the middle (e.g. due to a context switch) and + * thus leave the file partially written. */ + if (virFileRewrite(filename, 0644, virUdevMgrRewriter, state) < 0) { + virReportSystemError(errno, + _("Unable to save state file %s"), + filename); + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnlock(mgr); + VIR_FREE(state); + return ret; +} diff --git a/src/util/virudev.h b/src/util/virudev.h index 692b39b..1d5a23e 100644 --- a/src/util/virudev.h +++ b/src/util/virudev.h @@ -36,4 +36,9 @@ int virUdevMgrAddLabel(virUdevMgrPtr mgr, int virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, const char *device); +char *virUdevMgrDumpStr(virUdevMgrPtr mgr); + +int virUdevMgrDumpFile(virUdevMgrPtr mgr, + const char *filename); + #endif -- 2.8.4

Now that we have the virudev module somewhat working, lets introduce some testing of it. We also need mock for virRandomBits function. Without it, the order in which entries occur in the hash table would be random and thus test would fail some times (as we expect certain ordering). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/Makefile.am | 12 +++ tests/virudevmock.c | 29 +++++++ tests/virudevtest.c | 124 ++++++++++++++++++++++++++++++ tests/virudevtestdata/complex.json | 30 ++++++++ tests/virudevtestdata/empty.json | 5 ++ tests/virudevtestdata/simple-dac.json | 13 ++++ tests/virudevtestdata/simple-selinux.json | 13 ++++ 7 files changed, 226 insertions(+) create mode 100644 tests/virudevmock.c create mode 100644 tests/virudevtest.c create mode 100644 tests/virudevtestdata/complex.json create mode 100644 tests/virudevtestdata/empty.json create mode 100644 tests/virudevtestdata/simple-dac.json create mode 100644 tests/virudevtestdata/simple-selinux.json diff --git a/tests/Makefile.am b/tests/Makefile.am index 924029a..824796b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -150,6 +150,7 @@ EXTRA_DIST = \ virpcitestdata \ virscsidata \ virsh-uriprecedence \ + virudevtestdata \ virusbtestdata \ vmwareverdata \ vmx2xmldata \ @@ -193,6 +194,7 @@ test_programs = virshtest sockettest \ vircaps2xmltest \ virnetdevtest \ virtypedparamtest \ + virudevtest \ $(NULL) if WITH_REMOTE @@ -406,6 +408,7 @@ test_libraries = libshunload.la \ virhostcpumock.la \ nssmock.la \ domaincapsmock.la \ + virudevmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -1343,6 +1346,15 @@ virtypedparamtest_SOURCES = \ virtypedparamtest.c testutils.h testutils.c virtypedparamtest_LDADD = $(LDADDS) +virudevtest_SOURCES = \ + virudevtest.c testutils.h testutils.c +virudevtest_LDADD = $(LDADDS) + +virudevmock_la_SOURCES = virudevmock.c +virudevmock_la_CFLAGS = $(AM_CFLAGS) +virudevmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virudevmock_la_LIBADD = $(MOCKLIBS_LIBS) + if WITH_LINUX fchosttest_SOURCES = \ diff --git a/tests/virudevmock.c b/tests/virudevmock.c new file mode 100644 index 0000000..d01f1c9 --- /dev/null +++ b/tests/virudevmock.c @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2016 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, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "virrandom.h" + +uint64_t virRandomBits(int nbits ATTRIBUTE_UNUSED) +{ + return 4; /* chosen by fair dice roll. + guaranteed to be random. */ +} diff --git a/tests/virudevtest.c b/tests/virudevtest.c new file mode 100644 index 0000000..883d751 --- /dev/null +++ b/tests/virudevtest.c @@ -0,0 +1,124 @@ +/* + * Copyright (C) 2016 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, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "testutils.h" +#include "virudev.h" +#include "virjson.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct testUdevData { + const char *file; + const char *const *labels; +}; + + +static int +testDump(const void *opaque) +{ + const struct testUdevData *data = opaque; + virUdevMgrPtr mgr = NULL; + int ret = -1; + const char * const *tmp; + char *state = NULL; + char *filename = NULL; + + if (virAsprintf(&filename, "%s/virudevtestdata/%s.json", + abs_srcdir, data->file) < 0) + goto cleanup; + + if (!(mgr = virUdevMgrNew())) + goto cleanup; + + tmp = data->labels; + while (*tmp) { + const char *device; + const char *model; + const char *label; + virSecurityDeviceLabelDefPtr seclabel; + + device = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + model = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + label = *tmp; + tmp++; + + if (!(seclabel = virSecurityDeviceLabelDefNewLabel(model, label))) + goto cleanup; + + if (virUdevMgrAddLabel(mgr, device, seclabel) < 0) + goto cleanup; + virSecurityDeviceLabelDefFree(seclabel); + } + + if (!(state = virUdevMgrDumpStr(mgr))) + goto cleanup; + + if (virTestCompareToFile(state, filename) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(filename); + VIR_FREE(state); + virObjectUnref(mgr); + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST_DUMP(filename, ...) \ + do { \ + const char *labels[] = {__VA_ARGS__, NULL}; \ + struct testUdevData data = { \ + .file = filename, .labels = labels, \ + }; \ + if (virTestRun("Dump " filename, testDump, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_DUMP("empty", NULL); + DO_TEST_DUMP("simple-selinux", + "/dev/sda", "selinux", "someSELinuxLabel"); + DO_TEST_DUMP("simple-dac", + "/dev/sda", "dac", "someDACLabel"); + DO_TEST_DUMP("complex", + "/dev/sda", "dac", "someDACLabel", + "/dev/sda", "selinux", "someSELinuxLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virudevmock.so") diff --git a/tests/virudevtestdata/complex.json b/tests/virudevtestdata/complex.json new file mode 100644 index 0000000..e1c8e69 --- /dev/null +++ b/tests/virudevtestdata/complex.json @@ -0,0 +1,30 @@ +{ + "labels": [ + { + "device": "/dev/sda", + "labels": [ + { + "model": "dac", + "label": "someDACLabel" + }, + { + "model": "selinux", + "label": "someSELinuxLabel" + } + ] + }, + { + "device": "/dev/sdb", + "labels": [ + { + "model": "dac", + "label": "otherDACLabel" + }, + { + "model": "selinux", + "label": "otherSELinuxLabel" + } + ] + } + ] +} diff --git a/tests/virudevtestdata/empty.json b/tests/virudevtestdata/empty.json new file mode 100644 index 0000000..763c0f2 --- /dev/null +++ b/tests/virudevtestdata/empty.json @@ -0,0 +1,5 @@ +{ + "labels": [ + + ] +} diff --git a/tests/virudevtestdata/simple-dac.json b/tests/virudevtestdata/simple-dac.json new file mode 100644 index 0000000..bbda6bb --- /dev/null +++ b/tests/virudevtestdata/simple-dac.json @@ -0,0 +1,13 @@ +{ + "labels": [ + { + "device": "/dev/sda", + "labels": [ + { + "model": "dac", + "label": "someDACLabel" + } + ] + } + ] +} diff --git a/tests/virudevtestdata/simple-selinux.json b/tests/virudevtestdata/simple-selinux.json new file mode 100644 index 0000000..7398d7f --- /dev/null +++ b/tests/virudevtestdata/simple-selinux.json @@ -0,0 +1,13 @@ +{ + "labels": [ + { + "device": "/dev/sda", + "labels": [ + { + "model": "selinux", + "label": "someSELinuxLabel" + } + ] + } + ] +} -- 2.8.4

This is the cherry on the top. For given device all its security labels are fetched. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virudev.c | 43 +++++++++++++++++++++++ src/util/virudev.h | 4 +++ tests/virudevtest.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3369600..a0de4e9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2577,6 +2577,7 @@ virTypedParamsValidate; virUdevMgrAddLabel; virUdevMgrDumpFile; virUdevMgrDumpStr; +virUdevMgrLookupLabels; virUdevMgrNew; virUdevMgrNewFromFile; virUdevMgrNewFromStr; diff --git a/src/util/virudev.c b/src/util/virudev.c index d60fbf9..9940f5f 100644 --- a/src/util/virudev.c +++ b/src/util/virudev.c @@ -300,6 +300,49 @@ virUdevMgrAddLabel(virUdevMgrPtr mgr, int +virUdevMgrLookupLabels(virUdevMgrPtr mgr, + const char *device, + virSecurityDeviceLabelDefPtr **seclabels, + size_t *nseclabels) +{ + int ret = -1; + udevSeclabelPtr list; + size_t i; + + virObjectLock(mgr); + + if (!(list = virHashLookup(mgr->labels, device))) { + *seclabels = NULL; + *nseclabels = 0; + ret = 0; + goto cleanup; + } + + if (VIR_ALLOC_N(*seclabels, list->nseclabels) < 0) + goto cleanup; + + *nseclabels = list->nseclabels; + + for (i = 0; i < list->nseclabels; i++) { + if (!((*seclabels)[i] = virSecurityDeviceLabelDefCopy(list->seclabels[i]))) + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnlock(mgr); + if (ret < 0) { + if (*seclabels) { + for (i = 0; i < *nseclabels; i++) + virSecurityDeviceLabelDefFree((*seclabels)[i]); + VIR_FREE(*seclabels); + } + } + return ret; +} + + +int virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, const char *device) { diff --git a/src/util/virudev.h b/src/util/virudev.h index 82b2b4f..4e286bb 100644 --- a/src/util/virudev.h +++ b/src/util/virudev.h @@ -37,6 +37,10 @@ int virUdevMgrAddLabel(virUdevMgrPtr mgr, const virSecurityDeviceLabelDef *seclabel); int virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr, const char *device); +int virUdevMgrLookupLabels(virUdevMgrPtr mgr, + const char *device, + virSecurityDeviceLabelDefPtr **seclabels, + size_t *nseclabels); char *virUdevMgrDumpStr(virUdevMgrPtr mgr); diff --git a/tests/virudevtest.c b/tests/virudevtest.c index c395741..36a9077 100644 --- a/tests/virudevtest.c +++ b/tests/virudevtest.c @@ -124,6 +124,76 @@ testParse(const void *opaque) static int +testLookup(const void *opaque) +{ + const struct testUdevData *data = opaque; + virUdevMgrPtr mgr = NULL; + int ret = -1; + const char * const *tmp; + char *filename = NULL; + virSecurityDeviceLabelDefPtr *seclabels = NULL; + size_t i, nseclabels = 0; + + if (virAsprintf(&filename, "%s/virudevtestdata/%s.json", + abs_srcdir, data->file) < 0) + goto cleanup; + + if (!(mgr = virUdevMgrNewFromFile(filename))) + goto cleanup; + + tmp = data->labels; + while (*tmp) { + const char *device; + const char *model; + const char *label; + + device = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + model = *tmp; + if (!++tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels list"); + goto cleanup; + } + label = *tmp; + tmp++; + + if (virUdevMgrLookupLabels(mgr, device, &seclabels, &nseclabels) < 0) + goto cleanup; + + for (i = 0; i < nseclabels; i++) { + virSecurityDeviceLabelDefPtr seclabel = seclabels[i]; + + if (STREQ(seclabel->model, model) && + STREQ(seclabel->label, label)) + break; + } + + if (i == nseclabels) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Label not found"); + goto cleanup; + } + + for (i = 0; i < nseclabels; i++) + virSecurityDeviceLabelDefFree(seclabels[i]); + VIR_FREE(seclabels); + nseclabels = 0; + } + + ret = 0; + cleanup: + for (i = 0; i < nseclabels; i++) + virSecurityDeviceLabelDefFree(seclabels[i]); + VIR_FREE(seclabels); + VIR_FREE(filename); + virObjectUnref(mgr); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -147,6 +217,16 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_LOOKUP(filename, ...) \ + do { \ + const char *labels[] = {__VA_ARGS__, NULL}; \ + struct testUdevData data = { \ + .file = filename, .labels = labels, \ + }; \ + if (virTestRun("Lookup " filename, testLookup, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST_DUMP("empty", NULL); DO_TEST_DUMP("simple-selinux", "/dev/sda", "selinux", "someSELinuxLabel"); @@ -163,6 +243,17 @@ mymain(void) DO_TEST_PARSE("simple-dac"); DO_TEST_PARSE("complex"); + DO_TEST_LOOKUP("empty", NULL); + DO_TEST_LOOKUP("simple-selinux", + "/dev/sda", "selinux", "someSELinuxLabel"); + DO_TEST_LOOKUP("simple-dac", + "/dev/sda", "dac", "someDACLabel"); + DO_TEST_LOOKUP("complex", + "/dev/sda", "dac", "someDACLabel", + "/dev/sda", "selinux", "someSELinuxLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.8.4

Whenever a security driver wants to change label of some path, it should let virUdevMgr module know so that it can update its internal database too. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 2 ++ src/security/security_dac.c | 36 ++++++++++++++++++++++++++++--- src/security/security_manager.c | 16 ++++++++++++++ src/security/security_manager.h | 5 +++++ src/security/security_selinux.c | 47 ++++++++++++++++++++++++++++++++++++++--- 5 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a0de4e9..66df1f7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1143,6 +1143,7 @@ virSecurityManagerGetModel; virSecurityManagerGetMountOptions; virSecurityManagerGetNested; virSecurityManagerGetProcessLabel; +virSecurityManagerGetUdevManager; virSecurityManagerNew; virSecurityManagerNewDAC; virSecurityManagerNewStack; @@ -1167,6 +1168,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetUdevManager; virSecurityManagerStackAddNested; virSecurityManagerVerify; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 7f17124..54e59c7 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -356,7 +356,11 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr, gid_t gid) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virUdevMgrPtr udevMgr = virSecurityManagerGetUdevManager(mgr); + virSecurityDeviceLabelDefPtr seclabel = NULL; + char *label = NULL; struct stat sb; + int ret = -1; if (!path && src && src->path && virStorageSourceIsLocalStorage(src)) @@ -365,14 +369,36 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr, if (path) { if (stat(path, &sb) < 0) { virReportSystemError(errno, _("unable to stat: %s"), path); - return -1; + return ret; } if (virSecurityDACRememberLabel(priv, path, sb.st_uid, sb.st_gid) < 0) - return -1; + return ret; + + if (udevMgr) { + if (virAsprintf(&label, "%u %u", + (unsigned int) uid, + (unsigned int) gid) < 0) + goto cleanup; + + if (!(seclabel = virSecurityDeviceLabelDefNewLabel(SECURITY_DAC_NAME, label))) + goto cleanup; + VIR_FREE(label); + } } - return virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid); + if (virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid) < 0) + goto cleanup; + + if (udevMgr && path && + virUdevMgrAddLabel(udevMgr, path, seclabel) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(label); + virSecurityDeviceLabelDefFree(seclabel); + return ret; } @@ -382,6 +408,7 @@ virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr, const char *path) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virUdevMgrPtr udevMgr = virSecurityManagerGetUdevManager(mgr); int rv; uid_t uid = 0; /* By default return to root:root */ gid_t gid = 0; @@ -399,6 +426,9 @@ virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr, return -1; if (rv > 0) return 0; + + if (udevMgr) + virUdevMgrRemoveAllLabels(udevMgr, path); } return virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index ecb4a40..2f07d6a 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -39,6 +39,7 @@ struct _virSecurityManager { virSecurityDriverPtr drv; unsigned int flags; const char *virtDriver; + virUdevMgrPtr udevMgr; void *privateData; }; @@ -1001,3 +1002,18 @@ virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr, return 0; } + + +void +virSecurityManagerSetUdevManager(virSecurityManagerPtr mgr, + virUdevMgrPtr udevMgr) +{ + mgr->udevMgr = virObjectRef(udevMgr); +} + + +virUdevMgrPtr +virSecurityManagerGetUdevManager(virSecurityManagerPtr mgr) +{ + return mgr->udevMgr; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 4cbc2d8..8f565f7 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -26,6 +26,7 @@ # include "domain_conf.h" # include "vircommand.h" # include "virstoragefile.h" +# include "virudev.h" typedef struct _virSecurityManager virSecurityManager; typedef virSecurityManager *virSecurityManagerPtr; @@ -164,4 +165,8 @@ int virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, const char *path); +void virSecurityManagerSetUdevManager(virSecurityManagerPtr mgr, + virUdevMgrPtr udevMgr); +virUdevMgrPtr virSecurityManagerGetUdevManager(virSecurityManagerPtr mgr); + #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5dad22c..c85f500 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -947,7 +947,24 @@ virSecuritySELinuxSetFileconOptional(virSecurityManagerPtr mgr, const char *path, char *tcon) { bool privileged = virSecurityManagerGetPrivileged(mgr); - return virSecuritySELinuxSetFileconHelper(path, tcon, true, privileged); + virUdevMgrPtr udevMgr = virSecurityManagerGetUdevManager(mgr); + virSecurityDeviceLabelDefPtr seclabel = NULL; + int rc; + + if (udevMgr && + !(seclabel = virSecurityDeviceLabelDefNewLabel(SECURITY_SELINUX_NAME, tcon))) + return -1; + + rc = virSecuritySELinuxSetFileconHelper(path, tcon, true, privileged); + + if (udevMgr && + virUdevMgrAddLabel(udevMgr, path, seclabel) < 0) { + virSecurityDeviceLabelDefFree(seclabel); + return -1; + } + + virSecurityDeviceLabelDefFree(seclabel); + return rc; } static int @@ -955,7 +972,24 @@ virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr, const char *path, char *tcon) { bool privileged = virSecurityManagerGetPrivileged(mgr); - return virSecuritySELinuxSetFileconHelper(path, tcon, false, privileged); + virUdevMgrPtr udevMgr = virSecurityManagerGetUdevManager(mgr); + virSecurityDeviceLabelDefPtr seclabel = NULL; + int rc; + + if (udevMgr && + !(seclabel = virSecurityDeviceLabelDefNewLabel(SECURITY_SELINUX_NAME, tcon))) + return -1; + + rc = virSecuritySELinuxSetFileconHelper(path, tcon, false, privileged); + + if (udevMgr && + virUdevMgrAddLabel(udevMgr, path, seclabel) < 0) { + virSecurityDeviceLabelDefFree(seclabel); + return -1; + } + + virSecurityDeviceLabelDefFree(seclabel); + return rc; } static int @@ -1018,6 +1052,8 @@ static int virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr, const char *path) { + virUdevMgrPtr udevMgr = virSecurityManagerGetUdevManager(mgr); + bool privileged = virSecurityManagerGetPrivileged(mgr); struct stat buf; security_context_t fcon = NULL; int rc = -1; @@ -1038,6 +1074,11 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr, goto err; } + if (udevMgr) { + virUdevMgrRemoveAllLabels(udevMgr, path); + virUdevMgrRemoveAllLabels(udevMgr, newpath); + } + if (stat(newpath, &buf) != 0) { VIR_WARN("cannot stat %s: %s", newpath, virStrerror(errno, ebuf, sizeof(ebuf))); @@ -1051,7 +1092,7 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr, VIR_WARN("cannot lookup default selinux label for %s", newpath); rc = 0; } else { - rc = virSecuritySELinuxSetFilecon(mgr, newpath, fcon); + rc = virSecuritySELinuxSetFileconHelper(newpath, fcon, false, privileged); } err: -- 2.8.4

Now that security drivers are capable of writing into virUdevMgr module, we also need it to flush its internal database right after that. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 12 +++++++++++- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_driver.c | 9 +++++++-- src/qemu/qemu_hotplug.c | 35 ++++++++++++++++++++++++++++------- src/qemu/qemu_process.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_process.h | 3 +++ 6 files changed, 96 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 838e838..4422fbc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -641,13 +641,15 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv) /* qemuDomainMasterKeyRemove: + * @driver: qemu driver data * @priv: Pointer to the domain private object * * Remove the traces of the master key, clear the heap, clear the file, * delete the file. */ void -qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) +qemuDomainMasterKeyRemove(virQEMUDriverPtr driver, + qemuDomainObjPrivatePtr priv) { char *path = NULL; @@ -660,6 +662,8 @@ qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) /* Delete the master key file */ path = qemuDomainGetMasterKeyFilePath(priv->libDir); unlink(path); + if (driver->udevMgr) + virUdevMgrRemoveAllLabels(driver->udevMgr, path); VIR_FREE(path); } @@ -4935,6 +4939,9 @@ qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver, vm->def, elem) < 0) VIR_WARN("Unable to restore security label on %s", NULLSTR(elem->path)); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + if (qemuTeardownImageCgroup(vm, elem) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(elem->path)); @@ -4974,6 +4981,9 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver, elem) < 0) goto cleanup; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; + ret = 0; cleanup: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2ee1829..a381b59 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -710,7 +710,8 @@ int qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver, int qemuDomainMasterKeyCreate(virDomainObjPtr vm); -void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv); +void qemuDomainMasterKeyRemove(virQEMUDriverPtr driver, + qemuDomainObjPrivatePtr priv); void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7dbbc25..cc34782 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3864,8 +3864,11 @@ qemuDomainScreenshot(virDomainPtr dom, endjob: VIR_FORCE_CLOSE(tmp_fd); - if (unlink_tmp) + if (unlink_tmp) { + virSecurityManagerRestoreSavedStateLabel(driver->securityManager, vm->def, tmp); + qemuProcessFlushUdev(driver); unlink(tmp); + } VIR_FREE(tmp); qemuDomainObjEndJob(driver, vm); @@ -6682,6 +6685,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager, vm->def, path) < 0) VIR_WARN("failed to restore save state label on %s", path); + qemuProcessFlushUdev(driver); virObjectUnref(cfg); return ret; } @@ -16071,7 +16075,8 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, disk) < 0 || qemuSetupDiskCgroup(vm, disk) < 0 || virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, - disk) < 0)) + disk) < 0 || + qemuProcessFlushUdev(driver) < 0)) goto cleanup; disk->src = oldsrc; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9746a06..fdac464 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -113,6 +113,9 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, vm->def, disk) < 0) goto rollback_lock; + if (qemuProcessFlushUdev(driver) < 0) + goto rollback_label; + if (qemuSetupDiskCgroup(vm, disk) < 0) goto rollback_label; @@ -130,6 +133,9 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, VIR_WARN("Unable to restore security label on %s", virDomainDiskGetSource(disk)); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + rollback_lock: if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) VIR_WARN("Unable to release lock on %s", @@ -1427,6 +1433,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, goto error; if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) teardownlabel = true; + if (qemuProcessFlushUdev(driver) < 0) + goto error; if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto error; @@ -1476,10 +1484,10 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, hostdev, NULL) < 0 || + qemuProcessFlushUdev(driver) < 0)) VIR_WARN("Unable to restore host device labelling on hotplug fail"); - if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); @@ -2262,6 +2270,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, vm->def, hostdev, NULL) < 0) goto cleanup; teardownlabel = true; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto cleanup; @@ -2289,8 +2299,9 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, hostdev, NULL) < 0 || + qemuProcessFlushUdev(driver) < 0)) VIR_WARN("Unable to restore host device labelling on hotplug fail"); if (added) qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1); @@ -2364,6 +2375,9 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, goto cleanup; teardownlabel = true; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; + if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto cleanup; @@ -2407,8 +2421,9 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, hostdev, NULL) < 0 || + qemuProcessFlushUdev(driver) < 0)) VIR_WARN("Unable to restore host device labelling on hotplug fail"); } VIR_FREE(drivealias); @@ -3315,6 +3330,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + if (qemuTeardownDiskCgroup(vm, disk) < 0) VIR_WARN("Failed to tear down cgroup for disk path %s", src); @@ -3493,6 +3511,9 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, vm->def, hostdev, NULL) < 0) VIR_WARN("Failed to restore host device labelling"); + if (qemuProcessFlushUdev(driver) < 0) + VIR_WARN("Unable to clean up udev rules"); + if (qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Failed to remove host device cgroup ACL"); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1b67aee..10f29ff 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5499,6 +5499,9 @@ qemuProcessLaunch(virConnectPtr conn, */ ret = -2; + if (qemuProcessFlushUdev(driver) < 0) + goto cleanup; + if (incoming && incoming->fd != -1) { /* if there's an fd to migrate from, and it's a pipe, put the * proper security label on it @@ -5998,10 +6001,14 @@ void qemuProcessStop(virQEMUDriverPtr driver, } /* Remove the master key */ - qemuDomainMasterKeyRemove(priv); + qemuDomainMasterKeyRemove(driver, priv); virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); + if (driver->udevMgr) { + virUdevMgrRemoveAllLabels(driver->udevMgr, priv->libDir); + virUdevMgrRemoveAllLabels(driver->udevMgr, priv->channelTargetDir); + } qemuDomainClearPrivatePaths(vm); @@ -6033,10 +6040,12 @@ void qemuProcessStop(virQEMUDriverPtr driver, } /* Reset Security Labels unless caller don't want us to */ - if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL)) + if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL)) { virSecurityManagerRestoreAllLabel(driver->securityManager, vm->def, !!(flags & VIR_QEMU_PROCESS_STOP_MIGRATED)); + qemuProcessFlushUdev(driver); + } virSecurityManagerReleaseLabel(driver->securityManager, vm->def); for (i = 0; i < vm->def->ndisks; i++) { @@ -6587,3 +6596,37 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver, virHashFree(table); return ret; } + + +char * +qemuProcessGetUdevPath(virQEMUDriverPtr driver) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + char *file; + + ignore_value(virAsprintf(&file, "%s/devices.udev", cfg->stateDir)); + virObjectUnref(cfg); + return file; +} + + +int +qemuProcessFlushUdev(virQEMUDriverPtr driver) +{ + char *file = NULL; + int ret = -1; + + if (!driver->udevMgr) + return 0; + + if (!(file = qemuProcessGetUdevPath(driver))) + goto cleanup; + + if (virUdevMgrDumpFile(driver->udevMgr, file) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(file); + return ret; +} diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 21f3b0c..8d1e937 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -191,4 +191,7 @@ int qemuProcessRefreshDisks(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); +char * qemuProcessGetUdevPath(virQEMUDriverPtr driver); +int qemuProcessFlushUdev(virQEMUDriverPtr driver); + #endif /* __QEMU_PROCESS_H__ */ -- 2.8.4

In some cases callers might want to filter what devices are stored in this module (esp. when used in combination with udev who cares about nothing but "/dev/" prefixed paths). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virudev.c | 25 ++++++++++++++++++++++ src/util/virudev.h | 13 ++++++++++++ tests/virudevtest.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 66df1f7..60dc16b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2584,6 +2584,7 @@ virUdevMgrNew; virUdevMgrNewFromFile; virUdevMgrNewFromStr; virUdevMgrRemoveAllLabels; +virUdevMgrSetFilter; # util/viruri.h diff --git a/src/util/virudev.c b/src/util/virudev.c index 9940f5f..c17682c 100644 --- a/src/util/virudev.c +++ b/src/util/virudev.c @@ -36,6 +36,9 @@ struct _virUdevMgr { virObjectLockable parent; virHashTablePtr labels; + + virUdevMgrFilter filter; + void *opaque; }; struct _udevSeclabel { @@ -274,6 +277,17 @@ virUdevMgrAddLabel(virUdevMgrPtr mgr, virObjectLock(mgr); + if (mgr->filter) { + int rc = mgr->filter(device, seclabel, mgr->opaque); + if (rc < 0) + goto cleanup; + if (rc == 0) { + /* Claim success. */ + ret = 0; + goto cleanup; + } + } + if ((list = virHashLookup(mgr->labels, device))) { virSecurityDeviceLabelDefPtr entry = udevSeclabelFindByModel(list, seclabel->model); @@ -561,3 +575,14 @@ virUdevMgrNewFromFile(const char *filename) VIR_FREE(state); return NULL; } + +void +virUdevMgrSetFilter(virUdevMgrPtr mgr, + virUdevMgrFilter filter, + void *opaque) +{ + virObjectLock(mgr); + mgr->filter = filter; + mgr->opaque = opaque; + virObjectUnlock(mgr); +} diff --git a/src/util/virudev.h b/src/util/virudev.h index 4e286bb..2f035e3 100644 --- a/src/util/virudev.h +++ b/src/util/virudev.h @@ -28,6 +28,15 @@ typedef struct _virUdevMgr virUdevMgr; typedef virUdevMgr *virUdevMgrPtr; +/* Filter some devices out in virUdevMgrAddLabel. + * Return 0 to NOT record label for device, + * 1 to record the label for device, + * -1 on error. + */ +typedef int (*virUdevMgrFilter)(const char *device, + const virSecurityDeviceLabelDef *seclabel, + void *opaque); + virUdevMgrPtr virUdevMgrNew(void); virUdevMgrPtr virUdevMgrNewFromStr(const char *str); virUdevMgrPtr virUdevMgrNewFromFile(const char *filename); @@ -47,4 +56,8 @@ char *virUdevMgrDumpStr(virUdevMgrPtr mgr); int virUdevMgrDumpFile(virUdevMgrPtr mgr, const char *filename); +void virUdevMgrSetFilter(virUdevMgrPtr mgr, + virUdevMgrFilter filter, + void *opaque); + #endif diff --git a/tests/virudevtest.c b/tests/virudevtest.c index 36a9077..cd5d136 100644 --- a/tests/virudevtest.c +++ b/tests/virudevtest.c @@ -29,6 +29,7 @@ struct testUdevData { const char *file; const char *const *labels; + virUdevMgrFilter filter; }; @@ -49,6 +50,8 @@ testDump(const void *opaque) if (!(mgr = virUdevMgrNew())) goto cleanup; + virUdevMgrSetFilter(mgr, data->filter, NULL); + tmp = data->labels; while (*tmp) { const char *device; @@ -194,6 +197,25 @@ testLookup(const void *opaque) static int +filterAll(const char *device ATTRIBUTE_UNUSED, + const virSecurityDeviceLabelDef *seclabel ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int +filterAllowSda(const char *device, + const virSecurityDeviceLabelDef *seclabel ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + return STRPREFIX(device, "/dev/sda") ? 1 : 0; +} + + + +static int mymain(void) { int ret = 0; @@ -202,7 +224,7 @@ mymain(void) do { \ const char *labels[] = {__VA_ARGS__, NULL}; \ struct testUdevData data = { \ - .file = filename, .labels = labels, \ + .file = filename, .labels = labels, .filter = NULL, \ }; \ if (virTestRun("Dump " filename, testDump, &data) < 0) \ ret = -1; \ @@ -227,6 +249,17 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_FILTER(filename, fltr, ...) \ + do { \ + const char *labels[] = {__VA_ARGS__, NULL}; \ + struct testUdevData data = { \ + .file = filename, .labels = labels, .filter = fltr, \ + }; \ + if (virTestRun("Filter " filename, testDump, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_DUMP("empty", NULL); DO_TEST_DUMP("simple-selinux", "/dev/sda", "selinux", "someSELinuxLabel"); @@ -254,6 +287,25 @@ mymain(void) "/dev/sdb", "dac", "otherDACLabel", "/dev/sdb", "selinux", "otherSELinuxLabel"); + DO_TEST_FILTER("empty", filterAll, + "/dev/sda", "dac", "someDACLabel", + "/dev/sda", "selinux", "someSELinuxLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + DO_TEST_FILTER("simple-selinux", filterAllowSda, + "/dev/sda", "selinux", "someSELinuxLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + DO_TEST_FILTER("simple-dac", filterAllowSda, + "/dev/sda", "dac", "someDACLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + DO_TEST_FILTER("complex", NULL, + "/dev/sda", "dac", "someDACLabel", + "/dev/sda", "selinux", "someSELinuxLabel", + "/dev/sdb", "dac", "otherDACLabel", + "/dev/sdb", "selinux", "otherSELinuxLabel"); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.8.4