pool/libvirt
SHA256
2
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Accepting request 672890 from Virtualization

Browse Source 
OBS-URL: https://build.opensuse.org/request/show/672890
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/libvirt?expand=0&rev=272
This commit is contained in:
Dominique Leuenberger 2019-02-11 20:16:42 +00:00 committed by Git OBS Bridge
commit 46c59d603f
7 changed files with 408 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
17f6a257-security-dac-sev.patch Normal file
View File

@ -0,0 +1,103 @@
commit 17f6a257f1ea484489277f4da38be914b246a30b
Author: Erik Skultety <eskultet@redhat.com>
Date: Thu Jan 31 15:16:50 2019 +0100
security: dac: Relabel /dev/sev in the namespace
The default permissions (0600 root:root) are of no use to the qemu
process so we need to change the owner to qemu iff running with
namespaces.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Index: libvirt-5.0.0/src/security/security_dac.c
===================================================================
--- libvirt-5.0.0.orig/src/security/security_dac.c
+++ libvirt-5.0.0/src/security/security_dac.c
@@ -48,6 +48,7 @@
VIR_LOG_INIT("security.security_dac");
#define SECURITY_DAC_NAME "dac"
+#define DEV_SEV "/dev/sev"
typedef struct _virSecurityDACData virSecurityDACData;
typedef virSecurityDACData *virSecurityDACDataPtr;
@@ -1690,6 +1691,16 @@ virSecurityDACRestoreMemoryLabel(virSecu
static int
+virSecurityDACRestoreSEVLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+ virDomainDefPtr def ATTRIBUTE_UNUSED)
+{
+ /* we only label /dev/sev when running with namespaces, so we don't need to
+ * restore anything */
+ return 0;
+}
+
+
+static int
virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
bool migrated,
@@ -1759,6 +1770,11 @@ virSecurityDACRestoreAllLabel(virSecurit
rc = -1;
}
+ if (def->sev) {
+ if (virSecurityDACRestoreSEVLabel(mgr, def) < 0)
+ rc = -1;
+ }
+
if (def->os.loader && def->os.loader->nvram &&
virSecurityDACRestoreFileLabel(mgr, def->os.loader->nvram) < 0)
rc = -1;
@@ -1833,6 +1849,36 @@ virSecurityDACSetMemoryLabel(virSecurity
static int
+virSecurityDACSetSEVLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def)
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityLabelDefPtr seclabel;
+ uid_t user;
+ gid_t group;
+
+ /* Skip chowning /dev/sev if namespaces are disabled as we'd significantly
+ * increase the chance of a DOS attack on SEV
+ */
+ if (!priv->mountNamespace)
+ return 0;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+ if (seclabel && !seclabel->relabel)
+ return 0;
+
+ if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0)
+ return -1;
+
+ if (virSecurityDACSetOwnership(mgr, NULL, DEV_SEV,
+ user, group, false) < 0)
+ return -1;
+
+ return 0;
+}
+
+
+static int
virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *stdin_path ATTRIBUTE_UNUSED,
@@ -1902,6 +1948,11 @@ virSecurityDACSetAllLabel(virSecurityMan
return -1;
}
+ if (def->sev) {
+ if (virSecurityDACSetSEVLabel(mgr, def) < 0)
+ return -1;
+ }
+
if (virSecurityDACGetImageIds(secdef, priv, &user, &group))
return -1;

60
6fd4c8f8-qemu-domain-sev.patch Normal file
View File

