[libvirt] [PATCH] AppArmor: allow virt-aa-helper read access to Nova's qcow backing files.

[Christian Ehrhardt]

Hi Intrigeri and Michal,
IMHO this is the right path as it is stage one of the two stage process to
allow images for guests with apparmor.

Stage 1:
Virt-aa-helper has to be allowed to access the paths to evaluate images for
some attributes and e.g. backing chains.
Due to that virt-aa-helper needs access to the paths we expect the images
in.
Further more uncommon paths shall be handled by a local include #include
<local/usr.lib.libvirt.virt-aa-helper> where a user can add his special
cases.

Stage 2:
virt-aa-helper generated the guest apparmor profile, and in there will be
entries for all the used image files of a backing chain.
The guest can work due to the file allowing this.

With virt-aa-helper not able to read those paths in stage1 some entries
might be missing for stage2.
IMHO here the suggestion is just to open up stage1 a bit to get it working.

But if that is right and we are considering the change, the we should go
further and add more.
As a background, I had the proposed rule in Ubuntu for quite a while,
together with a bunch of other common (for us) paths.
I always considered them a downstream decision as nova could have been
somewhere else for other downstreams.
The same applies to paths for other tools.

In fact you'll see in the rules that there is some history to this with not
only nova, but also eucalyptus, then the ubuntu specific uvtool as well as
two snap specific paths which actually would be useful everywhere.

+  # nova base images (LP: #907269)
+  /var/lib/nova/images/** r,
+  /var/lib/nova/instances/_base/** r,
+  # nova snapshots (LP: #1244694)
+  /var/lib/nova/instances/snapshots/** r,
+  # nova base/snapshot files in snapped nova (LP: #1644507)
+  /var/snap/nova-hypervisor/common/instances/_base/** r,
+  /var/snap/nova-hypervisor/common/instances/snapshots/** r,
+  # eucalyptus (LP: #564914)
+  /var/lib/eucalyptus/instances/**/disk* r,
+  # eucalyptus loader (LP: #637544)
+  /var/lib/eucalyptus/instances/**/loader* r,
+  # for uvtool
+  /var/lib/uvtool/libvirt/images/** r,
+  # for multipass
+  /var/snap/multipass/common/data/multipassd/vault/instances/** r

Since this is only opening up the paths for virt-aa-helper and not the
guest it is rather safe.
And I always likes it more to explicitly list the paths we want instead of
the almost too global /**/disk{,.*} rule.

So I'm +0.75, lacking the final 0.25 only for the reason of considering it
a downstream thing so far.
OTOH if we can agree on the paths I'd totally love to see these paths
upstream, but then I'd prefer to add like all the paths I referred and not
only "the one just found".



On Sun, Jun 10, 2018 at 9:08 AM, Michal Prívozník <mprivozn at redhat.com>
wrote:

> On 06/09/2018 09:29 PM, intrigeri+libvirt at boum.org wrote:
> > From: intrigeri <intrigeri+libvirt at boum.org>
> >
> > As reported on https://bugs.debian.org/892431, without this rule, when
> launching
> > a QEMU KVM instance, an error occurs immediately upon launching the QEMU
> > process such as:
> >
> >   Could not open backing file: Could not open
> >   '/var/lib/nova/instances/_base/affe96668a4c64ef380ff1c71b4cae
> c17039080e':
> >   Permission denied
> >
> > The other instance disk images are already covered by the existing rule:
> >
> >   /**/disk{,.*} r
> > ---
> >  examples/apparmor/usr.lib.libvirt.virt-aa-helper | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> > index 6869685c05..e32402a904 100644
> > --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> > +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> > @@ -50,6 +50,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper
> {
> >    @{HOME}/** r,
> >    /var/lib/libvirt/images/ r,
> >    /var/lib/libvirt/images/** r,
> > +  /var/lib/nova/instances/_base/* r
> >    /{media,mnt,opt,srv}/** r,
> >    # For virt-sandbox
> >    /{,var/}run/libvirt/**/[sv]d[a-z] r,
> >
>
> I am not convinced this is correct fix. This would fix only some use
> cases where base image is under /var/lib/nova/.. path. The root cause
> seems to be virt-aa-helper not putting backing store into the profile
> but looking into the code it does. So we might need to debug
> virt-aa-helper adding disks with backing chain instead of blindly
> allowing some path.
>
> Michal
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>



-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180611/fcc183f3/attachment-0001.htm>