LibVirt Apparmor profile has qemu-bridge-helper listed in the wrong directory

This is on:
Description: Ubuntu Zesty Zapus (development branch)
Release: 17.04

Hi,
thank you so much for reporting and helping to make Ubuntu better.
It is the right place to report it.

I checked on last and current version, as well as the Debian counterpart.
It is true that the rule is not matching, but it kind of never did.

Zesty:
qemu-system-common: /usr/lib/qemu/qemu-bridge-helper
/etc/apparmor.d/usr.sbin.libvirtd:99: /usr/{lib,libexec}/qemu-bridge-helper rmix,

Yakkety:
qemu-system-common: /usr/lib/qemu/qemu-bridge-helper
/etc/apparmor.d/abstractions/libvirt-qemu:224: /usr/{lib,libexec}/qemu-bridge-helper rmix,

Debian:
qemu-system-common: /usr/lib/qemu/qemu-bridge-helper
/etc/apparmor.d/usr.sbin.libvirtd:86: /usr/{lib,libexec}/qemu-bridge-helper rmix,

Could you please describe the effect that you see due to that - is is an execution error of some sort. Because I'd like to use that when suggesting to upstream or Debian since they suffer of the same.

I think the upstream rule might not be broken in the upstream point of view. It is part of the .deb packaging that makes it end up in /usr/lib/qemu instead of /usr/lib/.
Maybe the right place to fix that is upstream, but in debian/pactches (instead of debian/patches/debian), but that is where I'd like to hear the Debian opinion as well.

If you would be open to go the extra mile, please feel free to report it to Debian with that extra info I added and you will provide on the actual effect. If not I can do that for you - eventually I want to link up the bugs and add the same solution, so if you do mention the Deb-bug number here.

So, TL;DR:
1. the rule never match the path of the tool
2. the path was always wrong
3. we followed upstream, I guess due to that now this actually is having an effect
4. caused by .deb packaging placing it in a subdir to /usr/lib
5. please share some more details on the effect

Good day,
The biggest impact I had is after updating and pulling in updated libvirt apparmor settings my VMs were unable to start due to app armor not allowing access to qemu-bridge-helper.

This may a regression bug as in the replaced configuration file it was correct, but that may have been from me editing it in the past.

That impact may be specific to me, but I have taken the default package install for both so hopefully they would know where the files are located.

Thank you,
Brian

Interesting, I definitely could start VMs I'd have found that issue with all the tests - so would have Debian.

As I listed above the path in the rules always was /usr/lib while the binary always was in /usr/lib/qemu. What really changed was which tool this rule applies to. These files are
Could you elaborate on what you meant with "as in the replaced configuration file it was correct"?

These files are considered conffiles, so if you modify them they are kept as-is. If an update would place a different content it would show you a diff and ask you what to keep.
You can run the following to identify any modified files
$ sudo dpkg -L libvirt-daemon-system
If there is one of the apparmor files in it please attach it here.

Eventually the bad path has to be fixed, but since you are the only one with a visible impact I'd want to understand that more before changing anything.

[Expired for libvirt (Ubuntu) because there has been no activity for 60 days.]

I'm using qemu-bridge-helper already for a while. Since updating to 19.10 libvirtd suddenly refused to start my VMs. Turned out to be caused by apparmor, I found these log messages:

audit: type=1400 audit(1580253669.262:100): apparmor="DENIED" operation="file_inherit" profile="libvirtd//qemu_bridge_helper" pid=5629 comm="qemu-bridge-hel" family="unix" sock_type="stream" protocol=0 requested_mask="send receive" denied_mask="send receive" addr=none peer_addr=none peer="libvirtd"
audit: type=1400 audit(1580253669.262:101): apparmor="DENIED" operation="file_inherit" profile="libvirtd" pid=5629 comm="qemu-bridge-hel" family="unix" sock_type="stream" protocol=0 requested_mask="send receive" denied_mask="send receive" addr=none peer_addr=none peer="libvirtd//qemu_bridge_helper"
audit: type=1400 audit(1580253669.262:102): apparmor="DENIED" operation="signal" profile="libvirtd" pid=3118 comm="libvirtd" requested_mask="send" denied_mask="send" signal=term peer="libvirtd//qemu_bridge_helper"

