Re: [PATCH] qemu: keep capabilities when running QEMU as root

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux