[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 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:
> >>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.

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.

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 :|


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