VM fails to start with dac security driver added

Bug #1633207 reported by bugproxy
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libvirt (Ubuntu)
Medium
Taco Screen team
Yakkety
Low
Unassigned

Bug Description

[Impact]

 * Due to an upstream change in libvirt 2.0 users of libvirt >=2.0
   (that is >=Yakkety) can't use non apparmor security labels anymore.

 * That means old guest definitions that should still work fail to start
   now

 * The issue was in virt-aa-helper, the proposed fix was tested and then
   brought upstream. This is a backport of the upstream accepted fix.

[Test Case]

 * Testcase with virt-aa-helper on a minimal xml:
  $ cat << EOF > /tmp/test.xml
  <domain type='kvm'>
    <name>test-seclabel</name>
    <uuid>12345678-9abc-def1-2345-6789abcdef00</uuid>
    <memory unit='KiB'>1</memory>
    <os><type arch='x86_64'>hvm</type></os>
    <seclabel type='dynamic' model='apparmor' relabel='yes'/>
    <seclabel type='dynamic' model='dac' relabel='yes'/>
  </domain>
  EOF
  $ /usr/lib/libvirt/virt-aa-helper -d -r -p 0 \
    -u libvirt-12345678-9abc-def1-2345-6789abcdef00 < /tmp/test.xml

  Current Result:
    virt-aa-helper: error: could not parse XML
    virt-aa-helper: error: could not get VM definition
  Expected Result is to emit a valid apparmor profile

* The more complex test is to create a guest (whatever way you like) and
  add an empty dac security label (as shown above) to then start the
  guest.

  Current Result:
    error: Failed to start domain yakkety-doubleseclabel
    error: internal error: cannot load AppArmor profile 'libvirt-8746b00d-aad1-4346-8784-2d4331465153'
  Expected Result:
    properly staring the guest

[Regression Potential]

 * The change is in the parsing of domain info in domain.conf. While no
   local nor upstream tests broke anything one could think of very special
   xml configuation that now might fail parsing. OTOH the new change now
   skips some of the parsing, so even if we miss to consider something it
   shouldn't fail, but instead "forget" to read some data correctly. The
   part that we skip are seclabels which are created dynamically anyway.

 * Also the changed flag is local to virt-aa-helper.c so and guarded by
   that flag in domain_conf.c so it should be a no-op to anybody but virt-
   aa-helper for sure.

[Other Info]

 * N/A

---Problem Description---
VM fails to start with dac security driver added

---uname output---
Linux ltc-test-ci1 4.4.0-9136-generic #55-Ubuntu SMP Fri Aug 26 05:56:24 UTC 2016 ppc64le ppc64le ppc64le GNU/Linux

Machine Type = power 8 ppc64le

---Steps to Reproduce---

VM fails to start with dac security driver added
1. Define a VM with both apparmor and dac security driver( Used XML as below)
#virsh dumpxml virt-tests-vm1
<domain type='kvm'>
  <name>virt-tests-vm1</name>
  <uuid>0491f0cd-eb14-4992-be4c-53a1adf1d314</uuid>
  <memory unit='KiB'>33554432</memory>
  <currentMemory unit='KiB'>33554432</currentMemory>
  <vcpu placement='static'>32</vcpu>
  <resource>
    <partition>/machine</partition>
  </resource>
  <os>
    <type arch='ppc64le' machine='pseries-2.6'>hvm</type>
    <boot dev='hd'/>
  </os>
  <cpu>
    <topology sockets='1' cores='32' threads='1'/>
  </cpu>
  <clock offset='utc'/>
  <on_poweroff>destroy</on_poweroff>
  <on_reboot>restart</on_reboot>
  <on_crash>restart</on_crash>
  <devices>
    <emulator>/usr/bin/kvm</emulator>
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2'/>
      <source file='/var/lib/libvirt/images/sath/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-16.10-ppc64le.qcow2'/>
      <target dev='sda' bus='scsi'/>
      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
    </disk>
    <controller type='usb' index='0'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
    </controller>
    <controller type='pci' index='0' model='pci-root'/>
    <controller type='scsi' index='0'>
      <address type='spapr-vio' reg='0x2000'/>
    </controller>
    <interface type='bridge'>
      <mac address='52:54:00:e2:e3:e4'/>
      <source bridge='virbr0'/>
      <model type='virtio'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
    </interface>
    <serial type='pty'>
      <target port='0'/>
      <address type='spapr-vio' reg='0x30000000'/>
    </serial>
    <console type='pty'>
      <target type='serial' port='0'/>
      <address type='spapr-vio' reg='0x30000000'/>
    </console>
    <memballoon model='virtio'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </memballoon>
    <panic model='pseries'/>
  </devices>
  <seclabel type='dynamic' model='apparmor' relabel='yes'/>
  <seclabel type='dynamic' model='dac' relabel='yes'/>
</domain>

2. virsh start virt-tests-vm1
#virsh start virt-tests-vm1
error: Failed to start domain virt-tests-vm1
error: internal error: cannot load AppArmor profile 'libvirt-0491f0cd-eb14-4992-be4c-53a1adf1d314'------NOK

3. After removing dac line from xml(<seclabel type='dynamic' model='dac' relabel='yes'/>) VM started fine
#virsh start virt-tests-vm1
Domain virt-tests-vm1 started

