Re: [libvirt] [PATCH v2 4/5] apparmor: allow qemu-smb access in /tmp

On Tue, 2018-08-14 at 08:18 +0200, Christian Ehrhardt wrote: While the facts are true, there is something omitted and I come to a different conclusion. It is true that the pid is unpredictable, but with the 'owner /{,var/}tmp/**/ r,' rule, it is easy for a confined process to find the directories. Also, the rule 'owner /tmp/qemu- smb.*/{,**} rw,' (below) allows writing /tmp/qemu-smb.*/ so symlink attacks are possible by a rogue VM against other VMs. Rogue VMs could also use the directory in coordinated file sharing (but I mention this only for completeness, not as an objection, since there are other rules in the policy that 'overlap' and allow this sort of thing). It is definitely a usability improvement to include the rule, but I disagree that it isn't safer without it-- your addition applies to all libvirt/apparmor users, many of whom will not use the smb feature that isn't supported by the domain xml. Now, you could argue that users that never use the smb feature aren't affected by the proposed global rule (which is true), but omitting this patch, users can add the glob rule to the per-VM site policy in /etc/apparmor.d/libvirt/libvirt-<uuid> for only the VMs they need it. This might be useful if they, for example, have one trusted VM that uses the smb feature and other untrusted VMs that do not. With the global rule, the untrusted VMs have access by default. These concerns are somewhat contrived and I'm ambivalent about the addition, but it bothers me that we are adding a rule for a use case that isn't directly supported by libvirt. +0 as is. If you can demonstrate that qemu guards against symlink attacks on the dir and is otherwise safe from attack (eg, open(..., O_CREAT|O_EXCL) followed by unlink(...)), etc, etc then I could be persuaded to give a +1. -- Jamie Strandboge | http://www.canonical.com