File bsc#1177916-CVE-2020-25654_pacemaker.patch of Package pacemaker

Overview Repositories Revisions Requests Users Attributes Meta

File bsc#1177916-CVE-2020-25654_pacemaker.patch of Package pacemaker (Revision f2bffd5939217c816ffd4d487bf9a9a4)

Currently displaying revision f2bffd5939217c816ffd4d487bf9a9a4 , Show latest

From 698679d276b26a044f33397f6e3833819f88be3d Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 09:56:03 -0500
Subject: [PATCH 1/7] Log: executor: show CRM_OP_REGISTER rc in debug message

Previously, process_lrmd_signon() would add the rc to the client reply
but not pass it back to process_lrmd_message(), which would always log "OK" in
its debug message, even if the sign-on was rejected.
---
 daemons/execd/execd_commands.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
index 81d64cd..6982098 100644
--- a/daemons/execd/execd_commands.c
+++ b/daemons/execd/execd_commands.c
@@ -1511,10 +1511,10 @@ free_rsc(gpointer data)
     free(rsc);
 }
 
-static xmlNode *
-process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id)
+static int
+process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id,
+                    xmlNode **reply)
 {
-    xmlNode *reply = NULL;
     int rc = pcmk_ok;
     const char *is_ipc_provider = crm_element_value(request, F_LRMD_IS_IPC_PROVIDER);
     const char *protocol_version = crm_element_value(request, F_LRMD_PROTOCOL_VERSION);
@@ -1525,18 +1525,19 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id)
         rc = -EPROTO;
     }
 
-    reply = create_lrmd_reply(__FUNCTION__, rc, call_id);
-    crm_xml_add(reply, F_LRMD_OPERATION, CRM_OP_REGISTER);
-    crm_xml_add(reply, F_LRMD_CLIENTID, client->id);
-    crm_xml_add(reply, F_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION);
-
     if (crm_is_true(is_ipc_provider)) {
         // This is a remote connection from a cluster node's controller
 #ifdef SUPPORT_REMOTE
         ipc_proxy_add_provider(client);
 #endif
     }
-    return reply;
+
+    *reply = create_lrmd_reply(__func__, rc, call_id);
+    crm_xml_add(*reply, F_LRMD_OPERATION, CRM_OP_REGISTER);
+    crm_xml_add(*reply, F_LRMD_CLIENTID, client->id);
+    crm_xml_add(*reply, F_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION);
+
+    return rc;
 }
 
 static int
@@ -1849,7 +1850,7 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request)
 #endif
         do_reply = 1;
     } else if (crm_str_eq(op, CRM_OP_REGISTER, TRUE)) {
-        reply = process_lrmd_signon(client, request, call_id);
+        rc = process_lrmd_signon(client, request, call_id, &reply);
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_REG, TRUE)) {
         rc = process_lrmd_rsc_register(client, id, request);
-- 
1.8.3.1

From 0e1672c87331465dc87862821f99e9f2140719f1 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 15:16:39 -0500
Subject: [PATCH 2/7] Low: executor: mark controller connections to
 pacemaker-remoted as privileged

Previously, pcmk__client_privileged was only set when local clients connected
(as root or hacluster). Now, set it when pacemaker-remoted successfully
completes the TLS handshake with a remote client (i.e., the controller on a
cluster node).

This has no effect as of this commit but will with later commits.
---
 daemons/execd/remoted_tls.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/daemons/execd/remoted_tls.c b/daemons/execd/remoted_tls.c
index 50cb502..a02ad33 100644
--- a/daemons/execd/remoted_tls.c
+++ b/daemons/execd/remoted_tls.c
@@ -72,6 +72,11 @@ remoted__read_handshake_data(pcmk__client_t *client)
     client->remote->tls_handshake_complete = TRUE;
     crm_notice("Remote client connection accepted");
 
+    /* Only a client with access to the TLS key can connect, so we can treat
+     * it as privileged.
+     */
+    set_bit(client->flags, pcmk__client_privileged);
+
     // Alert other clients of the new connection
     notify_of_new_client(client);
     return 0;
-- 
1.8.3.1

From 6a62b9652a62d91e6ce7484c772fd0b3f2e8e642 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 15 Oct 2020 15:33:13 -0500
Subject: [PATCH 3/7] Low: executor: return appropriate error code when no
 remote support

---
 daemons/execd/execd_commands.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
index 6982098..fc7e400 100644
--- a/daemons/execd/execd_commands.c
+++ b/daemons/execd/execd_commands.c
@@ -1526,9 +1526,11 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id,
     }
 
     if (crm_is_true(is_ipc_provider)) {
-        // This is a remote connection from a cluster node's controller
 #ifdef SUPPORT_REMOTE
+        // This is a remote connection from a cluster node's controller
         ipc_proxy_add_provider(client);
+#else
+        rc = -EPROTONOSUPPORT;
 #endif
     }
 
