Hello community, here is the log from the commit of package libvirt for openSUSE:Factory checked in at 2019-02-11 21:16:42 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/libvirt (Old) and /work/SRC/openSUSE:Factory/.libvirt.new.28833 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "libvirt" Mon Feb 11 21:16:42 2019 rev:272 rq:672890 version:5.0.0 Changes: -------- --- /work/SRC/openSUSE:Factory/libvirt/libvirt.changes 2019-02-04 21:23:58.047616900 +0100 +++ /work/SRC/openSUSE:Factory/.libvirt.new.28833/libvirt.changes 2019-02-11 21:16:43.663358229 +0100 @@ -1,0 +2,9 @@ +Fri Feb 8 21:32:29 UTC 2019 - James Fehlig <[email protected]> + +- 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 + +------------------------------------------------------------------- New: ---- 17f6a257-security-dac-sev.patch 6fd4c8f8-qemu-domain-sev.patch a2d3dea9-qemu-caps-dac-override-sev.patch a404ac34-qemu-cgroup-sev.patch b6440119-qemu-conf-sev.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ libvirt.spec ++++++ --- /var/tmp/diff_new_pack.iNRI7A/_old 2019-02-11 21:16:45.031357503 +0100 +++ /var/tmp/diff_new_pack.iNRI7A/_new 2019-02-11 21:16:45.043357497 +0100 @@ -335,6 +335,11 @@ 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 @@ %patch0 -p1 %patch1 -p1 %patch2 -p1 +%patch3 -p1 +%patch4 -p1 +%patch5 -p1 +%patch6 -p1 +%patch7 -p1 %patch100 -p1 %patch101 -p1 %patch150 -p1 ++++++ 17f6a257-security-dac-sev.patch ++++++ commit 17f6a257f1ea484489277f4da38be914b246a30b Author: Erik Skultety <[email protected]> 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 <[email protected]> Reviewed-by: Daniel P. Berrangé <[email protected]> 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; ++++++ 6fd4c8f8-qemu-domain-sev.patch ++++++ commit 6fd4c8f8785a063112c8161a3a3f5ad3cb6647ea Author: Erik Skultety <[email protected]> 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 <[email protected]> Reviewed-by: Daniel P. Berrangé <[email protected]> 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; ++++++ a2d3dea9-qemu-caps-dac-override-sev.patch ++++++ commit a2d3dea9d41dba313d9566120a8ec9d358567bd0 Author: Erik Skultety <[email protected]> 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 <[email protected]> Reviewed-by: Daniel P. Berrangé <[email protected]> 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. ++++++ a404ac34-qemu-cgroup-sev.patch ++++++ commit a404ac34768e975bd420d1eeac3811563da67e3f Author: Erik Skultety <[email protected]> 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 <[email protected]> Reviewed-by: Daniel P. Berrangé <[email protected]> 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); ++++++ b6440119-qemu-conf-sev.patch ++++++ commit b6440119185a4e307654a8d26d6d551a2675bf82 Author: Erik Skultety <[email protected]> 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 <[email protected]> Reviewed-by: Daniel P. Berrangé <[email protected]> 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" }
