Attachment 1722698 Details for Bug 1888191 – Fix for pacemaker CVE-2020-25654 (upstream master branch as of 2020-10-18)

[patch] Fix for pacemaker CVE-2020-25654 (upstream master branch as of 2020-10-18)

CVE-2020-25654_master.patch (text/plain), 19.00 KB, created by Ken Gaillot on 2020-10-19 20:20:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ken Gaillot

Created: 2020-10-19 20:20:57 UTC

Size: 19.00 KB

patch

obsolete

>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(__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);
>-
>     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 (pcmk__str_eq(op, CRM_OP_REGISTER, pcmk__str_none)) {
>-        reply = process_lrmd_signon(client, request, call_id);
>+        rc = process_lrmd_signon(client, request, call_id, &reply);
>         do_reply = 1;
>     } else if (pcmk__str_eq(op, LRMD_OP_RSC_REG, pcmk__str_none)) {
>         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.
>+     */
>+    pcmk__set_client_flags(client, 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 (pcmk__str_eq(op, CRM_OP_IPC_FWD, pcmk__str_none)) {
> #ifdef SUPPORT_REMOTE
>         ipc_proxy_forward_client(client, request);
>+#else
>+        rc = -EPROTONOSUPPORT;
> #endif
>         do_reply = 1;
>     } else if (pcmk__str_eq(op, CRM_OP_REGISTER, pcmk__str_none)) {
>-- 
>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 = pcmk_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 (pcmk__str_eq(op, CRM_OP_IPC_FWD, pcmk__str_none)) {
> #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 (pcmk__str_eq(op, LRMD_OP_RSC_REG, pcmk__str_none)) {
>-        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 (pcmk__str_eq(op, LRMD_OP_RSC_INFO, pcmk__str_none)) {
>-        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 (pcmk__str_eq(op, LRMD_OP_RSC_UNREG, pcmk__str_none)) {
>-        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 (pcmk__str_eq(op, LRMD_OP_RSC_EXEC, pcmk__str_none)) {
>-        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 (pcmk__str_eq(op, LRMD_OP_RSC_CANCEL, pcmk__str_none)) {
>-        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 (pcmk__str_eq(op, LRMD_OP_POKE, pcmk__str_none)) {
>         do_notify = 1;
>         do_reply = 1;
>     } else if (pcmk__str_eq(op, LRMD_OP_CHECK, pcmk__str_none)) {
>-        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 (pcmk__str_eq(op, LRMD_OP_ALERT_EXEC, pcmk__str_none)) {
>-        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 (pcmk__str_eq(op, LRMD_OP_GET_RECURRING, pcmk__str_none)) {
>-        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,9 +577,26 @@ pcmk_ipc_dispatch(qb_ipcs_connection_t * qbc, void *data, size_t size)
> 
>     task = crm_element_value(msg, F_CRM_TASK);
>     if (pcmk__str_eq(task, CRM_OP_QUIT, pcmk__str_none)) {
>-        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);
>+        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 = pcmk_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 (pcmk__str_eq(task, CRM_OP_RM_NODE_CACHE, pcmk__str_none)) {
>         crm_trace("Ignoring IPC request to purge node "
>-- 
>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 = pcmk_is_set(client->flags, pcmk__client_privileged);
>+    }
>+#endif
>+
>     crm_element_value_int(request, F_STONITH_CALLOPTS, &call_options);
> 
>     if (pcmk_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 (pcmk__str_eq(op, STONITH_OP_DEVICE_ADD, pcmk__str_none)) {
>         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 (pcmk__str_eq(op, STONITH_OP_DEVICE_DEL, pcmk__str_none)) {
>         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 (pcmk__str_eq(op, STONITH_OP_LEVEL_ADD, pcmk__str_none)) {
>         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 (pcmk__str_eq(op, STONITH_OP_LEVEL_DEL, pcmk__str_none)) {
>         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(pcmk__str_eq(op, CRM_OP_RM_NODE_CACHE, pcmk__str_casei)) {
>@@ -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
>

Actions: View | Diff

Attachments on bug 1888191: 1722698 | 1722699 | 1722700 | 1722701