@@ -1847,6 +1849,8 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request)
     if (crm_str_eq(op, CRM_OP_IPC_FWD, TRUE)) {
 #ifdef SUPPORT_REMOTE
         ipc_proxy_forward_client(client, request);
+#else
+        rc = -EPROTONOSUPPORT;
 #endif
         do_reply = 1;
     } else if (crm_str_eq(op, CRM_OP_REGISTER, TRUE)) {
-- 
1.8.3.1

From ac513f0a91a6ca9e25a8e00436c3c76a038fec23 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 15 Oct 2020 15:33:57 -0500
Subject: [PATCH 4/7] High: executor: restrict certain IPC requests to
 Pacemaker daemons

The executor IPC API allows clients to register resources, request agent
execution, and so forth.

If ACLs are enabled, this could allow an ACL-restricted user to bypass ACLs and
execute any code as root. (If ACLs are not enabled, users in the haclient group
have full access to the CIB, which already gives them that ability, so there is
no additional exposure in that case.)

When ACLs are supported, this commit effectively disables the executor IPC API
for clients that aren't connecting as root or hacluster. Such clients can only
register and poke now.
---
 daemons/execd/execd_commands.c | 91 +++++++++++++++++++++++++++++++++---------
 1 file changed, 73 insertions(+), 18 deletions(-)

diff --git a/daemons/execd/execd_commands.c b/daemons/execd/execd_commands.c
index fc7e400..54955c7 100644
--- a/daemons/execd/execd_commands.c
+++ b/daemons/execd/execd_commands.c
@@ -1527,8 +1527,12 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id,
 
     if (crm_is_true(is_ipc_provider)) {
 #ifdef SUPPORT_REMOTE
-        // This is a remote connection from a cluster node's controller
-        ipc_proxy_add_provider(client);
+        if ((client->remote != NULL) && client->remote->tls_handshake_complete) {
+            // This is a remote connection from a cluster node's controller
+            ipc_proxy_add_provider(client);
+        } else {
+            rc = -EACCES;
+        }
 #else
         rc = -EPROTONOSUPPORT;
 #endif
@@ -1843,12 +1847,26 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request)
     int do_notify = 0;
     xmlNode *reply = NULL;
 
+    bool allowed = true;
+
+#if ENABLE_ACL
+    /* Certain IPC commands may be done only by privileged users (i.e. root or
+     * hacluster) when ACLs are enabled, because they would otherwise provide a
+     * means of bypassing ACLs.
+     */
+    allowed = is_set(client->flags, pcmk__client_privileged);
+#endif
+
     crm_trace("Processing %s operation from %s", op, client->id);
     crm_element_value_int(request, F_LRMD_CALLID, &call_id);
 
     if (crm_str_eq(op, CRM_OP_IPC_FWD, TRUE)) {
 #ifdef SUPPORT_REMOTE
-        ipc_proxy_forward_client(client, request);
+        if (allowed) {
+            ipc_proxy_forward_client(client, request);
+        } else {
+            rc = -EACCES;
+        }
 #else
         rc = -EPROTONOSUPPORT;
 #endif
@@ -1857,38 +1875,70 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request)
         rc = process_lrmd_signon(client, request, call_id, &reply);
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_REG, TRUE)) {
-        rc = process_lrmd_rsc_register(client, id, request);
-        do_notify = 1;
+        if (allowed) {
+            rc = process_lrmd_rsc_register(client, id, request);
+            do_notify = 1;
+        } else {
+            rc = -EACCES;
+        }
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_INFO, TRUE)) {
-        reply = process_lrmd_get_rsc_info(request, call_id);
+        if (allowed) {
+            reply = process_lrmd_get_rsc_info(request, call_id);
+        } else {
+            rc = -EACCES;
+        }
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_UNREG, TRUE)) {
-        rc = process_lrmd_rsc_unregister(client, id, request);
-        /* don't notify anyone about failed un-registers */
-        if (rc == pcmk_ok || rc == -EINPROGRESS) {
-            do_notify = 1;
+        if (allowed) {
+            rc = process_lrmd_rsc_unregister(client, id, request);
+            /* don't notify anyone about failed un-registers */
+            if (rc == pcmk_ok || rc == -EINPROGRESS) {
+                do_notify = 1;
+            }
+        } else {
+            rc = -EACCES;
         }
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_EXEC, TRUE)) {
-        rc = process_lrmd_rsc_exec(client, id, request);
+        if (allowed) {
+            rc = process_lrmd_rsc_exec(client, id, request);
+        } else {
+            rc = -EACCES;
+        }
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_CANCEL, TRUE)) {
-        rc = process_lrmd_rsc_cancel(client, id, request);
+        if (allowed) {
+            rc = process_lrmd_rsc_cancel(client, id, request);
+        } else {
+            rc = -EACCES;
+        }
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_POKE, TRUE)) {
         do_notify = 1;
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_CHECK, TRUE)) {
-        xmlNode *data = get_message_xml(request, F_LRMD_CALLDATA); 
-        const char *timeout = crm_element_value(data, F_LRMD_WATCHDOG);
-        CRM_LOG_ASSERT(data != NULL);
-        pcmk__valid_sbd_timeout(timeout);
+        if (allowed) {
+            xmlNode *data = get_message_xml(request, F_LRMD_CALLDATA);
+
+            CRM_LOG_ASSERT(data != NULL);
+            pcmk__valid_sbd_timeout(crm_element_value(data, F_LRMD_WATCHDOG));
+        } else {
+            rc = -EACCES;
+        }
     } else if (crm_str_eq(op, LRMD_OP_ALERT_EXEC, TRUE)) {
-        rc = process_lrmd_alert_exec(client, id, request);
+        if (allowed) {
+            rc = process_lrmd_alert_exec(client, id, request);
+        } else {
+            rc = -EACCES;
+        }
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_GET_RECURRING, TRUE)) {
-        reply = process_lrmd_get_recurring(request, call_id);
+        if (allowed) {
+            reply = process_lrmd_get_recurring(request, call_id);
+        } else {
+            rc = -EACCES;
+        }
         do_reply = 1;
     } else {
         rc = -EOPNOTSUPP;
@@ -1896,6 +1946,11 @@ process_lrmd_message(pcmk__client_t *client, uint32_t id, xmlNode *request)
         crm_err("Unknown IPC request '%s' from %s", op, client->name);
     }
 