@ -0,0 +1,60 @@
commit 6fd4c8f8785a063112c8161a3a3f5ad3cb6647ea
Author: Erik Skultety <eskultet@redhat.com>
Date: Tue Jan 22 13:46:16 2019 +0100
qemu: domain: Add /dev/sev into the domain mount namespace selectively
Instead of exposing /dev/sev to every domain, do it selectively.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Index: libvirt-5.0.0/src/qemu/qemu_domain.c
===================================================================
--- libvirt-5.0.0.orig/src/qemu/qemu_domain.c
+++ libvirt-5.0.0/src/qemu/qemu_domain.c
@@ -116,6 +116,7 @@ VIR_ENUM_IMPL(qemuDomainNamespace, QEMU_
#define DEVPREFIX "/dev/"
#define DEV_VFIO "/dev/vfio/vfio"
#define DEVICE_MAPPER_CONTROL_PATH "/dev/mapper/control"
+#define DEV_SEV "/dev/sev"
struct _qemuDomainLogContext {
@@ -12018,6 +12019,26 @@ qemuDomainSetupLoader(virQEMUDriverConfi
}
+static int
+qemuDomainSetupLaunchSecurity(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
+ virDomainObjPtr vm,
+ const struct qemuDomainCreateDeviceData *data)
+{
+ virDomainSEVDefPtr sev = vm->def->sev;
+
+ if (!sev || sev->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV)
+ return 0;
+
+ VIR_DEBUG("Setting up launch security");
+
+ if (qemuDomainCreateDevice(DEV_SEV, data, false) < 0)
+ return -1;
+
+ VIR_DEBUG("Set up launch security");
+ return 0;
+}
+
+
int
qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
virSecurityManagerPtr mgr,
@@ -12089,6 +12110,9 @@ qemuDomainBuildNamespace(virQEMUDriverCo
if (qemuDomainSetupLoader(cfg, vm, &data) < 0)
goto cleanup;
+ if (qemuDomainSetupLaunchSecurity(cfg, vm, &data) < 0)
+ goto cleanup;
+
/* Save some mount points because we want to share them with the host */
for (i = 0; i < ndevMountsPath; i++) {
struct stat sb;

112
a2d3dea9-qemu-caps-dac-override-sev.patch Normal file
View File

@ -0,0 +1,112 @@
commit a2d3dea9d41dba313d9566120a8ec9d358567bd0
Author: Erik Skultety <eskultet@redhat.com>
Date: Thu Jan 24 10:33:01 2019 +0100
qemu: caps: Use CAP_DAC_OVERRIDE for probing to avoid permission issues
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>
Index: libvirt-5.0.0/src/qemu/qemu_capabilities.c
===================================================================
--- libvirt-5.0.0.orig/src/qemu/qemu_capabilities.c
+++ libvirt-5.0.0/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");
@@ -4521,6 +4525,13 @@ virQEMUCapsInitQMPCommandRun(virQEMUCaps
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);
Index: libvirt-5.0.0/src/util/virutil.c
===================================================================
--- libvirt-5.0.0.orig/src/util/virutil.c
+++ libvirt-5.0.0/src/util/virutil.c
@@ -1502,8 +1502,10 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gi
{
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 gi
*/
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 gi
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.

50
a404ac34-qemu-cgroup-sev.patch Normal file
View File

@ -0,0 +1,50 @@
commit a404ac34768e975bd420d1eeac3811563da67e3f
Author: Erik Skultety <eskultet@redhat.com>
Date: Mon Jan 21 14:50:11 2019 +0100
qemu: cgroup: Expose /dev/sev/ only to domains that require SEV
SEV has a limit on number of concurrent guests. From security POV we
should only expose resources (any resources for that matter) to domains
that truly need them.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Index: libvirt-5.0.0/src/qemu/qemu_cgroup.c
===================================================================
--- libvirt-5.0.0.orig/src/qemu/qemu_cgroup.c
+++ libvirt-5.0.0/src/qemu/qemu_cgroup.c
@@ -692,6 +692,22 @@ qemuTeardownChardevCgroup(virDomainObjPt
static int
+qemuSetupSEVCgroup(virDomainObjPtr vm)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret;
+
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
+ return 0;
+
+ ret = virCgroupAllowDevicePath(priv->cgroup, "/dev/sev",
+ VIR_CGROUP_DEVICE_RW, false);
+ virDomainAuditCgroupPath(vm, priv->cgroup, "allow", "/dev/sev",
+ "rw", ret);
+ return ret;
+}
+
+static int
qemuSetupDevicesCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -798,6 +814,9 @@ qemuSetupDevicesCgroup(virDomainObjPtr v
goto cleanup;
}
+ if (vm->def->sev && qemuSetupSEVCgroup(vm) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
virObjectUnref(cfg);

64
b6440119-qemu-conf-sev.patch Normal file
View File

@ -0,0 +1,64 @@
commit b6440119185a4e307654a8d26d6d551a2675bf82
Author: Erik Skultety <eskultet@redhat.com>
Date: Mon Jan 21 14:48:02 2019 +0100
qemu: conf: Remove /dev/sev from the default cgroup device acl list
We should not give domains access to something they don't necessarily
need by default. Remove it from the qemu driver docs too.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Index: libvirt-5.0.0/docs/drvqemu.html.in
===================================================================
--- libvirt-5.0.0.orig/docs/drvqemu.html.in
+++ libvirt-5.0.0/docs/drvqemu.html.in
@@ -396,8 +396,7 @@ chmod o+x /path/to/directory
/dev/null, /dev/full, /dev/zero,
/dev/random, /dev/urandom,
/dev/ptmx, /dev/kvm, /dev/kqemu,
-/dev/rtc, /dev/hpet, /dev/net/tun,
-/dev/sev
+/dev/rtc, /dev/hpet, /dev/net/tun
</pre>
<p>
Index: libvirt-5.0.0/src/qemu/qemu.conf
===================================================================
--- libvirt-5.0.0.orig/src/qemu/qemu.conf
+++ libvirt-5.0.0/src/qemu/qemu.conf
@@ -484,7 +484,7 @@
# "/dev/null", "/dev/full", "/dev/zero",
# "/dev/random", "/dev/urandom",
# "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
-# "/dev/rtc","/dev/hpet", "/dev/sev"
+# "/dev/rtc","/dev/hpet"
#]
#
# RDMA migration requires the following extra files to be added to the list:
Index: libvirt-5.0.0/src/qemu/qemu_cgroup.c
===================================================================
--- libvirt-5.0.0.orig/src/qemu/qemu_cgroup.c
+++ libvirt-5.0.0/src/qemu/qemu_cgroup.c
@@ -46,7 +46,7 @@ const char *const defaultDeviceACL[] = {
"/dev/null", "/dev/full", "/dev/zero",
"/dev/random", "/dev/urandom",
"/dev/ptmx", "/dev/kvm", "/dev/kqemu",
- "/dev/rtc", "/dev/hpet", "/dev/sev",
+ "/dev/rtc", "/dev/hpet",
NULL,
};
#define DEVICE_PTY_MAJOR 136
Index: libvirt-5.0.0/src/qemu/test_libvirtd_qemu.aug.in
===================================================================
--- libvirt-5.0.0.orig/src/qemu/test_libvirtd_qemu.aug.in
+++ libvirt-5.0.0/src/qemu/test_libvirtd_qemu.aug.in
@@ -62,7 +62,6 @@ module Test_libvirtd_qemu =
{ "8" = "/dev/kqemu" }
{ "9" = "/dev/rtc" }
{ "10" = "/dev/hpet" }
- { "11" = "/dev/sev" }
}
{ "save_image_format" = "raw" }
{ "dump_image_format" = "raw" }

9
libvirt.changes
View File

@ -1,3 +1,12 @@
-------------------------------------------------------------------
Fri Feb 8 21:32:29 UTC 2019 - James Fehlig <jfehlig@suse.com>
- qemu: fix issues related to restricted permissions on /dev/sev
b6440119-qemu-conf-sev.patch, a404ac34-qemu-cgroup-sev.patch,
6fd4c8f8-qemu-domain-sev.patch, 17f6a257-security-dac-sev.patch,
a2d3dea9-qemu-caps-dac-override-sev.patch
bsc#1124842
-------------------------------------------------------------------
Wed Jan 23 20:53:29 UTC 2019 - James Fehlig <jfehlig@suse.com>

10
libvirt.spec
View File

@ -335,6 +335,11 @@ Source100: %{name}-rpmlintrc
Patch0: 11c8aca9-libxl-set-mem-after-balloon.patch
Patch1: 70c2933d-apparmor-named-profiles.patch
Patch2: a3ab6d42-apparmor-conv-libvirtd-named-profile.patch
Patch3: b6440119-qemu-conf-sev.patch
Patch4: a404ac34-qemu-cgroup-sev.patch
Patch5: 6fd4c8f8-qemu-domain-sev.patch
Patch6: 17f6a257-security-dac-sev.patch
Patch7: a2d3dea9-qemu-caps-dac-override-sev.patch
# Patches pending upstream review
Patch100: libxl-dom-reset.patch
Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch
@ -871,6 +876,11 @@ libvirt plugin for NSS for translating domain names into IP addresses.
%patch0 -p1
%patch1 -p1
%patch2 -p1
%patch3 -p1
%patch4 -p1
%patch5 -p1
%patch6 -p1
%patch7 -p1
%patch100 -p1
%patch101 -p1
%patch150 -p1