From 0b90fc0c5d4bd3eecbf8b51ff996116bc137d199 Mon Sep 17 00:00:00 2001 Message-Id: <0b90fc0c5d4bd3eecbf8b51ff996116bc137d199@dist-git> From: Erik Skultety Date: Fri, 1 Feb 2019 17:21:58 +0100 Subject: [PATCH] qemu: caps: Use CAP_DAC_OVERRIDE for probing to avoid permission issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is mainly about /dev/sev and its default permissions 0600. Of course, rule of 'tinfoil' would be that we can't trust anything, but the probing code in QEMU is considered safe from security's perspective + we can't create an udev rule for this at the moment, because ioctls and file system permissions aren't cross-checked in kernel and therefore a user with read permissions could issue a 'privileged' operation on SEV which is currently only limited to root. https://bugzilla.redhat.com/show_bug.cgi?id=1665400 Signed-off-by: Erik Skultety Reviewed-by: Daniel P. Berrangé (cherry picked from commit a2d3dea9d41dba313d9566120a8ec9d358567bd0) Signed-off-by: Erik Skultety Reviewed-by: Ján Tomko --- src/qemu/qemu_capabilities.c | 11 +++++++++++ src/util/virutil.c | 31 +++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ba8c717e22..f71cd08f4d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -55,6 +55,10 @@ #include #include +#if WITH_CAPNG +# include +#endif + #define VIR_FROM_THIS VIR_FROM_QEMU VIR_LOG_INIT("qemu.qemu_capabilities"); @@ -4474,6 +4478,13 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd, NULL); virCommandAddEnvPassCommon(cmd->cmd); virCommandClearCaps(cmd->cmd); + +#if WITH_CAPNG + /* QEMU might run into permission issues, e.g. /dev/sev (0600), override + * them just for the purpose of probing */ + virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE); +#endif + virCommandSetGID(cmd->cmd, cmd->runGid); virCommandSetUID(cmd->cmd, cmd->runUid); diff --git a/src/util/virutil.c b/src/util/virutil.c index a908422feb..88e17e2c0f 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1474,8 +1474,10 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups, { size_t i; int capng_ret, ret = -1; - bool need_setgid = false, need_setuid = false; + bool need_setgid = false; + bool need_setuid = false; bool need_setpcap = false; + const char *capstr = NULL; /* First drop all caps (unless the requested uid is "unchanged" or * root and clearExistingCaps wasn't requested), then add back @@ -1484,14 +1486,18 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups, */ if (clearExistingCaps || (uid != (uid_t)-1 && uid != 0)) - capng_clear(CAPNG_SELECT_BOTH); + capng_clear(CAPNG_SELECT_BOTH); for (i = 0; i <= CAP_LAST_CAP; i++) { + capstr = capng_capability_to_name(i); + if (capBits & (1ULL << i)) { capng_update(CAPNG_ADD, CAPNG_EFFECTIVE|CAPNG_INHERITABLE| CAPNG_PERMITTED|CAPNG_BOUNDING_SET, i); + + VIR_DEBUG("Added '%s' to child capabilities' set", capstr); } } @@ -1551,6 +1557,27 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups, goto cleanup; } +# ifdef PR_CAP_AMBIENT + /* we couldn't do this in the loop earlier above, because the capabilities + * were not applied yet, since in order to add a capability into the AMBIENT + * set, it has to be present in both the PERMITTED and INHERITABLE sets + * (capabilities(7)) + */ + for (i = 0; i <= CAP_LAST_CAP; i++) { + capstr = capng_capability_to_name(i); + + if (capBits & (1ULL << i)) { + if (prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, i, 0, 0) < 0) { + virReportSystemError(errno, + _("prctl failed to enable '%s' in the " + "AMBIENT set"), + capstr); + goto cleanup; + } + } + } +# endif + /* Set bounding set while we have CAP_SETPCAP. Unfortunately we cannot * do this if we failed to get the capability above, so ignore the * return value. -- 2.20.1