[libvirt] [PATCH v2 1/2] m4: Run QEMU under a distro-specific user when possible

[Daniel P. Berrangé]

On Tue, Mar 26, 2019 at 05:06:15PM +0100, Andrea Bolognani wrote:
> Our current defaults are root:wheel on FreeBSD and macOS, root:root
> everywhere else.
> 
> Looking at what downstream distributions actually do, we can see that
> these defaults are overriden the vast majority of the time, with a
> number of variations showing up in the wild:
> 
>   * qemu:qemu -> Used by CentOS, Fedora, Gentoo, OpenSUSE, RHEL
>                  and... As it turns out, our very own spec file :)
> 
>   * libvirt-qemu:libvirt-qemu -> Used by Debian.
> 
>   * libvirt-qemu:kvm -> Used by Ubuntu.
> 
>   * nobody:nobody -> Used by Arch Linux.
> 
> Based on this information, we can do a better job at integrating with
> downstream packages: if the distro-specific user and group already
> exist on the system then we use them, and if not (or we're building
> on an unknown OS) we just use root:root as we would have before.
> 
> This change makes it less likely that people building from source
> will end up running their guests as root, which from the security
> point of view is a very desiderable outcome.
> 
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
> 
> Proof that I'm not making any of this up:
> 
>   * Alpine Linux
>     https://github.com/alpinelinux/aports/blob/master/main/libvirt/APKBUILD
> 
>   * Arch Linux
>     https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=libvirt-git#n113
> 
>   * CentOS
>     https://git.centos.org/blob/rpms!libvirt.git/8d86411e5109d791edf49c7f43c08a06b80896af/SPECS!libvirt.spec#L226
> 
>   * Debian
>     https://salsa.debian.org/libvirt-team/libvirt/blob/debian/sid/debian/rules#L94-95
> 
>   * Fedora
>     https://src.fedoraproject.org/rpms/libvirt/blob/f29/f/libvirt.spec#_204
> 
>   * FreeBSD
>     https://github.com/freebsd/freebsd-ports/blob/master/devel/libvirt/Makefile
> 
>   * Gentoo
>     https://github.com/gentoo/gentoo/blob/master/app-emulation/libvirt/libvirt-5.1.0.ebuild#L296-L297
> 
>   * macOS (Homebrew)
>     https://github.com/Homebrew/homebrew-core/blob/master/Formula/libvirt.rb
> 
>   * OpenSUSE
>     https://build.opensuse.org/package/view_file/openSUSE:Leap:15.0:Update/libvirt/libvirt.spec?expand=1
> 
>   * Ubuntu
>     https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/rules?h=ubuntu/disco#n99
> 
>   * Upstream :)
>     https://libvirt.org/git/?p=libvirt.git;a=blob;f=libvirt.spec.in;h=b7a35a0fb14f3360eb795c4ec9b0e46171d2e4ec;hb=HEAD#l196
> 
>  m4/virt-driver-qemu.m4 | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
> index cb05c34265..5d4ac66a4b 100644
> --- a/m4/virt-driver-qemu.m4
> +++ b/m4/virt-driver-qemu.m4
> @@ -44,8 +44,34 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
>      default_qemu_user=root
>      default_qemu_group=wheel
>    else
> -    default_qemu_user=root
> -    default_qemu_group=root

Either this needs to stay....

> +    # Try to integrate gracefully with downstream packages by running QEMU
> +    # processes under the same user and group they would
> +    case $(grep ^ID= /etc/os-release 2>/dev/null) in
> +      *arch*)
> +        default_qemu_user=nobody
> +        default_qemu_group=nobody
> +        ;;
> +      *centos*|*fedora*|*gentoo*|*rhel*|*suse*)
> +        default_qemu_user=qemu
> +        default_qemu_group=qemu
> +        ;;
> +      *debian*)
> +        default_qemu_user=libvirt-qemu
> +        default_qemu_group=libvirt-qemu
> +        ;;
> +      *ubuntu*)
> +        default_qemu_user=libvirt-qemu
> +        default_qemu_group=kvm
> +        ;;

Or this needs to gain...

  *)
       default_qemu_user=root
       default_qemu_group=root


> +    esac
> +    # If the expected user and group don't exist, or we haven't hit any
> +    # of the cases above because we're running on an unknown OS, the only
> +    # sensible fallback is root:root
> +    if ! getent passwd "$default_qemu_user" >/dev/null 2>&1 || \
> +       ! getent group "$default_qemu_group" >/dev/null 2>&1; then

otherwise these are passing an empty string to getent which
feels dubious. Even if empty string works I wouldn't want to
rely on that.

> +      default_qemu_user=root
> +      default_qemu_group=root

We ought to issue a warning if the expected user/group doesn't
actually exist.

> +    fi
>    fi
>  
>    if test "x$with_qemu_user" = "xplatform dependent" ; then


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 :|