3:38 a.m.
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(a)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 :|