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