Userspace tool common name: ii libvirt-bin 2.1.0-1ubuntu5 ppc64el programs for the libvirt library

The userspace tool has the following bit modes: both

Userspace package: ii libvirt-bin 2.1.0-1ubuntu5 ppc64el programs for the libvirt library

Revision history for this message
bugproxy (bugproxy) wrote : sosreport

Default Comment by Bridge

tags: added: architecture-ppc64le bugnameltc-145787 severity-critical targetmilestone-inin1610
Changed in ubuntu:
assignee: nobody → Taco Screen team (taco-screen-team)
affects: ubuntu → libvirt (Ubuntu)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
I tested a simple guest as created with uvt-kvm:
$ uvt-kvm create --memory 2048 --cpu 4 --password=ubuntu paelzer-yakkety-test-libvirt release=yakkety arch=ppc64el label=daily
plus the two lines:
  <seclabel type='dynamic' model='apparmor' relabel='yes'/>
  <seclabel type='dynamic' model='dac' relabel='yes'/>

That works on:
Xenial: ok
Yakkety: ok

I did the same on ppc64el, but only had a Xenial host available there.
Yet this worked just fine as well.

Quoting the report: "Linux ltc-test-ci1 4.4.0-9136-generic #55-Ubuntu SMP Fri Aug 26 05:56:24"
Since that seems to be a pre-release yakkety, could I ask you to retest with at least the released levels and report the versions of qemu&libvirt involved for you (dpkg -l '*qemu*' '*libvirt*')?

Changed in libvirt (Ubuntu):
status: New → Incomplete
Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla
Download full text (4.5 KiB)

