On Thu, Aug 14, 2014 at 10:07:34AM +0100, Daniel P. Berrange wrote:
On Thu, Aug 14, 2014 at 10:38:42AM +0200, Martin Kletzander wrote:On Wed, Aug 13, 2014 at 04:10:12PM +0100, Daniel P. Berrange wrote: >On Wed, Jul 23, 2014 at 04:27:13PM +0200, Martin Kletzander wrote: >>diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in >>new file mode 100644 >>index 0000000..86cc3f4 >>--- /dev/null >>+++ b/daemon/libvirtd.socket.in >>@@ -0,0 +1,6 @@ >>+[Socket] >>+ListenStream= runstatedir@/libvirt/libvirt-sock >>+ListenStream= runstatedir@/libvirt/libvirt-sock-ro >>+SocketMode=0777 >>+SocketUser=root >>+SocketGroup=root > >Perhaps add a comment in this file about Mode=0777 *only* being >safe if you have libvirtd.conf doing authentication (eg polkit) >on both UNIX sockets. > [I'm starting to regret that I wanted to fix some simple timeout-before-error issue :)] I can add the comment, but I just realized that we can't ship it this way. If someone has no authentication set up and the socket allowed only for root (for example), the machine would be vulnerable after update to the version with this libvirtd.socket. If, on the other hand, we put here 0700 for example, lot of applications may stop working, because they rely on authentication with 0777. And the daemon can do chmod() on the socket *only* to more lax permissions (not the other way around, as it would result in the same problem why we needed to add the comment in the first place). The solutions I came up with are: - Have SocketMode=0700 and do chmod() in the daemon to adjust the mode to permissions from the config file. And the better one: - Drop this whole socket starting stuff, because if there's a race, it's a systemd's problem. We call sd_notify(0, "READY=1") when everything is set up as systemd wants us to! I just discovered that now.Actually there's a 3rd option - Don't run 'systemctl enable libvirtd.socket' That way we provide the ability to use it, but don't turn it on - people have to explicitly opt-in.
I still don't see what would be the added value, but it certainly is. I'll do that (with the added comment) in next version. Martin
Description: Digital signature