apparmor violation for /sys/bus/usb/devices

I missed adding more violations:

audit: type=1400 audit(1666104252.556:36): apparmor="DENIED" operation="getattr" class="file" profile="libvirt-b3192f38-8bab-4424-8c45-1f6e88ddc903" name="/sys/bus/usb/devices/usb1" pid=1504 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0
audit: type=1400 audit(1666104252.556:37): apparmor="DENIED" operation="getattr" class="file" profile="libvirt-b3192f38-8bab-4424-8c45-1f6e88ddc903" name="/sys/bus/usb/devices/1-0:1.0" pid=1504 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0

Hi Jelle,
we have a while ago added a full set of rules which covered everything that USB devices needed.
In libvirt-qemu profile you should already have:

  # For hostdev access. The actual devices will be added dynamically
  /sys/bus/usb/devices/ r,
  /sys/devices/**/usb[0-9]*/** r,
  # libusb needs udev data about usb devices (~equal to content of lsusb -v)
  /run/udev/data/+usb* r,
  /run/udev/data/c16[6,7]* r,
  /run/udev/data/c18[0,8,9]* r,

This should already cover all, except the last denial which you reported on
  /sys/bus/usb/devices/1-0:1.0
But that should be a symlink (and apparmor rules have to list the target)

Here an example from my system:
$ ll /sys/bus/usb/devices/1-0\:1.0
lrwxrwxrwx 1 root root 0 Okt 18 10:21 /sys/bus/usb/devices/1-0:1.0 -> ../../../devices/pci0000:00/0000:00:14.0/usb1/1-0:1.0/
$ readlink -f /sys/bus/usb/devices/1-0:1.0
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-0:1.0

So that would also be covered.
And these rules landed in 2010 / 2014 so they should be present for you.

We need to find why you are not benefiting from those existing rules and why you do not have the sysfs layout others see.

Let me ask:
- which Ubuntu release are you using?
- which kernel are you using?
- how does your /etc/apparmor.d/abstractions/libvirt-qemu look like as it comes from the package

Note: if your release is recent and has #include <local/abstractions/libvirt-qemu> in /etc/apparmor.d/abstractions/libvirt-qemu then please edit /etc/apparmor.d/local/abstractions/libvirt-qemu which will survive package upgrades

Hi,

Sorry for the lack of information:

Ubuntu version: 22.10
Kernel: Linux ubuntu 5.19.0-19-generic

$ cat /etc/apparmor.d/abstractions/libvirt-qemu
  # For hostdev access. The actual devices will be added dynamically
  /sys/bus/usb/devices/ r,
  /sys/devices/**/usb[0-9]*/** r,
  # libusb needs udev data about usb devices (~equal to content of lsusb -v)
  /run/udev/data/+usb* r,
  /run/udev/data/c16[6,7]* r,
  /run/udev/data/c18[0,8,9]* r,

root@ubuntu:~# ls -lh /sys/bus/usb/devices/1-0:1.0
lrwxrwxrwx 1 root root 0 Oct 19 18:39 /sys/bus/usb/devices/1-0:1.0 -> ../../../devices/pci0000:00/0000:00:01.2/usb1/1-0:1.0

So that would be: /sys/devices/pci0000\:00/0000\:00\:01.2/usb1/

And indeed that should be handled by "/sys/devices/**/usb[0-9]*/** r,".

Fuller output:

[ 40.741731] audit: type=1400 audit(1666205557.536:35): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b" pid=7416 comm="apparmor_parser"
[ 40.775021] audit: type=1400 audit(1666205557.568:36): apparmor="DENIED" operation="getattr" class="file" profile="libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b" name="/sys/bus/usb/devices/usb1" pid=4814 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0
[ 40.775053] audit: type=1400 audit(1666205557.568:37): apparmor="DENIED" operation="getattr" class="file" profile="libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b" name="/sys/bus/usb/devices/1-0:1.0" pid=4814 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0

root@ubuntu:~# cat /etc/apparmor.d/libvirt/libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b
#include <tunables/global>

profile libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b flags=(attach_disconnected) {
  #include <abstractions/libvirt-qemu>
  #include <libvirt/libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b.files>

}

Note, this happened in Cockpit CI

After we updated our ubuntu 22.10 image:

https://logs.cockpit-project.org/logs/image-refresh-3953-20221014-083434/log (yes, this doensn't have a valid CA)

Notable changes:

  libvirt (8.6.0-0ubuntu1 -> 8.6.0-0ubuntu3)
  linux (5.19.0-15.15 -> 5.19.0-19.19)
  apparmor (3.0.7-1ubuntu1 -> 3.0.7-1ubuntu2)

Thanks Jelle for the details,
And since it used to work for me and you, but now seems to be not working the assumption is that this regressed in 22.10.

Working theory (based on the denials and other details):
Some change in libusb makes (we had a similar case in the past) it no more just iterate over, but read the actual files in /sys/bus/usb/devices/*. Since they are symlinks that was never meant to be needed but it is a getattr which might happen on the symllinks itself, so we'd need to probably just add:

  /sys/bus/usb/devices/* r,

@Jelle
1. could you check if just adding `/sys/bus/usb/devices/* r,` (and nothing else to any apparmor file) to /etc/apparmor.d/local/abstractions/libvirt-qemu is enough for your case?
2. I'd want to know if any dynamic rules got added before.
In the above case (please adapt uuid accordingly) can you compare 22.10 to 22.04?
The /etc/apparmor.d/libvirt/libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b actually includes the .files (it always does) so the question is if you have any *usb* rules that got rendered into /etc/apparmor.d/libvirt/libvirt-b14c0d3c-1c39-4d8e-ab57-6434b4efa59b.files in either release?

@Team - I'll be on travel the next two weeks, could someone please continue working on this once the further info was provided?

1. Applying that custom rule works
2. On 22.10 it's

root@ubuntu:~# cat /etc/apparmor.d/libvirt/libvirt-5ed35ab0-afe6-4d5b-b1dd-f249242260a6.files
# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.
  "/var/log/libvirt/**/subVmTest1.log" w,
  "/var/lib/libvirt/qemu/domain-subVmTest1/monitor.sock" rw,
  "/var/lib/libvirt/qemu/domain-1-subVmTest1/*" rw,
  "/run/libvirt/**/subVmTest1.pid" rwk,
  "/run/libvirt/**/*.tunnelmigrate.dest.subVmTest1" rw,
  "/var/lib/libvirt/images/subVmTest1-2.img" rwk,
  "/var/log/libvirt/console-subVmTest1.log" rw,
  "/var/log/libvirt/console-subVmTest1.log" rw,
  "/dev/vhost-net" rw,
  "/var/lib/libvirt/qemu/domain-1-subVmTest1/{,**}" rwk,
  "/var/lib/libvirt/qemu/channel/target/domain-1-subVmTest1/{,**}" rwk,
  "/var/lib/libvirt/qemu/domain-1-subVmTest1/master-key.aes" rwk,
  "/dev/net/tun" rwk,