For me the following change below the issue. I've barely any idea what I'm doing, never wrote apparmor profiles before. So probably this patch needs a bit of fine-tuning.

--- usr.sbin.libvirtd-orig 2020-01-29 22:52:27.257908332 +0100
+++ usr.sbin.libvirtd 2020-01-29 22:45:42.358642382 +0100
@@ -62,8 +62,10 @@
   signal (send) set=("kill", "term") peer=unconfined,

   # For communication/control to qemu-bridge-helper
- unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd//qemu_bridge_helper),
- signal (send) set=("term") peer=/usr/sbin/libvirtd//qemu_bridge_helper,
+ unix (send, receive) type=stream addr=none peer=(label=libvirtd//qemu_bridge_helper),
+ signal (send) set=("term") peer=libvirtd//qemu_bridge_helper,

   # allow connect with openGraphicsFD, direction reversed in newer versions
   unix (send, receive) type=stream addr=none peer=(label=libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*),
@@ -122,7 +124,8 @@
    network inet stream,

    # For communication/control from libvirtd
- unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd),
+ unix (send, receive) type=stream addr=none peer=(label=libvirtd),
    signal (receive) set=("term") peer=/usr/sbin/libvirtd,
    signal (receive) set=("term") peer=libvirtd,

The original bug here was fixed by changing the sub-element to

  /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper Cx -> qemu_bridge_helper,

Which covers the correct path as well.
Due to that it is "now" detected correctly.

This detection puts it under the label of libvirt and that is what triggers the new case of needing:
  label=libvirtd//qemu_bridge_helper

The change above was due to: https://libvirt.org/git/?p=libvirt.git;a=commit;h=123cc3e11c03442fd87f00a9089882ec65cffdb8

That was in 3.2 which maps to >=artful (17.10).
It fixes the original bug reported here (to get the helper confined)

But the named labelling was added later
https://libvirt.org/git/?p=libvirt.git;a=commit;h=a3ab6d42d825499af44b8f19f9299e150d9687bc

That maps to 5.1 which would be >=19.10 Eoan and reflects the latter bug.

I think now things make sense - thanks to both of you!

I'll keep this bug even thou the original report was fixed to address the second related issue.

@Bart - also your rule change seems absolutely right to me - well done.
I lack a full email to add you as reported-by in the patch - if you do mind about that let me know.

Submitted for upstream discussion as:
https://www.redhat.com/archives/libvir-list/2020-January/msg01329.html

The change is now upstream and I will make it part of the upload I'm preparing for Ubuntu 20.04

This bug was fixed in the package libvirt - 6.0.0-0ubuntu1

