From b7ed8ce981127d91300fa3e372c6ff87fa9fb6ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 4 Jul 2019 12:33:23 +0100 Subject: [PATCH] remote: introduce virtproxyd daemon to handle IP connectivity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The libvirtd daemon provides the traditional libvirt experience where all the drivers are in a single daemon, and is accessible over both local UNIX sockets and remote IP sockets. In the new world we're having a set of per-driver daemons which will primarily be accessed locally via their own UNIX sockets. We still, however, need to allow for case of applications which will connect to libvirt remotely. These remote connections can be done as TCP/TLS sockets, or by SSH tunnelling to the UNIX socket. In the later case, the old libvirt.so clients will only know about the path to the old libvirtd socket /var/run/libvirt/libvirt-sock, and not the new driver sockets /var/run/libvirt/virtqemud-sock. It is also not desirable to expose the main driver specific daemons over IP directly to minimize their attack service. Thus the virtproxyd daemon steps into place, to provide TCP/TLS sockets, and back compat for the old libvirtd UNIX socket path(s). It will then forward all RPC calls made to the appropriate driver specific daemon. Essentially it is equivalent to the old libvirtd with absolutely no drivers registered except for the remote driver (and other stateless drivers in libvirt.so). We could have modified libvirtd so none of the drivers are registed to get the same end result. We could even add a libvirtd.conf parameter to control whether the drivers are loaded to enable users to switch back to the old world if we discover bugs in the split-daemon model. Using a new daemon though has some advantages - We can make virtproxyd and the virtXXXd per-driver daemons all have "Conflicts: libvirtd.service" in their systemd unit files. This will guarantee that libvirtd is never started at the same time, as this would result in two daemons running the same driver. Fortunately drivers use locking to protect themselves, but it is better to avoid starting a daemon we know will conflict. - It allows us to break CLI compat to remove the --listen parameter. Both listen_tcp and listen_tls parameters in /etc/libvirtd/virtd.conf will default to zero. Either TLS or TCP can be enabled exclusively though virtd.conf without requiring the extra step of adding --listen. - It allows us to set a strict SELinux policy over virtproxyd. For back compat the libvirtd policy must continue to allow all drivers to run. We can't easily give a second policy to libvirtd which locks it down. By introducing a new virtproxyd we can set a strict policy for that daemon only. - It gets rid of the weird naming of having a daemon with "lib" in its name. Now all normal daemons libvirt ships will have "virt" as their prefix not "libvirt". - Distros can more easily choose their upgrade path. They can ship both sets of daemons in their packages, and choose to either enable libvirtd, or enable the per-driver daemons and virtproxyd out of the box. Users can easily override this if desired by just tweaking which systemd units are active. After some time we can deprecate use of libvirtd and after some more time delete it entirely, leaving us in a pretty world filled with prancing unicorns. The main downside with introducing a new daemon, and with the per-driver daemons in general, is figuring out the correct upgrade path. The conservative option is to leave libvirtd running if it was an existing installation. Only use the new daemons & virtproxyd on completely new installs. The aggressive option is to disable libvirtd if already running and activate all the new daemons. Reviewed-by: Andrea Bolognani Reviewed-by: Christophe de Dinechin Signed-off-by: Daniel P. Berrangé --- .gitignore | 4 ++ libvirt.spec.in | 10 +++ src/remote/Makefile.inc.am | 108 +++++++++++++++++++++++++++--- src/remote/remote_daemon.c | 30 ++++++--- src/remote/remote_daemon_config.c | 6 +- src/remote/virtproxyd.service.in | 24 +++++++ 6 files changed, 162 insertions(+), 20 deletions(-) create mode 100644 src/remote/virtproxyd.service.in diff --git a/.gitignore b/.gitignore index 35dc2c8bec..9ad159ecfd 100644 --- a/.gitignore +++ b/.gitignore @@ -161,6 +161,9 @@ /src/remote/libvirtd.aug /src/remote/libvirtd.conf /src/remote/test_libvirtd.aug +/src/remote/test_virtproxyd.aug +/src/remote/virtproxyd.aug +/src/remote/virtproxyd.conf /src/rpc/virkeepaliveprotocol.[ch] /src/rpc/virnetprotocol.[ch] /src/test*.aug @@ -169,6 +172,7 @@ /src/virt-aa-helper /src/virtlockd /src/virtlogd +/src/virtproxyd /src/virt-guest-shutdown.target /tests/*.log /tests/*.pid diff --git a/libvirt.spec.in b/libvirt.spec.in index 6f96fbec33..2acff40ea7 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1527,6 +1527,12 @@ exit 0 %{_unitdir}/libvirtd-admin.socket %{_unitdir}/libvirtd-tcp.socket %{_unitdir}/libvirtd-tls.socket +%{_unitdir}/virtproxyd.service +%{_unitdir}/virtproxyd.socket +%{_unitdir}/virtproxyd-ro.socket +%{_unitdir}/virtproxyd-admin.socket +%{_unitdir}/virtproxyd-tcp.socket +%{_unitdir}/virtproxyd-tls.socket %{_unitdir}/virt-guest-shutdown.target %{_unitdir}/virtlogd.service %{_unitdir}/virtlogd.socket @@ -1538,6 +1544,7 @@ exit 0 %config(noreplace) %{_sysconfdir}/sysconfig/virtlogd %config(noreplace) %{_sysconfdir}/sysconfig/virtlockd %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf +%config(noreplace) %{_sysconfdir}/libvirt/virtproxyd.conf %config(noreplace) %{_sysconfdir}/libvirt/virtlogd.conf %config(noreplace) %{_sysconfdir}/libvirt/virtlockd.conf %config(noreplace) %{_sysconfdir}/sasl2/libvirt.conf @@ -1565,6 +1572,8 @@ exit 0 %{_datadir}/augeas/lenses/tests/test_virtlogd.aug %{_datadir}/augeas/lenses/virtlockd.aug %{_datadir}/augeas/lenses/tests/test_virtlockd.aug +%{_datadir}/augeas/lenses/virtproxyd.aug +%{_datadir}/augeas/lenses/tests/test_virtproxyd.aug %{_datadir}/augeas/lenses/libvirt_lockd.aug %if %{with_qemu} %{_datadir}/augeas/lenses/tests/test_libvirt_lockd.aug @@ -1579,6 +1588,7 @@ exit 0 %attr(0755, root, root) %{_libexecdir}/libvirt_iohelper %attr(0755, root, root) %{_sbindir}/libvirtd +%attr(0755, root, root) %{_sbindir}/virtproxyd %attr(0755, root, root) %{_sbindir}/virtlogd %attr(0755, root, root) %{_sbindir}/virtlockd diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index c08d59df1f..0a3aef1ec1 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -89,18 +89,40 @@ SYSCONF_FILES += remote/libvirtd.sysconf PODFILES += remote/libvirtd.pod MANINFILES += libvirtd.8.in -LIBVIRTD_UNIT_FILES_IN = \ - remote/libvirtd.service.in \ +LIBVIRTD_SOCKET_UNIT_FILES_IN = \ remote/libvirtd.socket.in \ remote/libvirtd-ro.socket.in \ remote/libvirtd-admin.socket.in \ remote/libvirtd-tcp.socket.in \ remote/libvirtd-tls.socket.in \ + $(NULL) + +LIBVIRTD_SOCKET_UNIT_FILES = $(notdir $(LIBVIRTD_SOCKET_UNIT_FILES_IN:%.in=%)) + +LIBVIRTD_UNIT_FILES_IN = \ + remote/libvirtd.service.in \ + $(LIBVIRTD_SOCKET_UNIT_FILES_IN) \ + $(NULL) + +VIRTPROXYD_UNIT_FILES_IN = \ + remote/virtproxyd.service.in \ + $(NULL) + +GUEST_UNIT_FILES_IN = \ remote/virt-guest-shutdown.target.in \ $(NULL) -SYSTEMD_UNIT_FILES += $(notdir $(LIBVIRTD_UNIT_FILES_IN:%.in=%)) -SYSTEMD_UNIT_FILES_IN += $(LIBVIRTD_UNIT_FILES_IN) + +SYSTEMD_UNIT_FILES += \ + $(notdir $(LIBVIRTD_UNIT_FILES_IN:%.in=%)) \ + $(notdir $(LIBVIRTD_UNIT_FILES_IN:remote/libvirtd%.in=remote/virtproxyd%)) \ + $(notdir $(GUEST_UNIT_FILES_IN:%.in=%)) \ + $(NULL) +SYSTEMD_UNIT_FILES_IN += \ + $(LIBVIRTD_UNIT_FILES_IN) \ + $(VIRTPROXYD_UNIT_FILES_IN) \ + $(GUEST_UNIT_FILES_IN) \ + $(NULL) REMOTE_PROTOCOL = $(srcdir)/remote/remote_protocol.x LXC_PROTOCOL = $(srcdir)/remote/lxc_protocol.x @@ -138,6 +160,7 @@ MAINTAINERCLEANFILES += \ $(NULL) CLEANFILES += \ remote/libvirtd.conf \ + remote/virtproxyd.conf \ $(NULL) if WITH_REMOTE @@ -168,15 +191,27 @@ endif ! WITH_REMOTE if WITH_LIBVIRTD -sbin_PROGRAMS += libvirtd +sbin_PROGRAMS += libvirtd virtproxyd -augeas_DATA += remote/libvirtd.aug +augeas_DATA += \ + remote/libvirtd.aug \ + remote/virtproxyd.aug \ + $(NULL) -augeastest_DATA += remote/test_libvirtd.aug +augeastest_DATA += \ + remote/test_libvirtd.aug \ + remote/test_virtproxyd.aug \ + $(NULL) -nodist_conf_DATA += remote/libvirtd.conf +nodist_conf_DATA += \ + remote/libvirtd.conf \ + remote/virtproxyd.conf \ + $(NULL) -CLEANFILES += remote/libvirtd.aug +CLEANFILES += \ + remote/libvirtd.aug \ + remote/virtproxyd.aug \ + $(NULL) man8_MANS += libvirtd.8 @@ -187,12 +222,23 @@ libvirtd_CFLAGS = \ -DSOCK_PREFIX="\"libvirt\"" \ -DDAEMON_NAME="\"libvirtd\"" \ -DWITH_IP \ + -DLIBVIRTD \ $(NULL) libvirtd_LDFLAGS = $(REMOTE_DAEMON_LD_FLAGS) libvirtd_LDADD = $(REMOTE_DAEMON_LD_ADD) +virtproxyd_SOURCES = $(REMOTE_DAEMON_SOURCES) +virtproxyd_CFLAGS = \ + $(REMOTE_DAEMON_CFLAGS) \ + -DSOCK_PREFIX="\"libvirt\"" \ + -DDAEMON_NAME="\"virtproxyd\"" \ + -DENABLE_IP \ + $(NULL) +virtproxyd_LDFLAGS = $(REMOTE_DAEMON_LD_FLAGS) +virtproxyd_LDADD = $(REMOTE_DAEMON_LD_ADD) + remote/libvirtd.conf: remote/libvirtd.conf.in $(AM_V_GEN)$(SED) \ -e '/[@]CUT_ENABLE_IP[@]/d' \ @@ -201,6 +247,13 @@ remote/libvirtd.conf: remote/libvirtd.conf.in -e 's|[@]DAEMON_NAME[@]|libvirtd|' \ $< > $@ +remote/virtproxyd.conf: remote/libvirtd.conf.in + $(AM_V_GEN)sed \ + -e '/[@]CUT_ENABLE_IP[@]/d' \ + -e '/[@]END[@]/d' \ + -e 's/[@]DAEMON_NAME[@]/virtproxyd/' \ + $< > $@ + INSTALL_DATA_DIRS += remote install-data-remote: @@ -218,6 +271,14 @@ remote/libvirtd.aug: remote/libvirtd.aug.in -e 's|[@]DAEMON_NAME_UC[@]|Libvirtd|' \ $< > $@ +remote/virtproxyd.aug: remote/libvirtd.aug.in + $(AM_V_GEN)$(SED) \ + -e '/[@]CUT_ENABLE_IP[@]/d' \ + -e '/[@]END[@]/d' \ + -e 's/[@]DAEMON_NAME[@]/virtproxyd/' \ + -e 's/[@]DAEMON_NAME_UC[@]/Virtproxyd/' \ + $< > $@ + remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \ remote/libvirtd.conf $(AUG_GENTEST) $(AM_V_GEN)$(AUG_GENTEST) remote/libvirtd.conf \ @@ -230,6 +291,17 @@ remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \ -e 's|[@]DAEMON_NAME_UC[@]|Libvirtd|' \ > $@ || rm -f $@ +remote/test_virtproxyd.aug: remote/test_libvirtd.aug.in \ + remote/virtproxyd.conf $(AUG_GENTEST) + $(AM_V_GEN)$(AUG_GENTEST) remote/virtproxyd.conf \ + $(srcdir)/remote/test_libvirtd.aug.in | \ + $(SED) \ + -e '/[@]CUT_ENABLE_IP[@]/d' \ + -e '/[@]END[@]/d' \ + -e 's/[@]DAEMON_NAME[@]/virtproxyd/' \ + -e 's/[@]DAEMON_NAME_UC[@]/Virtproxyd/' \ + > $@ || rm -f $@ + if WITH_SYSCTL # Use $(prefix)/lib rather than $(libdir), since man sysctl.d insists on # /usr/lib/sysctl.d/ even when libdir is /usr/lib64 @@ -304,12 +376,30 @@ LIBVIRTD_UNIT_VARS = \ -e 's|[@]deps[@]||g' \ $(NULL) +VIRTD_UNIT_VARS = \ + $(COMMON_UNIT_VARS) \ + -e 's|[@]deps[@]|Conflicts=$(LIBVIRTD_SOCKET_UNIT_FILES)|g' \ + $(NULL) + +VIRTPROXYD_UNIT_VARS = \ + $(VIRTD_UNIT_VARS) \ + -e 's|[@]name[@]|Libvirt proxy|g' \ + -e 's|[@]service[@]|virtproxyd|g' \ + -e 's|[@]sockprefix[@]|libvirt|g' \ + $(NULL) + libvirtd.service: remote/libvirtd.service.in $(top_builddir)/config.status $(AM_V_GEN)$(SED) $(LIBVIRTD_UNIT_VARS) $< > $@-t && mv $@-t $@ libvirt%.socket: remote/libvirt%.socket.in $(top_builddir)/config.status $(AM_V_GEN)$(SED) $(LIBVIRTD_UNIT_VARS) $< > $@-t && mv $@-t $@ +virtproxyd.service: remote/virtproxyd.service.in $(top_builddir)/config.status + $(AM_V_GEN)$(SED) $(VIRTPROXYD_UNIT_VARS) $< > $@-t && mv $@-t $@ + +virtproxy%.socket: remote/libvirt%.socket.in $(top_builddir)/config.status + $(AM_V_GEN)$(SED) $(VIRTPROXYD_UNIT_VARS) $< > $@-t && mv $@-t $@ + virt-guest-shutdown.target: remote/virt-guest-shutdown.target.in \ $(top_builddir)/config.status $(AM_V_GEN)cp $< $@ diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index ad3c163120..1138485870 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -311,10 +311,16 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) static int daemonInitialize(void) { -#ifdef MODULE_NAME +#ifndef LIBVIRTD +# ifdef MODULE_NAME /* This a dedicated per-driver daemon build */ if (virDriverLoadModule(MODULE_NAME, MODULE_NAME "Register", true) < 0) return -1; +# else + /* This is virtproxyd which merely proxies to the per-driver + * daemons for back compat, and also allows IP connectivity. + */ +# endif #else /* This is the legacy monolithic libvirtd built with all drivers * @@ -906,9 +912,9 @@ daemonUsage(const char *argv0, bool privileged) { "-h | --help", N_("Display program help") }, { "-v | --verbose", N_("Verbose messages") }, { "-d | --daemon", N_("Run as a daemon & write PID file") }, -#ifdef WITH_IP +#if defined(WITH_IP) && defined(LIBVIRTD) { "-l | --listen", N_("Listen for TCP/IP connections") }, -#endif /* !WITH_IP */ +#endif /* !(WITH_IP && LIBVIRTD) */ { "-t | --timeout ", N_("Exit after timeout period") }, { "-f | --config ", N_("Configuration file") }, { "-V | --version", N_("Display version information") }, @@ -986,7 +992,11 @@ int main(int argc, char **argv) { int verbose = 0; int godaemon = 0; #ifdef WITH_IP +# ifdef LIBVIRTD int ipsock = 0; +# else /* ! LIBVIRTD */ + int ipsock = 1; /* listen_tcp/listen_tls default to 0 */ +# endif /* ! LIBVIRTD */ #endif /* ! WITH_IP */ struct daemonConfig *config; bool privileged = geteuid() == 0 ? true : false; @@ -997,9 +1007,9 @@ int main(int argc, char **argv) { struct option opts[] = { { "verbose", no_argument, &verbose, 'v'}, { "daemon", no_argument, &godaemon, 'd'}, -#ifdef WITH_IP +#if defined(WITH_IP) && defined(LIBVIRTD) { "listen", no_argument, &ipsock, 'l'}, -#endif /* ! WITH_IP */ +#endif /* !(WITH_IP && LIBVIRTD) */ { "config", required_argument, NULL, 'f'}, { "timeout", required_argument, NULL, 't'}, { "pid-file", required_argument, NULL, 'p'}, @@ -1022,11 +1032,11 @@ int main(int argc, char **argv) { int optidx = 0; int c; char *tmp; -#ifdef WITH_IP +#if defined(WITH_IP) && defined(LIBVIRTD) const char *optstr = "ldf:p:t:vVh"; -#else /* ! WITH_IP */ +#else /* !(WITH_IP && LIBVIRTD) */ const char *optstr = "df:p:t:vVh"; -#endif /* ! WITH_IP */ +#endif /* !(WITH_IP && LIBVIRTD) */ c = getopt_long(argc, argv, optstr, opts, &optidx); @@ -1044,11 +1054,11 @@ int main(int argc, char **argv) { godaemon = 1; break; -#ifdef WITH_IP +#if defined(WITH_IP) && defined(LIBVIRTD) case 'l': ipsock = 1; break; -#endif /* ! WITH_IP */ +#endif /* !(WITH_IP && LIBVIRTD) */ case 't': if (virStrToLong_i(optarg, &tmp, 10, &timeout) != 0 diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c index b74a1c242d..b224e75a18 100644 --- a/src/remote/remote_daemon_config.c +++ b/src/remote/remote_daemon_config.c @@ -108,7 +108,11 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) return NULL; #ifdef WITH_IP - data->listen_tls = 1; +# ifdef LIBVIRTD + data->listen_tls = 1; /* Only honoured if --listen is set */ +# else /* ! LIBVIRTD */ + data->listen_tls = 0; /* Always honoured, --listen doesn't exist. */ +# endif /* ! LIBVIRTD */ data->listen_tcp = 0; if (VIR_STRDUP(data->tls_port, LIBVIRTD_TLS_PORT) < 0 || diff --git a/src/remote/virtproxyd.service.in b/src/remote/virtproxyd.service.in new file mode 100644 index 0000000000..e99e2af19c --- /dev/null +++ b/src/remote/virtproxyd.service.in @@ -0,0 +1,24 @@ +[Unit] +Description=Virtualization daemon +Conflicts=libvirtd.service +Requires=virtproxyd.socket +Requires=virtproxyd-ro.socket +Requires=virtproxyd-admin.socket +After=network.target +After=dbus.service +After=apparmor.service +After=local-fs.target +Documentation=man:libvirtd(8) +Documentation=https://libvirt.org + +[Service] +Type=notify +ExecStart=@sbindir@/virtproxyd --timeout 120 +ExecReload=/bin/kill -HUP $MAINPID +Restart=on-failure + +[Install] +WantedBy=multi-user.target +Also=virtproxyd.socket +Also=virtproxyd-ro.socket +Also=virtproxyd-admin.socket