On filesystems supporting ACLs we don't need to do a chown but we
can just set ACLs to gain access for qemu. However, since we are
setting these on too low level, where we don't know if disk is
just a read only or read write, we set read write access
unconditionally.
>From implementation POV, a reference counter is introduced, so ACL is
restored only on the last restore attempt in order to not cut off other
domains. And since a file may had an ACL for a user already set, we need
to keep this as well. Both these, the reference counter and original ACL
are stored as extended attributes named trusted.refCount and
trusted.oldACL respectively.
---
diff to v2:
-basically squashed functionality of 2/4 and 4/4 from previous
round
src/security/security_dac.c | 209 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 182 insertions(+), 27 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0b274b7..46cc686 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_ACL "trusted.oldACL"
+#define SECURITY_DAC_XATTR_REFCOUNT "trusted.refCount"
typedef struct _virSecurityDACData virSecurityDACData;
typedef virSecurityDACData *virSecurityDACDataPtr;
@@ -234,6 +237,144 @@ int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
return 0;
}
+static void
+virSecurityDACCleanupLabel(const char *path)
+{
+ virFileRemoveAttr(path, SECURITY_DAC_XATTR_REFCOUNT);
+ virFileRemoveAttr(path, SECURITY_DAC_XATTR_OLD_ACL);
+}
+
+static int
+virSecurityDACSetACL(const char *path,
+ uid_t uid)
+{
+ int ret = -1;
+ char *refCountStr = NULL;
+ char *oldACL = NULL;
+ int refCount = 0;
+
+ if (virFileGetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, &refCountStr) < 0)
+ return ret;
+
+ if (!refCountStr) {
+ mode_t perms;
+
+ if (virFileGetACL(path, uid, &perms) < 0) {
+ /* error getting ACL entry for @uid */
+ goto cleanup;
+ }
+
+ if (virAsprintf(&oldACL, "%u:0%o", uid, perms) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virFileSetAttr(path, SECURITY_DAC_XATTR_OLD_ACL, oldACL) < 0)
+ goto cleanup;
+ } else if (virStrToLong_i(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 (virAsprintf(&refCountStr, "%u", ++refCount) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virFileSetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, refCountStr) < 0)
+ goto cleanup;
+
+ if (virFileSetACL(path, uid, S_IRUSR | S_IWUSR) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(oldACL);
+ VIR_FREE(refCountStr);
+ return ret;
+}
+
+static int
+virSecurityDACRestoreACL(const char *path)
+{
+ int ret = -1;
+ char *refCountStr = NULL, *oldACL = NULL, *c;
+ int refCount = 0;
+ uid_t uid;
+ mode_t perms;
+
+ 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_i(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, "%d", refCount) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virFileSetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, refCountStr) < 0)
+ goto cleanup;
+ } else {
+ if (virFileGetAttr(path, SECURITY_DAC_XATTR_OLD_ACL, &oldACL) < 0)
+ goto cleanup;
+
+ if (!oldACL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Attribute %s is missing"),
+ SECURITY_DAC_XATTR_OLD_ACL);
+ goto cleanup;
+ }
+
+ if (!(c = strchr(oldACL, ':'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed %s attribute: %s"),
+ SECURITY_DAC_XATTR_OLD_ACL, oldACL);
+ goto cleanup;
+ }
+
+ *c = '\0';
+ c++;
+
+ if (virStrToLong_ui(oldACL, NULL, 10, &uid) < 0 ||
+ virStrToLong_ui(c, NULL, 8, &perms) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed %s attribute: %s"),
+ SECURITY_DAC_XATTR_OLD_ACL, oldACL);
+ goto cleanup;
+ }
+
+ if ((perms && virFileSetACL(path, uid, perms) < 0) ||
+ (!perms && virFileRemoveACL(path, uid) < 0))
+ goto cleanup;
+
+ virSecurityDACCleanupLabel(path);
+ }
+
+ ret = 0;
+cleanup:
+ VIR_FREE(oldACL);
+ VIR_FREE(refCountStr);
+ return ret;
+}
static virSecurityDriverStatus
virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED)
@@ -265,11 +406,8 @@ static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNU
}
static int
-virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
+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);
-
if (chown(path, uid, gid) < 0) {
struct stat sb;
int chown_errno = errno;
@@ -306,6 +444,30 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
}
static int
+virSecurityDACSetOwnership(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);
+
+ if (virSecurityDACSetACL(path, uid) == 0) {
+ /* ACL set successfully */
+ return 0;
+ } else {
+ /* Oops, something went wrong. If ACL or XATTR are not
+ * supported fall back to chown then. */
+ virErrorPtr err = virGetLastError();
+ if (err && err->code != VIR_ERR_SYSTEM_ERROR && err->int1 != ENOSYS) {
+ virSecurityDACCleanupLabel(path);
+ return -1;
+ }
+ virSecurityDACCleanupLabel(path);
+ }
+
+ VIR_DEBUG("Falling back to chown");
+ return virSecurityDACChown(path, uid, gid);
+}
+
+static int
virSecurityDACRestoreSecurityFileLabel(const char *path)
{
struct stat buf;
@@ -317,16 +479,24 @@ 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;
+ goto cleanup;
+
+ if (virSecurityDACRestoreACL(newpath) == 0) {
+ /* ACL restored successfully */
+ rc = 0;
+ goto cleanup;
+ }
+
+ /* Oops, something went wrong. If ACL or XATTR are not
+ * supported fall back to chown then. */
- /* XXX record previous ownership */
- rc = virSecurityDACSetOwnership(newpath, 0, 0);
+ rc = virSecurityDACChown(path, 0, 0);
-err:
+cleanup:
VIR_FREE(newpath);
return rc;
}
@@ -384,24 +554,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
--
1.8.1.5
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list