------- Comment From <email address hidden> 2016-10-18 06:34 EDT-------
(In reply to comment #10)
> Hi,
> I tested a simple guest as created with uvt-kvm:
> $ uvt-kvm create --memory 2048 --cpu 4 --password=ubuntu
> paelzer-yakkety-test-libvirt release=yakkety arch=ppc64el label=daily
> plus the two lines:
> <seclabel type='dynamic' model='apparmor' relabel='yes'/>
> <seclabel type='dynamic' model='dac' relabel='yes'/>
>
> That works on:
> Xenial: ok
> Yakkety: ok
>
> I did the same on ppc64el, but only had a Xenial host available there.
> Yet this worked just fine as well.
>
> Quoting the report: "Linux ltc-test-ci1 4.4.0-9136-generic #55-Ubuntu SMP
> Fri Aug 26 05:56:24"
> Since that seems to be a pre-release yakkety, could I ask you to retest with
> at least the released levels and report the versions of qemu&libvirt
> involved for you (dpkg -l '*qemu*' '*libvirt*')?

I am still hitting with error
#virsh start vm1
error: Failed to start domain vm1
error: internal error: cannot load AppArmor profile 'libvirt-ac906bc7-db70-42fc-be34-1f3cf0fe5745'

After removing <seclabel type='dynamic' model='dac' relabel='yes'/> line, am able to start the same vm
# virsh start vm1
Domain vm1 started

qemu and libvirt Level in the system is matching http://cdimage.ubuntu.com/ubuntu/releases/16.10/release/ubuntu-16.10-server-ppc64el.list:

# dpkg -l '*qemu*' '*libvirt*'|grep ^ii
ii gir1.2-libvirt-glib-1.0:ppc64el 0.2.3-2 ppc64el GObject introspection files for the libvirt-glib library
ii gir1.2-libvirt-sandbox-1.0 0.5.1+git20151113-3 ppc64el GObject introspection files for the libvirt-sandbox library
ii ipxe-qemu 1.0.0+git-20150424.a25a16d-1ubuntu2 all PXE boot firmware - ROM images for qemu
ii libvirt-bin 2.1.0-1ubuntu9 ppc64el programs for the libvirt library
ii libvirt-clients 2.1.0-1ubuntu9 ppc64el Programs for the libvirt library
ii libvirt-daemon 2.1.0-1ubuntu9 ppc64el Virtualization daemon
ii libvirt-daemon-system 2.1.0-1ubuntu9 ppc64el Libvirt daemon configuration files
ii libvirt-dev:ppc64el 2.1.0-1ubuntu9 ppc64el development files for the libvirt library
ii libvirt-doc 2.1.0-1ubuntu9 all documentation for the libvirt library
ii libvirt-glib-1.0-0:ppc64el 0.2.3-2 ppc64el libvirt GLib and GObject mapping library
ii libvirt-glib-1.0-dev:ppc64el 0.2.3-2 ppc64el Development files for the libvirt-glib library
ii libvirt-ocaml 0.6.1.2-1build2 ppc64el OCaml bindings for libvirt
ii libvirt-ocaml-dev 0.6.1.2-1build2 ppc64el OCaml bindings for libvirt
ii libvirt-sandbox-1.0-5 0.5.1+git20151113-3 ppc64el Application sandbox toolkit shared library
ii libvirt-sandbox-1.0-dev 0.5.1+git20151113-3 ppc64el ...

Read more...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks satheera for the reply.
I wonder why it works for me than as I explicitly tested ppc as well just as you do ... ?

It works fine on x86 with Yakkety.
As well as fine on ppc64el with Xenial.
I don't have a Yakkety around yet and machines are scarce.

I assume the xml is how avocado creates it for you.
I compared our libvirt xml files and tried to remove any remaining delta.
That changes from mine to yours were:
+ resource partition
+ adding topology
- cpu features
+ on crash destroy -> restart
+ adding spapr-vio scsi controller

Still starting fine.

From you it would be great if you could enable debugging for libvirt service and virsh, run the failing start of the guest again and report the qemu log file and libvirt from journalctl here. See https://libvirt.org/logging.html for more.

It seems I have to try getting a ppc64el on Yakkety for this test next ... working on that ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I have to report that my usual trick to run KVM from inside a container doesn't work the same way on ppc64el. It might take a while for me to get a Yakkety ppc64el BM system, so more than before I'm dependent on you reporting the extended logs as I requested.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

2nd level kvm failed me as well :-/

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Ha - got my container trick working again.
So testing on Yakkety, adding the double seclabel.

Finally - able to reproduce - yeah!
Looking deeper into that now...

Changed in libvirt (Ubuntu):
status: Incomplete → Confirmed
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I realized that part of my former verification was caused by the kvm-in-lxd env I use to avoid needing too much metal. So I retried on x86 again as these code paths shouldn't be arch specific at all. And now I was able to recreate on x86 as well.

The summary looks like this now:
* - xenial - works
ppc4el - yakkety - fail
x86 - yakkety - fail
x86 - yakkety with 4.4 kernel - fail

Going on with debugging, but I'm on the Road the next few days - so it might take a bit unless someone else jumps in.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

While debugging I found the first level of oddities that I'll continue on and hopefully gives us a solution (or at least eliminate one roadblock).

I think I found that things work with the error described in the bug on Xenial->Yakkety upgraded systems. But on all others I see:
error: unsupported configuration: Unable to find security driver for model apparmor

That would explain the reproducibility fuzz a bit.

After realizing that I checked logs:
 internal error: template '/etc/apparmor.d/libvirt/TEMPLATE.qemu' does not exist
 internal error: template '/etc/apparmor.d/libvirt/TEMPLATE.qemu' does not exist
 unsupported configuration: Security driver apparmor not enabled
 internal error: template '/etc/apparmor.d/libvirt/TEMPLATE.qemu' does not exist

Now checking for those files is even more strange.

$ dpkg -S /etc/apparmor.d/libvirt/TEMPLATE.qemu
libvirt-daemon-system: /etc/apparmor.d/libvirt/TEMPLATE.qemu
sudo apt-get install --reinstall libvirt-daemon-system
ll /etc/apparmor.d/libvirt/TEMPLATE.qemu
ls: cannot access '/etc/apparmor.d/libvirt/TEMPLATE.qemu': No such file or directory

I guess we have those things here:
1. no proper handling of conffile changes due to the switch to the upstream provided apparmor profiles
2. on upgraded systems old&new somehow conflict
3. on new Yakkety apparmor seclabel doesn't work at all

Going on with debugging tomorrow.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (3.2 KiB)

Three way check on fresh installs:
dpkg -S $((find /etc/apparmor.d/ -name '*libvirt*' && find /etc/apparmor.d/ -name '*TEMPLATE*' )| xargs) | sort
X Y X-Y upgrade
libvirt-bin: /etc/apparmor.d/abstractions/libvirt-lxc libvirt-daemon-system: /etc/apparmor.d/abstractions/libvirt-lxc libvirt-daemon-system: /etc/apparmor.d/abstractions/libvirt-lxc
libvirt-bin: /etc/apparmor.d/abstractions/libvirt-qemu libvirt-daemon-system: /etc/apparmor.d/abstractions/libvirt-qemu libvirt-daemon-system: /etc/apparmor.d/abstractions/libvirt-qemu
libvirt-bin: /etc/apparmor.d/libvirt libvirt-daemon-system: /etc/apparmor.d/libvirt libvirt-daemon-system: /etc/apparmor.d/libvirt
libvirt-bin: /etc/apparmor.d/libvirt/TEMPLATE.lxc libvirt-daemon-system: /etc/apparmor.d/libvirt/TEMPLATE.lxc libvirt-daemon-system: /etc/apparmor.d/libvirt/TEMPLATE.lxc
libvirt-bin: /etc/apparmor.d/libvirt/TEMPLATE.qemu libvirt-daemon-system: /etc/apparmor.d/libvirt/TEMPLATE.qemu libvirt-daemon-system: /etc/apparmor.d/libvirt/TEMPLATE.qemu
                                                            libvirt-daemon-system: /etc/apparmor.d/local/usr.lib.libvirt.virt-aa-helper libvirt-daemon-system: /etc/apparmor.d/local/usr.lib.libvirt.virt-aa-helper
libvirt-bin: /etc/apparmor.d/local/usr.sbin.libvirtd libvirt-daemon-system: /etc/apparmor.d/local/usr.sbin.libvirtd libvirt-daemon-system: /etc/apparmor.d/local/usr.sbin.libvirtd
libvirt-bin: /etc/apparmor.d/usr.lib.libvirt.virt-aa-helper libvirt-daemon-system: /etc/apparmor.d/usr.lib.libvirt.virt-aa-helper libvirt-daemon-system: /etc/apparmor.d/usr.lib.libvirt.virt-aa-helper
libvirt-bin: /etc/apparmor.d/usr.sbin.libvirtd libvirt-daemon-system: /etc/apparmor.d/usr.sbin.libvirtd libvirt-daemon-system: /etc/apparmor.d/usr.sbin.libvirtd

In this case Y and the X-Y upgrade was equal.
Also the formerly missing TEMPLATE files were here.
Note that I still have that cae on my phys box - no matter how often I reinstall.

Checking content between X and Y:
Equal:
5f6aa836ced6b474dabfce46a8bfb5e4 /etc/apparmor.d/libvirt/TEMPLATE.lxc
b0dfa704c6297fd9a4e68f0137c6be88 /etc/apparmor.d/libvirt/TEMPLATE.qemu
7166fa490aaf905b7f71cb5407ef0696 /etc/apparmor.d/local/usr.sbin.libvirtd
No functional diff (only comments/reordering):
/etc/apparmor.d/abstractions/libvirt-lxc
/etc/apparmor.d/abstractions/libvirt-qemu
/etc/apparmor.d/usr.lib.libvirt.virt-aa-helper
New but non-functional (empty to carry overwrites)
/etc/apparmor.d/local/usr.lib.libvirt.virt-aa-helper
Changed:
/etc/apparmor.d/usr.sbin.libvirtd added "/usr/sbin/virtlogd pix"

Here the easy in container test stops, as the apparmor security driver really can't work in there.
I need to go back to my BM system and understand/fix why it does run into "error: unsupported configuration: Unable to find security driver for model apparmor" now.
While only partially related I still wanted to document he...

Read more...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Ok, I found why those templates didn't get on my BM test system to begin with.
They were conffiles and I had none of them modified, but some more in the same directories.
So while not that clear still the usual "protect custom conffiles" mechanism that blocked me.
A full purge + manual extra conffile clean + re-install made it working again.
Overall feels a bit touchy atm :-/

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Again at:
sudo virsh start yakkety-doubleseclabel
error: Failed to start domain yakkety-doubleseclabel
error: internal error: cannot load AppArmor profile 'libvirt-8746b00d-aad1-4346-8784-2d4331465153'

In the log I found the related:
Okt 27 13:45:50 horsea libvirtd[10370]: internal error: Child process (LIBVIRT_LOG_OUTPUTS=3:stderr /usr/lib/libvirt/virt-aa-helper -p 0 -r -u libvirt-8746b00d-aad1-4346-8784-2d4331465153) unexpected exit status 1: 2016-10-27 13:45:20.873+0000: 10640: info : libvirt version: 2.1.0, package: 1ubuntu10~ppa3 (Christian Ehrhardt <email address hidden> Mon, 24 Oct 2016 14:21:36 +0200)
                                        2016-10-27 13:45:20.873+0000: 10640: info : hostname: horsea
                                        2016-10-27 13:45:20.873+0000: 10640: error : virSecurityLabelDefParseXML:6473 : XML error: security label is missing
                                        virt-aa-helper: error: could not parse XML
                                        virt-aa-helper: error: could not get VM definition
Okt 27 13:45:50 horsea libvirtd[10370]: internal error: cannot load AppArmor profile 'libvirt-8746b00d-aad1-4346-8784-2d4331465153'
Okt 27 13:45:50 horsea virtlogd[7706]: End of file while reading data: Input/output error

I also found that adding dac alone is enough to trigger:

$ virsh dumpxml yakkety-doubleseclabel | grep -A 20 '<seclabel'
  <seclabel type='dynamic' model='apparmor' relabel='yes'/>
  <seclabel type='dynamic' model='dac' relabel='yes'/>
</domain>
=> Failing

$ virsh dumpxml yakkety-sec-app | grep -A 20 seclabel
  <seclabel type='dynamic' model='apparmor' relabel='yes'/>
</domain>
=> Working

$ virsh dumpxml yakkety-sec-dac | grep -A 20 seclabel
  <seclabel type='dynamic' model='dac' relabel='yes'/>
</domain>
=> Failing just as much as case 1, maybe because apparmor is default on.

Trying to check the /usr/lib/libvirt/virt-aa-helper in those cases, but since it is not meant to be called directly that is a bit tricky.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

After a bit of twiddling I found a somewhat reasonable repro with the virt-aa-helper tool.

diff -Naur yakkety-sec-dac.xml yakkety-sec-nodac.xml
--- yakkety-sec-dac.xml 2016-10-27 14:32:39.565995840 +0000
+++ yakkety-sec-nodac.xml 2016-10-27 14:32:45.097973456 +0000
@@ -60,6 +60,5 @@
       <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
     </memballoon>
   </devices>
- <seclabel type='dynamic' model='dac' relabel='yes'/>
 </domain>

So the only diff is if the dac seclabel is here or not.

$ sudo /usr/lib/libvirt/virt-aa-helper -d -r -p 0 -u libvirt-6e082f89-902c-413c-9d9e-f609089d3374 < yakkety-sec-dac.xml
virt-aa-helper: error: could not parse XML
virt-aa-helper: error: could not get VM definition

$ sudo /usr/lib/libvirt/virt-aa-helper -d -r -p 0 -u libvirt-6e082f89-902c-413c-9d9e-f609089d3374 < yakkety-sec-nodac.xml
virt-aa-helper:
/etc/apparmor.d/libvirt/libvirt-6e082f89-902c-413c-9d9e-f609089d3374.files
virt-aa-helper:
  "/var/log/libvirt/**/yakkety-sec-dac.log" w,
  "/var/lib/libvirt/qemu/domain-yakkety-sec-dac/monitor.sock" rw,
  "/var/lib/libvirt/qemu/domain--1-yakkety-sec-dac/*" rw,
  "/var/lib/libvirt/qemu/channel/target/domain--1-yakkety-sec-dac/*" rw,
  "/var/run/libvirt/**/yakkety-sec-dac.pid" rwk,
  "/run/libvirt/**/yakkety-sec-dac.pid" rwk,
  "/var/run/libvirt/**/*.tunnelmigrate.dest.yakkety-sec-dac" rw,
  "/run/libvirt/**/*.tunnelmigrate.dest.yakkety-sec-dac" rw,
  "/var/lib/uvtool/libvirt/images/yakkety-sec-dac.qcow" rw,
  "/var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTYuMTA6YW1kNjQgMjAxNjEwMjI=" r,
  "/var/lib/uvtool/libvirt/images/yakkety-sec-dac-ds.qcow" rw,
  # for qemu guest agent channel
  owner "/var/lib/libvirt/qemu/channel/target/domain-yakkety-sec-dac/**" rw,
  /dev/vhost-net rw,

New running debuild locally on xenial and yakkety libvirt to have the packaged aa-helper in a debuggable and recompilable fashion.

Changed in libvirt (Ubuntu):
importance: Undecided → Medium
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Once more confirmed that it worked in Xenial - adding regression-release

tags: added: regression-release
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (4.2 KiB)

TL;DR:
- a dac sec label is parsed
- it has no label, but due to a bug it searches one
- label can't be found for an inactive domain
- exit with Error
- expected fix is reverting part of dfbc9a83

Debug-Analysis:

Interesting part of the call chain:
get_definition -> virDomainDefParseString -> virDomainDefParse -> virDomainDefParseNode -> virDomainDefParseXML -> virSecurityLabelDefsParseXML -> virSecurityLabelDefParseXML

Compiled -O0 -g to see more to see where it is failing.
The code itself (of that failing function) didn't change since 1.3.1 (Xenial).

gdb ~/libvirt-2.1.0/debian/tmp/usr/lib/libvirt/virt-aa-helper
set env LD_LIBRARY_PATH /home/ubuntu/libvirt-2.1.0/debian/tmp/usr/lib/x86_64-linux-gnu/
set solib-search-path /home/ubuntu/libvirt-2.1.0/debian/tmp/usr/lib/x86_64-linux-gnu/
b virSecurityLabelDefsParseXML
run -d -r -p 0 -u libvirt-6e082f89-902c-413c-9d9e-f609089d3374 < yakkety-sec-dac.xml

virSecurityLabelDefParseXML (ctxt=0x5555557ddaf0, flags=1024) at ../../../src/conf/domain_conf.c:6384
n (number of labels) is 1
single def parse in virSecurityLabelDefParseXML
1. type dynamic = VIR_DOMAIN_SECLABEL_DYNAMIC
2. relabel yes
3-5 useless if/jumps
6. fails at parsing the actual label
   it doesn't find a label, but thinks it needs one
   check:
   6.1 seclabel->type == VIR_DOMAIN_SECLABEL_STATIC => it is not
   6.2 !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && => true
   6.3 seclabel->type != VIR_DOMAIN_SECLABEL_NONE => true
=> There is no label for the currently off machine, so it fails to find one and goes to error path

The function does right, but the flags suggest it would be alive.
Definiton:
    /* Parse only parts of the XML that would be present in an inactive libvirt
     * XML. Note that the flag does not imply that ABI incompatible
     * transformations can be used, since it's used to strip runtime info when
     * restoring save images/migration. */
    VIR_DOMAIN_DEF_PARSE_INACTIVE = 1 << 1,

The flag comes from the first in the call chain "get_definition"
   ctl->def = virDomainDefParseString(xmlStr, ctl->caps, ctl->xmlopt, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);

That exactly is a diff of the Ubuntu versions on that call:
     ctl->def = virDomainDefParseString(xmlStr,
                                        ctl->caps, ctl->xmlopt,
- VIR_DOMAIN_DEF_PARSE_INACTIVE);
+ VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);

