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

Christian Ehrhardt christian.ehrhardt at canonical.com
Mon Jun 11 06:43:06 UTC 2018


On Mon, Jun 11, 2018 at 8:12 AM, Michal Prívozník <mprivozn at redhat.com>
wrote:

> On 06/11/2018 07:34 AM, Christian Ehrhardt wrote:
> > 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".
>
> Thank you for your exhaustive explanation. You've convinced me that it's
> safe to merge this patch. However, what I still don't quite understand
> is: Nova uses that path for ages, doesn't it? How come we've hit the bug
> only now?
>

We didn't Ubuntu had this as downstream Delta as long as I can remember - I
guess only now someone drives Nova in Debian to that point.
And all that have done further have done local overrides.



> >
> > 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{,.*}
>
> Oh, I can't merge the patch as-is because it is missing S-O-B line which
> is required (https://libvirt.org/hacking.html). Also, it would be nice
> if you can use your real name.
>

We had the real name discussion before, but at least the S-O-B as agreed
last time should be added.

And I'd ask for an opinion on the "other" paths I listed - I can only
recommend adding as much as we can commonly agree to be useful.
To avoid coming back every few months adding another such line :-)



> Michal
>



-- 
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/b07bf87d/attachment-0001.htm>


More information about the libvir-list mailing list