more apparmor denials for opengl usage

Denies/log entries and their related solution:

XML snippet generated:
    <graphics type='spice'>
      <listen type='none'/>
      <image compression='off'/>
      <gl enable='yes'/>
    </graphics>
(no rendernode set and no other gl reference got added).

Generated profile (did gl detection trigger?):
Has no references to rendernodes that should be added by virt-aa-helper

guest log fails as almost expected:
2019-02-11T14:39:27.034392Z qemu-system-x86_64: Failed to initialize EGL render node for SPICE GL

Log:
[ 5585.656039] audit: type=1400 audit(1549895967.029:150): apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/dev/dri/renderD128" pid=12606 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=108 ouid=108

Guest XML as defined by virt-manager that does not trigger the GL detection in virt-aa-helper

until a proper fix for virt-aa-helper is available I hardcoded the rendernode, but still get some issues.

apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/usr/share/drirc.d/" pid=17033 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=108 ouid=0
apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/etc/drirc" pid=17033 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=108 ouid=0
apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/dev/dri/" pid=17033 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=108 ouid=0
apparmor="DENIED" operation="file_mmap" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/usr/lib/x86_64-linux-gnu/dri/i915_dri.so" pid=17033 comm="qemu-system-x86" requested_mask="m" denied_mask="m" fsuid=108 ouid=0

dri config - we should allow read
rmix to dri .so's we should allow as well
All of those should be "after" the detection of gl being in use

Note: as intended an unset rendernode (to libvirt) gets specified in commandline (to qemu)

-spice port=0,disable-ticketing,image-compression=off,gl=on,rendernode=/dev/dri/renderD128,seamless-migration=on

So we'd expect virt-aa-helper to trigger in the same case.

After one more round of manual fixes (for now) I got these:
apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/dev/dri/" pid=17275 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=108 ouid=0
apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/etc/glvnd/egl_vendor.d/" pid=17275 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=108 ouid=0
apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/usr/share/glvnd/egl_vendor.d/" pid=17275 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=108 ouid=0

Hmm, also hit virt-aa-helper deny like this:
apparmor="DENIED" operation="open" profile="virt-aa-helper" name="/dev/dri/" pid=17413 comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0

The detection with virt-aa-helper was fixed by the /dev/dri read to virt-aa-helper and I got this rule:
  "/dev/dri/renderD128" rw,

Yet as we learned above, we need way more.
I need to discuss:
- which ones we want to add statically
- which ones we want to add after gl is detected
- what pathing I could use to determine the /sys/devices/... paths at runtime or if we find a safe wildcard pattern (it is read only after all)

Well you don't need my rule for Fifa98-KVM obviously, but all the rest :-)

I confirmed two times now, even with an explicit rendernode set it will try to open the dir as well hitting

[83520.947120] audit: type=1400 audit(1549973902.951:298): apparmor="DENIED" operation="open" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/dev/dri/" pid=8268 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=108 ouid=0

So "/dev/dri/ r," is a requirement as well if gl is used.

I'm not entirely sure if the pathing for the XDG things is correct in libvirt.
The usual rule from mesa [1] would be:
  owner @{HOME}/.cache/ w, # if user clears all caches

But that does not work as user is libvirt-qemu which has a home in /var/lib/libvirt
  libvirt-qemu:x:108:135:Libvirt Qemu,,,:/var/lib/libvirt:/bin/false

But the rule above does not fix the following issue:
apparmor="DENIED" operation="mkdir" profile="libvirt-2f6bde7c-1d3d-498a-b96c-8920f165fa4c" name="/var/lib/libvirt/.cache/" pid=12056 comm="qemu-system-x86" requested_mask="c" denied_mask="c" fsuid=108 ouid=108

fsuid == ouid == 108 matches the user id.
The path matches what I'd expect

And the file for the guest has the rule rendered:
  owner "@{HOME}/.cache/" w

Why does this still fail?!

[1]: https://gitlab.com/apparmor/apparmor/blob/master/profiles/apparmor.d/abstractions/mesa

I thought it is the owner attribute, but the following works:
  owner "/var/lib/libvirt/.cache/" w,

So it must be the resolution of @{HOME} which fails

Which is odd as I thought it worked for
 owner @{HOME}/.drirc
But I realized this was a rule I copied over that isn't needed (dropped now).

Others might have other paths set, after all @{HOME} exists for a reason.
I'll ping the security team for some guidance ...

