[libvirt] [PATCH v1 1/2] security_dac: Keep original file label
Michal Privoznik
mprivozn at redhat.com
Thu Mar 13 09:02:44 UTC 2014
In the DAC security driver chown() is executed in order to allow
a domain accessing certain files. Later, when the domain is
shutting down, we are chown()-ing the file back. But we don't
remember the original owner anywhere, so we return the file to
root:root and hope for the best.
In this approach, a new file is created. It's in XML, so whenever
we want to extend it, we can. In the file, there's this structure:
<labels>
<label path='/home/zippy/work/tmp/gentoo.qcow2' uid='1000' gid='1000' refcount='1'/>
...
</labels>
The first chown() appends new <label/> element, each subsequent
increments the @refcount attribute. The file is parsed and
formatted prior to each call of chown(). It may seem like
overhead right now, but with locking introduction (not done here
though) we may allow other libvirtds to share the status.
Then, whenever we want to restore the DAC label, the corresponding
refcount is decremented. If the value is greater than zero, no
restoring is done (other domains using the file may lose access).
Therefore the original label is restored only if refcount hits value
of zero. Then it's removed from the label store file too.
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
src/security/security_dac.c | 335 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 295 insertions(+), 40 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85e6eec..b119825 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -34,6 +34,7 @@
#include "virstoragefile.h"
#include "virstring.h"
#include "virutil.h"
+#include "configmake.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
#define SECURITY_DAC_NAME "dac"
@@ -48,8 +49,140 @@ struct _virSecurityDACData {
int ngroups;
bool dynamicOwnership;
char *baselabel;
+ char *labelStore;
};
+typedef struct _virDACLabel virDACLabel;
+typedef virDACLabel *virDACLabelPtr;
+
+struct _virDACLabel {
+ char *path;
+ uid_t uid;
+ gid_t gid;
+ unsigned int refcount;
+};
+
+static void
+virSecurityDACLabelStoreFree(virDACLabelPtr labels,
+ size_t labels_size)
+{
+ size_t i;
+
+ for (i = 0; i < labels_size; i++) {
+ VIR_FREE(labels->path);
+ }
+ VIR_FREE(labels);
+}
+
+static int
+virSecurityDACLabelStoreParse(virSecurityDACDataPtr priv,
+ virDACLabelPtr *labels,
+ size_t *labels_size)
+{
+ int n = 0, ret = -1;
+ size_t i;
+ xmlDocPtr doc;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr *nodes = NULL;
+
+ if (!virFileExists(priv->labelStore)) {
+ *labels = NULL;
+ *labels_size = 0;
+ return 0;
+ }
+
+ if (!(doc = virXMLParseFileCtxt(priv->labelStore, &ctxt)))
+ goto cleanup;
+
+ n = virXPathNodeSet("./label", ctxt, &nodes);
+
+ *labels = NULL;
+ if (n && VIR_ALLOC_N(*labels, n) < 0)
+ goto cleanup;
+
+ for (i = 0; i < n; i++) {
+ virDACLabelPtr label = &(*labels)[i];
+ xmlNodePtr node = nodes[i];
+
+ ctxt->node = node;
+ if (!(label->path = virXPathString("string(./@path)", ctxt)))
+ goto cleanup;
+ if (virXPathUInt("string(./@uid)", ctxt, &label->uid) < 0)
+ goto cleanup;
+ if (virXPathUInt("string(./@gid)", ctxt, &label->gid) < 0)
+ goto cleanup;
+ if (virXPathUInt("string(./@refcount)", ctxt, &label->refcount) < 0)
+ goto cleanup;
+ }
+
+ *labels_size = n;
+ ret = 0;
+cleanup:
+ if (ret < 0)
+ virSecurityDACLabelStoreFree(*labels, n);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(doc);
+ return ret;
+}
+
+static virDACLabelPtr
+virSecurityDACLabelStoreFind(virDACLabelPtr labels,
+ size_t labels_size,
+ const char *path)
+{
+ size_t i;
+
+ for (i = 0; i < labels_size; i++) {
+ if (STREQ(labels[i].path, path))
+ return &labels[i];
+ }
+ return NULL;
+}
+
+static int
+virSecurityDACLabelStoreFormat(virSecurityDACDataPtr priv,
+ virDACLabelPtr labels,
+ size_t labels_size)
+{
+ int ret = -1;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *xml = NULL;
+ size_t i;
+
+ virBufferAddLit(&buf, "<labels>\n");
+ virBufferAdjustIndent(&buf, 2);
+ for (i = 0; i < labels_size; i++) {
+ virDACLabelPtr label = &labels[i];
+ if (label->refcount == 0)
+ continue;
+ virBufferAsprintf(&buf,
+ "<label path='%s' uid='%u' gid='%u' refcount='%u'/>\n",
+ label->path, label->uid, label->gid, label->refcount);
+ }
+ virBufferAdjustIndent(&buf, -2);
+ virBufferAddLit(&buf, "</labels>\n");
+
+ if (virBufferError(&buf))
+ goto cleanup;
+
+ xml = virBufferContentAndReset(&buf);
+
+ if (!xml)
+ goto cleanup;
+
+ if (virFileWriteStr(priv->labelStore, xml, 0600) < 0) {
+ virReportSystemError(errno,
+ _("unable to write label store file: %s"),
+ priv->labelStore);
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ VIR_FREE(xml);
+ return ret;
+}
+
/* returns -1 on error, 0 on success */
int
virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
@@ -210,8 +343,13 @@ virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED)
}
static int
-virSecurityDACOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+virSecurityDACOpen(virSecurityManagerPtr mgr)
{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+ if (VIR_STRDUP(priv->labelStore, LOCALSTATEDIR "/run/libvirt/labelStore.xml") < 0)
+ return -1;
+
return 0;
}
@@ -221,6 +359,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr)
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
VIR_FREE(priv->groups);
VIR_FREE(priv->baselabel);
+ VIR_FREE(priv->labelStore);
return 0;
}
@@ -253,7 +392,98 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr)
}
static int
-virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
+virSecurityDACRememberLabel(virSecurityManagerPtr mgr,
+ const char *path)
+{
+ int ret = -1;
+ virDACLabelPtr labels = NULL, label;
+ size_t labels_size = 0;
+ struct stat sb;
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+ /* TODO lock the labelStore file */
+ if (virSecurityDACLabelStoreParse(priv, &labels, &labels_size) < 0)
+ goto cleanup;
+
+ label = virSecurityDACLabelStoreFind(labels, labels_size, path);
+
+ if (!label) {
+ if (VIR_REALLOC_N(labels, labels_size + 1) < 0)
+ goto cleanup;
+
+ label = &labels[labels_size++];
+ if (VIR_STRDUP(label->path, path) < 0)
+ goto cleanup;
+
+ if (stat(path, &sb) < 0) {
+ virReportSystemError(errno,
+ _("unable to stat: %s"),
+ path);
+ goto cleanup;
+ }
+ label->uid = sb.st_uid;
+ label->gid = sb.st_gid;
+ label->refcount = 0;
+ }
+
+ label->refcount++;
+
+ if (virSecurityDACLabelStoreFormat(priv, labels, labels_size) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ /* TODO unlock the labelStore file */
+ virSecurityDACLabelStoreFree(labels, labels_size);
+ return ret;
+}
+
+static int
+virSecurityDACRecallLabel(virSecurityManagerPtr mgr,
+ const char *path,
+ uid_t *uid,
+ gid_t *gid,
+ unsigned int *refcount)
+{
+ int ret = -1;
+ virDACLabelPtr labels = NULL, label;
+ size_t labels_size = 0;
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+ /* TODO lock the labelStore file */
+ if (virSecurityDACLabelStoreParse(priv, &labels, &labels_size) < 0)
+ goto cleanup;
+
+ label = virSecurityDACLabelStoreFind(labels, labels_size, path);
+
+ if (!label) {
+ /* It's not an error if @path wasn't found */
+ VIR_DEBUG("path %s wasn't found in labelStore %s, using defaults",
+ path, priv->labelStore);
+ *uid = *gid = *refcount = 0;
+ ret = 0;
+ goto cleanup;
+ }
+
+ *refcount = --label->refcount;
+
+ if (!label->refcount) {
+ *uid = label->uid;
+ *gid = label->gid;
+ }
+
+ if (virSecurityDACLabelStoreFormat(priv, labels, labels_size) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ /* TODO unlock the labelStore file */
+ virSecurityDACLabelStoreFree(labels, labels_size);
+ return ret;
+}
+
+static int
+virSecurityDACChown(const char *path, uid_t uid, gid_t gid)
{
VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
path, (long) uid, (long) gid);
@@ -294,29 +524,53 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
}
static int
-virSecurityDACRestoreSecurityFileLabel(const char *path)
+virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
+ const char *path,
+ uid_t uid,
+ gid_t gid)
{
- struct stat buf;
- int rc = -1;
+ int ret = -1;
+
+ if (virSecurityDACRememberLabel(mgr, path) < 0)
+ goto cleanup;
+
+ ret = virSecurityDACChown(path, uid, gid);
+cleanup:
+ return ret;
+}
+
+static int
+virSecurityDACRestoreSecurityFileLabel(virSecurityManagerPtr mgr,
+ const char *path)
+{
+ int ret = -1;
char *newpath = NULL;
-
- VIR_INFO("Restoring DAC user and group on '%s'", path);
+ uid_t uid;
+ gid_t gid;
+ unsigned int refcount;
if (virFileResolveLink(path, &newpath) < 0) {
virReportSystemError(errno,
_("cannot resolve symlink %s"), path);
- goto err;
+ goto cleanup;
}
- if (stat(newpath, &buf) != 0)
- goto err;
+ if (virSecurityDACRecallLabel(mgr, path, &uid, &gid, &refcount) < 0)
+ goto cleanup;
- /* XXX record previous ownership */
- rc = virSecurityDACSetOwnership(newpath, 0, 0);
+ if (refcount > 0) {
+ VIR_DEBUG("Not restoring label on path %s refcount=%u", path, refcount);
+ } else {
+ VIR_INFO("Restoring DAC user and group on '%s'", path);
+ if (virSecurityDACChown(newpath, uid, gid) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
-err:
+cleanup:
VIR_FREE(newpath);
- return rc;
+ return ret;
}
@@ -336,7 +590,7 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
if (virSecurityDACGetImageIds(def, priv, &user, &group))
return -1;
- return virSecurityDACSetOwnership(path, user, group);
+ return virSecurityDACSetOwnership(mgr, path, user, group);
}
@@ -408,7 +662,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
}
}
- return virSecurityDACRestoreSecurityFileLabel(disk->src);
+ return virSecurityDACRestoreSecurityFileLabel(mgr, disk->src);
}
@@ -435,7 +689,7 @@ virSecurityDACSetSecurityHostdevLabelHelper(const char *file,
if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL))
return -1;
- return virSecurityDACSetOwnership(file, user, group);
+ return virSecurityDACSetOwnership(mgr, file, user, group);
}
@@ -563,27 +817,27 @@ done:
static int
virSecurityDACRestoreSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED,
const char *file,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
- return virSecurityDACRestoreSecurityFileLabel(file);
+ return virSecurityDACRestoreSecurityFileLabel(opaque, file);
}
static int
virSecurityDACRestoreSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
const char *file,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
- return virSecurityDACRestoreSecurityFileLabel(file);
+ return virSecurityDACRestoreSecurityFileLabel(opaque, file);
}
static int
virSecurityDACRestoreSecuritySCSILabel(virSCSIDevicePtr dev ATTRIBUTE_UNUSED,
const char *file,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
- return virSecurityDACRestoreSecurityFileLabel(file);
+ return virSecurityDACRestoreSecurityFileLabel(opaque, file);
}
@@ -696,7 +950,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
switch (dev->type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
- ret = virSecurityDACSetOwnership(dev->data.file.path, user, group);
+ ret = virSecurityDACSetOwnership(mgr, dev->data.file.path, user, group);
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
@@ -704,11 +958,11 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
(virAsprintf(&out, "%s.out", dev->data.file.path) < 0))
goto done;
if (virFileExists(in) && virFileExists(out)) {
- if ((virSecurityDACSetOwnership(in, user, group) < 0) ||
- (virSecurityDACSetOwnership(out, user, group) < 0)) {
+ if ((virSecurityDACSetOwnership(mgr, in, user, group) < 0) ||
+ (virSecurityDACSetOwnership(mgr, out, user, group) < 0)) {
goto done;
}
- } else if (virSecurityDACSetOwnership(dev->data.file.path,
+ } else if (virSecurityDACSetOwnership(mgr, dev->data.file.path,
user, group) < 0) {
goto done;
}
@@ -727,7 +981,7 @@ done:
}
static int
-virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
virDomainChrSourceDefPtr dev)
{
char *in = NULL, *out = NULL;
@@ -736,7 +990,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
switch (dev->type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
- ret = virSecurityDACRestoreSecurityFileLabel(dev->data.file.path);
+ ret = virSecurityDACRestoreSecurityFileLabel(mgr, dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
@@ -744,11 +998,12 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
(virAsprintf(&in, "%s.in", dev->data.file.path) < 0))
goto done;
if (virFileExists(in) && virFileExists(out)) {
- if ((virSecurityDACRestoreSecurityFileLabel(out) < 0) ||
- (virSecurityDACRestoreSecurityFileLabel(in) < 0)) {
+ if ((virSecurityDACRestoreSecurityFileLabel(mgr, out) < 0) ||
+ (virSecurityDACRestoreSecurityFileLabel(mgr, in) < 0)) {
goto done;
}
- } else if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path) < 0) {
+ } else if (virSecurityDACRestoreSecurityFileLabel(mgr,
+ dev->data.file.path) < 0) {
goto done;
}
ret = 0;
@@ -860,15 +1115,15 @@ virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
}
if (def->os.kernel &&
- virSecurityDACRestoreSecurityFileLabel(def->os.kernel) < 0)
+ virSecurityDACRestoreSecurityFileLabel(mgr, def->os.kernel) < 0)
rc = -1;
if (def->os.initrd &&
- virSecurityDACRestoreSecurityFileLabel(def->os.initrd) < 0)
+ virSecurityDACRestoreSecurityFileLabel(mgr, def->os.initrd) < 0)
rc = -1;
if (def->os.dtb &&
- virSecurityDACRestoreSecurityFileLabel(def->os.dtb) < 0)
+ virSecurityDACRestoreSecurityFileLabel(mgr, def->os.dtb) < 0)
rc = -1;
return rc;
@@ -933,15 +1188,15 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
return -1;
if (def->os.kernel &&
- virSecurityDACSetOwnership(def->os.kernel, user, group) < 0)
+ virSecurityDACSetOwnership(mgr, def->os.kernel, user, group) < 0)
return -1;
if (def->os.initrd &&
- virSecurityDACSetOwnership(def->os.initrd, user, group) < 0)
+ virSecurityDACSetOwnership(mgr, def->os.initrd, user, group) < 0)
return -1;
if (def->os.dtb &&
- virSecurityDACSetOwnership(def->os.dtb, user, group) < 0)
+ virSecurityDACSetOwnership(mgr, def->os.dtb, user, group) < 0)
return -1;
return 0;
@@ -960,7 +1215,7 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr,
if (virSecurityDACGetImageIds(def, priv, &user, &group))
return -1;
- return virSecurityDACSetOwnership(savefile, user, group);
+ return virSecurityDACSetOwnership(mgr, savefile, user, group);
}
@@ -974,7 +1229,7 @@ virSecurityDACRestoreSavedStateLabel(virSecurityManagerPtr mgr,
if (!priv->dynamicOwnership)
return 0;
- return virSecurityDACRestoreSecurityFileLabel(savefile);
+ return virSecurityDACRestoreSecurityFileLabel(mgr, savefile);
}
--
1.9.0
More information about the libvir-list
mailing list