remote: fix handling of systemd activation wrt socket ordering

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 <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 0dabd3dff898496f2114d501d2bec0e1501b1b7d..b5228e8176836a773fd1986e3c2c919d78ecae8c 100644 (file)
--- 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 },
+    };
+
+    if ((actmap[3].port = virSocketAddrResolveService(config->tcp_port)) < 0)
+        return -1;
+
+    if ((actmap[4].port = virSocketAddrResolveService(config->tls_port)) < 0)
+        return -1;
 
-    unsigned int cur_fd = STDERR_FILENO + 1;
-    unsigned int nfds = virGetListenFDs();
+    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;
+        }
 
+        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 315a4950dfd89319b6d9a7dd1560d6a9fd71dd99..d5df5d5c20b6db1cb44193994a3d14acb6135744 100644 (file)
--- 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,