<div dir="ltr">Hi Intrigeri and Michal,<div>IMHO this is the right path as it is stage one of the two stage process to allow images for guests with apparmor.</div><div><br></div><div>Stage 1:</div><div>Virt-aa-helper has to be allowed to access the paths to evaluate images for some attributes and e.g. backing chains.</div><div>Due to that virt-aa-helper needs access to the paths we expect the images in.</div><div>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.</div><div><br></div><div>Stage 2:</div><div>virt-aa-helper generated the guest apparmor profile, and in there will be entries for all the used image files of a backing chain.</div><div>The guest can work due to the file allowing this.</div><div><br></div><div>With virt-aa-helper not able to read those paths in stage1 some entries might be missing for stage2.</div><div>IMHO here the suggestion is just to open up stage1 a bit to get it working.</div><div><br></div><div>But if that is right and we are considering the change, the we should go further and add more.</div><div>As a background, I had the proposed rule in Ubuntu for quite a while, together with a bunch of other common (for us) paths.</div><div>I always considered them a downstream decision as nova could have been somewhere else for other downstreams.</div><div>The same applies to paths for other tools.</div><div><br></div><div>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.</div><div><br></div><div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">+  # nova base images (LP: #907269)
</span><br>+  /var/lib/nova/images/** r,
<br>+  /var/lib/nova/instances/_base/** r,
<br>+  # nova snapshots (LP: #1244694)
<br>+  /var/lib/nova/instances/snapshots/** r,
<br><span style="font-size:small;text-decoration-style:initial;text-decoration-color:initial;color:rgb(0,0,0);background-color:rgb(255,255,255)">+  # nova base/snapshot files in snapped nova (LP: #1644507)<span> </span></span><br style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">+  /var/snap/nova-hypervisor/common/instances/_base/** r,<span> </span></span><br style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">+  /var/snap/nova-hypervisor/common/instances/snapshots/** r,</span><br></span></div><div><span style="font-family:monospace">+  # eucalyptus (LP: #564914)
<br>+  /var/lib/eucalyptus/instances/**/disk* r,
<br>+  # eucalyptus loader (LP: #637544)
<br>+  /var/lib/eucalyptus/instances/**/loader* r,
<br>+  # for uvtool
<br>+  /var/lib/uvtool/libvirt/images/** r,
<br>+  # for multipass
<br>+  /var/snap/multipass/common/data/multipassd/vault/instances/** r<br></span><br></div><div>Since this is only opening up the paths for virt-aa-helper and not the guest it is rather safe.</div><div>And I always likes it more to explicitly list the paths we want instead of the almost too global <span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">/**/</span><span style="color:rgb(0,0,0);background-color:rgb(255,255,84)">disk</span><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">{,.*} rule.</span><br></span></div><div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)"><br></span></span></div>So I'm +0.75, lacking the final 0.25 only for the reason of considering it a downstream thing so far.<br>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".<div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Jun 10, 2018 at 9:08 AM, Michal Prívozník <span dir="ltr"><<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 06/09/2018 09:29 PM, <a href="mailto:intrigeri%2Blibvirt@boum.org">intrigeri+libvirt@boum.org</a> wrote:<br>
> From: intrigeri <<a href="mailto:intrigeri%2Blibvirt@boum.org">intrigeri+libvirt@boum.org</a>><br>
> <br>
> As reported on <a href="https://bugs.debian.org/892431" rel="noreferrer" target="_blank">https://bugs.debian.org/892431</a><wbr>, without this rule, when launching<br>
> a QEMU KVM instance, an error occurs immediately upon launching the QEMU<br>
> process such as:<br>
> <br>
>   Could not open backing file: Could not open<br>
>   '/var/lib/nova/instances/_<wbr>base/<wbr>affe96668a4c64ef380ff1c71b4cae<wbr>c17039080e':<br>
>   Permission denied<br>
> <br>
> The other instance disk images are already covered by the existing rule:<br>
> <br>
>   /**/disk{,.*} r<br>
> ---<br>
>  examples/apparmor/usr.lib.<wbr>libvirt.virt-aa-helper | 1 +<br>
>  1 file changed, 1 insertion(+)<br>
> <br>
> diff --git a/examples/apparmor/usr.lib.<wbr>libvirt.virt-aa-helper b/examples/apparmor/usr.lib.<wbr>libvirt.virt-aa-helper<br>
> index 6869685c05..e32402a904 100644<br>
> --- a/examples/apparmor/usr.lib.<wbr>libvirt.virt-aa-helper<br>
> +++ b/examples/apparmor/usr.lib.<wbr>libvirt.virt-aa-helper<br>
> @@ -50,6 +50,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-<wbr>aa-helper {<br>
>    @{HOME}/** r,<br>
>    /var/lib/libvirt/images/ r,<br>
>    /var/lib/libvirt/images/** r,<br>
> +  /var/lib/nova/instances/_base/<wbr>* r<br>
>    /{media,mnt,opt,srv}/** r,<br>
>    # For virt-sandbox<br>
>    /{,var/}run/libvirt/**/[sv]d[<wbr>a-z] r,<br>
> <br>
<br>
</div></div>I am not convinced this is correct fix. This would fix only some use<br>
cases where base image is under /var/lib/nova/.. path. The root cause<br>
seems to be virt-aa-helper not putting backing store into the profile<br>
but looking into the code it does. So we might need to debug<br>
virt-aa-helper adding disks with backing chain instead of blindly<br>
allowing some path.<br>
<span class="HOEnZb"><font color="#888888"><br>
Michal<br>
</font></span><br>--<br>
libvir-list mailing list<br>
<a href="mailto:libvir-list@redhat.com">libvir-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/libvir-list</a><br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="color:rgb(136,136,136);font-size:12.8px">Christian Ehrhardt</span><div style="color:rgb(136,136,136);font-size:12.8px">Software Engineer, Ubuntu Server</div><div style="color:rgb(136,136,136);font-size:12.8px">Canonical Ltd</div></div></div></div></div>
</div>