AppArmor security driver does not support backingstore

Bug #470636 reported by Olivier d.
36
This bug affects 5 people
Affects Status Importance Assigned to Milestone
libvirt (Ubuntu)
Fix Released
Medium
Jamie Strandboge
Declined for Maverick by Jamie Strandboge
Karmic
Won't Fix
Medium
Unassigned
Lucid
Fix Released
Medium
Jamie Strandboge

Bug Description

Binary package hint: libvirt-bin

System : Ubuntu 9.10 (x86_64)
Version of libvirt-bin : 0.7.0-1ubuntu13

When virt-aa-helper add a profile on apparmor, it does not check if the QEMU disk image is based on a read-only image.

This command create a copy-on-write image system.img based on readonly.img :

$ kvm-img create -b readonly.img -f qcow2 system.img

virt-aa-helper should allows read access on readonly.img, and also check that readonly.img is not a COW image. But it only add an access to system.img :

$ grep img /etc/apparmor.d/libvirt/libvirt-e1b4153d-9884-b3a2-2af0-b6bd051d6f56.files
  "/home/virtual/kvm/system.img" rw,
$

description: updated
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thank you for using Ubuntu and taking the time to report a bug. For now, you must add the readonly file to /etc/apparmor.d/libvirt/libvirt-<uuid> (don't add it to the dynamically generated libvirt-<uuid>.files file). After that you can shutdown the virtual machine and after that it will work as expected.

I am going to mark this wishlist for now. While this is a bug, libvirt does not support snapshotting by using cow files. This will be fixed when the upstream sVirt plugin framework supports snapshotting via cow files.

Changed in libvirt (Ubuntu):
importance: Undecided → Wishlist
status: New → Confirmed
tags: added: apparmor
Revision history for this message
Olivier d. (olivier-dembour) wrote :

Ok, thanks for the workaround, it should work (I've added them in an other files, and it works fine).

This command is not the snapshot feature (i.e: not the "kvm snapshot" command) which I knew was not supported actually.

Based on http://libvirt.org/news.html, copy-on-write is supported since 0.6.0, am I wrong ?

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

True, snapshot is different than copy-on-write. Can you explain how you are creating the cow files via libvirt and using/manipulating them from within libvirt (including the xml for the VM from 'virsh dumpxml <vm name>').

Revision history for this message
Olivier d. (olivier-dembour) wrote :

Here is a script that can exactly reproduce the bug.

First, a traditional VM is created and started without problem. Then a double copy-on-write vm is defined and (not) started.

When can see in the log :

Nov 3 10:48:36 wasabi kernel: [88903.924234] type=1503 audit(1257241716.474:1888): operation="open" pid=12022 parent=12021 profile="libvirt-9b026ac6-0e31-816c-580f-3af18fe5d375" requested_mask="::r" denied_mask="::r" fsuid=0 ouid=1000 name="/tmp/test-ro2.img"
Nov 3 10:49:06 wasabi kernel: [88934.163494] type=1505 audit(1257241746.714:1889): operation="profile_remove" pid=12038 name=libvirt-9b026ac6-0e31-816c-580f-3af18fe5d375 namespace=default

Revision history for this message
Olivier d. (olivier-dembour) wrote :

Because this feature is officially supported, can the importance be updated ?.

I think it should not be defined as a wishlist, actually it's a feature that no longer works with 9.10.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

While this is technically different than 'kvm snapshot', this is actually what I meant by 'snapshotting'. The problem is that there is nothing in your XML that shows that there is a base image in use (or a cow file for that matter). This is not expressed in your XML with the backing store being managed outside of libvirt, and therefore cannot be captured by the security driver.

The support that was added in 0.6.0 was for a backingstore when using a storage volume (see http://libvirt.org/formatstorage.html#StorageVolBacking). At this time, the AppArmor security driver does not support a backing store, but will in a future release. I'll update the bug accordingly.

summary: - virt-aa-helper fails to add copy-on-write images on apparmor profile
+ AppArmor security driver does not support backingstore
Changed in libvirt (Ubuntu Lucid):
status: Confirmed → Triaged
importance: Wishlist → High
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in libvirt (Ubuntu Karmic):
status: New → Triaged
importance: Undecided → Medium
Changed in libvirt (Ubuntu Lucid):
importance: High → Medium
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

This will be fixed in Lucid for sure. Once the fix is created, it may be possible to backport to Karmic. Reducing the priority since the issue can be working around by adjusting the profile.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

What needs to happen is virt-aa-helper needs to use virStorageFileGetMetadata(), find any backingStore entries and add them to the profile.

tags: added: regression-release
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I looked into this quite a bit, and the problem is that libvirt is unaware of the pristine cow/snapshot that the disk file is referencing. All that is in the machine definition is the <disk> entry, which points only to the snapshot/cow, not the underlying image. I can't (and won't) fix this because the only way to do that is to make libvirt peek into the disk file and see what its backing store is. This would allow an avenue for an attacker to escape the VM, or at least read/overwrite arbitrary files on the host. If the VM is compromised, the attacker has write access to the disk file, which could be modified to point to another file on the disk. Simply put, libvirt should not be looking at an attacker controlled file for determining access controls.

The support that was added in 0.6.0 that Olivier mentioned is for '<backingstore>', which can be used as a part of a volume definition in a storage pool. This also does not work right now, but I plan to fix it for Lucid, and possibly do an SRU for Karmic if the changes aren't too drastic (I'm hoping they won't be).

In the meantime, as a workaround, people can do one of two things:
1. modify /etc/apparmor.d/libvirt-<uuid> for the additional file. This will maintain guest isolation (ie, one VM cannot access files of another VM). This also needs to be done for each VM that uses a cow/snapshot file.
2. modify /etc/apparmor.d/abstractions/libvirt-qemu to allow access to all files in a particular directory, and put all your backing store files in that directory. This breaks guest isolation (ie, one VM can read/write to all of the files in this directory), but it does maintain host protection.

See /usr/share/doc/libvirt-bin/README.Debian.gz for details on the architecture of the sVirt AppArmor driver and how to customize it for your environment.

Revision history for this message
Imre Gergely (cemc) wrote :

If you're going with option 2., you can simply add the following line to /etc/apparmor.d/abstractions/libvirt-qemu file's end (after the HOME stuff)

/path/to/images_dir/** rw,

Then you can place you images for every guest in that folder or subfolders. Tested and working on Karmic.

Note that this is not as secure as option 1., but it is a quick fix if you're say on a private machine where only you have access to guests and you're doing a lot of guest creation/destroyingand you don't want to edit the profile for every one of them by hand.

Revision history for this message
Stefan Metzmacher (metze) wrote :

As the base images should be readonly anyway, you can add

/path/to/images_dir/** r,

Revision history for this message
Imre Gergely (cemc) wrote :

Yeah, that could work, too. I was going with 'rw' in case one keeps ALL the image files in one place, the base images AND the overlays. I, for example, have /path/to/images_dir/masters for the base images, and /path/to/images_dir for the overlays.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Marking karmic task as Won't Fix as this will likely be too intrusive to fix in Karmic's libvirt.

Changed in libvirt (Ubuntu Karmic):
status: Triaged → Won't Fix
Changed in libvirt (Ubuntu Lucid):
status: Triaged → In Progress
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Uploaded 0.7.5-5ubuntu18 which fixes this. Just needs to be approved.

Changed in libvirt (Ubuntu Lucid):
status: In Progress → Fix Committed
milestone: none → ubuntu-10.04-beta-2
Revision history for this message
Steve Langasek (vorlon) wrote :

Not respinning beta ISOs for this, so will accept immediately after beta2.

Changed in libvirt (Ubuntu Lucid):
milestone: ubuntu-10.04-beta-2 → ubuntu-10.04
Revision history for this message
Steve Langasek (vorlon) wrote :

libvirt 0.7.5-5ubuntu21 is accepted into lucid, but some of the intermediate versions were bounced out of the queue for simplicity's sake - so this didn't get autoclosed. Changelog entry:

libvirt (0.7.5-5ubuntu18) lucid; urgency=low

  * handle SDL graphics (LP: #545426). This can be dropped in 0.7.8
    - 9019-apparmor-fix-xauth.patch: adjust virt-aa-helper to handle SDL
      graphics, specifically Xauthority. Also remove a couple redundant
      checks.
    - debian/apparmor/libvirt-qemu: add comment about /dev/fb*
  * handle backingstore (LP: #470636). This can be dropped in 0.7.8
    - debian/patches/9020-apparmor-fix-backingstore.patch: adjust
      virt-aa-helper to handle disks with backing stores
    - debian/apparmor/usr.lib.libvirt.virt-aa-helper: allow access to
      user-tmp, non-hidden files in @{HOME} and storage pools

 -- Jamie Strandboge <email address hidden> Mon, 05 Apr 2010 16:56:25 -0500

Changed in libvirt (Ubuntu Lucid):
status: Fix Committed → Fix Released
Revision history for this message
Jamin W. Collins (jcollins) wrote :

This is still an issue in Maverick.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Actually, it supports backing stores just fine in both Lucid and Maverick (I use them all the time) for both storage pools and backed qemu disks (I use them all the time) and the issue you are seeing is a result of an upstream libvirt security fix, and not the AppArmor security driver.

What changed in maverick is that libvirt 0.8.3 won't automatically probe for the disk format (doing so causes various security problems), and therefore the backing store, unless the disk format is specified in the XML. Unfortunately this is not the case for virtinst, vmbuilder and any home-brewed scripts when using non-raw disk formats (raw is the default). See bug #656173 comment #7 for more details. I plan to fix virtinst in an SRU (bug #655392) as well as vmbuilder.

Revision history for this message
Jamin W. Collins (jcollins) wrote :

You are indeed correct that specifying the format of the disk image file corrected the issue. However, this is also a fairly large regression over previous functionality. Configurations that worked flawlessly under lucid will now break under maverick. Additionally, the reason for the breakage will not be clear. There aren't any error messages reported, at least none that I could find, that would guide the user to what the problem actually is.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.