Working version:

root@ubuntu:~# cat /etc/apparmor.d/libvirt/libvirt-a70443f4-ff95-480a-ad89-370cc5fce495.files
# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.
  "/var/log/libvirt/**/subVmTest1.log" w,
  "/var/lib/libvirt/qemu/domain-subVmTest1/monitor.sock" rw,
  "/var/lib/libvirt/qemu/domain-1-subVmTest1/*" rw,
  "/run/libvirt/**/subVmTest1.pid" rwk,
  "/run/libvirt/**/*.tunnelmigrate.dest.subVmTest1" rw,
  "/var/lib/libvirt/images/subVmTest1-2.img" rwk,
  "/var/log/libvirt/console-subVmTest1.log" rw,
  "/var/log/libvirt/console-subVmTest1.log" rw,
  "/var/lib/libvirt/qemu/domain-1-subVmTest1/{,**}" rwk,
  "/var/lib/libvirt/qemu/channel/target/domain-1-subVmTest1/{,**}" rwk,
  "/var/lib/libvirt/qemu/domain-1-subVmTest1/master-key.aes" rwk,
  "/dev/net/tun" rwk,

Ok, so it really isn't (and wasn't) in the dynamic part of it.
Thanks!

And you confirmed it will work fine with the suggested rule.
I need to propose that upstream, but be on a very busy business trip for two weeks.
Do not be concerned if updates on this are a bit delayed.

Next:
- propose rule extension to upstream (explain getent on links)
- Upload fix to L
- SRU that change to J/K

@Team - if you want to get this rolling in the meantime feel welcome to ... :-)

Submitted upstream in:
  https://listman.redhat.com/archives/libvir-list/2022-November/235823.html