Almost all other changes do OR it in:
- int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+ int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+ VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;

Check upstream for the reasons:

commit b394af162a3871575d9f9c28f72331f198aafa25
Author: Peter Krempa <email address hidden>
Date: Thu May 26 15:58:53 2016 +0200
    conf: Add infrastructure for adding configuration validation

On the critical place before there was a 0, so setting fix was like ORing in.
Why was there a 0 and not VIR_DOMAIN_DEF_PARSE_INACTIVE like in the past?

That was the reason there was a 0 before b394af16 came in:
commit dfbc9a8382adc0495bf0e034ae6add92bed4822b
Author: Guido Günther <a...

Read more...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I made an experimental fix available at https://launchpad.net/~paelzer/+archive/ubuntu/libvirt-bug-1633207

In the pure aa-helper tests that continues to work with all my usual minor tests and it fixes dac and dac+apparmor label issues.
But I seem to run into issues with doing full guests:
qemu-system-x86_64: -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-2-yakkety-sec-testfix/master-key.aes: Unable to read /var/lib/libvirt/qemu/domain-2-yakkety-sec-testfix/master-key.aes: Failed to open file '/var/lib/libvirt/qemu/domain-2-yakkety-sec-testfix/master-key.aes': Permission denie

That is due to an apparmor deny and might be that this was the issue that was tried to be fixed with the breaking change in libvirt?

