[libvirt] [PATCH v4 3/3] security_dac: Favour ACLs over chown()

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. 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.libvirt.dac.refCount and trusted.libvirt.dac.oldACL respectively. However, some filesystems doesn't support ACLs, XATTRs, or both. So the code is made to favour ACLs among with tracking the reference count. If this fails, we fall back to chown() with best effort to remember the original owner of file. --- diff to v3: -Dan's suggestions worked in diff to v2: -basically squashed functionality of 2/4 and 4/4 from previous round src/security/security_dac.c | 302 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 273 insertions(+), 29 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 0b274b7..4914baa 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,9 @@ #define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_DAC_NAME "dac" +#define SECURITY_DAC_XATTR_OLD_ACL "trusted.libvirt.dac.oldACL" +#define SECURITY_DAC_XATTR_OLD_OWNER "trusted.libvirt.dac.oldOwner" +#define SECURITY_DAC_XATTR_REFCOUNT "trusted.libvirt.dac.refCount" typedef struct _virSecurityDACData virSecurityDACData; typedef virSecurityDACData *virSecurityDACDataPtr; @@ -234,6 +238,196 @@ int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, return 0; } +static int +virSecurityDACGetXATTRRefcount(const char *path, + int *refCount) +{ + int ret = -1; + char *refCountStr; + + if (virFileGetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, &refCountStr) < 0) + return ret; + + VIR_DEBUG("path=%s refCountStr=%s", path, NULLSTR(refCountStr)); + + if (!refCountStr) { + *refCount = 0; + 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; + } + + ret = 0; +cleanup: + VIR_FREE(refCountStr); + return ret; +} + +static int +virSecurityDACSetXATTRRefcount(const char *path, + int refCount) +{ + int ret = -1; + char *refCountStr; + + VIR_DEBUG("path=%s refCount=%d", path, refCount); + + if (refCount == 0) { + virFileRemoveAttr(path, SECURITY_DAC_XATTR_REFCOUNT); + return 0; + } + + if (virAsprintf(&refCountStr, "%u", refCount) < 0) { + virReportOOMError(); + return ret; + } + + if (virFileSetAttr(path, SECURITY_DAC_XATTR_REFCOUNT, refCountStr) < 0) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(refCountStr); + return ret; +} + +static int +virSecurityDACSetACL(const char *path, + uid_t uid) +{ + int ret = -1; + char *oldACL = NULL; + mode_t perms; + + VIR_DEBUG("path=%s uid=%u", path, uid); + + 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; + + if (virFileSetACL(path, uid, S_IRUSR | S_IWUSR) < 0) + goto cleanup; + + ret = 0; +cleanup: + if (ret < 0) + virFileRemoveAttr(path, SECURITY_DAC_XATTR_OLD_ACL); + VIR_FREE(oldACL); + return ret; +} + +static int +virSecurityDACRestoreACL(const char *path) +{ + int ret = -1; + char *oldACL = NULL, *c; + uid_t uid; + mode_t perms; + + VIR_DEBUG("path=%s", path); + + if (virFileGetAttr(path, SECURITY_DAC_XATTR_OLD_ACL, &oldACL) < 0) + return ret; + + if (!oldACL) { + VIR_WARN("Attribute %s is missing", SECURITY_DAC_XATTR_OLD_ACL); + return ret; + } + + 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; + + virFileRemoveAttr(path, SECURITY_DAC_XATTR_OLD_ACL); + ret = 0; +cleanup: + VIR_FREE(oldACL); + return ret; +} + +static int +virSecurityDACRememberLabel(const char *path) +{ + int ret = -1; + struct stat sb; + char *oldOwner = NULL; + + VIR_DEBUG("path=%s", path); + + 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; + + ret = 0; +cleanup: + VIR_FREE(oldOwner); + return ret; +} + +static int +virSecurityDACGetRememberedLabel(const char *path, + uid_t *user, + uid_t *group) +{ + int ret = -1; + char *oldOwner = NULL; + + 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_OLD_OWNER); + ret = 0; +cleanup: + VIR_FREE(oldOwner); + return ret; +} static virSecurityDriverStatus virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED) @@ -265,11 +459,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,29 +497,97 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) } static int +virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) +{ + int refCount = 0; + bool xattrSupported = true; + + VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", + path, (long) uid, (long) gid); + + if (virSecurityDACGetXATTRRefcount(path, &refCount) < 0) { + if (errno != ENOSYS && errno != ENOTSUP) + return -1; + xattrSupported = false; + } + + if (refCount || virSecurityDACSetACL(path, uid) == 0) { + if (xattrSupported && + virSecurityDACSetXATTRRefcount(path, refCount + 1) < 0) { + /* Clear out oldACL XATTR */ + return -1; + } + return 0; + } + + /* Setting ACL failed. If the cause is libvirt was build without ACL + * support, or filesystem does not support ACLs fall back to chown */ + if (errno != ENOSYS && errno != ENOTSUP) + return -1; + + VIR_DEBUG("Falling back to chown"); + if (xattrSupported && virSecurityDACRememberLabel(path) < 0) + return -1; + + if (virSecurityDACChown(path, uid, gid) < 0 || + (xattrSupported && + virSecurityDACSetXATTRRefcount(path, refCount + 1) < 0)) { + /* XXX Clear our oldOwner XATTR */ + return -1; + } + return 0; +} + +static int virSecurityDACRestoreSecurityFileLabel(const char *path) { struct stat buf; - int rc = -1; + int ret = -1; + int refCount; char *newpath = NULL; + uid_t uid = 0; + gid_t gid = 0; + bool xattrSupported = true; VIR_INFO("Restoring DAC user and group on '%s'", path); if (virFileResolveLink(path, &newpath) < 0) { virReportSystemError(errno, _("cannot resolve symlink %s"), path); - goto err; + return ret; } if (stat(newpath, &buf) != 0) - goto err; + goto cleanup; + + if (virSecurityDACGetXATTRRefcount(newpath, &refCount) < 0) { + if (errno != ENOSYS && errno != ENOTSUP) + return ret; + xattrSupported = false; + } + + /* Backward compatibility. If domain was started with older libvirt, + * it doesn't have any XATTR set, which means refCount == 0 */ + if (refCount) + refCount--; + + if (refCount || virSecurityDACRestoreACL(newpath) == 0) { + if (xattrSupported) + ret = virSecurityDACSetXATTRRefcount(newpath, refCount); + else + ret = 0; + goto cleanup; + } + + if (virSecurityDACGetRememberedLabel(newpath, &uid, &gid) == 0) + virSecurityDACSetXATTRRefcount(newpath, refCount); - /* XXX record previous ownership */ - rc = virSecurityDACSetOwnership(newpath, 0, 0); + VIR_DEBUG("Falling back to chown"); + ret = virSecurityDACChown(newpath, uid, gid); -err: +cleanup: VIR_FREE(newpath); - return rc; + return ret; } @@ -384,24 +643,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