[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 9/9] daemon: use socket activation with systemd



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:
Signed-off-by: Martin Kletzander <mkletzan redhat com>
---
 .gitignore                 |  1 +
 daemon/Makefile.am         | 14 ++++++++++++--
 daemon/libvirtd.conf       |  5 +++++
 daemon/libvirtd.service.in |  5 -----
 daemon/libvirtd.socket.in  |  6 ++++++
 libvirt.spec.in            | 26 +++++++++++++++++++++-----
 6 files changed, 45 insertions(+), 12 deletions(-)
 create mode 100644 daemon/libvirtd.socket.in

diff --git a/.gitignore b/.gitignore
index 90fee91..9776ea1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -60,6 +60,7 @@
 /daemon/libvirtd.pod
 /daemon/libvirtd.policy
 /daemon/libvirtd.service
+/daemon/libvirtd.socket
 /daemon/test_libvirtd.aug
 /docs/aclperms.htmlinc
 /docs/apibuild.py.stamp
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 00221ab..70b7655 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -55,6 +55,7 @@ EXTRA_DIST =						\
 	libvirtd.policy.in				\
 	libvirtd.sasl					\
 	libvirtd.service.in				\
+	libvirtd.socket.in				\
 	libvirtd.sysconf				\
 	libvirtd.sysctl					\
 	libvirtd.aug                                    \
@@ -388,15 +389,18 @@ endif ! LIBVIRT_INIT_SCRIPT_UPSTART
 if LIBVIRT_INIT_SCRIPT_SYSTEMD

 SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system
-BUILT_SOURCES += libvirtd.service
+BUILT_SOURCES += libvirtd.service libvirtd.socket

-install-init-systemd: install-sysconfig libvirtd.service
+install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket
 	$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
 	$(INSTALL_DATA) libvirtd.service \
 	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
+	$(INSTALL_DATA) libvirtd.socket \
+	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket

 uninstall-init-systemd: uninstall-sysconfig
 	rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
+	rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
 	rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || :
 else ! LIBVIRT_INIT_SCRIPT_SYSTEMD
 install-init-systemd:
@@ -420,6 +424,12 @@ libvirtd.service: libvirtd.service.in $(top_builddir)/config.status
 	    < $< > $ -t &&					\
 	    mv $ -t $@

+libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status
+	$(AM_V_GEN)sed						\
+	    -e 's|[ ]runstatedir[@]|$(runstatedir)|g'		\
+	    < $< > $ -t &&					\
+	    mv $ -t $@
+

 check-local: check-augeas

diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index e5856d4..b644e81 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -77,6 +77,11 @@
 # UNIX socket access controls
 #

+# Beware that if you are changing *any* of these options, and you use
+# socket activation with systemd, you need to adjust the settings in
+# the libvirtd.socket file as well since it could impose a security
+# risk if you rely on file permission checking only.
+
 # Set the UNIX domain socket group ownership. This can be used to
 # allow a 'trusted' set of users access to management capabilities
 # without becoming root.
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index 086da36..1759ac8 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -1,8 +1,3 @@
-# NB we don't use socket activation. When libvirtd starts it will
-# spawn any virtual machines registered for autostart. We want this
-# to occur on every boot, regardless of whether any client connects
-# to a socket. Thus socket activation doesn't have any benefit
-
 [Unit]
 Description=Virtualization daemon
 Before=libvirt-guests.service
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.


Martin

ACK to the chagne though

Regards,
Daniel
--
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]