ping On Wed, Dec 04, 2019 at 10:11:47AM +0000, Daniel P. Berrangé wrote: > When QEMU uid/gid is set to non-root this is pointless as if we just > used a regular setuid/setgid call, the process will have all its > capabilities cleared anyway by the kernel. > > When QEMU uid/gid is set to root, this is almost (always?) never > what people actually want. People make QEMU run as root in order > to access some privileged resource that libvirt doesn't support > yet and this often requires capabilities. As a result they have > to go find the qemu.conf param to turn this off. This is not > viable for libguestfs - they want to control everything via the > XML security label to request running as root regardless of the > qemu.conf settings for user/group. > > Clearing capabilities was implemented originally because there > was a proposal in Fedora to change permissions such that root, > with no capabilities would not be able to compromise the system. > ie a locked down root account. This never went anywhere though, > and as a result clearing capabilities when running as root does > not really get us any security benefit AFAICT. The root user > can easily do something like create a cronjob, which will then > faithfully be run with full capabilities, trivially bypassing > the restriction we place. > > IOW, our clearing of capabilities is both useless from a security > POV, and breaks valid use cases when people need to run as root. > > This removes the clear_emulator_capabilities configuration > option from qemu.conf, and always runs QEMU with capabilities > when root. The behaviour when non-root is unchanged. > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > --- > docs/drvqemu.html.in | 46 +++++++++++------------------- > src/qemu/libvirtd_qemu.aug | 8 +++++- > src/qemu/qemu.conf | 11 ------- > src/qemu/qemu_conf.c | 4 --- > src/qemu/qemu_conf.h | 1 - > src/qemu/qemu_domain.c | 3 +- > src/qemu/qemu_process.c | 5 ---- > src/qemu/test_libvirtd_qemu.aug.in | 1 - > 8 files changed, 25 insertions(+), 54 deletions(-) > > diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in > index 294117ee1f..8beb28655c 100644 > --- a/docs/drvqemu.html.in > +++ b/docs/drvqemu.html.in > @@ -187,41 +187,29 @@ chmod o+x /path/to/directory > </li> > </ul> > > - <h3><a id="securitycap">Linux process capabilities</a></h3> > - > <p> > - The libvirt QEMU driver has a build time option allowing it to use > - the <a href="http://people.redhat.com/sgrubb/libcap-ng/index.html";>libcap-ng</a> > - library to manage process capabilities. If this build option is > - enabled, then the QEMU driver will use this to ensure that all > - process capabilities are dropped before executing a QEMU virtual > - machine. Process capabilities are what gives the 'root' account > - its high power, in particular the CAP_DAC_OVERRIDE capability > - is what allows a process running as 'root' to access files owned > - by any user. > + The libvirt maintainers <strong>strongly recommend against</strong> > + running QEMU as the root user/group. This should not be required > + in most supported usage scenarios, as libvirt will generally do the > + right thing to grant QEMU access to files it is permitted to > + use when it is running non-root. > </p> > > + <h3><a id="securitycap">Linux process capabilities</a></h3> > + > <p> > - If the QEMU driver is configured to run virtual machines as non-root, > - then they will already lose all their process capabilities at time > - of startup. The Linux capability feature is thus aimed primarily at > - the scenario where the QEMU processes are running as root. In this > - case, before launching a QEMU virtual machine, libvirtd will use > - libcap-ng APIs to drop all process capabilities. It is important > - for administrators to note that this implies the QEMU process will > - <strong>only</strong> be able to access files owned by root, and > - not files owned by any other user. > + In versions of libvirt prior to 6.0.0, even if QEMU was configured > + to run as the root user / group, libvirt would strip all process > + capabilities. This meant that QEMU could only read/write files > + owned by root, or with open permissions. In reality, stripping > + capabilities did not have any security benefit, as it was trivial > + to get commands to run in another context with full capabilities, > + for example, by creating a cronjob. > </p> > - > <p> > - Thus, if a vendor / distributor has configured their libvirt package > - to run as 'qemu' by default, a number of changes will be required > - before an administrator can change a host to run guests as root. > - In particular it will be necessary to change ownership on the > - directories <code>/var/run/libvirt/qemu/</code>, > - <code>/var/lib/libvirt/qemu/</code> and > - <code>/var/cache/libvirt/qemu/</code> back to root, in addition > - to changing the <code>/etc/libvirt/qemu.conf</code> settings. > + Thus since 6.0.0, if QEMU is running as root, it will keep all > + process capabilities. Behaviour when QEMU is running non-root > + is unchanged, it still has no capabilities. > </p> > > <h3><a id="securityselinux">SELinux basic confinement</a></h3> > diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug > index 7d7844dc09..86bea2a32a 100644 > --- a/src/qemu/libvirtd_qemu.aug > +++ b/src/qemu/libvirtd_qemu.aug > @@ -86,7 +86,6 @@ module Libvirtd_qemu = > | bool_entry "auto_start_bypass_cache" > > let process_entry = str_entry "hugetlbfs_mount" > - | bool_entry "clear_emulator_capabilities" > | str_entry "bridge_helper" > | str_entry "pr_helper" > | str_entry "slirp_helper" > @@ -129,6 +128,12 @@ module Libvirtd_qemu = > let swtpm_entry = str_entry "swtpm_user" > | str_entry "swtpm_group" > > + (* Entries that used to exist in the config which are now > + * deleted. We keep on parsing them so we don't break > + * ability to parse old configs after upgrade > + *) > + let obsolete_entry = bool_entry "clear_emulator_capabilities" > + > let capability_filters_entry = str_array_entry "capability_filters" > > (* Each entry in the config is one of the following ... *) > @@ -153,6 +158,7 @@ module Libvirtd_qemu = > | nbd_entry > | swtpm_entry > | capability_filters_entry > + | obsolete_entry > > let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] > let empty = [ label "#empty" . eol ] > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf > index 7a056b037e..b6805ffc41 100644 > --- a/src/qemu/qemu.conf > +++ b/src/qemu/qemu.conf > @@ -583,17 +583,6 @@ > #bridge_helper = "/usr/libexec/qemu-bridge-helper" > > > - > -# If clear_emulator_capabilities is enabled, libvirt will drop all > -# privileged capabilities of the QEMU/KVM emulator. This is enabled by > -# default. > -# > -# Warning: Disabling this option means that a compromised guest can > -# exploit the privileges and possibly do damage to the host. > -# > -#clear_emulator_capabilities = 1 > - > - > # If enabled, libvirt will have QEMU set its process name to > # "qemu:VM_NAME", where VM_NAME is the name of the VM. The QEMU > # process will appear as "qemu:VM_NAME" in process listings and > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index 30637b21ac..0e14d60bc9 100644 > --- a/src/qemu/qemu_conf.c > +++ b/src/qemu/qemu_conf.c > @@ -234,8 +234,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) > cfg->prHelperName = g_strdup(QEMU_PR_HELPER); > cfg->slirpHelperName = g_strdup(QEMU_SLIRP_HELPER); > > - cfg->clearEmulatorCapabilities = true; > - > cfg->securityDefaultConfined = true; > cfg->securityRequireConfined = false; > > @@ -602,8 +600,6 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, > } > } > > - if (virConfGetValueBool(conf, "clear_emulator_capabilities", &cfg->clearEmulatorCapabilities) < 0) > - return -1; > if (virConfGetValueString(conf, "bridge_helper", &cfg->bridgeHelperName) < 0) > return -1; > > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h > index 95b33a1093..a641bbd983 100644 > --- a/src/qemu/qemu_conf.h > +++ b/src/qemu/qemu_conf.h > @@ -161,7 +161,6 @@ struct _virQEMUDriverConfig { > bool relaxedACS; > bool vncAllowHostAudio; > bool nogfxAllowHostAudio; > - bool clearEmulatorCapabilities; > bool setProcessName; > > unsigned int maxProcesses; > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 470d342afc..0dadca4894 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -9345,8 +9345,7 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, > bool custom_hypervisor_feat = false; > > if (virQEMUDriverIsPrivileged(driver) && > - (!cfg->clearEmulatorCapabilities || > - cfg->user == 0 || > + (cfg->user == 0 || > cfg->group == 0)) > qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logCtxt); > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 75ee3893c6..979ea36f0f 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -6811,11 +6811,6 @@ qemuProcessLaunch(virConnectPtr conn, > if (qemuDomainCreateNamespace(driver, vm) < 0) > goto cleanup; > > - VIR_DEBUG("Clear emulator capabilities: %d", > - cfg->clearEmulatorCapabilities); > - if (cfg->clearEmulatorCapabilities) > - virCommandClearCaps(cmd); > - > VIR_DEBUG("Setting up raw IO"); > if (qemuProcessSetupRawIO(driver, vm, cmd) < 0) > goto cleanup; > diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in > index b9cf8d6688..dd90edf687 100644 > --- a/src/qemu/test_libvirtd_qemu.aug.in > +++ b/src/qemu/test_libvirtd_qemu.aug.in > @@ -72,7 +72,6 @@ module Test_libvirtd_qemu = > { "auto_start_bypass_cache" = "0" } > { "hugetlbfs_mount" = "/dev/hugepages" } > { "bridge_helper" = "/usr/libexec/qemu-bridge-helper" } > -{ "clear_emulator_capabilities" = "1" } > { "set_process_name" = "1" } > { "max_processes" = "0" } > { "max_files" = "0" } > -- > 2.21.0 > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