---------------
libvirt (6.0.0-0ubuntu1) focal; urgency=medium

  * Merged with Debian 5.6.0-4 from experimental and v6.0.0 from upstream
    Among many other new features and fixes this includes fixes for:
    - LP: #1859253 - rbd driver fails to create a new volume
    - LP: #1858341 - rbd driver does not list all volumes in pool
    - LP: #1845506 - Libvirt snapshot doesn't update apparmor profile
    - LP: #1854653 - slow libvirt-guests.sh during shutdown if service is off
    - LP: #1848229 - enable ppc64el to use ccf-assist feature
    - LP: #1853315 - Enable CPU Model Comparison and Baselining on s390x
    - LP: #1853317 - CCW IPL support to boot from ECKD DASDs
    - LP: #1859506 - security: AppArmor profile fixes for swtpm
    Remaining changes:
    - Disable libssh2 support (universe dependency)
    - Disable firewalld support (universe dependency)
    - Set qemu-group to kvm (for compat with older ubuntu)
    - Additional apport package-hook
    - Autostart default bridged network (As upstream does, but not Debian).
      In addition to just enabling it our solution provides:
      + do not autostart if subnet is already taken (e.g. in guests).
      + iterate some alternative subnets before giving up
    - d/p/ubuntu/Allow-libvirt-group-to-access-the-socket.patch: This is
      the group based access to libvirt functions as it was used in Ubuntu
      for quite long.
      + d/p/ubuntu/daemon-augeas-fix-expected.patch fix some related tests
        due to the group access change.
      + d/libvirt-daemon-system.postinst: add users in sudo to the libvirt
        group.
    - ubuntu/parallel-shutdown.patch: set parallel shutdown by default.
    - Update Vcs-Git and Vcs-Browser fields to point to launchpad
    - Update README.Debian with Ubuntu changes
    - Enable some additional features on ppc64el and s390x (for arch parity)
      + systemtap, zfs, numa and numad on s390x.
      + systemtap on ppc64el.
    - d/p/ubuntu/ubuntu_machine_type.patch: accept ubuntu types as pci440fx
    - Further upstreamed apparmor Delta, especially any new one
      Our former delta is split into logical pieces and is either Ubuntu only
      or is part of a continuous upstreaming effort.
      Listing related remaining changes in debian/patches/ubuntu-aa/:
    - fix autopkgtests
      + d/t/control, d/t/smoke-qemu-session: fixup smoke-qemu-session by making
        vmlinuz available and accessible (Debian bug 848314)
      + d/t/control: fix smoke-qemu-session by ensuring the service will run
        installing libvirt-daemon-system
      + d/t/smoke-lxc: fix smoke-lxc by ignoring potential issues on destroy as
        long as the following undefine succeeds
      + d/t/smoke-lxc: use systemd instead of sysV to restart the service
    - dnsmasq related enhancements
      + run dnsmasq as libvirt-dnsmasq (LP: 1743718)
      + d/libvirt-daemon-system.postinst: add libvirt-dnsmasq user and group
      + d/libvirt-daemon-system.postrm: remove libvirt-dnsmasq user and group
        on purge
      + d/p/ubuntu/dnsmasq-as-priv-user: write dnsmasq config with user
        libvirt-dnsmasq and adapt t...

Now that Focal is done check older releases.
Pre 5.1 that wasn't an issue (no named labels), Eoan has 5.4 so mark that as affected for a potential SRU.

I'm adding an SRU Template to the bug.

MP: https://code.launchpad.net/~paelzer/ubuntu/+source/libvirt/+git/libvirt/+merge/380765
Test build at: https://launchpad.net/~paelzer/+archive/ubuntu/lp-1655111-libvirt-eoan-bridge-rule

Note: [1] is in Eoan proposed right now, this has to complete first before this new change here can be accepted by the SRU Team.

[1]: https://launchpad.net/ubuntu/+source/libvirt/5.4.0-0ubuntu5.1

Hello bluedogs, or anyone else affected,

Accepted libvirt into eoan-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/5.4.0-0ubuntu5.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-eoan to verification-done-eoan. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-eoan. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

1. Upgrade to the version in proposed worked fine without issues.

2. static test
root@e:~# grep qemu_bridge_helper /etc/apparmor.d/usr.sbin.libvirtd
  unix (send, receive) type=stream addr=none peer=(label=/usr/sbin/libvirtd//qemu_bridge_helper),
  signal (send) set=("term") peer=/usr/sbin/libvirtd//qemu_bridge_helper,
  /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper Cx -> qemu_bridge_helper,
  profile qemu_bridge_helper {
post:
root@e:~# grep qemu_bridge_helper /etc/apparmor.d/usr.sbin.libvirtd
  unix (send, receive) type=stream addr=none peer=(label=libvirtd//qemu_bridge_helper),
  signal (send) set=("term") peer=libvirtd//qemu_bridge_helper,
  /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper Cx -> qemu_bridge_helper,
  profile qemu_bridge_helper {
=> good

3. dynamic test is no more triggering the reported apparmor denials.

=> verified

This bug was fixed in the package libvirt - 5.4.0-0ubuntu5.2

---------------
libvirt (5.4.0-0ubuntu5.2) eoan; urgency=medium

  * d/p/u/lp-1655111-apparmor-fix-qemu_bridge_helper-for-named-profile.patch:
    fix qemu_bridge_helper to work with named profiles (LP: #1655111)

 -- Christian Ehrhardt <email address hidden> Tue, 17 Mar 2020 09:09:01 +0100

The verification of the Stable Release Update for libvirt has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.