Comment 5 for bug 1786159

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks Jamie as always for your "security pair of eyes"!
The denies are already gone for multiple reasons - so yes.

So I read from you that the dir reads themselve as you outlined above, but not all of user-temp abstraction is a safe compromise - ok I'lll go for that.

Yeah qemu-smb functionality in particular is not readable to libvirt currently.
But wit hthe argument that users can add it later we could as well remove half of the rules in there.
It at least has a clear "qemu-smb" part in its name - wouldn't that be acceptable.
So it would overall look like:

I'm following your guidance and drop the user-temp abstraction, instead the suggested change would now look like:

+Description: allow expected /tmp access patterns
+Several cases were found needing this, for example ceph will try to list /tmp
+and the samba feature of qemu will place things in /tmp/qemu-smb.*.
+This is sort of safe because:
+ - While /tmp could contain anything it is not recommended to put critical
+ data there anyway
+ - We restrict general access to dir listing and reading of files owned
+ - While it would be hard to predict the PID as part of the string for the
+ qemu smb feature (this is not exposed through XML so virt-aa-helper
+ can't help) it is guarded by the "owner" statement and a pretty clear
+ qemu-smb infix in the path.
+
+Forwarded: no
+Forward-info: Needs to be proven to work reliably first
+Author: Christian Ehrhardt <email address hidden>
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1786159
+Last-Update: 2018-08-09
+--- a/examples/apparmor/libvirt-qemu
++++ b/examples/apparmor/libvirt-qemu
+@@ -203,6 +203,16 @@
+ /var/lib/charm/*/ceph.conf r,
+ /run/ceph/rbd-client-*.asok rw,
+
++ # various functions will need /tmp, allow the base dir and a few known functions
++ # we want to avoid to give blanket read or even write to everything in /tmp
++ # so users are expected to add site specific addons for more uncommon cases
++ # allow dir listing, but no general file read
++ /{,var/}tmp/ r,
++ # only owner file read
++ owner /{,var/}tmp/**/ r,
++ # qemu smb feature specific path with write access
++ owner /tmp/qemu-smb.*/{,**} rw,
++
+ # kvm.powerpc executes/accesses this
+ /{usr/,}bin/uname rmix,
+ /{usr/,}sbin/ppc64_cpu rmix,

It would make ceph and smb happy already.

Others like the snapshot issue we had at least would get a proper log error (since no more silently denied) and people - as you said can add site specific addons. That is an example with unpredictable paths that I won't add.

Is that an acceptable tradeoff for known features to work?