[libvirt] [PATCH RFC 1/3] security_dac: Remember owner prior chown() and restore on relabel
Michal Privoznik
mprivozn at redhat.com
Tue Feb 26 16:08:40 UTC 2013
Currently, if we label a file to match qemu process DAC label, we
do not store the original owner anywhere. So when relabeling
back, the only option we have is to relabel to root:root
which is obviously wrong.
However, bare remembering is not enough. We need to keep track of
how many times we labeled a file so only the last restore
chown()-s file back to the original owner.
In order to not pollute domain XML, this info is kept in driver's
private data in a hash table with path being key and pair
<oldLabel, refcount> being value.
---
src/security/security_dac.c | 351 ++++++++++++++++++++++++++++++++++-------
src/security/security_driver.h | 3 +
2 files changed, 296 insertions(+), 58 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0b274b7..4b8f0a2 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -42,8 +42,121 @@ struct _virSecurityDACData {
uid_t user;
gid_t group;
bool dynamicOwnership;
+ virHashTablePtr oldOwners; /* to hold pairs <path, virOldLabelPtr> */
};
+struct _virOldLabel {
+ char *owner;
+ int refCount;
+};
+
+static void virOldLabelFree(virOldLabelPtr label)
+{
+ if (!label)
+ return;
+
+ VIR_FREE(label->owner);
+ VIR_FREE(label);
+}
+
+static void
+hashDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
+{
+ virOldLabelFree(payload);
+}
+
+/**
+ * virSecurityDACRememberLabel:
+ * @priv: private DAC driver data
+ * @path: path which is about to be relabelled
+ *
+ * Store the original DAC label of @path.
+ * Returns: number of references of @path, or -1 on error
+ */
+static int
+virSecurityDACRememberLabel(virSecurityDACDataPtr priv,
+ const char *path)
+{
+ struct stat sb;
+ virOldLabelPtr oldLabel = NULL;
+ char *user = NULL, *group = NULL;
+
+ oldLabel = virHashLookup(priv->oldOwners, path);
+
+ if (oldLabel) {
+ /* just increment ref counter */
+ oldLabel->refCount++;
+ goto cleanup;
+ }
+
+ if (stat(path, &sb) < 0) {
+ virReportSystemError(errno, _("Unable to stat %s"), path);
+ goto cleanup;
+ }
+
+ user = virGetUserName(sb.st_uid);
+ group = virGetGroupName(sb.st_gid);
+ if ((!user && (virAsprintf(&user, "+%ld", (long) sb.st_uid) < 0)) ||
+ (!group && (virAsprintf(&group, "+%ld", (long) sb.st_gid) < 0)) ||
+ (VIR_ALLOC(oldLabel) < 0) ||
+ (virAsprintf(&oldLabel->owner, "%s:%s", user, group) < 0)) {
+ virReportOOMError();
+ VIR_FREE(oldLabel);
+ goto cleanup;
+ }
+
+ oldLabel->refCount = 1;
+ if (virHashUpdateEntry(priv->oldOwners, path, oldLabel) < 0) {
+ virOldLabelFree(oldLabel);
+ oldLabel = NULL;
+ }
+
+cleanup:
+ VIR_FREE(user);
+ VIR_FREE(group);
+ return oldLabel ? oldLabel->refCount : -1;
+}
+
+static int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr);
+
+/**
+ * virSecurityDACGetRememberedLabel:
+ * @priv: private DAC driver data
+ * @path: path which we want to restore label on
+ * @user: original owner of @path
+ * @group: original owner of @path
+ *
+ * Decrements reference counter on @path. If this was the last
+ * reference, we need to restore the original label, in which
+ * case @user and @group are updated.
+ * Returns: the number of references of @path
+ * 0 if we need to restore the label
+ * -1 on error
+ */
+static int
+virSecurityDACGetRememberedLabel(virSecurityDACDataPtr priv,
+ const char *path,
+ uid_t *user,
+ gid_t *group)
+{
+ int ret = -1;
+ virOldLabelPtr oldLabel = NULL;
+
+ oldLabel = virHashLookup(priv->oldOwners, path);
+ if (!oldLabel)
+ goto cleanup;
+
+ ret = --oldLabel->refCount;
+
+ if (!ret) {
+ ret = parseIds(oldLabel->owner, user, group);
+ virHashRemoveEntry(priv->oldOwners, path);
+ }
+
+cleanup:
+ return ret;
+}
+
void virSecurityDACSetUser(virSecurityManagerPtr mgr,
uid_t user)
{
@@ -242,14 +355,20 @@ virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED)
}
static int
-virSecurityDACOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+virSecurityDACOpen(virSecurityManagerPtr mgr)
{
- return 0;
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+ priv->oldOwners = virHashCreate(0, hashDataFree);
+ return priv->oldOwners ? 0 : -1;
}
static int
-virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+virSecurityDACClose(virSecurityManagerPtr mgr)
{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+ virHashFree(priv->oldOwners);
return 0;
}
@@ -306,7 +425,9 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
}
static int
-virSecurityDACRestoreSecurityFileLabel(const char *path)
+virSecurityDACRestoreSecurityFileLabel(const char *path,
+ uid_t user,
+ gid_t group)
{
struct stat buf;
int rc = -1;
@@ -323,8 +444,7 @@ virSecurityDACRestoreSecurityFileLabel(const char *path)
if (stat(newpath, &buf) != 0)
goto err;
- /* XXX record previous ownership */
- rc = virSecurityDACSetOwnership(newpath, 0, 0);
+ rc = virSecurityDACSetOwnership(newpath, user, group);
err:
VIR_FREE(newpath);
@@ -345,7 +465,8 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
uid_t user;
gid_t group;
- if (virSecurityDACGetImageIds(def, priv, &user, &group))
+ if (virSecurityDACGetImageIds(def, priv, &user, &group) ||
+ (virSecurityDACRememberLabel(priv, path) < 0))
return -1;
return virSecurityDACSetOwnership(path, user, group);
@@ -382,26 +503,14 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
virDomainDiskDefPtr disk,
int migrated)
{
+ int ret;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ uid_t user = 0;
+ gid_t group = 0;
- 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
@@ -411,7 +520,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
*/
if (migrated) {
int rc = virStorageFileIsSharedFS(disk->src);
- if (rc < 0)
+ if (rc > 0)
return -1;
if (rc == 1) {
VIR_DEBUG("Skipping image label restore on %s because FS is shared",
@@ -420,7 +529,15 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
}
}
- return virSecurityDACRestoreSecurityFileLabel(disk->src);
+ ret = virSecurityDACGetRememberedLabel(priv, disk->src, &user, &group);
+ if (ret < 0)
+ return -1;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping image label restore on %s "
+ "because the image is still in use", disk->src);
+ return 0;
+ }
+ return virSecurityDACRestoreSecurityFileLabel(disk->src, user, group);
}
@@ -445,7 +562,8 @@ virSecurityDACSetSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED,
uid_t user;
gid_t group;
- if (virSecurityDACGetIds(def, priv, &user, &group))
+ if (virSecurityDACGetIds(def, priv, &user, &group) ||
+ (virSecurityDACRememberLabel(priv, file) < 0))
return -1;
return virSecurityDACSetOwnership(file, user, group);
@@ -464,7 +582,8 @@ virSecurityDACSetSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
uid_t user;
gid_t group;
- if (virSecurityDACGetIds(def, priv, &user, &group))
+ if (virSecurityDACGetIds(def, priv, &user, &group) ||
+ (virSecurityDACRememberLabel(priv, file) < 0))
return -1;
return virSecurityDACSetOwnership(file, user, group);
@@ -536,18 +655,46 @@ done:
static int
virSecurityDACRestoreSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED,
const char *file,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
- return virSecurityDACRestoreSecurityFileLabel(file);
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(opaque);
+ uid_t user = 0;
+ gid_t group = 0;
+ int ret;
+
+ ret = virSecurityDACGetRememberedLabel(priv, file, &user, &group);
+ if (ret < 0)
+ return -1;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the PCI device is still in use", file);
+ return 0;
+ }
+
+ return virSecurityDACRestoreSecurityFileLabel(file, user, group);
}
static int
virSecurityDACRestoreSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED,
const char *file,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
- return virSecurityDACRestoreSecurityFileLabel(file);
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(opaque);
+ uid_t user = 0;
+ gid_t group = 0;
+ int ret;
+
+ ret = virSecurityDACGetRememberedLabel(priv, file, &user, &group);
+ if (ret < 0)
+ return -1;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the USB device is still in use", file);
+ return 0;
+ }
+
+ return virSecurityDACRestoreSecurityFileLabel(file, user, group);
}
@@ -630,6 +777,8 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
switch (dev->type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
+ if (virSecurityDACRememberLabel(priv, dev->data.file.path) < 0)
+ goto cleanup;
ret = virSecurityDACSetOwnership(dev->data.file.path, user, group);
break;
@@ -637,16 +786,22 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) ||
(virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) {
virReportOOMError();
- goto done;
+ goto cleanup;
}
if (virFileExists(in) && virFileExists(out)) {
+ if ((virSecurityDACRememberLabel(priv, in) < 0) ||
+ (virSecurityDACRememberLabel(priv, out) < 0))
+ goto cleanup;
if ((virSecurityDACSetOwnership(in, user, group) < 0) ||
(virSecurityDACSetOwnership(out, user, group) < 0)) {
- goto done;
+ goto cleanup;
}
- } else if (virSecurityDACSetOwnership(dev->data.file.path,
- user, group) < 0) {
- goto done;
+ } else {
+ if (virSecurityDACRememberLabel(priv, dev->data.file.path) < 0)
+ goto cleanup;
+ if (virSecurityDACSetOwnership(dev->data.file.path,
+ user, group) < 0)
+ goto cleanup;
}
ret = 0;
break;
@@ -656,38 +811,85 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
break;
}
-done:
+cleanup:
VIR_FREE(in);
VIR_FREE(out);
return ret;
}
static int
-virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
virDomainChrSourceDefPtr dev)
{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
char *in = NULL, *out = NULL;
int ret = -1;
+ uid_t user = 0;
+ gid_t group = 0;
switch (dev->type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
- ret = virSecurityDACRestoreSecurityFileLabel(dev->data.file.path);
+ ret = virSecurityDACGetRememberedLabel(priv, dev->data.file.path,
+ &user, &group);
+ if (ret < 0)
+ return -1;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the chardev device is still in use",
+ dev->data.file.path);
+ return 0;
+ }
+
+ ret = virSecurityDACRestoreSecurityFileLabel(dev->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)) {
virReportOOMError();
- goto done;
+ goto cleanup;
}
if (virFileExists(in) && virFileExists(out)) {
- if ((virSecurityDACRestoreSecurityFileLabel(out) < 0) ||
- (virSecurityDACRestoreSecurityFileLabel(in) < 0)) {
- goto done;
+ ret = virSecurityDACGetRememberedLabel(priv, in, &user, &group);
+ if (ret < 0)
+ goto cleanup;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the chardev device is still in use", in);
+ goto cleanup;
+ }
+ ret = virSecurityDACGetRememberedLabel(priv, out, &user, &group);
+ if (ret < 0)
+ goto cleanup;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the chardev device is still in use", out);
+ goto cleanup;
+ }
+ if ((virSecurityDACRestoreSecurityFileLabel(out, user, group) < 0) ||
+ (virSecurityDACRestoreSecurityFileLabel(in, user, group) < 0)) {
+ ret = -1;
+ goto cleanup;
+ }
+ } else {
+ ret = virSecurityDACGetRememberedLabel(priv, dev->data.file.path,
+ &user, &group);
+ if (ret < 0)
+ goto cleanup;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the chardev device is still in use",
+ dev->data.file.path);
+ goto cleanup;
+ }
+
+ if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path,
+ user, group) < 0) {
+ ret = -1;
+ goto cleanup;
}
- } else if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path) < 0) {
- goto done;
}
ret = 0;
break;
@@ -697,7 +899,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
break;
}
-done:
+cleanup:
VIR_FREE(in);
VIR_FREE(out);
return ret;
@@ -723,6 +925,8 @@ virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
int i;
int rc = 0;
+ uid_t user = 0;
+ gid_t group = 0;
if (!priv->dynamicOwnership)
return 0;
@@ -752,13 +956,29 @@ virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
mgr) < 0)
rc = -1;
- if (def->os.kernel &&
- virSecurityDACRestoreSecurityFileLabel(def->os.kernel) < 0)
- rc = -1;
+ if (def->os.kernel) {
+ i = virSecurityDACGetRememberedLabel(priv, def->os.kernel, &user, &group);
+ if (i < 0)
+ rc = -1;
+ else if (i > 0)
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the kernel image is still in use", def->os.kernel);
+ else if (virSecurityDACRestoreSecurityFileLabel(def->os.kernel,
+ user, group) < 0)
+ rc = -1;
+ }
- if (def->os.initrd &&
- virSecurityDACRestoreSecurityFileLabel(def->os.initrd) < 0)
- rc = -1;
+ if (def->os.initrd) {
+ i = virSecurityDACGetRememberedLabel(priv, def->os.initrd, &user, &group);
+ if (i < 0)
+ rc = -1;
+ else if (i > 0)
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the initrd image is still in use", def->os.initrd);
+ else if (virSecurityDACRestoreSecurityFileLabel(def->os.initrd,
+ user, group) < 0)
+ rc = -1;
+ }
return rc;
}
@@ -815,11 +1035,13 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
return -1;
if (def->os.kernel &&
- virSecurityDACSetOwnership(def->os.kernel, user, group) < 0)
+ ((virSecurityDACRememberLabel(priv, def->os.kernel) < 0) ||
+ (virSecurityDACSetOwnership(def->os.kernel, user, group) < 0)))
return -1;
if (def->os.initrd &&
- virSecurityDACSetOwnership(def->os.initrd, user, group) < 0)
+ ((virSecurityDACRememberLabel(priv, def->os.initrd) < 0) ||
+ (virSecurityDACSetOwnership(def->os.initrd, user, group) < 0)))
return -1;
return 0;
@@ -835,7 +1057,8 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr,
gid_t group;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
- if (virSecurityDACGetImageIds(def, priv, &user, &group))
+ if (virSecurityDACGetImageIds(def, priv, &user, &group) ||
+ (virSecurityDACRememberLabel(priv, savefile) < 0))
return -1;
return virSecurityDACSetOwnership(savefile, user, group);
@@ -848,11 +1071,23 @@ virSecurityDACRestoreSavedStateLabel(virSecurityManagerPtr mgr,
const char *savefile)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ uid_t user = 0;
+ gid_t group = 0;
+ int ret;
if (!priv->dynamicOwnership)
return 0;
- return virSecurityDACRestoreSecurityFileLabel(savefile);
+ ret = virSecurityDACGetRememberedLabel(priv, savefile, &user, &group);
+ if (ret < 0)
+ return -1;
+ else if (ret > 0) {
+ VIR_DEBUG("Skipping security label restore on %s "
+ "because the saved state file is still in use", savefile);
+ return 0;
+ }
+
+ return virSecurityDACRestoreSecurityFileLabel(savefile, user, group);
}
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index cc401e1..d54f754 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -40,6 +40,9 @@ typedef enum {
typedef struct _virSecurityDriver virSecurityDriver;
typedef virSecurityDriver *virSecurityDriverPtr;
+typedef struct _virOldLabel virOldLabel;
+typedef virOldLabel *virOldLabelPtr;
+
typedef virSecurityDriverStatus (*virSecurityDriverProbe) (const char *virtDriver);
typedef int (*virSecurityDriverOpen) (virSecurityManagerPtr mgr);
typedef int (*virSecurityDriverClose) (virSecurityManagerPtr mgr);
--
1.8.1.4
More information about the libvir-list
mailing list