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@xxxxxxxxxx> > >>--- > >> .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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list