Applied upstream at
https://gitlab.com/libvirt/libvirt/-/commit/d6ecd766aa95028b35b6da0d709721720c75c7c1

Ready for Lunar + SRU backports from there.

Already uploaded to Lunar-proposed.
Now tests and review is complete for SRUs as well so I uploaded it to J+K -unapproved too.

This bug was fixed in the package libvirt - 8.6.0-0ubuntu5

---------------
libvirt (8.6.0-0ubuntu5) lunar; urgency=medium

  * d/p/u/tests-Fix-libxlxml2domconfigtest-with-latest-xen.patch: fix FTBFS
    with latest libxl

libvirt (8.6.0-0ubuntu4) lunar; urgency=medium

  [ Lena Voytek ]
  * d/p/u/fix-swtpm-pid-duplication.patch: Clean up swtpm pids after a vm
    shuts down (LP: #1997269)

  [Christian Ehrhardt ]
  * d/p/u/lp-1993304-apparmor-allow-getattr-on-usb-devices.patch: prevent
    apparmor denials on USB forwarding (LP: #1993304)
  * d/p/u/lp-1996176-nodedev-ignore-EINVAL-from-libudev-in-udevEventHandl.patch:
    tolerate the impact of too large udev data avoiding a busy loop
    (LP: #1996176)

 -- Christian Ehrhardt <email address hidden> Tue, 22 Nov 2022 16:13:36 +0100

Hello Jelle, or anyone else affected,

Accepted libvirt into kinetic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/8.6.0-0ubuntu3.1 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, what testing has been performed on the package and change the tag from verification-needed-kinetic to verification-done-kinetic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-kinetic. 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.

Hello Jelle, or anyone else affected,

Accepted libvirt into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/8.0.0-1ubuntu7.4 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, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. 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.

FYI: Upgrading to proposed on jammy (tested in 1996176) worked just fine, reproducing the issue itself and some up/downgrades needs a bit more time.

I've up- (and down-) graded the system in jammy to the versions of proposed.
I as able to verify that with the change applied I no more see the denial.

In addition I verified that the former assumptions that were made on the upstream submission about the content of that path are correct and they are. Just a redirection link collection that is now - depending on the system setup - read to get attributes.

Upgrading to Kinetic for the next test ...

Same behavior on kinetic, works fine for me when attaching an USB device.

Upgrading worked fine, but there was so much in proposed that the output was messy (to much to copy and paste). see bug 1996176 for at least an excerpt of it.

ubuntu@node-horsea:~$ virsh attach-device --domain j --file usbdev.xml
Device attached successfully
ubuntu@node-horsea:~$ virsh detach-device --domain j --file usbdev.xml
Device detached successfully
ubuntu@node-horsea:~$ sudo dmesg | grep DENI | grep usb
^^ is empty

=> verified the bug (and proven that the related function is also still working in general)

This bug was fixed in the package libvirt - 8.0.0-1ubuntu7.4

---------------
libvirt (8.0.0-1ubuntu7.4) jammy; urgency=medium

  * d/p/u/lp-1993304-apparmor-allow-getattr-on-usb-devices.patch: prevent
    apparmor denials on USB forwarding (LP: #1993304)
  * d/p/u/lp-1996176-nodedev-ignore-EINVAL-from-libudev-in-udevEventHandl.patch:
    tolerate the impact of too large udev data avoiding a busy loop
    (LP: #1996176)

 -- Christian Ehrhardt <email address hidden> Tue, 22 Nov 2022 15:59:28 +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.

This bug was fixed in the package libvirt - 8.6.0-0ubuntu3.1

---------------
libvirt (8.6.0-0ubuntu3.1) kinetic; urgency=medium

  [ Lena Voytek ]
  * d/p/u/fix-swtpm-pid-duplication.patch: Clean up swtpm pids after a vm
    shuts down (LP: #1997269)

  [Christian Ehrhardt ]
  * d/p/u/lp-1993304-apparmor-allow-getattr-on-usb-devices.patch: prevent
    apparmor denials on USB forwarding (LP: #1993304)
  * d/p/u/lp-1996176-nodedev-ignore-EINVAL-from-libudev-in-udevEventHandl.patch:
    tolerate the impact of too large udev data avoiding a busy loop
    (LP: #1996176)

 -- Christian Ehrhardt <email address hidden> Tue, 22 Nov 2022 11:21:30 +0100