From 975724044df52f558119008352a19b790ce874cd Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 27 Mar 2018 15:05:47 -0500
Subject: [PATCH 01/10] Refactor: lrmd: functionize creating reply
reduces duplication, and enforces consistency in reply XML and logging
---
lrmd/lrmd.c | 94 ++++++++++++++++++++++++++-----------------------------------
1 file changed, 40 insertions(+), 54 deletions(-)
diff --git a/lrmd/lrmd.c b/lrmd/lrmd.c
index 2e8ea41..ceeb10b 100644
--- a/lrmd/lrmd.c
+++ b/lrmd/lrmd.c
@@ -373,23 +373,15 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd)
}
}
-static void
-send_reply(crm_client_t * client, int rc, uint32_t id, int call_id)
+static xmlNode *
+create_lrmd_reply(const char *origin, int rc, int call_id)
{
- int send_rc = 0;
- xmlNode *reply = NULL;
+ xmlNode *reply = create_xml_node(NULL, T_LRMD_REPLY);
- reply = create_xml_node(NULL, T_LRMD_REPLY);
- crm_xml_add(reply, F_LRMD_ORIGIN, __FUNCTION__);
+ crm_xml_add(reply, F_LRMD_ORIGIN, origin);
crm_xml_add_int(reply, F_LRMD_RC, rc);
crm_xml_add_int(reply, F_LRMD_CALLID, call_id);
-
- send_rc = lrmd_server_send_reply(client, id, reply);
-
- free_xml(reply);
- if (send_rc < 0) {
- crm_warn("LRMD reply to %s failed: %d", client->name, send_rc);
- }
+ return reply;
}
static void
@@ -1396,23 +1388,24 @@ free_rsc(gpointer data)
free(rsc);
}
-static int
-process_lrmd_signon(crm_client_t * client, uint32_t id, xmlNode * request)
+static xmlNode *
+process_lrmd_signon(crm_client_t *client, xmlNode *request, int call_id)
{
- xmlNode *reply = create_xml_node(NULL, "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);
if (compare_version(protocol_version, LRMD_MIN_PROTOCOL_VERSION) < 0) {
crm_err("Cluster API version must be greater than or equal to %s, not %s",
LRMD_MIN_PROTOCOL_VERSION, protocol_version);
- crm_xml_add_int(reply, F_LRMD_RC, -EPROTO);
+ 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);
- lrmd_server_send_reply(client, id, reply);
if (crm_is_true(is_ipc_provider)) {
/* this is a remote connection from a cluster nodes crmd */
@@ -1420,9 +1413,7 @@ process_lrmd_signon(crm_client_t * client, uint32_t id, xmlNode * request)
ipc_proxy_add_provider(client);
#endif
}
-
- free_xml(reply);
- return pcmk_ok;
+ return reply;
}
static int
@@ -1450,52 +1441,34 @@ process_lrmd_rsc_register(crm_client_t * client, uint32_t id, xmlNode * request)
return rc;
}
-static void
-process_lrmd_get_rsc_info(crm_client_t * client, uint32_t id, xmlNode * request)
+static xmlNode *
+process_lrmd_get_rsc_info(xmlNode *request, int call_id)
{
int rc = pcmk_ok;
- int send_rc = 0;
- int call_id = 0;
xmlNode *rsc_xml = get_xpath_object("//" F_LRMD_RSC, request, LOG_ERR);
const char *rsc_id = crm_element_value(rsc_xml, F_LRMD_RSC_ID);
xmlNode *reply = NULL;
lrmd_rsc_t *rsc = NULL;
- crm_element_value_int(request, F_LRMD_CALLID, &call_id);
-
- if (!rsc_id) {
- rc = -ENODEV;
- goto get_rsc_done;
- }
-
- if (!(rsc = g_hash_table_lookup(rsc_list, rsc_id))) {
- crm_info("Resource '%s' not found (%d active resources)",
- rsc_id, g_hash_table_size(rsc_list));
+ if (rsc_id == NULL) {
rc = -ENODEV;
- goto get_rsc_done;
+ } else {
+ rsc = g_hash_table_lookup(rsc_list, rsc_id);
+ if (rsc == NULL) {
+ crm_info("Resource '%s' not found (%d active resources)",
+ rsc_id, g_hash_table_size(rsc_list));
+ rc = -ENODEV;
+ }
}
- get_rsc_done:
-
- reply = create_xml_node(NULL, T_LRMD_REPLY);
- crm_xml_add(reply, F_LRMD_ORIGIN, __FUNCTION__);
- crm_xml_add_int(reply, F_LRMD_RC, rc);
- crm_xml_add_int(reply, F_LRMD_CALLID, call_id);
-
+ reply = create_lrmd_reply(__FUNCTION__, rc, call_id);
if (rsc) {
crm_xml_add(reply, F_LRMD_RSC_ID, rsc->rsc_id);
crm_xml_add(reply, F_LRMD_CLASS, rsc->class);
crm_xml_add(reply, F_LRMD_PROVIDER, rsc->provider);
crm_xml_add(reply, F_LRMD_TYPE, rsc->type);
}
-
- send_rc = lrmd_server_send_reply(client, id, reply);
-
- if (send_rc < 0) {
- crm_warn("LRMD reply to %s failed: %d", client->name, send_rc);
- }
-
- free_xml(reply);
+ return reply;
}
static int
@@ -1675,6 +1648,7 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
const char *op = crm_element_value(request, F_LRMD_OPERATION);
int do_reply = 0;
int do_notify = 0;
+ xmlNode *reply = NULL;
crm_trace("Processing %s operation from %s", op, client->id);
crm_element_value_int(request, F_LRMD_CALLID, &call_id);
@@ -1685,13 +1659,15 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
#endif
do_reply = 1;
} else if (crm_str_eq(op, CRM_OP_REGISTER, TRUE)) {
- rc = process_lrmd_signon(client, id, request);
+ reply = process_lrmd_signon(client, request, call_id);
+ 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;
do_reply = 1;
} else if (crm_str_eq(op, LRMD_OP_RSC_INFO, TRUE)) {
- process_lrmd_get_rsc_info(client, id, request);
+ reply = process_lrmd_get_rsc_info(request, call_id);
+ 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 */
@@ -1727,7 +1703,17 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
op, client->id, rc, do_reply, do_notify);
if (do_reply) {
- send_reply(client, rc, id, call_id);
+ int send_rc = pcmk_ok;
+
+ if (reply == NULL) {
+ reply = create_lrmd_reply(__FUNCTION__, rc, call_id);
+ }
+ send_rc = lrmd_server_send_reply(client, id, reply);
+ free_xml(reply);
+ if (send_rc < 0) {
+ crm_warn("Reply to client %s failed: %s " CRM_XS " %d",
+ client->name, pcmk_strerror(send_rc), send_rc);
+ }
}
if (do_notify) {
--
1.8.3.1
From 6ed12ce8ace8e0a1cc5b8ca233684ba4e70a021c Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 09:56:03 -0500
Subject: [PATCH 02/10] 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.
---
lrmd/lrmd.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/lrmd/lrmd.c b/lrmd/lrmd.c
index ceeb10b..a20f165 100644
--- a/lrmd/lrmd.c
+++ b/lrmd/lrmd.c
@@ -1388,10 +1388,10 @@ free_rsc(gpointer data)
free(rsc);
}
-static xmlNode *
-process_lrmd_signon(crm_client_t *client, xmlNode *request, int call_id)
+static int
+process_lrmd_signon(crm_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);
@@ -1402,18 +1402,19 @@ process_lrmd_signon(crm_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 nodes crmd */
#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
@@ -1659,7 +1660,7 @@ process_lrmd_message(crm_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 fd9ee715ce26a2bc2f7546afe8e7fbfb778cb861 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 15:16:39 -0500
Subject: [PATCH 03/10] Low: executor: mark controller connections to
pacemaker-remoted as privileged
Previously, crm_client_flag_ipc_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.
---
lrmd/tls_backend.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lrmd/tls_backend.c b/lrmd/tls_backend.c
index 8934ae3..509f4fa 100644
--- a/lrmd/tls_backend.c
+++ b/lrmd/tls_backend.c
@@ -84,6 +84,11 @@ remoted__read_handshake_data(crm_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, crm_client_flag_ipc_privileged);
+
// Alert other clients of the new connection
notify_of_new_client(client);
return 0;
--
1.8.3.1
From 755290ec01e0f955700e6e726461eca9cc27edab Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 15 Oct 2020 15:33:13 -0500
Subject: [PATCH 04/10] Low: executor: return appropriate error code when no
remote support
---
lrmd/lrmd.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lrmd/lrmd.c b/lrmd/lrmd.c
index a20f165..07f68ed 100644
--- a/lrmd/lrmd.c
+++ b/lrmd/lrmd.c
@@ -1403,9 +1403,11 @@ process_lrmd_signon(crm_client_t *client, xmlNode *request, int call_id,
}
if (crm_is_true(is_ipc_provider)) {
- /* this is a remote connection from a cluster nodes crmd */
#ifdef SUPPORT_REMOTE
+ // This is a remote connection from a cluster node's controller
ipc_proxy_add_provider(client);
+#else
+ rc = -EPROTONOSUPPORT;
#endif
}
@@ -1657,6 +1659,8 @@ process_lrmd_message(crm_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 540b7d4f5689b90ac466844f31fd161bdf446bed Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 15 Oct 2020 15:33:57 -0500
Subject: [PATCH 05/10] High: executor: restrict certain IPC requests to
Pacemaker daemons
(partial fix for CVE-2020-25654 with two later commits)
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.
---
lrmd/lrmd.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 69 insertions(+), 18 deletions(-)
diff --git a/lrmd/lrmd.c b/lrmd/lrmd.c
index 07f68ed..774b45b 100644
--- a/lrmd/lrmd.c
+++ b/lrmd/lrmd.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2012-2019 the Pacemaker project contributors
+ * Copyright 2012-2020 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -1404,8 +1404,12 @@ process_lrmd_signon(crm_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
@@ -1653,12 +1657,26 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
int do_notify = 0;
xmlNode *reply = NULL;
+#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.
+ */
+ bool allowed = is_set(client->flags, crm_client_flag_ipc_privileged);
+#else
+ bool allowed = true;
+#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
@@ -1667,35 +1685,63 @@ process_lrmd_message(crm_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);
- check_sbd_timeout(timeout);
+ if (allowed) {
+ xmlNode *data = get_message_xml(request, F_LRMD_CALLDATA);
+
+ CRM_LOG_ASSERT(data != NULL);
+ check_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 {
rc = -EOPNOTSUPP;
@@ -1704,6 +1750,11 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
crm_log_xml_warn(request, "UnknownOp");
}
+ if (rc == -EACCES) {
+ crm_warn("Rejecting IPC request '%s' from unprivileged client %s",
+ op, crm_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 34605b42d32e6f63eacd72e0ed971e8286102a98 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 11:16:43 -0500
Subject: [PATCH 06/10] Low: pacemakerd: check client for NULL before using it
... to guard against bugs in client tracking
---
mcp/pacemaker.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mcp/pacemaker.c b/mcp/pacemaker.c
index 05515c8..736c25c 100644
--- a/mcp/pacemaker.c
+++ b/mcp/pacemaker.c
@@ -567,9 +567,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;
crm_client_t *c = crm_client_get(qbc);
- xmlNode *msg = crm_ipcs_recv(c, data, size, &id, &flags);
+ CRM_CHECK(c != NULL, return 0);
+
+ msg = crm_ipcs_recv(c, data, size, &id, &flags);
crm_ipcs_send_ack(c, id, flags, "ack", __FUNCTION__, __LINE__);
if (msg == NULL) {
return 0;
--
1.8.3.1
From 3388b4f76dfb951f9d7e27e39f7cea6be63c3f5c Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 11:17:18 -0500
Subject: [PATCH 07/10] High: pacemakerd: ignore shutdown requests from
unprivileged users
(partial fix for CVE-2020-25654 with one previous commit and one later commit)
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.
---
mcp/pacemaker.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/mcp/pacemaker.c b/mcp/pacemaker.c
index 736c25c..94ef949 100644
--- a/mcp/pacemaker.c
+++ b/mcp/pacemaker.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2010-2019 the Pacemaker project contributors
+ * Copyright 2010-2020 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -580,10 +580,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);
+#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.
+ */
+ bool allowed = is_set(c->flags, crm_client_flag_ipc_privileged);
+#else
+ bool allowed = true;
+#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",
+ crm_client_name(c));
+ }
} else if (crm_str_eq(task, CRM_OP_RM_NODE_CACHE, TRUE)) {
/* Send to everyone */
--
1.8.3.1
From 7babd406e7195fcce57850a8589b06e095642c33 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 9 Oct 2020 11:55:26 -0500
Subject: [PATCH 08/10] Fix: fencer: restrict certain IPC requests to
privileged users
(partial fix for CVE-2020-25654 along with 2 previous commits)
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.
---
fencing/commands.c | 44 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/fencing/commands.c b/fencing/commands.c
index 26952e9..a2abcdd 100644
--- a/fencing/commands.c
+++ b/fencing/commands.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2009-2018 Andrew Beekhof <andrew@beekhof.net>
+ * Copyright 2009-2020 the Pacemaker project contributors
*
* This source code is licensed under the GNU General Public License version 2
* or later (GPLv2+) WITHOUT ANY WARRANTY.
@@ -2575,6 +2575,19 @@ handle_request(crm_client_t * client, uint32_t id, uint32_t flags, xmlNode * req
const char *op = crm_element_value(request, F_STONITH_OPERATION);
const char *client_id = crm_element_value(request, F_STONITH_CLIENTID);
+#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 no client was given, this is a peer request, which is always allowed.
+ */
+ bool allowed = (client == NULL)
+ || is_set(client->flags, crm_client_flag_ipc_privileged);
+#else
+ bool allowed = true;
+#endif
+
crm_element_value_int(request, F_STONITH_CALLOPTS, &call_options);
if (is_set(call_options, st_opt_sync_call)) {
@@ -2731,27 +2744,43 @@ handle_request(crm_client_t * client, uint32_t id, uint32_t flags, xmlNode * req
} 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 (crm_str_eq(op, STONITH_OP_CONFIRM, TRUE)) {
@@ -2782,6 +2811,11 @@ handle_request(crm_client_t * client, uint32_t id, uint32_t flags, xmlNode * req
done:
+ if (rc == -EACCES) {
+ crm_warn("Rejecting IPC request '%s' from unprivileged client %s",
+ crm_str(op), crm_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
From 74f3866cc23dad76fe914c341924ed17ece6b80b Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 1 Jul 2020 20:38:16 -0500
Subject: [PATCH 09/10] Test: CTS: libqb shared memory creates directories now
... so use "rm -rf" instead of "rm -f"
---
cts/CTS.py | 2 +-
cts/CTSaudits.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/cts/CTS.py b/cts/CTS.py
index cca2697..28e0e9c 100644
--- a/cts/CTS.py
+++ b/cts/CTS.py
@@ -635,7 +635,7 @@ class ClusterManager(UserDict):
if self.rsh(node, self.templates["StopCmd"]) == 0:
# Make sure we can continue even if corosync leaks
# fdata-* is the old name
- #self.rsh(node, "rm -f /dev/shm/qb-* /dev/shm/fdata-*")
+ #self.rsh(node, "rm -rf /dev/shm/qb-* /dev/shm/fdata-*")
self.ShouldBeStatus[node] = "down"
self.cluster_stable(self.Env["DeadTime"])
return 1
diff --git a/cts/CTSaudits.py b/cts/CTSaudits.py
index d9fbeb9..572d3ab 100755
--- a/cts/CTSaudits.py
+++ b/cts/CTSaudits.py
@@ -245,7 +245,7 @@ class FileAudit(ClusterAudit):
for line in lsout:
self.CM.debug("ps[%s]: %s" % (node, line))
- self.CM.rsh(node, "rm -f /dev/shm/qb-*")
+ self.CM.rsh(node, "rm -rf /dev/shm/qb-*")
else:
self.CM.debug("Skipping %s" % node)
--
1.8.3.1
From aa4ebe27b392a4b613d0ddd4480470659d89106f Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 2 Jul 2020 16:09:20 -0500
Subject: [PATCH 10/10] Test: CTS: ignore error logged by recent pcs versions
... because it is expected when a node is fenced, and we should already see
pacemaker errors if a node is unexpectedly fenced
---
cts/patterns.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/cts/patterns.py b/cts/patterns.py
index e908714..47a9067 100644
--- a/cts/patterns.py
+++ b/cts/patterns.py
@@ -12,6 +12,10 @@ class BasePatterns:
# Logging bug in some versions of libvirtd
r"libvirtd.*: internal error: Failed to parse PCI config address",
+
+ # pcs can log this when node is fenced, but fencing is OK in some
+ # tests (and we will catch it in pacemaker logs when not OK)
+ r"pcs.daemon:No response from: .* request: get_configs, error:",
]
self.BadNews = []
self.components = {}
--
1.8.3.1