I'll run more tests on it on my own.
But I'd really like to coordinate with the author of the former change what the test case was that made him create the patch.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I subscribed agx, the author of the conflicting patch upstream.

Questions:
agx - Could you please comment on my finding?
agx - Please describe your case that let you write dfbc9a83?
IBM - I don't think it helps yet, but if you can please try to verify the ppa I provide at https://launchpad.net/~paelzer/+archive/ubuntu/libvirt-bug-1633207

I'm available on freenode e.g. in #ubuntu-server as cpaelzer.
This likely is an extended weekend for both of us, but please feel free to try to catch me there.

Revision history for this message
Guido Günther (agx) wrote :

dfbc9a83 was necessary since libvirt changed the paths of the monitor socket in a89f05ba8df095875f5ec8a9065a585af63a010b. We had to switch from VIR_DOMAIN_DEF_PARSE_INACTIVE to active since we need the domain id (ctl->def->id) as it is part of the socket path now. It would probably o.k. to skip validation but we need to parse the active domain config to get the id.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks a lot Guido for your feedback - it helped me better "reading the case".

I see the same issue throughout latest libvirt upstream as of today.
So I'm gonna submit the fix upstream for discussion as I could easily overlook something here.
E.g. parseOpaque is quite close as it is passed up to virDomainDefParseXML, but I think that would be even more misuse than a new flag.

