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

[Michal Prívozník]

On 06/11/2018 08:43 AM, Christian Ehrhardt wrote:
> 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:
>>>
>>> <snip/>
>> 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.

Ah, that explains it.
> 
>>>
>>> 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.

To my recollection even the usage of real name is a must (although we
don't have any means to verify that). Anyway, the point being we don't
allow any nicknames, TLAs (three letter acronyms), and so on. IANAL, but
I guess the reason for that is to protect libvirt from any future lawsuits.

> 
> 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 :-)

Well, yes we can add them. However, at that point I guess we might want
to revisit virt-aa-helper design? I mean, it's nice that we have two
step permission building, but then we might end up chasing changing
consumers of libvirt. So I guess if we want to preserve the two step
process we should encourage libvirt consumers to write their own rules
and have virt-aa-helper match against that. Of course, libvirt would
still provide some sensible default so that it works out of box, but in
this case OpenStack would provide an additional file to enable access to
/var/lib/nova/... (which is a private path to Nova and could be
considered internal implementation of Nova and therefore libvirt should
not care). Also, whenever there's a patch changing the path in Nova code
base it can be coupled with apparmor profile adjustment.

I'm not sure if there's a way to achieve this. Perhaps we could have a
separate directory and all other projects would store their profiles
there and libvirt would just include them all (among with the defaults
file described above). Then virt-aa-helper would generate its own rules
in second step.

Michal