From e2f012b8f0d2ba2f2355a8bbd961527836e11732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Tue, 25 Jun 2019 17:27:04 +0100 Subject: [PATCH] remote: fix handling of systemd activation wrt socket ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current libvirtd code for systemd socket activation assumes socket FDs are passed in the order unix-rw, unix-ro, unix-admin. There is in fact no ordering guarantee made by systemd. Applications are expected to check the address or name associated with each FD to figure out its identity. This rewrites libvirtd to make use of the new systemd activation APIs to make it robust wrt socket ordering changes. Reviewed-by: Michal Privoznik Signed-off-by: Daniel P. Berrangé --- src/remote/remote_daemon.c | 248 ++++++++++++++++------------------ src/rpc/virnetserverservice.c | 7 + 2 files changed, 127 insertions(+), 128 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 0dabd3dff8..b5228e8176 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -56,6 +56,7 @@ #include "virutil.h" #include "virgettext.h" #include "util/virnetdevopenvswitch.h" +#include "virsystemd.h" #include "driver.h" @@ -367,30 +368,34 @@ daemonSetupNetworking(virNetServerPtr srv, bool ipsock, bool privileged) { - virNetServerServicePtr svc = NULL; - virNetServerServicePtr svcAdm = NULL; - virNetServerServicePtr svcRO = NULL; - virNetServerServicePtr svcTCP = NULL; - virNetServerServicePtr svcTLS = NULL; gid_t unix_sock_gid = 0; int unix_sock_ro_mask = 0; int unix_sock_rw_mask = 0; int unix_sock_adm_mask = 0; int ret = -1; + VIR_AUTOPTR(virSystemdActivation) act = NULL; + virSystemdActivationMap actmap[] = { + { .name = "libvirtd.socket", .family = AF_UNIX, .path = sock_path }, + { .name = "libvirtd-ro.socket", .family = AF_UNIX, .path = sock_path_ro }, + { .name = "libvirtd-admin.socket", .family = AF_UNIX, .path = sock_path_adm }, + { .name = "libvirtd-tcp.socket", .family = AF_INET }, + { .name = "libvirtd-tls.socket", .family = AF_INET }, + }; - unsigned int cur_fd = STDERR_FILENO + 1; - unsigned int nfds = virGetListenFDs(); + if ((actmap[3].port = virSocketAddrResolveService(config->tcp_port)) < 0) + return -1; + + if ((actmap[4].port = virSocketAddrResolveService(config->tls_port)) < 0) + return -1; + + if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), &act) < 0) + return -1; if (config->unix_sock_group) { if (virGetGroupID(config->unix_sock_group, &unix_sock_gid) < 0) return ret; } - if (nfds > (sock_path_ro ? 2 : 1)) { - VIR_ERROR(_("Too many (%u) FDs passed from caller"), nfds); - return ret; - } - if (virStrToLong_i(config->unix_sock_ro_perms, NULL, 8, &unix_sock_ro_mask) != 0) { VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_ro_perms); goto cleanup; @@ -406,148 +411,135 @@ daemonSetupNetworking(virNetServerPtr srv, goto cleanup; } - if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path, - unix_sock_rw_mask, - unix_sock_gid, - config->auth_unix_rw, - NULL, - false, - config->max_queued_clients, - config->max_client_requests, - nfds, &cur_fd))) + if (virNetServerAddServiceUNIX(srv, + act, + "libvirtd.socket", + sock_path, + unix_sock_rw_mask, + unix_sock_gid, + config->auth_unix_rw, + NULL, + false, + config->max_queued_clients, + config->max_client_requests) < 0) goto cleanup; - if (sock_path_ro) { - if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro, - unix_sock_ro_mask, - unix_sock_gid, - config->auth_unix_ro, - NULL, - true, - config->max_queued_clients, - config->max_client_requests, - nfds, &cur_fd))) - goto cleanup; - } - - if (virNetServerAddService(srv, svc) < 0) + if (sock_path_ro && + virNetServerAddServiceUNIX(srv, + act, + "libvirtd-ro.socket", + sock_path_ro, + unix_sock_ro_mask, + unix_sock_gid, + config->auth_unix_ro, + NULL, + true, + config->max_queued_clients, + config->max_client_requests) < 0) goto cleanup; - if (svcRO && - virNetServerAddService(srv, svcRO) < 0) + if (sock_path_adm && + virNetServerAddServiceUNIX(srvAdm, + act, + "libvirtd-admin.socket", + sock_path_adm, + unix_sock_adm_mask, + unix_sock_gid, + REMOTE_AUTH_NONE, + NULL, + false, + config->admin_max_queued_clients, + config->admin_max_client_requests) < 0) goto cleanup; - if (sock_path_adm) { - VIR_DEBUG("Registering unix socket %s", sock_path_adm); - if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm, - unix_sock_adm_mask, - unix_sock_gid, - REMOTE_AUTH_NONE, - NULL, - false, - config->admin_max_queued_clients, - config->admin_max_client_requests))) - goto cleanup; + if (((ipsock && config->listen_tcp) || act) && + virNetServerAddServiceTCP(srv, + act, + "libvirtd-tcp.socket", + config->listen_addr, + config->tcp_port, + AF_UNSPEC, + config->auth_tcp, + NULL, + false, + config->max_queued_clients, + config->max_client_requests) < 0) + goto cleanup; - if (virNetServerAddService(srvAdm, svcAdm) < 0) - goto cleanup; - } + if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, "ip-tls")))) { + virNetTLSContextPtr ctxt = NULL; - if (ipsock) { - if (config->listen_tcp) { - VIR_DEBUG("Registering TCP socket %s:%s", - config->listen_addr, config->tcp_port); - if (!(svcTCP = virNetServerServiceNewTCP(config->listen_addr, - config->tcp_port, - AF_UNSPEC, - config->auth_tcp, - NULL, - false, - config->max_queued_clients, - config->max_client_requests))) + if (config->ca_file || + config->cert_file || + config->key_file) { + if (!config->ca_file) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("No CA certificate path set to match server key/cert")); goto cleanup; - - if (virNetServerAddService(srv, svcTCP) < 0) + } + if (!config->cert_file) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("No server certificate path set to match server key")); goto cleanup; - } - - if (config->listen_tls) { - virNetTLSContextPtr ctxt = NULL; - - if (config->ca_file || - config->cert_file || - config->key_file) { - if (!config->ca_file) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No CA certificate path set to match server key/cert")); - goto cleanup; - } - if (!config->cert_file) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No server certificate path set to match server key")); - goto cleanup; - } - if (!config->key_file) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No server key path set to match server cert")); - goto cleanup; - } - VIR_DEBUG("Using CA='%s' cert='%s' key='%s'", - config->ca_file, config->cert_file, config->key_file); - if (!(ctxt = virNetTLSContextNewServer(config->ca_file, - config->crl_file, - config->cert_file, - config->key_file, + } + if (!config->key_file) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("No server key path set to match server cert")); + goto cleanup; + } + VIR_DEBUG("Using CA='%s' cert='%s' key='%s'", + config->ca_file, config->cert_file, config->key_file); + if (!(ctxt = virNetTLSContextNewServer(config->ca_file, + config->crl_file, + config->cert_file, + config->key_file, + (const char *const*)config->tls_allowed_dn_list, + config->tls_priority, + config->tls_no_sanity_certificate ? false : true, + config->tls_no_verify_certificate ? false : true))) + goto cleanup; + } else { + if (!(ctxt = virNetTLSContextNewServerPath(NULL, + !privileged, (const char *const*)config->tls_allowed_dn_list, config->tls_priority, config->tls_no_sanity_certificate ? false : true, config->tls_no_verify_certificate ? false : true))) - goto cleanup; - } else { - if (!(ctxt = virNetTLSContextNewServerPath(NULL, - !privileged, - (const char *const*)config->tls_allowed_dn_list, - config->tls_priority, - config->tls_no_sanity_certificate ? false : true, - config->tls_no_verify_certificate ? false : true))) - goto cleanup; - } - - VIR_DEBUG("Registering TLS socket %s:%s", - config->listen_addr, config->tls_port); - if (!(svcTLS = - virNetServerServiceNewTCP(config->listen_addr, - config->tls_port, - AF_UNSPEC, - config->auth_tls, - ctxt, - false, - config->max_queued_clients, - config->max_client_requests))) { - virObjectUnref(ctxt); goto cleanup; - } - if (virNetServerAddService(srv, svcTLS) < 0) - goto cleanup; - - virObjectUnref(ctxt); } + + VIR_DEBUG("Registering TLS socket %s:%s", + config->listen_addr, config->tls_port); + if (virNetServerAddServiceTCP(srv, + act, + "libvirtd-tls.socket", + config->listen_addr, + config->tls_port, + AF_UNSPEC, + config->auth_tls, + ctxt, + false, + config->max_queued_clients, + config->max_client_requests) < 0) { + virObjectUnref(ctxt); + goto cleanup; + } + virObjectUnref(ctxt); } + if (act && + virSystemdActivationComplete(act) < 0) + goto cleanup; + #if WITH_SASL if (virNetServerNeedsAuth(srv, REMOTE_AUTH_SASL) && !(saslCtxt = virNetSASLContextNewServer( (const char *const*)config->sasl_allowed_username_list))) - goto cleanup; + goto cleanup; #endif ret = 0; cleanup: - virObjectUnref(svcTLS); - virObjectUnref(svcTCP); - virObjectUnref(svcRO); - virObjectUnref(svcAdm); - virObjectUnref(svc); return ret; } diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 315a4950df..d5df5d5c20 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -28,9 +28,12 @@ #include "viralloc.h" #include "virerror.h" #include "virthread.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_RPC +VIR_LOG_INIT("rpc.netserverservice"); + struct _virNetServerService { virObject parent; @@ -201,6 +204,8 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, virNetSocketPtr *socks; size_t nsocks; + VIR_DEBUG("Creating new TCP server nodename='%s' service='%s'", + NULLSTR(nodename), NULLSTR(service)); if (virNetSocketNewListenTCP(nodename, service, family, @@ -236,6 +241,8 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, virNetServerServicePtr svc; virNetSocketPtr sock; + VIR_DEBUG("Creating new UNIX server path='%s' mask=%o gid=%u", + path, mask, grp); if (virNetSocketNewListenUNIX(path, mask, -1,