If accepted there (one way or the other) I intend to create a diff to upload for latest Debian and Ubuntu and consider SRUs from there.

The created domain while active has both seclabels and valid content in them just as it had back on libvirt 1.3:
[...]
  <seclabel type='dynamic' model='apparmor' relabel='yes'>
    <label>libvirt-956134c4-d91d-417e-b68f-1d8d492419d6</label>
    <imagelabel>libvirt-956134c4-d91d-417e-b68f-1d8d492419d6</imagelabel>
  </seclabel>
  <seclabel type='dynamic' model='dac' relabel='yes'>
    <label>+112:+116</label>
    <imagelabel>+112:+116</imagelabel>
  </seclabel>

@AGX - I'll set you on cc on that upstream discussion.

@IBM - I have a new version (2.1.0-1ubuntu10~ppa5) in the ppa that works for me, It would be great if you could verify this one for you as well.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2016-11-03 02:33 EDT-------
(In reply to comment #25)
> Thanks a lot Guido for your feedback - it helped me better "reading the
> case".
>
> I see the same issue throughout latest libvirt upstream as of today.
> So I'm gonna submit the fix upstream for discussion as I could easily
> overlook something here.
> E.g. parseOpaque is quite close as it is passed up to virDomainDefParseXML,
> but I think that would be even more misuse than a new flag.
>
> If accepted there (one way or the other) I intend to create a diff to upload
> for latest Debian and Ubuntu and consider SRUs from there.
>
> The created domain while active has both seclabels and valid content in them
> just as it had back on libvirt 1.3:
> [...]
> <seclabel type='dynamic' model='apparmor' relabel='yes'>
> <label>libvirt-956134c4-d91d-417e-b68f-1d8d492419d6</label>
> <imagelabel>libvirt-956134c4-d91d-417e-b68f-1d8d492419d6</imagelabel>
> </seclabel>
> <seclabel type='dynamic' model='dac' relabel='yes'>
> <label>+112:+116</label>
> <imagelabel>+112:+116</imagelabel>
> </seclabel>
>
> @AGX - I'll set you on cc on that upstream discussion.
>
> @IBM - I have a new version (2.1.0-1ubuntu10~ppa5) in the ppa that works for
> me, It would be great if you could verify this one for you as well.
>
Tried to install update and could not found any update in ppa.
Looks like it has not been built for ppc64el(https://launchpad.net/~paelzer/+archive/ubuntu/libvirt-bug-1633207/+packages)
Can you add it for me to test?
Thanks in advance.

#add-apt-repository ppa:paelzer/libvirt-bug-1633207

Early verification of libvirt-bug-1633207
More info: https://launchpad.net/~paelzer/+archive/ubuntu/libvirt-bug-1633207
Press [ENTER] to continue or ctrl-c to cancel adding it
gpg: keybox '/tmp/tmpypsm5vem/pubring.gpg' created
gpg: /tmp/tmpypsm5vem/trustdb.gpg: trustdb created
gpg: key 42C3EB37B6832E30: public key "Launchpad PPA for ChristianEhrhardt" imported
gpg: Total number processed: 1
gpg: imported: 1
OK
#apt-get update
Hit:1 http://us.ports.ubuntu.com/ubuntu-ports yakkety InRelease
Hit:2 http://us.ports.ubuntu.com/ubuntu-ports yakkety-updates InRelease
Hit:3 http://ports.ubuntu.com/ubuntu-ports yakkety-security InRelease
Hit:4 http://us.ports.ubuntu.com/ubuntu-ports yakkety-backports InRelease
Get:5 http://ppa.launchpad.net/paelzer/libvirt-bug-1633207/ubuntu yakkety InRelease [18.1 kB]
Get:6 http://ppa.launchpad.net/paelzer/libvirt-bug-1633207/ubuntu yakkety/main ppc64el Packages [516 B]
Get:7 http://ppa.launchpad.net/paelzer/libvirt-bug-1633207/ubuntu yakkety/main Translation-en [984 B]
Fetched 19.6 kB in 1s (14.6 kB/s)
Reading package lists... Done

# apt upgrade
Reading package lists... Done
Building dependency tree
Reading state information... Done
Calculating upgrade... Done
0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.

> FYI discussion started at
> https://www.redhat.com/archives/libvir-list/2016-October/msg01297.html

Revision history for this message
Christian Ehrhardt  (paelzer) wrote : Re: [Bug 1633207] Comment bridged from LTC Bugzilla

On Thu, Nov 3, 2016 at 7:40 AM, bugproxy <email address hidden> wrote:

> Tried to install update and could not found any update in ppa.
> Looks like it has not been built for ppc64el(https://launchpad.net/
> ~paelzer/+archive/ubuntu/libvirt-bug-1633207/+packages)
> Can you add it for me to test?
> Thanks in advance.
>

I beg a pardon, I tested all on amd64 where it obviously worked.
Of course I can enable and upload a new rev.
Rebuilding in the ppa for ppc64el this very moment - please retest once it
is built.

Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla

------- Comment From <email address hidden> 2016-11-03 07:03 EDT-------
(In reply to comment #27)
> On Thu, Nov 3, 2016 at 7:40 AM, bugproxy <email address hidden> wrote:
>
> > Tried to install update and could not found any update in ppa.
> > Looks like it has not been built for ppc64el(https://launchpad.net/
> > ~paelzer/+archive/ubuntu/libvirt-bug-1633207/+packages)
> > Can you add it for me to test?
> > Thanks in advance.
> >
>
> I beg a pardon, I tested all on amd64 where it obviously worked.
> Of course I can enable and upload a new rev.
> Rebuilding in the ppa for ppc64el this very moment - please retest once it
> is built.

Have tested with the given packages, and now VM starts with dac secuity driver added in the xml.
# dpkg -l |grep libvirt
ii libvirt-bin 2.1.0-1ubuntu10~ppa6 ppc64el programs for the libvirt library
ii libvirt-clients 2.1.0-1ubuntu10~ppa6 ppc64el Programs for the libvirt library
ii libvirt-daemon 2.1.0-1ubuntu10~ppa6 ppc64el Virtualization daemon
ii libvirt-daemon-system 2.1.0-1ubuntu10~ppa6 ppc64el Libvirt daemon configuration files
ii libvirt-dev:ppc64el 2.1.0-1ubuntu10~ppa6 ppc64el development files for the libvirt library
ii libvirt-glib-1.0-0:ppc64el 0.2.3-2 ppc64el libvirt GLib and GObject mapping library
ii libvirt0:ppc64el 2.1.0-1ubuntu10~ppa6 ppc64el library for interfacing with different virtualization systems
ii python-libvirt 2.0.0-1 ppc64el libvirt Python bindings

# uname -a
Linux ltc-test-ci1 4.8.0-26-generic #28-Ubuntu SMP Tue Oct 18 14:41:40 UTC 2016 ppc64le ppc64le ppc64le GNU/Linux

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thank you a lot for verifying the ppa.

Since this isn't critically urgent I'll wait with a fix upload to the package until the upstream discussion settled (better than to revert in two weeks again).

Once https://www.redhat.com/archives/libvir-list/2016-October/msg01297.html
followed in November by thread https://www.redhat.com/archives/libvir-list/2016-November/msg00229.html conclude I'll go forward on this.

Changed in libvirt (Ubuntu):
status: Confirmed → In Progress
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Unfortunately upstream response is super slow on this.
I think the patch is right and therefore I'm willing to put it into zesty as being a dev release in development.
That will also give us more coverage if there is anything we might have missed.

That said pushed it to Zesty now the way it was tested.by me and the reporter.

Since it is not an issue for Xenial there is not SRU need there but for Yakkety I'd only consider an SRU once upstream discussion settled and accepted it.
That said @IBM - if you want to request an SRU on this into Yakkety I'd ask you to join the upstream discussion on libvirt to give it some weight by a third party pushing for it.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

What worked last week doesn't have to this week - I ran into an FTBFS - please wait a bit until resolved.

Revision history for this message
bugproxy (bugproxy) wrote :

dpkg -S | xargs) | sort

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libvirt - 2.1.0-1ubuntu13

---------------
libvirt (2.1.0-1ubuntu13) zesty; urgency=medium

  * drop d/p/ubuntu/fix-ftbfs-for-gnutls-3-5-6.patch as the offending change
    in gnutls has been reverted (LP: #1641615)
  * Build depend on gnutls >= 3.5.6-4ubuntu2 to build after the gnutls fix
    migrated

 -- Christian Ehrhardt <email address hidden> Thu, 17 Nov 2016 08:43:10 +0100

Changed in libvirt (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI - The backport SRU to Yakkety will have to wait until we have a upstream accepted solution.

Changed in libvirt (Ubuntu Yakkety):
status: New → Triaged
importance: Undecided → Low
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

The refreshed upstream accepted fix is now available in Zesty as 2.1.0-1ubuntu14
With that ready now preparing the SRU into Yakkety.

description: updated
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Prepared SRU Template and Uploaded into the (Y) SRU review queue.

Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello bugproxy, or anyone else affected,

Accepted libvirt into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/2.1.0-1ubuntu9.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 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 to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

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

Changed in libvirt (Ubuntu Yakkety):
status: Triaged → Fix Committed
tags: added: verification-needed
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Since there was no reply to verify in a week I felt I had to clear the queue and tested it myself again.

@bugproxy: In general - a.k.a. for next time - I'd really like to have 3rd party verification. Not to save me the 15 minutes, but to make sure it really addresses your issue and get further verification if anything else was broken by the SRU. If it doesn't fit with your current plans that is fine, but then let me know an estimate when you expect you get to it.

tags: added: verification-done
removed: verification-needed
Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla
Download full text (4.0 KiB)

------- Comment From <email address hidden> 2016-12-12 03:13 EDT-------
Tested in my env aswell with the proposed stream,it is working as expected and issue is fixed
# dpkg -l|grep libvirt
ii libvirt-bin 2.1.0-1ubuntu9.1 ppc64el programs for the libvirt library
ii libvirt-clients 2.1.0-1ubuntu9.1 ppc64el Programs for the libvirt library
ii libvirt-daemon 2.1.0-1ubuntu9.1 ppc64el Virtualization daemon
ii libvirt-daemon-system 2.1.0-1ubuntu9.1 ppc64el Libvirt daemon configuration files
ii libvirt-dev:ppc64el 2.1.0-1ubuntu9.1 ppc64el development files for the libvirt library
ii libvirt-glib-1.0-0:ppc64el 0.2.3-2 ppc64el libvirt GLib and GObject mapping library
ii libvirt0:ppc64el 2.1.0-1ubuntu9.1 ppc64el library for interfacing with different virtualization systems
ii python-libvirt 2.0.0-1 ppc64el libvirt Python bindings

#virsh dumpxml virt-tests-vm1
<domain type='kvm' id='4'>
<name>virt-tests-vm1</name>
<uuid>e0e543b0-c05d-4653-8436-b6c4e4eca978</uuid>
<memory unit='KiB'>33554432</memory>
<currentMemory unit='KiB'>33554432</currentMemory>
<vcpu placement='static'>32</vcpu>
<resource>
<partition>/machine</partition>
</resource>
<os>
<type arch='ppc64le' machine='pseries-yakkety'>hvm</type>
<boot dev='hd'/>
</os>
<cpu>
<topology sockets='1' cores='4' threads='8'/>
</cpu>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/bin/kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/var/lib/libvirt/images/workspace/runAvocadoFVTTest/avocado-fvt-wrapper/data/avocado-vt/images/ubuntu-16.10-ppc64le.qcow2'/>
<backingStore/>
<target dev='sda' bus='scsi'/>
<alias name='scsi0-0-0-0'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</controller>
<controller type='usb' index='0'>
<alias name='usb'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</controller>
<controller type='pci' index='0' model='pci-root'>
<alias name='pci.0'/>
</controller>
<interface type='bridge'>
<mac address='52:54:00:23:24:25'/>
<source bridge='virbr0'/>
<target dev='vnet0'/>
<model type='virtio'/>
<alias name='net0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
</interface>
<serial type='pty'>
<source path='/dev/pts/1'/>
<target port='0'/>
<alias name='serial0'/>
<address type='spapr-vio' reg='0x30000000'/>
</serial>
<console type='pty' tty='/dev/pts/1'>
<source path='/dev/pts/1'/>
<target type='serial' port='0'/>
<alias name='serial0'/>
<address type='spapr-vio' reg='0x300...

Read more...

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libvirt - 2.1.0-1ubuntu9.1

---------------
libvirt (2.1.0-1ubuntu9.1) yakkety; urgency=medium

  * d/p/u/apparmor-fix-other-seclabels.patch fixes an issue parsing non
    apparmor security labels (LP: #1633207).

 -- Christian Ehrhardt <email address hidden> Thu, 01 Dec 2016 09:44:12 +0100

Changed in libvirt (Ubuntu Yakkety):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of the Stable Release Update for libvirt has completed successfully and the package has now been 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.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Local creation with a apparmor seclabel fails the same as the migration, so we can ignore all "migration specials".

To test that add:
    <seclabel type='dynamic' model='apparmor' relabel='yes'/>
    <seclabel type='dynamic' model='dac' relabel='yes'/>
to /usr/share/uvtool/libvirt/template.xml
And run uvt-kvm create again

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

Other bug subscribers

Bug attachments