128 lines
4.5 KiB
Diff
128 lines
4.5 KiB
Diff
From 0b90fc0c5d4bd3eecbf8b51ff996116bc137d199 Mon Sep 17 00:00:00 2001
|
|
Message-Id: <0b90fc0c5d4bd3eecbf8b51ff996116bc137d199@dist-git>
|
|
From: Erik Skultety <eskultet@redhat.com>
|
|
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 <eskultet@redhat.com>
|
|
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
(cherry picked from commit a2d3dea9d41dba313d9566120a8ec9d358567bd0)
|
|
|
|
Signed-off-by: Erik Skultety <eskultet@redhat.com>
|
|
Reviewed-by: Ján Tomko <jtomko@redhat.com>
|
|
---
|
|
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 <stdarg.h>
|
|
#include <sys/utsname.h>
|
|
|
|
+#if WITH_CAPNG
|
|
+# include <cap-ng.h>
|
|
+#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
|
|
|