After consulting the apparmor people, HOME isn't a users home dir but the list of all defined homedirs in /etc/apparmor.d/tunables/home
Eventually this would only catch classic home dirs in /home by
@{HOME}=@{HOMEDIRS}/*/ /root/

But not any special ones like this.
Lets add the path we need then and others can chime in to add more as/if needed.

Ok, first revision of the patches is complete, submitting upstream for review ...

I checked Suse's config for the qemu user it is literal "qemu" and it has "/" set as home dir. But I must admit I'm not liking to add /.config to the rules, therefore I'm skipping this rather unspecific path from the rules for now.

Testable PPA for Disco in [1] and the upstreaming of the changes started at [2]

[1]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3644/+packages
[2]: https://www.redhat.com/archives/libvir-list/2019-February/msg00704.html

Unless the security Team really dislikes the idea of opening it up I'd want to at least SRu this change to Bionic - further back I'm not so sure (the further we go back the less hardening/fixes the interface will have).

Adding bug tasks.

I tried getting this working on my nVidia card, but wasn't able to.

Added abstractions to local/abstractions/libvirt-qemu.
  /proc/modules r,
  /proc/driver/nvidia/ r,
  /proc/driver/nvidia/** r,
  /usr/share/egl/ r,
  /usr/share/egl/** r,
  /sys/devices/** r,
  /sys/devices/ r,
  /dev/nvidiactl rw,

Error starting domain: internal error: qemu unexpectedly closed the monitor: qemu-system-x86_64: ../src/gallium/drivers/llvmpipe/lp_texture.c:499: llvmpipe_resource_get_handle: Assertion `lpr->dt' failed.

Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in cb_wrapper
    callback(asyncjob, *args, **kwargs)
  File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in tmpcb
    callback(*args, **kwargs)
  File "/usr/share/virt-manager/virtManager/libvirtobject.py", line 66, in newfn
    ret = fn(self, *args, **kwargs)
  File "/usr/share/virt-manager/virtManager/domain.py", line 1400, in startup
    self._backend.create()
  File "/usr/lib/python3/dist-packages/libvirt.py", line 1080, in create
    if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
libvirt.libvirtError: internal error: qemu unexpectedly closed the monitor: qemu-system-x86_64: ../src/gallium/drivers/llvmpipe/lp_texture.c:499: llvmpipe_resource_get_handle: Assertion `lpr->dt' failed.

On another note, for Disco+ should we be defaulting to settings that are closer to what is needed, for instance Virtio and no ports by default? I've reduced my graphical issues in VMs just by setting those.

> I tried getting this working on my nVidia card, but wasn't able to.

Hi Brian, thanks for the test.
The gl features are still new and I enable them one by one (i915, mdev
usage, nvidia).
It is good to know that there are more issues lurking, I'll at some
point do the same tests and then try to fix them up.

I assume you added the apparmor rules since you hit denies.
Would you mind filing a new bug listing exactly what config (guest
xml) you have used and what apparmor dnies (dmesg) you have hit.
That usually is enough confirmation to then check with the security
Team if it is enough.

> Error starting domain: internal error: qemu unexpectedly closed the
> monitor: qemu-system-x86_64:
> ../src/gallium/drivers/llvmpipe/lp_texture.c:499:
> llvmpipe_resource_get_handle: Assertion `lpr->dt' failed.

Interesting, haven't seen that yet with any of the combinations I had so far.
[...]

> On another note, for Disco+ should we be defaulting to settings that are closer to what is needed, for instance Virtio and no ports by default? I've reduced my graphical issues in VMs just by setting those.

That is a tricky discussion, libvirt naturally has an API contract of
giving the default that it has given all the time.
Also not all guests will have virtio graphics support, so you need
some insight libvirt as man-in-the middle can't have.
Users of libvirt are supposed to make that decision consciously
instead of being forced - you might see the discussion around q35 as
default machine type for the same.
But you are right - we should try to open and discuss bugs for uvtool,
multipass and probably even openstack to make their default model more
recent.
uvtool/multipass know they start Ubuntu and can make assumptions on
the version of it, Openstack I'll leave to the Openstack Team.
I have added a Trello card for this as other things are more urgent
right now - but I like the idea doing that in "E" to check how those
things work out.

Next steps:
- fix this issue around known apparmor denials (this bug)
- fix crash when using gl with some guest kernels (bug 1815889)
- once you reported the extra denials you have triggered lets tackle
those as well (BTW OS team will get HW for that which I hope to can
test as well)

Comment #18 hit the wrong bug - sorry - removing the B/C bug tasks.
This is intended for Disco and later only.

Opened a new bug as requested - https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943

Thanks for the detailed explanation on libvirtd's limitations in regards to defaults. I'm using virt-manager, which looks like it could be pursued for that seperately.

Fix for this and bug 1817943 now uploaded to Disco - lets see how it migrates.

GL seems still shaky in different use cases aside the most default path - therefore I'm not making it easier to use it on older releases - therefore no SRU of the change.

This bug was fixed in the package libvirt - 5.0.0-1ubuntu2

---------------
libvirt (5.0.0-1ubuntu2) disco; urgency=medium

  * Implement further apparmor rules for usage of gl enabled
    graphics (LP: #1815452)
    - d/p/ubuntu-aa/lp-1815452-more-gl-rules.patch
    - d/p/ubuntu-aa/lp-1815452-virt-aa-helper-rule.patch
  * Implement further apparmor rules for usage of gl enabled
    graphics with nvidia cards (LP: #1817943)
    - d/p/ubuntu-aa/lp-1817943-nvidia-gl-rules.patch
    - d/p/ubuntu-aa/lp-1817943-devices-in-sysfs.patch
  * d/p/ubuntu-aa/lp-1804766-*: updated to the upstream accepted
    version (no functional change, LP: 1804766)

 -- Christian Ehrhardt <email address hidden> Tue, 12 Feb 2019 11:27:14 +0100