[Bug 1726804] Re: rules for images on attach-device not containing lock permission

[Corey Bryant]

** Also affects: cloud-archive/pike
   Importance: Undecided
       Status: New

** Changed in: cloud-archive/pike
       Status: New => Triaged

** Changed in: cloud-archive
       Status: New => Triaged

** Changed in: cloud-archive/pike
   Importance: Undecided => Critical

** Changed in: cloud-archive
   Importance: Undecided => Critical

** Also affects: cloud-archive/queens
   Importance: Critical
       Status: Triaged

-- 
You received this bug notification because you are a member of Ubuntu
OpenStack, which is subscribed to Ubuntu Cloud Archive.
https://bugs.launchpad.net/bugs/1726804

Title:
  rules for images on attach-device not containing lock permission

Status in Ubuntu Cloud Archive:
  Triaged
Status in Ubuntu Cloud Archive pike series:
  Triaged
Status in Ubuntu Cloud Archive queens series:
  Triaged
Status in libvirt package in Ubuntu:
  Fix Committed
Status in libvirt source package in Artful:
  Fix Committed

Bug description:
  [Impact]

   * Qemu 2.10 started to lock image files to ensure no data corruption 
     occurs. Unfurtunately that isn't covered by the apparmor rules we had 
     for images so far - it need to add "k" permission.

   * This was spotted and done in Artful, but the tests for the hot-add of
     disks were hidden behind some other known not-too-bad issues. So by 
     fixing those tests I realized that hot-add of disks is currently broken
     in Artful.

  [Test Case]

  # Get a very minimal Testguest that keeps running to attach something
  $ qemu-img create /tmp/A.img 1M
  cat <<EOF > testguest.xml
  <domain type='kvm'>
          <name>testguest</name>
          <uuid>deadbeef-dead-beef-dead-beefdeadbeef</uuid>
          <memory unit='KiB'>1024</memory>
          <vcpu placement='static'>1</vcpu>
          <os>
                  <type arch='x86_64' machine='pc-i440fx-zesty'>hvm</type>
                  <boot dev='hd'/>
          </os>
          <features>
                  <acpi/>
                  <apic/>
                  <pae/>
          </features>
          <devices>
                  <emulator>/usr/bin/kvm-spice</emulator>
                  <disk type='file' device='disk'>
                          <driver name='qemu'/>
                          <source file='/tmp/A.img'/>
                          <target dev='vda'/>
                  </disk>
          </devices>
          <seclabel type='dynamic' model='apparmor' relabel='yes'/>
  </domain>
  EOF
  $ virsh define testguest.xml
  $ virsh start testguest

  # Prepare Disk
  $ qemu-img create /tmp/F.img 1M
  $ cat <<EOF >diskF.xml
  <disk type='file'>
    <driver name='qemu'/>
    <source file='/tmp/F.img'/>
    <target dev='sdc'/>
  </disk>
  EOF

  # Then attach:
  $ virsh attach-device testguest diskF.xml

  * This should work, but fails without the fix as:
     error: internal error: unable to execute QEMU command 'device_add': 
  Property 'scsi-hd.drive' can't find value 'drive-scsi0-0-0-1'
    With a related apparmor denial:
     apparmor="DENIED" operation="file_lock" profile="libvirt-7d781722-69b7-8801-fe96-caf37b7a8969" name="/tmp/tmpKzZQR0/device_disk.img" pid=17582 comm="qemu" requested_mask="k" denied_mask="k" fsuid=0 ouid=0

  * With the fix the file is rwk and works to be attached

  [Regression Potential]

   * This is only adding apparmor lock permissions to files added after 
     start. Thereby the only thing that comes to mind is if now things are 
     locked that were not before, and thereby cause issues. But OTOH no one 
     but qemu should lock the image files in use - and if someone else does 
     he now correctly sees qemu holding the lock. Seems safe to me.

  [Other Info]
   
   * This is an release/upgrade-regression which should be fixed 
     asap. I already wrote and submitted a fix to upstream, but given that 
     this can break a lot of use cases we ahve to fix fast and reroll in 
     case upstream decides to modify.

  ---

  
  On something like:
   $ virsh attach-device <guest> <xml>

  The rule rendered is:
  "/tmp/B.img" rw,

  This is missing the k flag needed on qemu >=2.10.

  This applies to block and file definitions:
  <disk type='block'>
    <driver name='qemu'/>
    <source dev='/tmp/B.img'/>
    <target dev='sdb'/>
  </disk>
  <disk type='file'>
    <driver name='qemu'/>
    <source file='/tmp/F.img'/>
    <target dev='sdc'/>
  </disk>

  Both are rendered correctly as:
  "/tmp/F.img" rwk,
  If being part of the domain xml instead of being a hot-add.

To manage notifications about this bug go to:
https://bugs.launchpad.net/cloud-archive/+bug/1726804/+subscriptions