+    if (rc == -EACCES) {
+        crm_warn("Rejecting IPC request '%s' from unprivileged client %s",
+                 op, pcmk__client_name(client));
+    }
+
     crm_debug("Processed %s operation from %s: rc=%d, reply=%d, notify=%d",
               op, client->id, rc, do_reply, do_notify);
 
-- 
1.8.3.1

From 816c90c01fa3f29dce15e3682545e16ee65ee468 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 11:16:43 -0500
Subject: [PATCH 5/7] Low: pacemakerd: check client for NULL before using it

... to guard against bugs in client tracking
---
 daemons/pacemakerd/pacemakerd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c
index 48254dd..3f99430 100644
--- a/daemons/pacemakerd/pacemakerd.c
+++ b/daemons/pacemakerd/pacemakerd.c
@@ -564,9 +564,12 @@ pcmk_ipc_dispatch(qb_ipcs_connection_t * qbc, void *data, size_t size)
     uint32_t id = 0;
     uint32_t flags = 0;
     const char *task = NULL;
+    xmlNode *msg = NULL;
     pcmk__client_t *c = pcmk__find_client(qbc);
-    xmlNode *msg = pcmk__client_data2xml(c, data, &id, &flags);
 
+    CRM_CHECK(c != NULL, return 0);
+
+    msg = pcmk__client_data2xml(c, data, &id, &flags);
     pcmk__ipc_send_ack(c, id, flags, "ack");
     if (msg == NULL) {
         return 0;
-- 
1.8.3.1

From a8c9f5b0d80f25c5e884f4282f5f8b8d82404576 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 11:17:18 -0500
Subject: [PATCH 6/7] High: pacemakerd: ignore shutdown requests from
 unprivileged users

The pacemakerd IPC API supports a shutdown request, along with a
command-line interface for using it (pacemakerd --shutdown).

Only the haclient group has access to the IPC. Without ACLs, that group can
already shut down Pacemaker via the CIB, so there's no security implication.

However, it might not be desired to allow ACL-restricted users to shut down
Pacemaker, so block users other than root or hacluster if ACLs are supported.
---
 daemons/pacemakerd/pacemakerd.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c
index 3f99430..0c7b413 100644
--- a/daemons/pacemakerd/pacemakerd.c
+++ b/daemons/pacemakerd/pacemakerd.c
@@ -577,10 +577,26 @@ pcmk_ipc_dispatch(qb_ipcs_connection_t * qbc, void *data, size_t size)
 
     task = crm_element_value(msg, F_CRM_TASK);
     if (crm_str_eq(task, CRM_OP_QUIT, TRUE)) {
-        /* Time to quit */
-        crm_notice("Shutting down in response to ticket %s (%s)",
-                   crm_element_value(msg, F_CRM_REFERENCE), crm_element_value(msg, F_CRM_ORIGIN));
-        pcmk_shutdown(15);
+        bool allowed = true;
+
+#if ENABLE_ACL
+        /* Only allow privileged users (i.e. root or hacluster)
+         * to shut down Pacemaker from the command line (or direct IPC).
+         *
+         * We only check when ACLs are enabled, because without them, any client
+         * with IPC access could shut down Pacemaker via the CIB anyway.
+         */
+        allowed = is_set(c->flags, pcmk__client_privileged);
+#endif
+        if (allowed) {
+            crm_notice("Shutting down in response to IPC request %s from %s",
+                       crm_element_value(msg, F_CRM_REFERENCE),
+                       crm_element_value(msg, F_CRM_ORIGIN));
+            pcmk_shutdown(15);
+        } else {
+            crm_warn("Ignoring shutdown request from unprivileged client %s",
+                     pcmk__client_name(c));
+        }
 
     } else if (crm_str_eq(task, CRM_OP_RM_NODE_CACHE, TRUE)) {
         /* Send to everyone */
-- 
1.8.3.1

From c5e7c04f95815ddf1fdf050806e493df478273ab Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 11:55:26 -0500
Subject: [PATCH 7/7] Fix: fencer: restrict certain IPC requests to privileged
 users

The fencer IPC API allows clients to register fence devices.

If ACLs are enabled, this could allow an ACL-restricted user to bypass ACLs to
configure fencing. If the user is able to install executables to the standard
fencing agent locations, have arbitrary code executed as root (the standard
locations generally require root for write access, so that is unlikely to be an
issue).

If ACLs are not enabled, users in the haclient group have full access to the
CIB, which already gives them these capabilities, so there is no additional
exposure in that case.

This commit does not restrict unprivileged users from using other fencing API,
such as requesting actual fencing.
---
 daemons/fenced/fenced_commands.c | 41 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c
index 6162150..2558eac 100644
--- a/daemons/fenced/fenced_commands.c
+++ b/daemons/fenced/fenced_commands.c
@@ -2547,6 +2547,18 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,
     const char *op = crm_element_value(request, F_STONITH_OPERATION);
     const char *client_id = crm_element_value(request, F_STONITH_CLIENTID);
 
+    bool allowed = true;
+
+#if ENABLE_ACL
+    /* IPC commands related to fencing configuration may be done only by
+     * privileged users (i.e. root or hacluster) when ACLs are supported,
+     * because all other users should go through the CIB to have ACLs applied.
+     */
+    if (client != NULL) {
+        allowed = is_set(client->flags, pcmk__client_privileged);
+    }
+#endif
+
     crm_element_value_int(request, F_STONITH_CALLOPTS, &call_options);
 
     if (is_set(call_options, st_opt_sync_call)) {
@@ -2705,27 +2717,43 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,
     } else if (crm_str_eq(op, STONITH_OP_DEVICE_ADD, TRUE)) {
         const char *device_id = NULL;
 
-        rc = stonith_device_register(request, &device_id, FALSE);
+        if (allowed) {
+            rc = stonith_device_register(request, &device_id, FALSE);
+        } else {
+            rc = -EACCES;
+        }
         do_stonith_notify_device(call_options, op, rc, device_id);
 
     } else if (crm_str_eq(op, STONITH_OP_DEVICE_DEL, TRUE)) {
         xmlNode *dev = get_xpath_object("//" F_STONITH_DEVICE, request, LOG_ERR);
         const char *device_id = crm_element_value(dev, XML_ATTR_ID);
 
-        rc = stonith_device_remove(device_id, FALSE);
+        if (allowed) {
+            rc = stonith_device_remove(device_id, FALSE);
+        } else {
+            rc = -EACCES;
+        }
         do_stonith_notify_device(call_options, op, rc, device_id);
 
     } else if (crm_str_eq(op, STONITH_OP_LEVEL_ADD, TRUE)) {
         char *device_id = NULL;
 
-        rc = stonith_level_register(request, &device_id);
+        if (allowed) {
+            rc = stonith_level_register(request, &device_id);
+        } else {
+            rc = -EACCES;
+        }
         do_stonith_notify_level(call_options, op, rc, device_id);
         free(device_id);
 
     } else if (crm_str_eq(op, STONITH_OP_LEVEL_DEL, TRUE)) {
         char *device_id = NULL;
 
-        rc = stonith_level_remove(request, &device_id);
+        if (allowed) {
+            rc = stonith_level_remove(request, &device_id);
+        } else {
+            rc = -EACCES;
+        }
         do_stonith_notify_level(call_options, op, rc, device_id);
 
     } else if(safe_str_eq(op, CRM_OP_RM_NODE_CACHE)) {
@@ -2745,6 +2773,11 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,
 
   done:
 
+    if (rc == -EACCES) {
+        crm_warn("Rejecting IPC request '%s' from unprivileged client %s",
+                 crm_str(op), pcmk__client_name(client));
+    }
+
     /* Always reply unless the request is in process still.
      * If in progress, a reply will happen async after the request
      * processing is finished */
-- 
1.8.3.1

Places

File bsc#1177916-CVE-2020-25654_pacemaker.patch of Package pacemaker (Revision f2bffd5939217c816ffd4d487bf9a9a4)

Places