Blob Blame History Raw
From f7389ac6f67804f20393951462a59a0b505dfe03 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 21 Jul 2020 16:41:18 -0500
Subject: [PATCH] Fix: executor: only send executor notifications to executor
 clients

This bug has existed since Pacemaker Remote was first implemented, but was
hidden until crm_node -l/-p was recently modified to go through controller IPC,
because other command-line IPC API clients either fire-and-forget IPC requests
or merely count replies, rather than parse the content of replies.

Previously, when the executor sent notifications of results, it broadcast the
notification to all IPC clients. Normally this behavior makes sense, but for
the executor in particular, it may be running as pacemaker-remoted, in which
case its IPC clients include not only clients that connected to the executor
IPC, but clients that connected via proxy to other IPC APIs on the cluster node
hosting the remote connection.

With crm_node -l/-p, this meant that it was possible for an executor API
notification to arrive while crm_node was waiting for a controller IPC reply.
It would not find the information it needed and would report a protocol
violation error.

The fix is to send executor notifications only to executor clients.
---
 daemons/execd/execd_commands.c    | 9 +++++++++
 daemons/execd/remoted_proxy.c     | 5 +++++
 include/crm/common/ipc_internal.h | 5 +++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
index aaf2976..685fcc7 100644
--- a/daemons/execd/execd_commands.c
+++ b/daemons/execd/execd_commands.c
@@ -507,6 +507,15 @@ send_client_notify(gpointer key, gpointer value, gpointer user_data)
         crm_trace("Skipping notification to client without name");
         return;
     }
+    if (is_set(client->flags, pcmk__client_to_proxy)) {
+        /* We only want to notify clients of the executor IPC API. If we are
+         * running as Pacemaker Remote, we may have clients proxied to other
+         * IPC services in the cluster, so skip those.
+         */
+        crm_trace("Skipping executor API notification to %s IPC client",
+                  client->name);
+        return;
+    }
 
     rc = lrmd_server_send_notify(client, update_msg);
     if (rc == pcmk_rc_ok) {
diff --git a/daemons/execd/remoted_proxy.c b/daemons/execd/remoted_proxy.c
index dda7eed..5c58de4 100644
--- a/daemons/execd/remoted_proxy.c
+++ b/daemons/execd/remoted_proxy.c
@@ -88,6 +88,11 @@ ipc_proxy_accept(qb_ipcs_connection_t * c, uid_t uid, gid_t gid, const char *ipc
     client->userdata = strdup(ipc_proxy->id);
     client->name = crm_strdup_printf("proxy-%s-%d-%.8s", ipc_channel, client->pid, client->id);
 
+    /* Allow remote executor to distinguish between proxied local clients and
+     * actual executor API clients
+     */
+    set_bit(client->flags, pcmk__client_to_proxy);
+
     g_hash_table_insert(ipc_clients, client->id, client);
 
     msg = create_xml_node(NULL, T_LRMD_IPC_PROXY);
diff --git a/include/crm/common/ipc_internal.h b/include/crm/common/ipc_internal.h
index 6a1fcf3..91b3435 100644
--- a/include/crm/common/ipc_internal.h
+++ b/include/crm/common/ipc_internal.h
@@ -121,8 +121,9 @@ struct pcmk__remote_s {
 };
 
 enum pcmk__client_flags {
-    pcmk__client_proxied    = 0x00001, /* ipc_proxy code only */
-    pcmk__client_privileged = 0x00002, /* root or cluster user */
+    pcmk__client_proxied    = (1 << 0), // Remote client behind proxy
+    pcmk__client_privileged = (1 << 1), // root or cluster user
+    pcmk__client_to_proxy   = (1 << 2), // Local client to be proxied
 };
 
 struct pcmk__client_s {
-- 
1.8.3.1