[libvirt] [PATCH v3 9/9] daemon: use socket activation with systemd
Daniel P. Berrange
berrange at redhat.com
Thu Aug 14 09:07:34 UTC 2014
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 at 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 :|
More information about the libvir-list
mailing list