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
filesystem permisions are cross checked in kernel and therefore a user
with read permisions could issue a 'privileged' operation on SEV which
is currently only limited to root.
Signed-off-by: Erik Skultety <eskultet(a)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 5cf4b617c6..2e84c965e8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -53,6 +53,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");
@@ -4515,6 +4519,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 5251b66454..02de92061c 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1502,8 +1502,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
@@ -1512,14 +1514,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);
}
}
@@ -1579,6 +1585,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