[libvirt] [PATCH v2 2/4] security_dac: Remember owner prior chown() and restore on relabel
Michal Privoznik
mprivozn at redhat.com
Wed Mar 6 14:05:54 UTC 2013
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 | 187 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 153 insertions(+), 34 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0b274b7..76a1dc6 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -25,6 +25,7 @@
#include "security_dac.h"
#include "virerror.h"
+#include "virfile.h"
#include "virutil.h"
#include "viralloc.h"
#include "virlog.h"
@@ -34,6 +35,8 @@
#define VIR_FROM_THIS VIR_FROM_SECURITY
#define SECURITY_DAC_NAME "dac"
+#define SECURITY_DAC_XATTR_OLD_OWNER "trusted.oldOwner"
+#define SECURITY_DAC_XATTR_REFCOUNT "trusted.refCount"
typedef struct _virSecurityDACData virSecurityDACData;
typedef virSecurityDACData *virSecurityDACDataPtr;
@@ -234,6 +237,117 @@ int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
return 0;
}
+static int
+virSecurityDACRememberLabel(const char *path)
+{
+ int ret = -1;
+ char *refCountStr = NULL;
+ char *oldOwner = NULL;
+ unsigned int refCount = 0;
+
+ if (virFileGetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, &refCountStr) < 0) {
+ /* Not all filesystems support XATTR */
+ if (virStorageFileIsSharedFS(path))
+ return 0;
+ else
+ return ret;
+ }
+
+ if (!refCountStr) {
+ struct stat sb;
+
+ if (stat(path, &sb) < 0) {
+ virReportSystemError(errno, _("Unable to stat '%s'"), path);
+ return ret;
+ }
+
+ if (virAsprintf(&oldOwner, "+%u:+%u",
+ (unsigned) sb.st_uid, (unsigned) sb.st_gid) < 0) {
+ virReportOOMError();
+ return ret;
+ }
+
+ if (virFileSetAttr(path, SECURITY_DAC_XATTR_OLD_OWNER, oldOwner) < 0)
+ goto cleanup;
+ } else if (virStrToLong_ui(refCountStr, NULL, 10, &refCount) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed %s attribute: %s"),
+ SECURITY_DAC_XATTR_REFCOUNT,
+ refCountStr);
+ goto cleanup;
+ }
+
+ if (virAsprintf(&refCountStr, "%u", refCount+1) < 0) {
+ virReportOOMError();
+ return ret;
+ }
+
+ if (virFileSetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, refCountStr) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(oldOwner);
+ VIR_FREE(refCountStr);
+ return 0;
+}
+
+static int
+virSecurityDACGetRememberedLabel(const char *path,
+ uid_t *user,
+ uid_t *group)
+{
+ int ret = -1;
+ char *refCountStr = NULL;
+ char *oldOwner = NULL;
+ unsigned int refCount;
+
+ if (virFileGetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, &refCountStr) < 0)
+ return ret;
+
+ if (!refCountStr) {
+ /* Backward compatibility. If domain was started with older libvirt,
+ * it doesn't have any XATTR set so we should not fail here */
+ return 0;
+ }
+
+ if (virStrToLong_ui(refCountStr, NULL, 10, &refCount) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed %s attribute: %s"),
+ SECURITY_DAC_XATTR_REFCOUNT,
+ refCountStr);
+ goto cleanup;
+ }
+ VIR_FREE(refCountStr);
+
+ if (--refCount) {
+ if (virAsprintf(&refCountStr, "%u", refCount) < 0) {
+ virReportOOMError();
+ return ret;
+ }
+ if (virFileSetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, refCountStr) < 0)
+ goto cleanup;
+ ret = refCount;
+ } else {
+ if (virFileGetAttr(path, SECURITY_DAC_XATTR_OLD_OWNER, &oldOwner) < 0 ||
+ !oldOwner)
+ return ret;
+
+ if (parseIds(oldOwner, user, group) < 0)
+ goto cleanup;
+
+ virFileRemoveAttr(path, SECURITY_DAC_XATTR_REFCOUNT);
+ virFileRemoveAttr(path, SECURITY_DAC_XATTR_OLD_OWNER);
+ }
+
+ ret = refCount;
+
+cleanup:
+ VIR_FREE(oldOwner);
+ VIR_FREE(refCountStr);
+ return ret;
+}
+
static virSecurityDriverStatus
virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED)
@@ -265,11 +379,18 @@ static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNU
}
static int
-virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
+virSecurityDACSetOwnership(const char *path,
+ uid_t uid,
+ gid_t gid,
+ bool remember)
{
VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
path, (long) uid, (long) gid);
+ if (remember &&
+ virSecurityDACRememberLabel(path) < 0)
+ return -1;
+
if (chown(path, uid, gid) < 0) {
struct stat sb;
int chown_errno = errno;
@@ -310,23 +431,35 @@ virSecurityDACRestoreSecurityFileLabel(const char *path)
{
struct stat buf;
int rc = -1;
+ int ret;
char *newpath = NULL;
+ uid_t user = (uid_t) -1;
+ gid_t group = (gid_t) -1;
VIR_INFO("Restoring DAC user and group on '%s'", path);
if (virFileResolveLink(path, &newpath) < 0) {
virReportSystemError(errno,
_("cannot resolve symlink %s"), path);
- goto err;
+ goto cleanup;
}
if (stat(newpath, &buf) != 0)
- goto err;
+ goto cleanup;
- /* XXX record previous ownership */
- rc = virSecurityDACSetOwnership(newpath, 0, 0);
+ ret = virSecurityDACGetRememberedLabel(newpath, &user, &group);
+ if (ret < 0)
+ goto cleanup;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping label restore on '%s' as it is still "
+ "in use (refCount=%d)", newpath, ret);
+ rc = 0;
+ goto cleanup;
+ }
+
+ rc = virSecurityDACSetOwnership(newpath, user, group, false);
-err:
+cleanup:
VIR_FREE(newpath);
return rc;
}
@@ -348,7 +481,7 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
if (virSecurityDACGetImageIds(def, priv, &user, &group))
return -1;
- return virSecurityDACSetOwnership(path, user, group);
+ return virSecurityDACSetOwnership(path, user, group, true);
}
@@ -384,24 +517,9 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
- if (!priv->dynamicOwnership)
- return 0;
-
- if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
- return 0;
-
- /* Don't restore labels on readoly/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
- * this would not work for clustered filesystems, since
- * 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)
- return 0;
-
- if (!disk->src)
+ if (!priv->dynamicOwnership ||
+ disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK ||
+ !disk->src)
return 0;
/* If we have a shared FS & doing migrated, we must not
@@ -448,7 +566,7 @@ virSecurityDACSetSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED,
if (virSecurityDACGetIds(def, priv, &user, &group))
return -1;
- return virSecurityDACSetOwnership(file, user, group);
+ return virSecurityDACSetOwnership(file, user, group, true);
}
@@ -467,7 +585,7 @@ virSecurityDACSetSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
if (virSecurityDACGetIds(def, priv, &user, &group))
return -1;
- return virSecurityDACSetOwnership(file, user, group);
+ return virSecurityDACSetOwnership(file, user, group, true);
}
@@ -630,7 +748,8 @@ 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(dev->data.file.path,
+ user, group, true);
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
@@ -640,12 +759,12 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
goto done;
}
if (virFileExists(in) && virFileExists(out)) {
- if ((virSecurityDACSetOwnership(in, user, group) < 0) ||
- (virSecurityDACSetOwnership(out, user, group) < 0)) {
+ if ((virSecurityDACSetOwnership(in, user, group, true) < 0) ||
+ (virSecurityDACSetOwnership(out, user, group, true) < 0)) {
goto done;
}
} else if (virSecurityDACSetOwnership(dev->data.file.path,
- user, group) < 0) {
+ user, group, true) < 0) {
goto done;
}
ret = 0;
@@ -815,11 +934,11 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
return -1;
if (def->os.kernel &&
- virSecurityDACSetOwnership(def->os.kernel, user, group) < 0)
+ virSecurityDACSetOwnership(def->os.kernel, user, group, true) < 0)
return -1;
if (def->os.initrd &&
- virSecurityDACSetOwnership(def->os.initrd, user, group) < 0)
+ virSecurityDACSetOwnership(def->os.initrd, user, group, true) < 0)
return -1;
return 0;
@@ -838,7 +957,7 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr,
if (virSecurityDACGetImageIds(def, priv, &user, &group))
return -1;
- return virSecurityDACSetOwnership(savefile, user, group);
+ return virSecurityDACSetOwnership(savefile, user, group, true);
}
--
1.8.1.5
More information about the libvir-list
mailing list