Bug 1070916 - virt-aa-helper: AppArmor denial with non-default disk image paths
virt-aa-helper: AppArmor denial with non-default disk image paths
Status: RESOLVED FIXED
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Virtualization:Other
Current
Other openSUSE 42.2
: P5 - None : Normal (vote)
: ---
Assigned To: James Fehlig
E-mail List
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-12-03 17:47 UTC by Christian Boltz
Modified: 2017-12-20 20:20 UTC (History)
2 users (show)

See Also:
Found By: Beta-Customer
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
/etc/libvirt/qemu/123-wiki.xml (3.93 KB, text/plain)
2017-12-06 12:18 UTC, Christian Boltz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Boltz 2017-12-03 17:47:32 UTC
After enabling security_default_confined in /etc/libvirt/qemu.conf, I see this AppArmor denial:

type=AVC msg=audit(1512320837.992:6275): apparmor="DENIED" operation="open" profile="virt-aa-helper" name="/filespace/images/salt-test/wiki.raw" pid=15636 comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0

Interestingly the VM boots and works, so I wonder if virt-aa-helper really needs read access to the disk images.


If you think that access should be granted:
I know I use a non-default path for my disk images, but it could still be solved:
a) perfect solution: automatically update the profile based on the VM config 
   (see samba for an example - /usr/share/samba/update-apparmor-samba-profile
   gets called by smb.service before every samba (re)start)
b) by adding a tunables/libvirt that sets a @{VM_DISK_PATH} variable
   (has to be edited by the user if using non-default paths)
c) by adding a local/ include, as already done by lots of profiles
d) by telling me that *.raw isn't a perfect filename and that I should
   rename my disks to *.img ;-)

Ideally you should implement a), but b) would also be ok.

You might want to additionally implement c) because that allows profile adjustments without "risking" *.rpmnew files.


After that, you could even remove some of the broad rules in this profile, like the rules for /**.img and /{media,mnt,opt,srv}/**
Comment 1 Cédric Bosdonnat 2017-12-05 15:48:49 UTC
Could you provide the libvirt XML description of the guest to help us see where those are coming from?
Comment 2 Christian Boltz 2017-12-06 12:18:25 UTC
Created attachment 751716 [details]
/etc/libvirt/qemu/123-wiki.xml

Here's the requested XML file. The short version is that /filespace/images/salt-test/wiki.raw is the virtual disk used by that VM.
Comment 3 Cédric Bosdonnat 2017-12-07 20:47:11 UTC
(In reply to Christian Boltz from comment #0)
> Interestingly the VM boots and works, so I wonder if virt-aa-helper really
> needs read access to the disk images.

it needs read access to the image to resolve potential symlinks.

> If you think that access should be granted:
> I know I use a non-default path for my disk images, but it could still be
> solved:
> a) perfect solution: automatically update the profile based on the VM config 
>    (see samba for an example - /usr/share/samba/update-apparmor-samba-profile
>    gets called by smb.service before every samba (re)start)

That one sounds like chicken & egg problem since virt-aa-helper is the one updating the profile for the VM.

> b) by adding a tunables/libvirt that sets a @{VM_DISK_PATH} variable
>    (has to be edited by the user if using non-default paths)
> c) by adding a local/ include, as already done by lots of profiles
> d) by telling me that *.raw isn't a perfect filename and that I should
>    rename my disks to *.img ;-)

All of these are perfectly fine. The virt-aa-helper profile also has rules to give read access to all *.img files anywhere... I can add the *.raw extension to fix your problem, though that may not be a perfect fix.

I have a similar one with virt-sandbox with no extension at all in /run/libvirt/...

> Ideally you should implement a), but b) would also be ok.

I'ld be more inclined to implement c) if really needed since the user could need to whitelist several paths for different VMs / devices.

> You might want to additionally implement c) because that allows profile
> adjustments without "risking" *.rpmnew files.

And would let it add as many rules as needed.

> After that, you could even remove some of the broad rules in this profile,
> like the rules for /**.img and /{media,mnt,opt,srv}/**

or should we add one more for /**.raw? Removing those rules will surely bring us a few "regression" bugs.
Comment 4 Cédric Bosdonnat 2017-12-20 19:45:37 UTC
patch upstream, and submitted to factory. Thanks for the report.
Comment 5 Swamp Workflow Management 2017-12-20 20:20:05 UTC
This is an autogenerated message for OBS integration:
This bug (1070916) was mentioned in
https://build.opensuse.org/request/show/558920 Factory / libvirt