[libvirt] Change virQEMUDriverConfig defaults?

[Daniel P. Berrangé]

On Thu, Nov 28, 2019 at 09:34:36AM +0100, Michal Privoznik wrote:
> Dear list,
> 
> lately I was confronted with the way how KubeVirt uses libvirt. They run it
> inside a docker container with UID mapping and some capabilities dropped.
> See [1] for more info. Problem is that because of the way we initialize our
> defaults they had to disable some features.

Is that really "a problem" though ? The config file exists precisely so
that admins/apps can customize the behaviour of libvirtd, so this sounds
like it is working as intended. If you are runing libvirtd privileged
but restricting capabilities, I think it is not unreasonable to turn off
features in libvirtd.

Or to put it another way. Kubevirt has decided they don't need certain
features in libvirtd so they wish to disable their use. With them disabled
they now have the ability to turn off some capabilities when running
libvirtd.

>                                             Specifically,
> virQEMUDriverConfigNew() is passed @privileged argument which equals
> geteuid() == 0. Apparently, due to UID mapping this leads us to create not
> valid defaults. For instance, rememberOwner will be set to true in this
> case, but trying to write any XATTR is going to fail because the container
> and subsequently libvirtd does have CAP_SYS_ADMIN (and writing trusted.*
> XATTRs is reserve to CAP_SYS_ADMIN). Similarly, dynamicOwnership won't work
> unless CAP_CHOWN is granted and if SELinux driver is enabled then we will
> also need CAP_SYS_ADMIN because setfilecon_raw() does nothing more than
> write XATTR [2].
> So instead of using @privileged we could use capabilities to enable/disable
> features by default. There is one problem with this though - do we know the
> full set of capabilities we need for given feature? For instance, does
> rememberOwner needs just CAP_SYS_ADMIN or also some other capability?

It is technically possible to check capabilities & dynamically change
features use. As you say though, the challenge with checking capabilities
is figuring out exactly which ones we want/need to have for each feature,
especially if we're calling out to 3rd party libraries.

I would also note that capabilities are Linux specific, so we'll still
need the fallback to UID checks.

> Users could still override our defaults through qemu.conf, but I can see us
> throwing a warning if they are trying to enable say rememberOwner while the
> libvird lacks CAP_SYS_ADMIN.
> 
> What is your opinion?

We certainly could do something but I'm not convinced there's a real
problem here that needs addressing.

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