Comment 58 for bug 1393842

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

Hi,
thanks for the ping, this brought it to my attention, taking a look now ...

First of all to get Fedora/Virt-manager/... out of scope here a guest without virt-manager or anything else.
Using uvtool to create a very basic guest based on daily cloud images

$ uvt-simplestreams-libvirt --verbose sync --source http://cloud-images.ubuntu.com/daily arch=amd64 label=daily release=xenial
$ uvt-kvm create --password=ubuntu kvmguest-testgachannel release=xenial arch=amd64 label=daily

In later versions of libvirt/qemu in Xenial for example the apparmor rules automatically get a rule for channels of the guest agent:
   # for qemu guest agent channel
   owner "/var/lib/libvirt/qemu/channel/target/domain-kvmguest-testchannel/**" rw,
The individual guests are namespaced by the domain name (?now?) which makes it easier as there is no rule-per-channel needed as in the past.

But lets try to add a manual channel to see its path and behavior.
The raw basics of a guest channel would be:
    <channel type='unix'>
      <source mode='bind' />
      <target type='virtio' name='org.qemu.guest_agent.0'/>
    </channel>
Adding that (and not more) lets libvirt fill in the extra defaults.

The path that gets auto-assigned does not match the expected guest agent paths above.
That gets expanded on Xenial to:
    <channel type='unix'>
      <source mode='bind'/>
      <target type='virtio' name='org.qemu.guest_agent.0'/>
      <address type='virtio-serial' controller='0' bus='0' port='1'/>
    </channel>
But on Trusty it becomes:
    <channel type='unix'>
      <source mode='bind' path='/var/lib/libvirt/qemu/channel/target/kvmguest-testgachannel.org.qemu.guest_agent.0'/>
      <target type='virtio' name='org.qemu.guest_agent.0'/>
      <address type='virtio-serial' controller='0' bus='0' port='1'/>
    </channel>

See the path out that is still explicit and also not following the namespaceing of later versions.
The Xenial version generates the path on guest instantiation on the expected path and works as-is.
"-chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-kvmguest-testgachannel/org.qemu.guest_agent.0,server,nowait"

On Trusty this fails as reported:
$ virsh start kvmguest-testgachannel
error: Failed to start domain kvmguest-testgachannel
error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/kvmguest-testgachannel.org.qemu.guest_agent.0,server,nowait: Failed to bind socket: No such file or directory
qemu-system-x86_64: -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/kvmguest-testgachannel.org.qemu.guest_agent.0,server,nowait: chardev: opening backend "socket" failed

Yet there is no apparmor deny associated, it seems it just fails due to some of the underlying paths being non-existant (maybe it apparmor-fails later once they exist).
Yes that is step 1, after:
$ mkdir /var/lib/libvirt/qemu/channel
$ mkdir /var/lib/libvirt/qemu/channel/target
$ chown libvirt-qemu:kvm /var/lib/libvirt/qemu/channel
$ chown libvirt-qemu:kvm /var/lib/libvirt/qemu/channel/target/

Things get further and only then block on the apparmor denial:
apparmor="DENIED" operation="mknod" namespace="root//lxd-trusty-test_<var-lib-lxd>" profile="libvirt-4ec6a091-8aef-4bab-b8a4-ca46e91ed18f" name="/var/lib/libvirt/qemu/channel/target/kvmguest-testgachannel.org.qemu.guest_agent.0" pid=10517 comm="qemu-system-x86" requested_mask="c" denied_mask="c" fsuid=108 ouid=108

That could explain why fixing the rule did not solve it for the reporters - see comment #51 and #52. That in turn led to a non verified SRU and made it being removed from the SRU.

In some of the cases a user or tool might have created those.
But in fact if libvirt is defaulting to these paths it has to ensure they exist and also are of the right user/group.

Only then can the apparmor fix that was supposed help.
This was more or less described as early as comment #2, but then due to different setups and configs appeared one or the other way to different users discussing on this bug.

The rule suggested in #52 is almost safe in terms of following the guest-namespacing in the rule.
Obviously if one has full control of a guest definition he could create a name/path combo that will end up being on the same path, but in case of full guest definition control one can also just create a path="" entry to any of the paths he wants to attach.
So one could create a guest named "a" and and add a path to another guest named abc channel like
path=/var/lib/libvirt/qemu/channel/target/abc.channelname".
To make this a bit more safe the rule could contain the "." that is added on the namespacing.
"." is an allowed character on guest names, but one can name his guest then
name => rule
a => ../a.**
a. => ../a..**
Target that has to be "non reachable" is
abc => ../abc.**
So with this there should be no guest name that lets anyone other than "abc" get a rule that can access "abc.{channelname}" - more or less what in never versions the directories achieve in a nicer way.

I think (hope) what should help to fix this is an SRU that will:
1. add the directories /var/lib/libvirt/qemu/channel + /var/lib/libvirt/qemu/channel/target with proper ownership of libvirt-qemu:kvm on upgrade/install
   (and not fail if there are already directories)
   (also if there are these directories it shouldn't touch the perm, whatever a user has set up should be retained)
2. add the "old libvirt namespace" to generated rules of a trusty guest but with a tailing "." before the wildcard which would be:
   /var/lib/libvirt/qemu/channel/target/${domain-name}.**

Given the old issues of non-verification and mis-reading I'll not push an SRU right away.
Instead I'll try build something into a ppa that I want (myself and) others to try first.
Will update once I have something.