[libvirt] [PATCH 0/2] Restore DAC labels
If libvirt is doing labeling on a domain startup, the original owner of files is not remembered. So later, when the domain is shutting down and re-labelling is done, we have no other option, just to fall back to 0:0. These patches are solving this issue for DAC driver. I am sending them just to know if the path I wen through is right so I don't bother with selinux if it is not. Michal Privoznik (2): conf: Add oldlabel field to virSecurityDeviceLabelDef security driver: Remember the original DAC label src/conf/domain_conf.c | 20 ++- src/conf/domain_conf.h | 1 + src/security/security_dac.c | 340 +++++++++++++++++++++++++++++++------------- 3 files changed, 260 insertions(+), 101 deletions(-) -- 1.8.0.2
The field is there to store the original label of device, so we can restore it when domain is shutting down. --- src/conf/domain_conf.c | 20 +++++++++++++++----- src/conf/domain_conf.h | 1 + 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7a2b012..d83330a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -957,6 +957,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def) return; VIR_FREE(def->model); VIR_FREE(def->label); + VIR_FREE(def->oldlabel); VIR_FREE(def); } @@ -3639,14 +3640,15 @@ static int virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn, size_t *nseclabels_rtn, virSecurityLabelDefPtr *vmSeclabels, - int nvmSeclabels, xmlXPathContextPtr ctxt) + int nvmSeclabels, xmlXPathContextPtr ctxt, + unsigned int flags) { virSecurityDeviceLabelDefPtr *seclabels; size_t nseclabels = 0; int n, i, j; xmlNodePtr *list = NULL; virSecurityLabelDefPtr vmDef = NULL; - char *model, *relabel, *label; + char *model, *relabel, *label, *oldlabel; if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) < 0) goto error; @@ -3717,6 +3719,13 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn, NULLSTR(seclabels[i]->model)); goto error; } + + /* only parse oldlabel when parsing domain status XML */ + if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) { + oldlabel = virXPathStringLimit("string(./oldlabel)", + VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + seclabels[i]->oldlabel = oldlabel; + } } VIR_FREE(list); @@ -4299,7 +4308,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, &def->nseclabels, vmSeclabels, nvmSeclabels, - ctxt) < 0) + ctxt, flags) < 0) goto error; ctxt->node = saved_node; } @@ -5926,7 +5935,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, &chr_def->nseclabels, vmSeclabels, nvmSeclabels, - ctxt) < 0) { + ctxt, flags) < 0) { ctxt->node = saved_node; goto error; } @@ -12344,10 +12353,11 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes"); - if (def->label) { + if (def->label || def->oldlabel) { virBufferAddLit(buf, ">\n"); virBufferEscapeString(buf, " <label>%s</label>\n", def->label); + virBufferEscapeString(buf, " <oldlabel>%s</oldlabel>\n", def->oldlabel); virBufferAddLit(buf, "</seclabel>\n"); } else { virBufferAddLit(buf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9232ff9..b7f4b38 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -327,6 +327,7 @@ typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr; struct _virSecurityDeviceLabelDef { char *model; char *label; /* image label string */ + char *oldlabel; /* the original label to return to */ bool norelabel; }; -- 1.8.0.2
On Mon, Feb 18, 2013 at 12:29:03PM +0100, Michal Privoznik wrote:
The field is there to store the original label of device, so we can restore it when domain is shutting down. --- src/conf/domain_conf.c | 20 +++++++++++++++----- src/conf/domain_conf.h | 1 + 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7a2b012..d83330a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -957,6 +957,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def) return; VIR_FREE(def->model); VIR_FREE(def->label); + VIR_FREE(def->oldlabel); VIR_FREE(def); }
@@ -3639,14 +3640,15 @@ static int virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn, size_t *nseclabels_rtn, virSecurityLabelDefPtr *vmSeclabels, - int nvmSeclabels, xmlXPathContextPtr ctxt) + int nvmSeclabels, xmlXPathContextPtr ctxt, + unsigned int flags) { virSecurityDeviceLabelDefPtr *seclabels; size_t nseclabels = 0; int n, i, j; xmlNodePtr *list = NULL; virSecurityLabelDefPtr vmDef = NULL; - char *model, *relabel, *label; + char *model, *relabel, *label, *oldlabel;
if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) < 0) goto error; @@ -3717,6 +3719,13 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn, NULLSTR(seclabels[i]->model)); goto error; } + + /* only parse oldlabel when parsing domain status XML */ + if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) { + oldlabel = virXPathStringLimit("string(./oldlabel)", + VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + seclabels[i]->oldlabel = oldlabel; + } } VIR_FREE(list);
@@ -4299,7 +4308,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, &def->nseclabels, vmSeclabels, nvmSeclabels, - ctxt) < 0) + ctxt, flags) < 0) goto error; ctxt->node = saved_node; } @@ -5926,7 +5935,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, &chr_def->nseclabels, vmSeclabels, nvmSeclabels, - ctxt) < 0) { + ctxt, flags) < 0) { ctxt->node = saved_node; goto error; } @@ -12344,10 +12353,11 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes");
- if (def->label) { + if (def->label || def->oldlabel) { virBufferAddLit(buf, ">\n"); virBufferEscapeString(buf, " <label>%s</label>\n", def->label); + virBufferEscapeString(buf, " <oldlabel>%s</oldlabel>\n", def->oldlabel); virBufferAddLit(buf, "</seclabel>\n"); } else { virBufferAddLit(buf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9232ff9..b7f4b38 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -327,6 +327,7 @@ typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr; struct _virSecurityDeviceLabelDef { char *model; char *label; /* image label string */ + char *oldlabel; /* the original label to return to */ bool norelabel; };
This is storing driver specific state in the XML configuration description which is bad practice in general. Any such state should be maintained inside the DAC driver itself. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 02/18/2013 12:38 PM, Daniel P. Berrange wrote:
On Mon, Feb 18, 2013 at 12:29:03PM +0100, Michal Privoznik wrote:
The field is there to store the original label of device, so we can restore it when domain is shutting down. --- src/conf/domain_conf.c | 20 +++++++++++++++----- src/conf/domain_conf.h | 1 + 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7a2b012..d83330a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -957,6 +957,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def) return; VIR_FREE(def->model); VIR_FREE(def->label); + VIR_FREE(def->oldlabel); VIR_FREE(def); }
@@ -3639,14 +3640,15 @@ static int virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn, size_t *nseclabels_rtn, virSecurityLabelDefPtr *vmSeclabels, - int nvmSeclabels, xmlXPathContextPtr ctxt) + int nvmSeclabels, xmlXPathContextPtr ctxt, + unsigned int flags) { virSecurityDeviceLabelDefPtr *seclabels; size_t nseclabels = 0; int n, i, j; xmlNodePtr *list = NULL; virSecurityLabelDefPtr vmDef = NULL; - char *model, *relabel, *label; + char *model, *relabel, *label, *oldlabel;
if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) < 0) goto error; @@ -3717,6 +3719,13 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn, NULLSTR(seclabels[i]->model)); goto error; } + + /* only parse oldlabel when parsing domain status XML */ + if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) { + oldlabel = virXPathStringLimit("string(./oldlabel)", + VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + seclabels[i]->oldlabel = oldlabel; + } } VIR_FREE(list);
@@ -4299,7 +4308,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, &def->nseclabels, vmSeclabels, nvmSeclabels, - ctxt) < 0) + ctxt, flags) < 0) goto error; ctxt->node = saved_node; } @@ -5926,7 +5935,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, &chr_def->nseclabels, vmSeclabels, nvmSeclabels, - ctxt) < 0) { + ctxt, flags) < 0) { ctxt->node = saved_node; goto error; } @@ -12344,10 +12353,11 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes");
- if (def->label) { + if (def->label || def->oldlabel) { virBufferAddLit(buf, ">\n"); virBufferEscapeString(buf, " <label>%s</label>\n", def->label);
Unnecessary label when def->label == NULL but def->oldlabel is specified (is this the case when seclabel is not specified, but we relabel some files so we need to store the old seclabel?).
+ virBufferEscapeString(buf, " <oldlabel>%s</oldlabel>\n", def->oldlabel); virBufferAddLit(buf, "</seclabel>\n"); } else { virBufferAddLit(buf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9232ff9..b7f4b38 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -327,6 +327,7 @@ typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr; struct _virSecurityDeviceLabelDef { char *model; char *label; /* image label string */ + char *oldlabel; /* the original label to return to */ bool norelabel; };
This is storing driver specific state in the XML configuration description which is bad practice in general. Any such state should be maintained inside the DAC driver itself.
I'm not sure how Michal wants to continue with this, but I'd say it's pretty reasonable if all drivers understand this element (even though it will not get to another driver thanks to the 'model' attribute) as I was under the impression that Michal wants to continue with selinux and other labels as well. Since this is saved only in the state XML and not parsed when not understood, it solves the problem with libvirt being restarted and doesn't make any other libvirt versions fail with such XML. And it gets transferred when migrating. Martin
Currently, on domain startup a labeling of domain resources (e.g. disk images, sockets, ...) is done. However, we don't store the original owner anywhere. So when domain is shutting down, we cannot restore the original owner of files we have chown()-ed. This patch resolves this issue for DAC driver. --- src/security/security_dac.c | 340 +++++++++++++++++++++++++++++++------------- 1 file changed, 244 insertions(+), 96 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index b115bb0..c0ec707 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -265,50 +265,185 @@ static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNU } static int -virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) +virSecurityDACAddOldlabel(virSecurityDeviceLabelDefPtr label, + uid_t user, gid_t group) { - VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", - path, (long) uid, (long) gid); + int ret = -1; + char *userStr = NULL, *groupStr = NULL; - if (chown(path, uid, gid) < 0) { - struct stat sb; - int chown_errno = errno; - - if (stat(path, &sb) >= 0) { - if (sb.st_uid == uid && - sb.st_gid == gid) { - /* It's alright, there's nothing to change anyway. */ - return 0; + userStr = virGetUserName(user); + groupStr = virGetGroupName(group); + + if ((!userStr && (virAsprintf(&userStr, "+%ld", (long) user) < 0)) || + (!groupStr && (virAsprintf(&groupStr, "+%ld", (long) group) < 0)) || + (virAsprintf(&label->oldlabel, "%s:%s", userStr, groupStr) < 0)) { + virReportOOMError(); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(groupStr); + VIR_FREE(userStr); + return ret; +} + +static int +virSecurityDACSaveOwnership(virSecurityDeviceLabelDefPtr **labels, + size_t *nseclabels, + uid_t user, gid_t group) +{ + int ret = -1; + size_t i; + + if ((user == (uid_t) -1) && + (group == (gid_t) -1)) { + /* nothing to save */ + return 0; + } + + /* find DAC seclabel or create one */ + for (i = 0; i < *nseclabels; i++) { + virSecurityDeviceLabelDefPtr label = (*labels)[i]; + + if (STREQ(label->model, SECURITY_DAC_NAME)) + return virSecurityDACAddOldlabel(label, user, group); + } + + if ((VIR_REALLOC_N(*labels, *nseclabels + 1) < 0) || + (VIR_ALLOC((*labels)[*nseclabels]) < 0)) { + virReportOOMError(); + goto cleanup; + } + + if (!((*labels)[*nseclabels]->model = strdup(SECURITY_DAC_NAME))) { + VIR_FREE((*labels)[*nseclabels]); + virReportOOMError(); + goto cleanup; + } + + if (virSecurityDACAddOldlabel((*labels)[*nseclabels], user, group) < 0) { + VIR_FREE((*labels)[*nseclabels]->model); + VIR_FREE((*labels)[*nseclabels]); + if (VIR_REALLOC_N(*labels, *nseclabels) < 0) { + /* ignore harmless */ + } + goto cleanup; + } + + (*nseclabels)++; + + ret = 0; + +cleanup: + return ret; +} + +static int +virSecurityDACGetOriginalOwnerwhip(virSecurityDeviceLabelDefPtr **labels, + size_t *nseclabels, + uid_t *user, gid_t *group) +{ + int ret; + size_t i; + + for (i = 0; i < *nseclabels; i++) { + virSecurityDeviceLabelDefPtr label = (*labels)[i]; + + if (STREQ(label->model, SECURITY_DAC_NAME) && label->oldlabel) { + /* steal oldlabel */ + char *oldlabel = label->oldlabel; + label->oldlabel = NULL; + + /* Moreover, if this is just a dummy seclabel to hold just the + * oldlabel, it is the one created by virSecurityDACSaveOwnership + * so clear it out */ + if (!label->label) { + VIR_FREE(label); + if (*nseclabels > 1) { + memmove(*labels + i, + *labels + 1, + sizeof(*labels) * (*nseclabels) - (i + 1)); + (*nseclabels)--; + if (VIR_REALLOC_N(*labels, *nseclabels) < 0) { + /* ignore harmless */ + } + } else { + VIR_FREE(*labels); + *nseclabels = 0; + } } + + ret = parseIds(oldlabel, user, group); + VIR_FREE(oldlabel); + return ret; } + } + + return -1; +} + +static int +virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid, + uid_t *olduser, gid_t *oldgroup) +{ + struct stat sb; + VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", + path, (long) uid, (long) gid); - if (chown_errno == EOPNOTSUPP || chown_errno == EINVAL) { + if (stat(path, &sb) < 0) + goto error; + + if (sb.st_uid == uid && + sb.st_gid == gid) { + /* It's alright, there's nothing to change anyway. And since we are not + * chown()-ing it, don't neither save the original owner */ + if (olduser) + *olduser = (uid_t) -1; + if (oldgroup) + *oldgroup = (gid_t) -1; + return 0; + } + + if (chown(path, uid, gid) < 0) { + if (errno == EOPNOTSUPP || errno == EINVAL) { VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " "supported by filesystem", (long) uid, (long) gid, path); - } else if (chown_errno == EPERM) { + } else if (errno == EPERM) { VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " "permitted", (long) uid, (long) gid, path); - } else if (chown_errno == EROFS) { + } else if (errno == EROFS) { VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not " "possible on readonly filesystem", (long) uid, (long) gid, path); - } else { - virReportSystemError(chown_errno, - _("unable to set user and group to '%ld:%ld' " - "on '%s'"), - (long) uid, (long) gid, path); - return -1; - } + } else + goto error; + } else { + /* save the original owner */ + if (olduser) + *olduser = sb.st_uid; + if (oldgroup) + *oldgroup = sb.st_gid; } + return 0; + +error: + virReportSystemError(errno, + _("unable to set user and group to '%ld:%ld' " + "on '%s'"), + (long) uid, (long) gid, path); + return -1; } static int -virSecurityDACRestoreSecurityFileLabel(const char *path) +virSecurityDACRestoreSecurityFileLabel(const char *path, + uid_t user, + gid_t group) { - struct stat buf; int rc = -1; char *newpath = NULL; @@ -317,23 +452,19 @@ virSecurityDACRestoreSecurityFileLabel(const char *path) if (virFileResolveLink(path, &newpath) < 0) { virReportSystemError(errno, _("cannot resolve symlink %s"), path); - goto err; + goto cleanup; } - if (stat(newpath, &buf) != 0) - goto err; - - /* XXX record previous ownership */ - rc = virSecurityDACSetOwnership(newpath, 0, 0); + rc = virSecurityDACSetOwnership(newpath, user, group, NULL, NULL); -err: +cleanup: VIR_FREE(newpath); return rc; } static int -virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, +virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk, const char *path, size_t depth ATTRIBUTE_UNUSED, void *opaque) @@ -342,13 +473,18 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, virSecurityManagerPtr mgr = params[0]; virDomainDefPtr def = params[1]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - uid_t user; - gid_t group; + uid_t user, olduser; + gid_t group, oldgroup; if (virSecurityDACGetImageIds(def, priv, &user, &group)) return -1; - return virSecurityDACSetOwnership(path, user, group); + if (virSecurityDACSetOwnership(path, user, group, &olduser, &oldgroup) < 0) + return -1; + + return virSecurityDACSaveOwnership(&disk->seclabels, + &disk->nseclabels, + olduser, oldgroup); } @@ -383,6 +519,8 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, int migrated) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user; + gid_t group; if (!priv->dynamicOwnership) return 0; @@ -390,7 +528,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; - /* Don't restore labels on readoly/shared disks, because + /* Don't restore labels on shared disks, because * other VMs may still be accessing these * Alternatively we could iterate over all running * domains and try to figure out if it is in use, but @@ -398,7 +536,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * we can't see running VMs using the file on other nodes * Safest bet is thus to skip the restore step. */ - if (disk->readonly || disk->shared) + if (disk->shared) return 0; if (!disk->src) @@ -420,7 +558,15 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, } } - return virSecurityDACRestoreSecurityFileLabel(disk->src); + if (virSecurityDACGetOriginalOwnerwhip(&disk->seclabels, + &disk->nseclabels, + &user, &group) < 0) { + VIR_INFO("Unable to find the original owner of %s. " + "Falling back to root:root", disk->src); + user = group = 0; + } + + return virSecurityDACRestoreSecurityFileLabel(disk->src, user, group); } @@ -448,7 +594,7 @@ virSecurityDACSetSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED, if (virSecurityDACGetIds(def, priv, &user, &group)) return -1; - return virSecurityDACSetOwnership(file, user, group); + return virSecurityDACSetOwnership(file, user, group, NULL, NULL); } @@ -467,7 +613,7 @@ virSecurityDACSetSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED, if (virSecurityDACGetIds(def, priv, &user, &group)) return -1; - return virSecurityDACSetOwnership(file, user, group); + return virSecurityDACSetOwnership(file, user, group, NULL, NULL); } @@ -538,7 +684,8 @@ virSecurityDACRestoreSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED, const char *file, void *opaque ATTRIBUTE_UNUSED) { - return virSecurityDACRestoreSecurityFileLabel(file); + /* restoring seclabel on PCI devices is not supported yet */ + return virSecurityDACRestoreSecurityFileLabel(file, 0, 0); } @@ -547,7 +694,8 @@ virSecurityDACRestoreSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED, const char *file, void *opaque ATTRIBUTE_UNUSED) { - return virSecurityDACRestoreSecurityFileLabel(file); + /* restoring seclabel on USB devices is not supported yet */ + return virSecurityDACRestoreSecurityFileLabel(file, 0, 0); } @@ -613,39 +761,43 @@ done: static int -virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def, - virDomainChrSourceDefPtr dev) - +virSecurityDACSetChardevCallback(virDomainDefPtr def, + virDomainChrDefPtr dev, + void *opaque) { - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(opaque); + virDomainChrSourceDefPtr src = &dev->source; char *in = NULL, *out = NULL; int ret = -1; - uid_t user; - gid_t group; + uid_t user, olduser = (uid_t) -1; + gid_t group, oldgroup = (gid_t) -1; if (virSecurityDACGetIds(def, priv, &user, &group)) return -1; - switch (dev->type) { + switch (src->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACSetOwnership(dev->data.file.path, user, group); + ret = virSecurityDACSetOwnership(src->data.file.path, user, group, + &olduser, &oldgroup); break; case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + if ((virAsprintf(&in, "%s.in", src->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", src->data.file.path) < 0)) { virReportOOMError(); goto done; } if (virFileExists(in) && virFileExists(out)) { - if ((virSecurityDACSetOwnership(in, user, group) < 0) || - (virSecurityDACSetOwnership(out, user, group) < 0)) { + if ((virSecurityDACSetOwnership(in, user, group, + &olduser, &oldgroup) < 0) || + (virSecurityDACSetOwnership(out, user, group, + &olduser, &oldgroup) < 0)) { goto done; } - } else if (virSecurityDACSetOwnership(dev->data.file.path, - user, group) < 0) { + } else if (virSecurityDACSetOwnership(src->data.file.path, + user, group, + &olduser, &oldgroup) < 0) { goto done; } ret = 0; @@ -656,6 +808,10 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, break; } + ret = virSecurityDACSaveOwnership(&dev->seclabels, + &dev->nseclabels, + olduser, oldgroup); + done: VIR_FREE(in); VIR_FREE(out); @@ -663,30 +819,44 @@ done: } static int -virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainChrSourceDefPtr dev) +virSecurityDACRestoreChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainChrDefPtr dev, + void *opaque ATTRIBUTE_UNUSED) { + virDomainChrSourceDefPtr src = &dev->source; char *in = NULL, *out = NULL; int ret = -1; + uid_t user; + gid_t group; - switch (dev->type) { + if (virSecurityDACGetOriginalOwnerwhip(&dev->seclabels, + &dev->nseclabels, + &user, &group) < 0) { + VIR_INFO("Unable to find the original owner of %s. " + "Falling back to root:root", src->data.file.path); + user = group = 0; + } + + switch (src->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACRestoreSecurityFileLabel(dev->data.file.path); + ret = virSecurityDACRestoreSecurityFileLabel(src->data.file.path, + user, group); break; case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) || - (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) { + if ((virAsprintf(&out, "%s.out", src->data.file.path) < 0) || + (virAsprintf(&in, "%s.in", src->data.file.path) < 0)) { virReportOOMError(); goto done; } if (virFileExists(in) && virFileExists(out)) { - if ((virSecurityDACRestoreSecurityFileLabel(out) < 0) || - (virSecurityDACRestoreSecurityFileLabel(in) < 0)) { + if ((virSecurityDACRestoreSecurityFileLabel(out, user, group) < 0) || + (virSecurityDACRestoreSecurityFileLabel(in, user, group) < 0)) { goto done; } - } else if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path) < 0) { + } else if (virSecurityDACRestoreSecurityFileLabel(src->data.file.path, + user, group) < 0) { goto done; } ret = 0; @@ -703,18 +873,6 @@ done: return ret; } - -static int -virSecurityDACRestoreChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainChrDefPtr dev, - void *opaque) -{ - virSecurityManagerPtr mgr = opaque; - - return virSecurityDACRestoreChardevLabel(mgr, &dev->source); -} - - static int virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, @@ -753,28 +911,16 @@ virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr, rc = -1; if (def->os.kernel && - virSecurityDACRestoreSecurityFileLabel(def->os.kernel) < 0) + virSecurityDACRestoreSecurityFileLabel(def->os.kernel, 0, 0) < 0) rc = -1; if (def->os.initrd && - virSecurityDACRestoreSecurityFileLabel(def->os.initrd) < 0) + virSecurityDACRestoreSecurityFileLabel(def->os.initrd, 0, 0) < 0) rc = -1; return rc; } - -static int -virSecurityDACSetChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainChrDefPtr dev, - void *opaque) -{ - virSecurityManagerPtr mgr = opaque; - - return virSecurityDACSetChardevLabel(mgr, def, &dev->source); -} - - static int virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, @@ -815,11 +961,13 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, return -1; if (def->os.kernel && - virSecurityDACSetOwnership(def->os.kernel, user, group) < 0) + virSecurityDACSetOwnership(def->os.kernel, user, group, + NULL, NULL) < 0) return -1; if (def->os.initrd && - virSecurityDACSetOwnership(def->os.initrd, user, group) < 0) + virSecurityDACSetOwnership(def->os.initrd, user, group, + NULL, NULL) < 0) return -1; return 0; @@ -838,7 +986,7 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetImageIds(def, priv, &user, &group)) return -1; - return virSecurityDACSetOwnership(savefile, user, group); + return virSecurityDACSetOwnership(savefile, user, group, NULL, NULL); } @@ -852,7 +1000,7 @@ virSecurityDACRestoreSavedStateLabel(virSecurityManagerPtr mgr, if (!priv->dynamicOwnership) return 0; - return virSecurityDACRestoreSecurityFileLabel(savefile); + return virSecurityDACRestoreSecurityFileLabel(savefile, 0, 0); } -- 1.8.0.2
participants (3)
-
Daniel P. Berrange -
Martin Kletzander -
Michal Privoznik