a125f5
From 32ded3e0172e0fae89cf70965e1c0406c1db883b Mon Sep 17 00:00:00 2001
a125f5
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
a125f5
Date: Tue, 2 Apr 2019 10:13:21 +0200
a125f5
Subject: [PATCH 1/7] High: libservices: fix use-after-free wrt. alert handling
a125f5
a125f5
This could possibly lead to unsolicited information disclosure by the
a125f5
means of standard output of the immediately preceding agent/resource
a125f5
execution leaking into the log stream under some circumstances.
a125f5
It was hence assigned CVE-2019-3885.
a125f5
a125f5
The provoked pathological state of pacemaker-execd daemon progresses
a125f5
towards crashing it for hitting segmentation fault.
a125f5
---
a125f5
 lib/services/services.c       | 40 +---------------------------------------
a125f5
 lib/services/services_linux.c | 35 +++++++++++++++++++++++++++++++----
a125f5
 2 files changed, 32 insertions(+), 43 deletions(-)
a125f5
a125f5
diff --git a/lib/services/services.c b/lib/services/services.c
a125f5
index 313567f..d3537d0 100644
a125f5
--- a/lib/services/services.c
a125f5
+++ b/lib/services/services.c
a125f5
@@ -373,35 +373,6 @@ services_action_user(svc_action_t *op, const char *user)
a125f5
     return crm_user_lookup(user, &(op->opaque->uid), &(op->opaque->gid));
a125f5
 }
a125f5
 
a125f5
-static void
a125f5
-set_alert_env(gpointer key, gpointer value, gpointer user_data)
a125f5
-{
a125f5
-    int rc;
a125f5
-
a125f5
-    if (value) {
a125f5
-        rc = setenv(key, value, 1);
a125f5
-    } else {
a125f5
-        rc = unsetenv(key);
a125f5
-    }
a125f5
-
a125f5
-    if (rc < 0) {
a125f5
-        crm_perror(LOG_ERR, "setenv %s=%s",
a125f5
-                  (char*)key, (value? (char*)value : ""));
a125f5
-    } else {
a125f5
-        crm_trace("setenv %s=%s", (char*)key, (value? (char*)value : ""));
a125f5
-    }
a125f5
-}
a125f5
-
a125f5
-static void
a125f5
-unset_alert_env(gpointer key, gpointer value, gpointer user_data)
a125f5
-{
a125f5
-    if (unsetenv(key) < 0) {
a125f5
-        crm_perror(LOG_ERR, "unset %s", (char*)key);
a125f5
-    } else {
a125f5
-        crm_trace("unset %s", (char*)key);
a125f5
-    }
a125f5
-}
a125f5
-
a125f5
 /*!
a125f5
  * \brief Execute an alert agent action
a125f5
  *
a125f5
@@ -416,18 +387,9 @@ unset_alert_env(gpointer key, gpointer value, gpointer user_data)
a125f5
 gboolean
a125f5
 services_alert_async(svc_action_t *action, void (*cb)(svc_action_t *op))
a125f5
 {
a125f5
-    gboolean responsible;
a125f5
-
a125f5
     action->synchronous = false;
a125f5
     action->opaque->callback = cb;
a125f5
-    if (action->params) {
a125f5
-        g_hash_table_foreach(action->params, set_alert_env, NULL);
a125f5
-    }
a125f5
-    responsible = services_os_action_execute(action);
a125f5
-    if (action->params) {
a125f5
-        g_hash_table_foreach(action->params, unset_alert_env, NULL);
a125f5
-    }
a125f5
-    return responsible;
a125f5
+    return services_os_action_execute(action);
a125f5
 }
a125f5
 
a125f5
 #if SUPPORT_DBUS
a125f5
diff --git a/lib/services/services_linux.c b/lib/services/services_linux.c
a125f5
index a04a8f9..90c1f44 100644
a125f5
--- a/lib/services/services_linux.c
a125f5
+++ b/lib/services/services_linux.c
a125f5
@@ -160,6 +160,25 @@ set_ocf_env_with_prefix(gpointer key, gpointer value, gpointer user_data)
a125f5
     set_ocf_env(buffer, value, user_data);
a125f5
 }
a125f5
 
a125f5
+static void
a125f5
+set_alert_env(gpointer key, gpointer value, gpointer user_data)
a125f5
+{
a125f5
+    int rc;
a125f5
+
a125f5
+    if (value != NULL) {
a125f5
+        rc = setenv(key, value, 1);
a125f5
+    } else {
a125f5
+        rc = unsetenv(key);
a125f5
+    }
a125f5
+
a125f5
+    if (rc < 0) {
a125f5
+        crm_perror(LOG_ERR, "setenv %s=%s",
a125f5
+                  (char*)key, (value? (char*)value : ""));
a125f5
+    } else {
a125f5
+        crm_trace("setenv %s=%s", (char*)key, (value? (char*)value : ""));
a125f5
+    }
a125f5
+}
a125f5
+
a125f5
 /*!
a125f5
  * \internal
a125f5
  * \brief Add environment variables suitable for an action
a125f5
@@ -169,12 +188,20 @@ set_ocf_env_with_prefix(gpointer key, gpointer value, gpointer user_data)
a125f5
 static void
a125f5
 add_action_env_vars(const svc_action_t *op)
a125f5
 {
a125f5
-    if (safe_str_eq(op->standard, PCMK_RESOURCE_CLASS_OCF) == FALSE) {
a125f5
-        return;
a125f5
+    void (*env_setter)(gpointer, gpointer, gpointer) = NULL;
a125f5
+    if (op->agent == NULL) {
a125f5
+        env_setter = set_alert_env;  /* we deal with alert handler */
a125f5
+
a125f5
+    } else if (safe_str_eq(op->standard, PCMK_RESOURCE_CLASS_OCF)) {
a125f5
+        env_setter = set_ocf_env_with_prefix;
a125f5
     }
a125f5
 
a125f5
-    if (op->params) {
a125f5
-        g_hash_table_foreach(op->params, set_ocf_env_with_prefix, NULL);
a125f5
+    if (env_setter != NULL && op->params != NULL) {
a125f5
+        g_hash_table_foreach(op->params, env_setter, NULL);
a125f5
+    }
a125f5
+
a125f5
+    if (env_setter == NULL || env_setter == set_alert_env) {
a125f5
+        return;
a125f5
     }
a125f5
 
a125f5
     set_ocf_env("OCF_RA_VERSION_MAJOR", "1", NULL);
a125f5
-- 
a125f5
1.8.3.1
a125f5
a125f5
a125f5
From 912f5d9ce983339e939e4cc55f27791f8c9baa18 Mon Sep 17 00:00:00 2001
a125f5
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
a125f5
Date: Mon, 15 Apr 2019 17:09:50 +0200
a125f5
Subject: [PATCH 2/7] High: pacemakerd vs. IPC/procfs confused deputy
a125f5
 authenticity issue (0/4)
a125f5
a125f5
[0/4: make crm_pid_active more precise as to when detections fail]
a125f5
a125f5
It would be bad if the function claimed the process is not active
a125f5
when the only obstacle in the detection process was that none of the
a125f5
detection methods worked for a plain lack of permissions to apply
a125f5
them.  Also, do some other minor cleanup of the function and add its
a125f5
documentation.  As an additional measure, log spamming is kept at
a125f5
minimum for repeated queries about the same PID.
a125f5
---
a125f5
 include/crm/common/internal.h | 21 +++++++++++++
a125f5
 lib/common/pid.c              | 68 +++++++++++++++++++++++++++----------------
a125f5
 2 files changed, 64 insertions(+), 25 deletions(-)
a125f5
a125f5
diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h
a125f5
index f2944f5..6775d14 100644
a125f5
--- a/include/crm/common/internal.h
a125f5
+++ b/include/crm/common/internal.h
a125f5
@@ -49,7 +49,28 @@ void crm_schema_cleanup(void);
a125f5
 
a125f5
 /* internal functions related to process IDs (from pid.c) */
a125f5
 
a125f5
+/*!
a125f5
+ * \internal
a125f5
+ * \brief Detect if process per PID and optionally exe path (component) exists
a125f5
+ *
a125f5
+ * \param[in] pid     PID of process assumed alive, disproving of which to try
a125f5
+ * \param[in] daemon  exe path (component) to possibly match with procfs entry
a125f5
+ *
a125f5
+ * \return -1 on invalid PID specification, -2 when the calling process has no
a125f5
+ *         (is refused an) ability to (dis)prove the predicate,
a125f5
+ *         0 if the negation of the predicate is confirmed (check-through-kill
a125f5
+ *         indicates so, or the subsequent check-through-procfs-match on
a125f5
+ *         \p daemon when provided and procfs available at the standard path),
a125f5
+ *         1 if it cannot be disproved (reliably [modulo race conditions]
a125f5
+ *         when \p daemon provided, procfs available at the standard path
a125f5
+ *         and the calling process has permissions to access the respective
a125f5
+ *         procfs location, less so otherwise, since mere check-through-kill
a125f5
+ *         is exercised without powers to exclude PID recycled in the interim).
a125f5
+ *
a125f5
+ * \note This function cannot be used to verify \e authenticity of the process.
a125f5
+ */
a125f5
 int crm_pid_active(long pid, const char *daemon);
a125f5
+
a125f5
 long crm_pidfile_inuse(const char *filename, long mypid, const char *daemon);
a125f5
 long crm_read_pidfile(const char *filename);
a125f5
 int crm_lock_pidfile(const char *filename, const char *name);
a125f5
diff --git a/lib/common/pid.c b/lib/common/pid.c
a125f5
index 803799e..2439680 100644
a125f5
--- a/lib/common/pid.c
a125f5
+++ b/lib/common/pid.c
a125f5
@@ -1,5 +1,7 @@
a125f5
 /*
a125f5
- * Copyright 2004-2018 Andrew Beekhof <andrew@beekhof.net>
a125f5
+ * Copyright 2004-2019 the Pacemaker project contributors
a125f5
+ *
a125f5
+ * The version control history for this file may have further details.
a125f5
  *
a125f5
  * This source code is licensed under the GNU Lesser General Public License
a125f5
  * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
a125f5
@@ -20,17 +22,21 @@
a125f5
 int
a125f5
 crm_pid_active(long pid, const char *daemon)
a125f5
 {
a125f5
+    static int last_asked_pid = 0;  /* log spam prevention */
a125f5
+#if SUPPORT_PROCFS
a125f5
     static int have_proc_pid = 0;
a125f5
+#else
a125f5
+    static int have_proc_pid = -1;
a125f5
+#endif
a125f5
+    int rc = 0;
a125f5
 
a125f5
     if (have_proc_pid == 0) {
a125f5
+        /* evaluation of /proc/PID/exe applicability via self-introspection */
a125f5
         char proc_path[PATH_MAX], exe_path[PATH_MAX];
a125f5
-
a125f5
-        // Make sure pid hasn't been reused by another process
a125f5
         snprintf(proc_path, sizeof(proc_path), "/proc/%lu/exe",
a125f5
-                 (long unsigned int)getpid());
a125f5
-
a125f5
+                 (long unsigned int) getpid());
a125f5
         have_proc_pid = 1;
a125f5
-        if (readlink(proc_path, exe_path, PATH_MAX - 1) < 0) {
a125f5
+        if (readlink(proc_path, exe_path, sizeof(exe_path) - 1) < 0) {
a125f5
             have_proc_pid = -1;
a125f5
         }
a125f5
     }
a125f5
@@ -38,40 +44,52 @@ crm_pid_active(long pid, const char *daemon)
a125f5
     if (pid <= 0) {
a125f5
         return -1;
a125f5
 
a125f5
-    } else if ((kill(pid, 0) < 0) && (errno == ESRCH)) {
a125f5
-        return 0;
a125f5
+    } else if ((rc = kill(pid, 0)) < 0 && errno == ESRCH) {
a125f5
+        return 0;  /* no such PID detected */
a125f5
 
a125f5
-    } else if ((daemon == NULL) || (have_proc_pid == -1)) {
a125f5
-        return 1;
a125f5
+    } else if (rc < 0 && have_proc_pid == -1) {
a125f5
+        if (last_asked_pid != pid) {
a125f5
+            crm_info("Cannot examine PID %ld: %s", pid, strerror(errno));
a125f5
+            last_asked_pid = pid;
a125f5
+        }
a125f5
+        return -2;  /* errno != ESRCH */
a125f5
+
a125f5
+    } else if (rc == 0 && (daemon == NULL || have_proc_pid == -1)) {
a125f5
+        return 1;  /* kill as the only indicator, cannot double check */
a125f5
 
a125f5
     } else {
a125f5
-        int rc = 0;
a125f5
+        /* make sure PID hasn't been reused by another process
a125f5
+           XXX: might still be just a zombie, which could confuse decisions */
a125f5
+        bool checked_through_kill = (rc == 0);
a125f5
         char proc_path[PATH_MAX], exe_path[PATH_MAX], myexe_path[PATH_MAX];
a125f5
-
a125f5
-        // Make sure pid hasn't been reused by another process
a125f5
         snprintf(proc_path, sizeof(proc_path), "/proc/%ld/exe", pid);
a125f5
 
a125f5
-        rc = readlink(proc_path, exe_path, PATH_MAX - 1);
a125f5
+        rc = readlink(proc_path, exe_path, sizeof(exe_path) - 1);
a125f5
         if ((rc < 0) && (errno == EACCES)) {
a125f5
-            crm_perror(LOG_INFO, "Could not read from %s", proc_path);
a125f5
-            return 1;
a125f5
+            if (last_asked_pid != pid) {
a125f5
+                crm_info("Could not read from %s: %s", proc_path,
a125f5
+                         strerror(errno));
a125f5
+                last_asked_pid = pid;
a125f5
+            }
a125f5
+            return checked_through_kill ? 1 : -2;
a125f5
         } else if (rc < 0) {
a125f5
-            crm_perror(LOG_ERR, "Could not read from %s", proc_path);
a125f5
-            return 0;
a125f5
+            if (last_asked_pid != pid) {
a125f5
+                crm_err("Could not read from %s: %s (%d)", proc_path,
a125f5
+                        strerror(errno), errno);
a125f5
+                last_asked_pid = pid;
a125f5
+            }
a125f5
+            return 0;  /* most likely errno == ENOENT */
a125f5
         }
a125f5
-
a125f5
-        exe_path[rc] = 0;
a125f5
+        exe_path[rc] = '\0';
a125f5
 
a125f5
         if (daemon[0] != '/') {
a125f5
-            rc = snprintf(myexe_path, sizeof(proc_path), CRM_DAEMON_DIR"/%s",
a125f5
+            rc = snprintf(myexe_path, sizeof(myexe_path), CRM_DAEMON_DIR"/%s",
a125f5
                           daemon);
a125f5
-            myexe_path[rc] = 0;
a125f5
         } else {
a125f5
-            rc = snprintf(myexe_path, sizeof(proc_path), "%s", daemon);
a125f5
-            myexe_path[rc] = 0;
a125f5
+            rc = snprintf(myexe_path, sizeof(myexe_path), "%s", daemon);
a125f5
         }
a125f5
 
a125f5
-        if (strcmp(exe_path, myexe_path) == 0) {
a125f5
+        if (rc > 0 && rc < sizeof(myexe_path) && !strcmp(exe_path, myexe_path)) {
a125f5
             return 1;
a125f5
         }
a125f5
     }
a125f5
-- 
a125f5
1.8.3.1
a125f5
a125f5
a125f5
From 1148f45da977113dff588cdd1cfebb7a47760b32 Mon Sep 17 00:00:00 2001
a125f5
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
a125f5
Date: Mon, 15 Apr 2019 17:10:07 +0200
a125f5
Subject: [PATCH 3/7] High: pacemakerd vs. IPC/procfs confused deputy
a125f5
 authenticity issue (1/4)
a125f5
a125f5
[1/4: new helpers to allow IPC client side to authenticate the server]
a125f5
a125f5
The title problem here could possibly lead to local privilege escalation
a125f5
up to the root's level (and implicitly unguarded by some additional
a125f5
protection layers like SELinux unless the defaults constrained further).
a125f5
a125f5
Main problem is that the authenticity assumptions were built on two,
a125f5
seemingly mutually supporting legs leading to two CVEs assigned:
a125f5
a125f5
* procfs (mere process existence and right path to binary check)
a125f5
  used to verify (this part was assigned CVE-2018-16878), and
a125f5
a125f5
* one-way only client-server authentication, putting the client
a125f5
  here at the mercy of the server not necessarily cross-verified
a125f5
  per the above point if at all (this part was assigned
a125f5
  CVE-2018-16877)
a125f5
a125f5
whereas these two were in fact orthogonal, tearing security
a125f5
assumptions about the "passive influencers" in the pacemaker's daemon
a125f5
resilience-friendly constellation (orchestrated by the main of them,
a125f5
pacemakerd) apart.  Moreover, procfs-based approach is discouraged
a125f5
for other reasons.
a125f5
a125f5
The layout of the basic fix is as follows:
a125f5
* 1/4: new helpers to allow IPC client side to authenticate the server
a125f5
       (this commit, along with unifying subsequent solution for
a125f5
       both CVEs)
a125f5
* 2/4: pacemakerd to trust pre-existing processes via new checks instead
a125f5
       (along with unifying solution for both CVEs)
a125f5
* 3/4: other daemons to authenticate IPC servers of fellow processes
a125f5
       (along with addressing CVE-2018-16877 alone, for parts of
a125f5
       pacemaker not covered earlier)
a125f5
* 4/4: CPG users to be careful about now-more-probable rival processes
a125f5
       (this is merely to mitigate corner case fallout from the new
a125f5
       approaches taken to face CVE-2018-16878 in particular;
a125f5
       courtesy of Yan Gao of SUSE for the reporting this)
a125f5
a125f5
With "basic", it is meant that it constitutes a self-contained best
a125f5
effort solution with some compromises that can only be overcome with the
a125f5
assistance of IPC library, libqb, as is also elaborated in messages of
a125f5
remaining "fix" commits.  Beside that, also conventional encapsulation
a125f5
of server-by-client authentication would be useful, but lack thereof
a125f5
is not an obstacle (more so should there by any security related
a125f5
neglectations on the IPC client side and its connection initiating
a125f5
arrangement within libqb that has a potential to strike as early as
a125f5
when the authenticity of the server side is yet to be examined).
a125f5
a125f5
One extra kludge that's introduced for FreeBSD lacking Unix socket to
a125f5
remote peer PID mapping is masquerading such an unspecified PID with
a125f5
value of 1, since that shall always be around as "init" task and,
a125f5
deferring to proof by contradiction, cannot be pacemakerd-spawned
a125f5
child either even if PID 1 was pacemakerd (and running such a child
a125f5
alone is rather nonsensical).  The code making decisions based on that
a125f5
value must acknowledge this craze and refrain from killing/signalling
a125f5
the underlying process on this platform (but shall in general follow
a125f5
the same elsewhere, keep in mind systemd socket-based activation for
a125f5
instance, which would end up in such a situation easily!).
a125f5
---
a125f5
 configure.ac                      |  43 +++++++++++
a125f5
 include/crm/common/Makefile.am    |   3 +-
a125f5
 include/crm/common/ipc.h          |  40 ++++++++++-
a125f5
 include/crm/common/ipc_internal.h |  69 ++++++++++++++++++
a125f5
 lib/common/ipc.c                  | 145 +++++++++++++++++++++++++++++++++++++-
a125f5
 5 files changed, 296 insertions(+), 4 deletions(-)
a125f5
 create mode 100644 include/crm/common/ipc_internal.h
a125f5
a125f5
diff --git a/configure.ac b/configure.ac
a125f5
index f272587..7228989 100644
a125f5
--- a/configure.ac
a125f5
+++ b/configure.ac
a125f5
@@ -425,6 +425,48 @@ do
a125f5
     fi
a125f5
 done
a125f5
 
a125f5
+us_auth=
a125f5
+AC_CHECK_HEADER([sys/socket.h], [
a125f5
+    AC_CHECK_DECL([SO_PEERCRED], [
a125f5
+        # Linux
a125f5
+        AC_CHECK_TYPE([struct ucred], [
a125f5
+            us_auth=peercred_ucred;
a125f5
+            AC_DEFINE([US_AUTH_PEERCRED_UCRED], [1],
a125f5
+                      [Define if Unix socket auth method is
a125f5
+                       getsockopt(s, SO_PEERCRED, &ucred, ...)])
a125f5
+        ], [
a125f5
+            # OpenBSD
a125f5
+            AC_CHECK_TYPE([struct sockpeercred], [
a125f5
+                us_auth=localpeercred_sockepeercred;
a125f5
+                AC_DEFINE([US_AUTH_PEERCRED_SOCKPEERCRED], [1],
a125f5
+                          [Define if Unix socket auth method is
a125f5
+                           getsockopt(s, SO_PEERCRED, &sockpeercred, ...)])
a125f5
+            ], [], [[#include <sys/socket.h>]])
a125f5
+        ], [[#define _GNU_SOURCE
a125f5
+             #include <sys/socket.h>]])
a125f5
+    ], [], [[#include <sys/socket.h>]])
a125f5
+])
a125f5
+
a125f5
+if test -z "${us_auth}"; then
a125f5
+    # FreeBSD
a125f5
+    AC_CHECK_DECL([getpeereid], [
a125f5
+        us_auth=getpeereid;
a125f5
+        AC_DEFINE([US_AUTH_GETPEEREID], [1],
a125f5
+                  [Define if Unix socket auth method is
a125f5
+                   getpeereid(s, &uid, &gid)])
a125f5
+    ], [
a125f5
+        # Solaris/OpenIndiana
a125f5
+        AC_CHECK_DECL([getpeerucred], [
a125f5
+            us_auth=getpeerucred;
a125f5
+            AC_DEFINE([US_AUTH_GETPEERUCRED], [1],
a125f5
+                      [Define if Unix socket auth method is
a125f5
+                       getpeercred(s, &ucred)])
a125f5
+        ], [
a125f5
+            AC_MSG_ERROR([No way to authenticate a Unix socket peer])
a125f5
+        ], [[#include <ucred.h>]])
a125f5
+    ])
a125f5
+fi
a125f5
+
a125f5
 dnl This OS-based decision-making is poor autotools practice;
a125f5
 dnl feature-based mechanisms are strongly preferred.
a125f5
 dnl
a125f5
@@ -1845,3 +1887,4 @@ AC_MSG_RESULT([  LDFLAGS_HARDENED_EXE     = ${LDFLAGS_HARDENED_EXE}])
a125f5
 AC_MSG_RESULT([  LDFLAGS_HARDENED_LIB     = ${LDFLAGS_HARDENED_LIB}])
a125f5
 AC_MSG_RESULT([  Libraries                = ${LIBS}])
a125f5
 AC_MSG_RESULT([  Stack Libraries          = ${CLUSTERLIBS}])
a125f5
+AC_MSG_RESULT([  Unix socket auth method  = ${us_auth}])
a125f5
diff --git a/include/crm/common/Makefile.am b/include/crm/common/Makefile.am
a125f5
index 7400152..be19e5e 100644
a125f5
--- a/include/crm/common/Makefile.am
a125f5
+++ b/include/crm/common/Makefile.am
a125f5
@@ -14,3 +14,4 @@ headerdir=$(pkgincludedir)/crm/common
a125f5
 header_HEADERS = xml.h ipc.h util.h iso8601.h mainloop.h logging.h results.h
a125f5
 noinst_HEADERS = cib_secrets.h ipcs.h internal.h alerts_internal.h \
a125f5
-		 iso8601_internal.h remote_internal.h xml_internal.h
a125f5
+		 iso8601_internal.h remote_internal.h xml_internal.h \
a125f5
+		 ipc_internal.h
a125f5
diff --git a/include/crm/common/ipc.h b/include/crm/common/ipc.h
a125f5
index 84a3a71..3249662 100644
a125f5
--- a/include/crm/common/ipc.h
a125f5
+++ b/include/crm/common/ipc.h
a125f5
@@ -1,5 +1,7 @@
a125f5
 /*
a125f5
- * Copyright 2013-2018 Andrew Beekhof <andrew@beekhof.net>
a125f5
+ * Copyright 2004-2019 the Pacemaker project contributors
a125f5
+ *
a125f5
+ * The version control history for this file may have further details.
a125f5
  *
a125f5
  * This source code is licensed under the GNU Lesser General Public License
a125f5
  * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
a125f5
@@ -75,6 +75,44 @@ uint32_t crm_ipc_buffer_flags(crm_ipc_t * client);
a125f5
 const char *crm_ipc_name(crm_ipc_t * client);
a125f5
 unsigned int crm_ipc_default_buffer_size(void);
a125f5
 
a125f5
+/*!
a125f5
+ * \brief Check the authenticity of the IPC socket peer process
a125f5
+ *
a125f5
+ * If everything goes well, peer's authenticity is verified by the means
a125f5
+ * of comparing against provided referential UID and GID (either satisfies),
a125f5
+ * and the result of this check can be deduced from the return value.
a125f5
+ * As an exception, detected UID of 0 ("root") satisfies arbitrary
a125f5
+ * provided referential daemon's credentials.
a125f5
+ *
a125f5
+ * \param[in]  sock    IPC related, connected Unix socket to check peer of
a125f5
+ * \param[in]  refuid  referential UID to check against
a125f5
+ * \param[in]  refgid  referential GID to check against
a125f5
+ * \param[out] gotpid  to optionally store obtained PID of the peer
a125f5
+ *                     (not available on FreeBSD, special value of 1
a125f5
+ *                     used instead, and the caller is required to
a125f5
+ *                     special case this value respectively)
a125f5
+ * \param[out] gotuid  to optionally store obtained UID of the peer
a125f5
+ * \param[out] gotgid  to optionally store obtained GID of the peer
a125f5
+ *
a125f5
+ * \return 0 if IPC related socket's peer is not authentic given the
a125f5
+ *         referential credentials (see above), 1 if it is,
a125f5
+ *         negative value on error (generally expressing -errno unless
a125f5
+ *         it was zero even on nonhappy path, -pcmk_err_generic is
a125f5
+ *         returned then; no message is directly emitted)
a125f5
+ *
a125f5
+ * \note While this function is tolerant on what constitutes authorized
a125f5
+ *       IPC daemon process (its effective user matches UID=0 or \p refuid,
a125f5
+ *       or at least its group matches \p refroup), either or both (in case
a125f5
+ *       of UID=0) mismatches on the expected credentials of such peer
a125f5
+ *       process \e shall be investigated at the caller when value of 1
a125f5
+ *       gets returned there, since higher-than-expected privileges in
a125f5
+ *       respect to the expected/intended credentials possibly violate
a125f5
+ *       the least privilege principle and may pose an additional risk
a125f5
+ *       (i.e. such accidental inconsistency shall be eventually fixed).
a125f5
+ */
a125f5
+int crm_ipc_is_authentic_process(int sock, uid_t refuid, gid_t refgid,
a125f5
+                                 pid_t *gotpid, uid_t *gotuid, gid_t *gotgid);
a125f5
+
a125f5
 /* Utils */
a125f5
 xmlNode *create_hello_message(const char *uuid, const char *client_name,
a125f5
                               const char *major_version, const char *minor_version);
a125f5
diff --git a/include/crm/common/ipc_internal.h b/include/crm/common/ipc_internal.h
a125f5
new file mode 100644
a125f5
index 0000000..41a6653
a125f5
--- /dev/null
a125f5
+++ b/include/crm/common/ipc_internal.h
a125f5
@@ -0,0 +1,69 @@
a125f5
+/*
a125f5
+ * Copyright 2019 the Pacemaker project contributors
a125f5
+ *
a125f5
+ * The version control history for this file may have further details.
a125f5
+ *
a125f5
+ * This source code is licensed under the GNU Lesser General Public License
a125f5
+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
a125f5
+ */
a125f5
+
a125f5
+#ifndef PCMK__IPC_INTERNAL_H
a125f5
+#define PCMK__IPC_INTERNAL_H
a125f5
+
a125f5
+#include <sys/types.h>
a125f5
+
a125f5
+#include <crm_config.h>  /* US_AUTH_GETPEEREID */
a125f5
+
a125f5
+
a125f5
+/* denotes "non yieldable PID" on FreeBSD, or actual PID1 in scenarios that
a125f5
+   require a delicate handling anyway (socket-based activation with systemd);
a125f5
+   we can be reasonably sure that this PID is never possessed by the actual
a125f5
+   child daemon, as it gets taken either by the proper init, or by pacemakerd
a125f5
+   itself (i.e. this precludes anything else); note that value of zero
a125f5
+   is meant to carry "unset" meaning, and better not to bet on/conditionalize
a125f5
+   over signedness of pid_t */
a125f5
+#define PCMK__SPECIAL_PID  1
a125f5
+
a125f5
+#if defined(US_AUTH_GETPEEREID)
a125f5
+/* on FreeBSD, we don't want to expose "non-yieldable PID" (leading to
a125f5
+   "IPC liveness check only") as its nominal representation, which could
a125f5
+   cause confusion -- this is unambiguous as long as there's no
a125f5
+   socket-based activation like with systemd (very improbable) */
a125f5
+#define PCMK__SPECIAL_PID_AS_0(p)  (((p) == PCMK__SPECIAL_PID) ? 0 : (p))
a125f5
+#else
a125f5
+#define PCMK__SPECIAL_PID_AS_0(p)  (p)
a125f5
+#endif
a125f5
+
a125f5
+/*!
a125f5
+ * \internal
a125f5
+ * \brief Check the authenticity and liveness of the process via IPC end-point
a125f5
+ *
a125f5
+ * When IPC daemon under given IPC end-point (name) detected, its authenticity
a125f5
+ * is verified by the means of comparing against provided referential UID and
a125f5
+ * GID, and the result of this check can be deduced from the return value.
a125f5
+ * As an exception, referential UID of 0 (~ root) satisfies arbitrary
a125f5
+ * detected daemon's credentials.
a125f5
+ *
a125f5
+ * \param[in]  name    IPC name to base the search on
a125f5
+ * \param[in]  refuid  referential UID to check against
a125f5
+ * \param[in]  refgid  referential GID to check against
a125f5
+ * \param[out] gotpid  to optionally store obtained PID of the found process
a125f5
+ *                     upon returning 1 or -2
a125f5
+ *                     (not available on FreeBSD, special value of 1,
a125f5
+ *                     see PCMK__SPECIAL_PID, used instead, and the caller
a125f5
+ *                     is required to special case this value respectively)
a125f5
+ *
a125f5
+ * \return 0 if no trace of IPC peer's liveness detected, 1 if it was,
a125f5
+ *         -1 on error, and -2 when the IPC blocked with unauthorized
a125f5
+ *         process (log message emitted in both latter cases)
a125f5
+ *
a125f5
+ * \note This function emits a log message also in case there isn't a perfect
a125f5
+ *       match in respect to \p reguid and/or \p refgid, for a possible
a125f5
+ *       least privilege principle violation.
a125f5
+ *
a125f5
+ * \see crm_ipc_is_authentic_process
a125f5
+ */
a125f5
+int pcmk__ipc_is_authentic_process_active(const char *name, uid_t refuid,
a125f5
+                                          gid_t refgid, pid_t *gotpid);
a125f5
+
a125f5
+#endif
a125f5
diff --git a/lib/common/ipc.c b/lib/common/ipc.c
a125f5
index c582ccf..aa055d3 100644
a125f5
--- a/lib/common/ipc.c
a125f5
+++ b/lib/common/ipc.c
a125f5
@@ -1,5 +1,7 @@
a125f5
 /*
a125f5
- * Copyright 2004-2018 Andrew Beekhof <andrew@beekhof.net>
a125f5
+ * Copyright 2004-2019 the Pacemaker project contributors
a125f5
+ *
a125f5
+ * The version control history for this file may have further details.
a125f5
  *
a125f5
  * This source code is licensed under the GNU Lesser General Public License
a125f5
  * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
a125f5
@@ -7,6 +9,17 @@
a125f5
 
a125f5
 #include <crm_internal.h>
a125f5
 
a125f5
+#if defined(US_AUTH_PEERCRED_UCRED) || defined(US_AUTH_PEERCRED_SOCKPEERCRED)
a125f5
+#  ifdef US_AUTH_PEERCRED_UCRED
a125f5
+#    ifndef _GNU_SOURCE
a125f5
+#      define _GNU_SOURCE
a125f5
+#    endif
a125f5
+#  endif
a125f5
+#  include <sys/socket.h>
a125f5
+#elif defined(US_AUTH_GETPEERUCRED)
a125f5
+#  include <ucred.h>
a125f5
+#endif
a125f5
+
a125f5
 #include <sys/param.h>
a125f5
 
a125f5
 #include <stdio.h>
a125f5
@@ -19,11 +32,13 @@
a125f5
 #include <fcntl.h>
a125f5
 #include <bzlib.h>
a125f5
 
a125f5
-#include <crm/crm.h>
a125f5
+#include <crm/crm.h>   /* indirectly: pcmk_err_generic */
a125f5
 #include <crm/msg_xml.h>
a125f5
 #include <crm/common/ipc.h>
a125f5
 #include <crm/common/ipcs.h>
a125f5
 
a125f5
+#include <crm/common/ipc_internal.h>  /* PCMK__SPECIAL_PID* */
a125f5
+
a125f5
 #define PCMK_IPC_VERSION 1
a125f5
 
a125f5
 /* Evict clients whose event queue grows this large (by default) */
a125f5
@@ -1394,6 +1409,132 @@ crm_ipc_send(crm_ipc_t * client, xmlNode * message, enum crm_ipc_flags flags, in
a125f5
     return rc;
a125f5
 }
a125f5
 
a125f5
+int
a125f5
+crm_ipc_is_authentic_process(int sock, uid_t refuid, gid_t refgid,
a125f5
+                             pid_t *gotpid, uid_t *gotuid, gid_t *gotgid) {
a125f5
+    int ret = 0;
a125f5
+    pid_t found_pid = 0; uid_t found_uid = 0; gid_t found_gid = 0;
a125f5
+#if defined(US_AUTH_PEERCRED_UCRED)
a125f5
+    struct ucred ucred;
a125f5
+    socklen_t ucred_len = sizeof(ucred);
a125f5
+
a125f5
+    if (!getsockopt(sock, SOL_SOCKET, SO_PEERCRED,
a125f5
+                    &ucred, &ucred_len)
a125f5
+                && ucred_len == sizeof(ucred)) {
a125f5
+        found_pid = ucred.pid; found_uid = ucred.uid; found_gid = ucred.gid;
a125f5
+
a125f5
+#elif defined(US_AUTH_PEERCRED_SOCKPEERCRED)
a125f5
+    struct sockpeercred sockpeercred;
a125f5
+    socklen_t sockpeercred_len = sizeof(sockpeercred);
a125f5
+
a125f5
+    if (!getsockopt(sock, SOL_SOCKET, SO_PEERCRED,
a125f5
+                    &sockpeercred, &sockpeercred_len)
a125f5
+                && sockpeercred_len == sizeof(sockpeercred_len)) {
a125f5
+        found_pid = sockpeercred.pid;
a125f5
+        found_uid = sockpeercred.uid; found_gid = sockpeercred.gid;
a125f5
+
a125f5
+#elif defined(US_AUTH_GETPEEREID)
a125f5
+    if (!getpeereid(sock, &found_uid, &found_gid)) {
a125f5
+        found_pid = PCMK__SPECIAL_PID;  /* cannot obtain PID (FreeBSD) */
a125f5
+
a125f5
+#elif defined(US_AUTH_GETPEERUCRED)
a125f5
+    ucred_t *ucred;
a125f5
+    if (!getpeerucred(sock, &ucred)) {
a125f5
+        errno = 0;
a125f5
+        found_pid = ucred_getpid(ucred);
a125f5
+        found_uid = ucred_geteuid(ucred); found_gid = ucred_getegid(ucred);
a125f5
+        ret = -errno;
a125f5
+        ucred_free(ucred);
a125f5
+        if (ret) {
a125f5
+            return (ret < 0) ? ret : -pcmk_err_generic;
a125f5
+        }
a125f5
+
a125f5
+#else
a125f5
+#  error "No way to authenticate a Unix socket peer"
a125f5
+    errno = 0;
a125f5
+    if (0) {
a125f5
+#endif
a125f5
+        if (gotpid != NULL) {
a125f5
+            *gotpid = found_pid;
a125f5
+        }
a125f5
+        if (gotuid != NULL) {
a125f5
+            *gotuid = found_uid;
a125f5
+        }
a125f5
+        if (gotgid != NULL) {
a125f5
+            *gotgid = found_gid;
a125f5
+        }
a125f5
+        ret = (found_uid == 0 || found_uid == refuid || found_gid == refgid);
a125f5
+    } else {
a125f5
+        ret = (errno > 0) ? -errno : -pcmk_err_generic;
a125f5
+    }
a125f5
+
a125f5
+    return ret;
a125f5
+}
a125f5
+
a125f5
+int
a125f5
+pcmk__ipc_is_authentic_process_active(const char *name, uid_t refuid,
a125f5
+                                      gid_t refgid, pid_t *gotpid) {
a125f5
+    static char last_asked_name[PATH_MAX / 2] = "";  /* log spam prevention */
a125f5
+    int fd, ret = 0;
a125f5
+    pid_t found_pid = 0; uid_t found_uid = 0; gid_t found_gid = 0;
a125f5
+    qb_ipcc_connection_t *c;
a125f5
+
a125f5
+    if ((c = qb_ipcc_connect(name, 0)) == NULL) {
a125f5
+        crm_info("Could not connect to %s IPC: %s", name, strerror(errno));
a125f5
+
a125f5
+    } else if ((ret = qb_ipcc_fd_get(c, &fd))) {
a125f5
+        crm_err("Could not get fd from %s IPC: %s (%d)", name,
a125f5
+                strerror(-ret), -ret);
a125f5
+        ret = -1;
a125f5
+
a125f5
+    } else if ((ret = crm_ipc_is_authentic_process(fd, refuid, refgid,
a125f5
+                                                   &found_pid, &found_uid,
a125f5
+                                                   &found_gid)) < 0) {
a125f5
+        if (ret == -pcmk_err_generic) {
a125f5
+            crm_err("Could not get peer credentials from %s IPC", name);
a125f5
+        } else {
a125f5
+            crm_err("Could not get peer credentials from %s IPC: %s (%d)",
a125f5
+                    name, strerror(-ret), -ret);
a125f5
+        }
a125f5
+        ret = -1;
a125f5
+
a125f5
+    } else {
a125f5
+        if (gotpid != NULL) {
a125f5
+            *gotpid = found_pid;
a125f5
+        }
a125f5
+
a125f5
+        if (!ret) {
a125f5
+            crm_err("Daemon (IPC %s) effectively blocked with unauthorized"
a125f5
+                    " process %lld (uid: %lld, gid: %lld)",
a125f5
+                    name, (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                    (long long) found_uid, (long long) found_gid);
a125f5
+            ret = -2;
a125f5
+        } else if ((found_uid != refuid || found_gid != refgid)
a125f5
+                && strncmp(last_asked_name, name, sizeof(last_asked_name))) {
a125f5
+            if (!found_uid && refuid) {
a125f5
+                crm_warn("Daemon (IPC %s) runs as root, whereas the expected"
a125f5
+                         " credentials are %lld:%lld, hazard of violating"
a125f5
+                         " the least privilege principle",
a125f5
+                         name, (long long) refuid, (long long) refgid);
a125f5
+            } else {
a125f5
+                crm_notice("Daemon (IPC %s) runs as %lld:%lld, whereas the"
a125f5
+                           " expected credentials are %lld:%lld, which may"
a125f5
+                           " mean a different set of privileges than expected",
a125f5
+                           name, (long long) found_uid, (long long) found_gid,
a125f5
+                           (long long) refuid, (long long) refgid);
a125f5
+            }
a125f5
+            memccpy(last_asked_name, name, '\0', sizeof(last_asked_name));
a125f5
+        }
a125f5
+    }
a125f5
+
a125f5
+    if (ret) {  /* here, !ret only when we could not initially connect */
a125f5
+        qb_ipcc_disconnect(c);
a125f5
+    }
a125f5
+
a125f5
+    return ret;
a125f5
+}
a125f5
+
a125f5
+
a125f5
 /* Utils */
a125f5
 
a125f5
 xmlNode *
a125f5
-- 
a125f5
1.8.3.1
a125f5
a125f5
a125f5
From 970736b1c7ad5c78cc5295a4231e546104d55893 Mon Sep 17 00:00:00 2001
a125f5
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
a125f5
Date: Tue, 16 Apr 2019 00:13:06 +0200
a125f5
Subject: [PATCH 4/7] High: pacemakerd vs. IPC/procfs confused deputy
a125f5
 authenticity issue (2/4)
a125f5
a125f5
[2/4: pacemakerd to trust pre-existing processes via new checks instead]
a125f5
a125f5
In pacemakerd in the context of entrusting pre-existing processes,
a125f5
we now resort to procfs-based solution only in boundary, fouled cases,
a125f5
and primarily examine the credentials of the processes already
a125f5
occupying known IPC end-points before adopting them.
a125f5
a125f5
The commit applies the new helpers from 1/1 so as to close the both
a125f5
related sensitive problems, CVE-2018-16877 and CVE-2018-16878, in
a125f5
a unified manner, this time limited to the main daemon of pacemaker
a125f5
(pacemakerd).
a125f5
a125f5
To be noted that it is clearly not 100% for this purpose for still
a125f5
allowing for TOCTTOU, but that's what commit (3/3) is meant to solve
a125f5
for the most part, plus there may be optimizations solving this concern
a125f5
as a side effect, but that requires an active assistance on the libqb
a125f5
side (https://github.com/ClusterLabs/libqb/issues/325) since any
a125f5
improvement on pacemaker side in isolation would be very
a125f5
cumbersome if generally possible at all, but either way
a125f5
means a new, soft compatibility encumberment.
a125f5
a125f5
As a follow-up to what was put in preceding 1/3 commit, PID of 1 tracked
a125f5
as child's identification on FreeBSD (or when socket-based activation is
a125f5
used with systemd) is treated specially, incl. special precaution with
a125f5
child's PID discovered as 1 elsewhere.
a125f5
a125f5
v2: courtesy of Yan Gao of SUSE for early discovery and report for
a125f5
    what's primarily solved with 4/4 commit, in extension, child
a125f5
    daemons in the initialization phase coinciding with IPC-feasibility
a125f5
    based process scan in pacemakerd in a way that those are missed
a125f5
    (although they are to come up fully just moments later only
a125f5
    to interfere with naturally spawned ones) are now considered so
a125f5
    that if any native children later fail for said clash, the
a125f5
    pre-existing counterpart may get adopted instead of ending up
a125f5
    with repeated spawn-bury loop ad nauseam without real progress
a125f5
    (note that PCMK_fail_fast=true could possibly help, but that's
a125f5
    rather a big hammer not suitable for all the use cases, not
a125f5
    the ones we try to deal with gracefully here)
a125f5
---
a125f5
 daemons/pacemakerd/pacemakerd.c | 406 ++++++++++++++++++++++++++++++++++------
a125f5
 1 file changed, 345 insertions(+), 61 deletions(-)
a125f5
a125f5
diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c
a125f5
index eca7f4d..bebf938 100644
a125f5
--- a/daemons/pacemakerd/pacemakerd.c
a125f5
+++ b/daemons/pacemakerd/pacemakerd.c
a125f5
@@ -12,17 +12,23 @@
a125f5
 
a125f5
 #include <pwd.h>
a125f5
 #include <grp.h>
a125f5
+#include <poll.h>
a125f5
 #include <sys/stat.h>
a125f5
 #include <sys/types.h>
a125f5
 #include <sys/time.h>
a125f5
 #include <sys/resource.h>
a125f5
 #include <sys/reboot.h>
a125f5
 
a125f5
+#include <crm/crm.h>  /* indirectly: CRM_EX_* */
a125f5
+#include <crm/cib/internal.h>  /* cib_channel_ro */
a125f5
 #include <crm/msg_xml.h>
a125f5
 #include <crm/common/ipcs.h>
a125f5
 #include <crm/common/mainloop.h>
a125f5
 #include <crm/cluster/internal.h>
a125f5
 #include <crm/cluster.h>
a125f5
+
a125f5
+#include <crm/common/ipc_internal.h>  /* PCMK__SPECIAL_PID*, ... */
a125f5
+
a125f5
 #ifdef SUPPORT_COROSYNC
a125f5
 #include <corosync/cfg.h>
a125f5
 #endif
a125f5
@@ -33,6 +39,7 @@
a125f5
 static gboolean pcmk_quorate = FALSE;
a125f5
 static gboolean fatal_error = FALSE;
a125f5
 static GMainLoop *mainloop = NULL;
a125f5
+static bool global_keep_tracking = false;
a125f5
 
a125f5
 #define PCMK_PROCESS_CHECK_INTERVAL 5
a125f5
 
a125f5
@@ -50,6 +57,7 @@ typedef struct pcmk_child_s {
a125f5
     const char *name;
a125f5
     const char *uid;
a125f5
     const char *command;
a125f5
+    const char *endpoint;  /* IPC server name */
a125f5
 
a125f5
     gboolean active_before_startup;
a125f5
 } pcmk_child_t;
a125f5
@@ -64,30 +72,38 @@ static pcmk_child_t pcmk_children[] = {
a125f5
     },
a125f5
     {
a125f5
         0, crm_proc_execd,      3, 0, TRUE,  "pacemaker-execd",
a125f5
-        NULL, CRM_DAEMON_DIR "/pacemaker-execd"
a125f5
+        NULL, CRM_DAEMON_DIR "/pacemaker-execd",
a125f5
+        CRM_SYSTEM_LRMD
a125f5
     },
a125f5
     {
a125f5
         0, crm_proc_based,      1, 0, TRUE,  "pacemaker-based",
a125f5
-        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-based"
a125f5
+        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-based",
a125f5
+        CIB_CHANNEL_RO
a125f5
     },
a125f5
     {
a125f5
         0, crm_proc_controld,   6, 0, TRUE, "pacemaker-controld",
a125f5
-        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-controld"
a125f5
+        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-controld",
a125f5
+        CRM_SYSTEM_CRMD
a125f5
     },
a125f5
     {
a125f5
         0, crm_proc_attrd,      4, 0, TRUE, "pacemaker-attrd",
a125f5
-        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-attrd"
a125f5
+        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-attrd",
a125f5
+        T_ATTRD
a125f5
     },
a125f5
     {
a125f5
         0, crm_proc_schedulerd, 5, 0, TRUE, "pacemaker-schedulerd",
a125f5
-        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-schedulerd"
a125f5
+        CRM_DAEMON_USER, CRM_DAEMON_DIR "/pacemaker-schedulerd",
a125f5
+        CRM_SYSTEM_PENGINE
a125f5
     },
a125f5
     {
a125f5
         0, crm_proc_fenced,     2, 0, TRUE, "pacemaker-fenced",
a125f5
-        NULL, CRM_DAEMON_DIR "/pacemaker-fenced"
a125f5
+        NULL, CRM_DAEMON_DIR "/pacemaker-fenced",
a125f5
+        "stonith-ng"
a125f5
     },
a125f5
 };
a125f5
 
a125f5
+static gboolean check_active_before_startup_processes(gpointer user_data);
a125f5
+static int pcmk_child_active(pcmk_child_t *child);
a125f5
 static gboolean start_child(pcmk_child_t * child);
a125f5
 static gboolean update_node_processes(uint32_t id, const char *uname,
a125f5
                                       uint32_t procs);
a125f5
@@ -130,14 +146,31 @@ pcmk_process_exit(pcmk_child_t * child)
a125f5
     }
a125f5
 
a125f5
     if (shutdown_trigger) {
a125f5
+        /* resume step-wise shutdown (returned TRUE yields no parallelizing) */
a125f5
         mainloop_set_trigger(shutdown_trigger);
a125f5
+        /* intended to speed up propagating expected lay-off of the daemons? */
a125f5
         update_node_processes(local_nodeid, NULL, get_process_list());
a125f5
 
a125f5
-    } else if (child->respawn && crm_is_true(getenv("PCMK_fail_fast"))) {
a125f5
+    } else if (!child->respawn) {
a125f5
+        /* nothing to do */
a125f5
+
a125f5
+    } else if (crm_is_true(getenv("PCMK_fail_fast"))) {
a125f5
         crm_err("Rebooting system because of %s", child->name);
a125f5
         pcmk_panic(__FUNCTION__);
a125f5
 
a125f5
-    } else if (child->respawn) {
a125f5
+    } else if (pcmk_child_active(child) == 1) {
a125f5
+        crm_warn("One-off suppressing strict respawning of a child process %s,"
a125f5
+                 " appears alright per %s IPC end-point",
a125f5
+                 child->name, child->endpoint);
a125f5
+        /* need to monitor how it evolves, and start new process if badly */
a125f5
+        child->active_before_startup = TRUE;
a125f5
+        if (!global_keep_tracking) {
a125f5
+            global_keep_tracking = true;
a125f5
+            g_timeout_add_seconds(PCMK_PROCESS_CHECK_INTERVAL,
a125f5
+                                  check_active_before_startup_processes, NULL);
a125f5
+        }
a125f5
+
a125f5
+    } else {
a125f5
         crm_notice("Respawning failed child process: %s", child->name);
a125f5
         start_child(child);
a125f5
     }
a125f5
@@ -218,8 +251,13 @@ stop_child(pcmk_child_t * child, int signal)
a125f5
         signal = SIGTERM;
a125f5
     }
a125f5
 
a125f5
-    if (child->command == NULL) {
a125f5
-        crm_debug("Nothing to do for child \"%s\"", child->name);
a125f5
+    /* why to skip PID of 1?
a125f5
+       - FreeBSD ~ how untrackable process behind IPC is masqueraded as
a125f5
+       - elsewhere: how "init" task is designated; in particular, in systemd
a125f5
+         arrangement of socket-based activation, this is pretty real */
a125f5
+    if (child->command == NULL || child->pid == PCMK__SPECIAL_PID) {
a125f5
+        crm_debug("Nothing to do for child \"%s\" (process %lld)",
a125f5
+                  child->name, (long long) PCMK__SPECIAL_PID_AS_0(child->pid));
a125f5
         return TRUE;
a125f5
     }
a125f5
 
a125f5
@@ -244,6 +282,11 @@ stop_child(pcmk_child_t * child, int signal)
a125f5
 static char *opts_default[] = { NULL, NULL };
a125f5
 static char *opts_vgrind[] = { NULL, NULL, NULL, NULL, NULL };
a125f5
 
a125f5
+/* TODO once libqb is taught to juggle with IPC end-points carried over as
a125f5
+        bare file descriptor (https://github.com/ClusterLabs/libqb/issues/325)
a125f5
+        it shall hand over these descriptors here if/once they are successfully
a125f5
+        pre-opened in (presumably) pcmk_child_active, to avoid any remaining
a125f5
+        room for races */
a125f5
 static gboolean
a125f5
 start_child(pcmk_child_t * child)
a125f5
 {
a125f5
@@ -380,7 +423,10 @@ escalate_shutdown(gpointer data)
a125f5
 
a125f5
     pcmk_child_t *child = data;
a125f5
 
a125f5
-    if (child->pid) {
a125f5
+    if (child->pid == PCMK__SPECIAL_PID) {
a125f5
+        pcmk_process_exit(child);
a125f5
+
a125f5
+    } else if (child->pid) {
a125f5
         /* Use SIGSEGV instead of SIGKILL to create a core so we can see what it was up to */
a125f5
         crm_err("Child %s not terminating in a timely manner, forcing", child->name);
a125f5
         stop_child(child, SIGSEGV);
a125f5
@@ -388,6 +434,8 @@ escalate_shutdown(gpointer data)
a125f5
     return FALSE;
a125f5
 }
a125f5
 
a125f5
+#define SHUTDOWN_ESCALATION_PERIOD 180000  /* 3m */
a125f5
+
a125f5
 static gboolean
a125f5
 pcmk_shutdown_worker(gpointer user_data)
a125f5
 {
a125f5
@@ -416,11 +464,24 @@ pcmk_shutdown_worker(gpointer user_data)
a125f5
                 time_t now = time(NULL);
a125f5
 
a125f5
                 if (child->respawn) {
a125f5
+                    if (child->pid == PCMK__SPECIAL_PID) {
a125f5
+                        crm_warn("The process behind %s IPC cannot be"
a125f5
+                                 " terminated, so either wait the graceful"
a125f5
+                                 " period of %ld s for its native termination"
a125f5
+                                 " if it vitally depends on some other daemons"
a125f5
+                                 " going down in a controlled way already,"
a125f5
+                                 " or locate and kill the correct %s process"
a125f5
+                                 " on your own; set PCMK_fail_fast=1 to avoid"
a125f5
+                                 " this altogether next time around",
a125f5
+                                 child->name, (long) SHUTDOWN_ESCALATION_PERIOD,
a125f5
+                                 child->command);
a125f5
+                    }
a125f5
                     next_log = now + 30;
a125f5
                     child->respawn = FALSE;
a125f5
                     stop_child(child, SIGTERM);
a125f5
                     if (phase < pcmk_children[PCMK_CHILD_CONTROLD].start_seq) {
a125f5
-                        g_timeout_add(180000 /* 3m */ , escalate_shutdown, child);
a125f5
+                        g_timeout_add(SHUTDOWN_ESCALATION_PERIOD,
a125f5
+                                      escalate_shutdown, child);
a125f5
                     }
a125f5
 
a125f5
                 } else if (now >= next_log) {
a125f5
@@ -702,7 +763,102 @@ mcp_chown(const char *path, uid_t uid, gid_t gid)
a125f5
     }
a125f5
 }
a125f5
 
a125f5
-#if SUPPORT_PROCFS
a125f5
+/*!
a125f5
+ * \internal
a125f5
+ * \brief Check the liveness of the child based on IPC name and PID if tracked
a125f5
+ *
a125f5
+ * \param[inout] child  Child tracked data
a125f5
+ *
a125f5
+ * \return 0 if no trace of child's liveness detected (while it is detectable
a125f5
+ *         to begin with, at least according to one of the two properties),
a125f5
+ *         1 if everything is fine, 2 if it's up per PID, but not per IPC
a125f5
+ *         end-point (still starting?), -1 on error, and -2 when the child
a125f5
+ *         (its IPC) blocked with an unauthorized process (log message
a125f5
+ *         emitted in both latter cases)
a125f5
+ *
a125f5
+ * \note This function doesn't modify any of \p child members but \c pid,
a125f5
+ *       and is not actively toying with processes as such but invoking
a125f5
+ *       \c stop_child in one particular case (there's for some reason
a125f5
+ *       a different authentic holder of the IPC end-point).
a125f5
+ */
a125f5
+static int
a125f5
+pcmk_child_active(pcmk_child_t *child) {
a125f5
+    static uid_t cl_uid = 0;
a125f5
+    static gid_t cl_gid = 0;
a125f5
+    const uid_t root_uid = 0;
a125f5
+    const gid_t root_gid = 0;
a125f5
+    const uid_t *ref_uid;
a125f5
+    const gid_t *ref_gid;
a125f5
+    int ret = 0;
a125f5
+    pid_t ipc_pid = 0;
a125f5
+
a125f5
+    if (child->endpoint == NULL
a125f5
+            && (child->pid <= 0 || child->pid == PCMK__SPECIAL_PID)) {
a125f5
+        crm_err("Cannot track child %s for missing both API end-point and PID",
a125f5
+                child->name);
a125f5
+        ret = -1;  /* misuse of the function when child is not trackable */
a125f5
+
a125f5
+    } else if (child->endpoint != NULL) {
a125f5
+
a125f5
+        ref_uid = (child->uid != NULL) ? &cl_uid : &root_uid;
a125f5
+        ref_gid = (child->uid != NULL) ? &cl_gid : &root_gid;
a125f5
+
a125f5
+        if (child->uid != NULL && !cl_uid && !cl_gid
a125f5
+                && crm_user_lookup(CRM_DAEMON_USER, &cl_uid, &cl_gid) < 0) {
a125f5
+            crm_err("Could not find user and group IDs for user %s",
a125f5
+                    CRM_DAEMON_USER);
a125f5
+            ret = -1;
a125f5
+        } else if ((ret = pcmk__ipc_is_authentic_process_active(child->endpoint,
a125f5
+                                                                *ref_uid, *ref_gid,
a125f5
+                                                                &ipc_pid)) < 0) {
a125f5
+            /* game over */
a125f5
+        } else if (child->pid <= 0) {
a125f5
+            /* hit new child to be initialized, or reset to zero
a125f5
+               and investigate further for ret == 0 */
a125f5
+            child->pid = ipc_pid;
a125f5
+        } else if (ipc_pid && child->pid != ipc_pid) {
a125f5
+            /* ultimately strange for ret == 1; either way, investigate */
a125f5
+            ret = 0;
a125f5
+        }
a125f5
+    }
a125f5
+
a125f5
+    if (!ret) {
a125f5
+        /* when no IPC based liveness detected (incl. if ever a child without
a125f5
+           IPC is tracked), or detected for a different _authentic_ process;
a125f5
+           safe on FreeBSD since the only change possible from a proper child's
a125f5
+           PID into "special" PID of 1 behind more loosely related process */
a125f5
+        ret = crm_pid_active(child->pid, child->name);
a125f5
+        if (ipc_pid && (ret != 1
a125f5
+                        || ipc_pid == PCMK__SPECIAL_PID
a125f5
+                        || crm_pid_active(ipc_pid, child->name) == 1)) {
a125f5
+            if (ret == 1) {
a125f5
+                /* assume there's no forking-while-retaining-IPC-socket
a125f5
+                   involved in the "children's" lifecycle, hence that the
a125f5
+                   tracking got out of sync purely because of some external
a125f5
+                   (esotheric?) forces (user initiated process "refresh" by
a125f5
+                   force? or intentionally racing on start-up, even?), and
a125f5
+                   that switching over to this other detected, authentic
a125f5
+                   instance with an IPC already in possession is a better
a125f5
+                   trade-off than "neutralizing" it first so as to give
a125f5
+                   either the original or possibly a new to-be-spawned
a125f5
+                   daemon process a leeway for operation, which would
a125f5
+                   otherwise have to be carried out */
a125f5
+                /* not possessing IPC, afterall (what about corosync CPG?) */
a125f5
+                stop_child(child, SIGKILL);
a125f5
+            } else {
a125f5
+                ret = 1;
a125f5
+            }
a125f5
+            child->pid = ipc_pid;
a125f5
+        } else if (ret == 1) {
a125f5
+            ret = 2;  /* up per PID, but not per IPC (still starting?) */
a125f5
+        } else if (!child->pid && ret == -1) {
a125f5
+            ret = 0;  /* correct -1 on FreeBSD from above back to 0 */
a125f5
+        }
a125f5
+    }
a125f5
+
a125f5
+    return ret;
a125f5
+}
a125f5
+
a125f5
 static gboolean
a125f5
 check_active_before_startup_processes(gpointer user_data)
a125f5
 {
a125f5
@@ -719,12 +875,41 @@ check_active_before_startup_processes(gpointer user_data)
a125f5
                 continue;
a125f5
             } else {
a125f5
                 const char *name = pcmk_children[lpc].name;
a125f5
-
a125f5
-                if (crm_pid_active(pcmk_children[lpc].pid, name) != 1) {
a125f5
-                    crm_notice("Process %s terminated (pid=%d)",
a125f5
-                           name, pcmk_children[lpc].pid);
a125f5
-                    pcmk_process_exit(&(pcmk_children[lpc]));
a125f5
-                    continue;
a125f5
+                int ret;
a125f5
+
a125f5
+                switch ((ret = pcmk_child_active(&pcmk_children[lpc]))) {
a125f5
+                    case 1:
a125f5
+                        break;
a125f5
+                    case 0:
a125f5
+                    case 2:  /* this very case: it was OK once already */
a125f5
+                        if (pcmk_children[lpc].respawn == TRUE) {
a125f5
+                            /* presumably after crash, hence critical */
a125f5
+                            crm_crit("Process %s terminated (pid=%lld)%s", \
a125f5
+                                     name, (long long)
a125f5
+                                     PCMK__SPECIAL_PID_AS_0(pcmk_children[lpc].pid),
a125f5
+                                     ret ? ", at least per IPC end-point that went AWOL"
a125f5
+                                         : "");
a125f5
+                        } else {
a125f5
+                            /* orderly shutdown */
a125f5
+                            crm_notice("Process %s terminated (pid=%lld)%s", \
a125f5
+                                       name, (long long)
a125f5
+                                       PCMK__SPECIAL_PID_AS_0(pcmk_children[lpc].pid),
a125f5
+                                       ret ? ", at least per IPC end-point that went AWOL"
a125f5
+                                           : "");
a125f5
+                        }
a125f5
+                        pcmk_process_exit(&(pcmk_children[lpc]));
a125f5
+                        continue;
a125f5
+                    default:
a125f5
+                        crm_crit("Unexpected value from pcmk_child_active:"
a125f5
+                                 " %d (pid=%lld)", ret,
a125f5
+                                 (long long) PCMK__SPECIAL_PID_AS_0(
a125f5
+                                                 pcmk_children[lpc].pid));
a125f5
+                        /* fall through */
a125f5
+                    case -1:
a125f5
+                    case -2:
a125f5
+                        /* message(s) already emitted */
a125f5
+                        crm_exit(CRM_EX_FATAL);
a125f5
+                        break;  /* static analysis/noreturn */
a125f5
                 }
a125f5
             }
a125f5
             /* at least one of the processes found at startup
a125f5
@@ -733,57 +918,147 @@ check_active_before_startup_processes(gpointer user_data)
a125f5
         }
a125f5
     }
a125f5
 
a125f5
+    global_keep_tracking = keep_tracking;
a125f5
     return keep_tracking;
a125f5
 }
a125f5
-#endif // SUPPORT_PROCFS
a125f5
 
a125f5
-static void
a125f5
+/*!
a125f5
+ * \internal
a125f5
+ * \brief Initial one-off check of the pre-existing "child" processes
a125f5
+ *
a125f5
+ * With "child" process, we mean the subdaemon that defines an API end-point
a125f5
+ * (all of them do as of the comment) -- the possible complement is skipped
a125f5
+ * as it is deemed it has no such shared resources to cause conflicts about,
a125f5
+ * hence it can presumably be started anew without hesitation.
a125f5
+ * If that won't hold true in the future, the concept of a shared resource
a125f5
+ * will have to be generalized beyond the API end-point.
a125f5
+ *
a125f5
+ * For boundary cases that the "child" is still starting (IPC end-point is yet
a125f5
+ * to be witnessed), or more rarely (practically FreeBSD only), when there's
a125f5
+ * a pre-existing "untrackable" authentic process, we give the situation some
a125f5
+ * time to possibly unfold in the right direction, meaning that said socket
a125f5
+ * will appear or the unattainable process will disappear per the observable
a125f5
+ * IPC, respectively.
a125f5
+ *
a125f5
+ * \return 0 if no such "child" process found, positive number X when X
a125f5
+ *         "children" detected, -1 on an internal error, -2 when any
a125f5
+ *         would-be-used IPC is blocked with an unauthorized process
a125f5
+ *
a125f5
+ * \note Since this gets run at the very start, \c respawn_count fields
a125f5
+ *       for particular children get temporarily overloaded with "rounds
a125f5
+ *       of waiting" tracking, restored once we are about to finish with
a125f5
+ *       success (i.e. returning value >=0) and will remain unrestored
a125f5
+ *       otherwise.  One way to suppress liveness detection logic for
a125f5
+ *       particular child is to set the said value to a negative number.
a125f5
+ */
a125f5
+#define WAIT_TRIES 4  /* together with interleaved sleeps, worst case ~ 1s */
a125f5
+static int
a125f5
 find_and_track_existing_processes(void)
a125f5
 {
a125f5
-#if SUPPORT_PROCFS
a125f5
-    DIR *dp;
a125f5
-    struct dirent *entry;
a125f5
-    bool start_tracker = FALSE;
a125f5
-    char entry_name[16];
a125f5
-
a125f5
-    dp = opendir("/proc");
a125f5
-    if (!dp) {
a125f5
-        /* no proc directory to search through */
a125f5
-        crm_notice("Can not read /proc directory to track existing components");
a125f5
-        return;
a125f5
-    }
a125f5
-
a125f5
-    while ((entry = readdir(dp)) != NULL) {
a125f5
-        int pid;
a125f5
-        int max = SIZEOF(pcmk_children);
a125f5
-        int i;
a125f5
-
a125f5
-        if (crm_procfs_process_info(entry, entry_name, &pid) < 0) {
a125f5
-            continue;
a125f5
-        }
a125f5
-        for (i = 0; i < max; i++) {
a125f5
-            if ((pcmk_children[i].start_seq != 0)
a125f5
-                && !strncmp(entry_name, pcmk_children[i].name, 15)
a125f5
-                && (crm_pid_active(pid, NULL) == 1)) {
a125f5
-
a125f5
-                crm_notice("Tracking existing %s process (pid=%d)",
a125f5
-                           pcmk_children[i].name, pid);
a125f5
-                pcmk_children[i].pid = pid;
a125f5
-                pcmk_children[i].active_before_startup = TRUE;
a125f5
-                start_tracker = TRUE;
a125f5
-                break;
a125f5
+    unsigned tracking = 0U;
a125f5
+    bool wait_in_progress;
a125f5
+    int cur;
a125f5
+    size_t i, rounds;
a125f5
+
a125f5
+    for (rounds = 1; rounds <= WAIT_TRIES; rounds++) {
a125f5
+        wait_in_progress = false;
a125f5
+        for (i = 0; i < SIZEOF(pcmk_children); i++) {
a125f5
+            if (!pcmk_children[i].endpoint
a125f5
+                    || pcmk_children[i].respawn_count < 0
a125f5
+                    || !(cur = pcmk_child_active(&pcmk_children[i]))) {
a125f5
+                /* as a speculation, don't give up in the context of
a125f5
+                   pcmk_child_active check if there are more rounds to
a125f5
+                   come for other reasons, but don't artificially wait just
a125f5
+                   because of this, since we would preferably start ASAP */
a125f5
+                continue;
a125f5
+            }
a125f5
+            pcmk_children[i].respawn_count = rounds;
a125f5
+            switch (cur) {
a125f5
+                case 1:
a125f5
+                    if (pcmk_children[i].pid == PCMK__SPECIAL_PID) {
a125f5
+                        if (crm_is_true(getenv("PCMK_fail_fast"))) {
a125f5
+                            crm_crit("Cannot reliably track pre-existing"
a125f5
+                                     " authentic process behind %s IPC on this"
a125f5
+                                     " platform and PCMK_fail_fast requested",
a125f5
+                                     pcmk_children[i].endpoint);
a125f5
+                            return -1;
a125f5
+                        } else if (pcmk_children[i].respawn_count == WAIT_TRIES) {
a125f5
+                            crm_notice("Assuming pre-existing authentic, though"
a125f5
+                                       " on this platform untrackable, process"
a125f5
+                                       " behind %s IPC is stable (was in %d"
a125f5
+                                       " previous samples) so rather than"
a125f5
+                                       " bailing out (PCMK_fail_fast not"
a125f5
+                                       " requested), we just switch to a less"
a125f5
+                                       " optimal IPC liveness monitoring"
a125f5
+                                       " (not very suitable for heavy load)",
a125f5
+                                       pcmk_children[i].name, WAIT_TRIES - 1);
a125f5
+                            crm_warn("The process behind %s IPC cannot be"
a125f5
+                                     " terminated, so the overall shutdown"
a125f5
+                                     " will get delayed implicitly (%ld s),"
a125f5
+                                     " which serves as a graceful period for"
a125f5
+                                     " its native termination if it vitally"
a125f5
+                                     " depends on some other daemons going"
a125f5
+                                     " down in a controlled way already",
a125f5
+                                     pcmk_children[i].name,
a125f5
+                                     (long) SHUTDOWN_ESCALATION_PERIOD);
a125f5
+                        } else {
a125f5
+                            wait_in_progress = true;
a125f5
+                            crm_warn("Cannot reliably track pre-existing"
a125f5
+                                     " authentic process behind %s IPC on this"
a125f5
+                                     " platform, can still disappear in %d"
a125f5
+                                     " attempt(s)", pcmk_children[i].endpoint,
a125f5
+                                     WAIT_TRIES - pcmk_children[i].respawn_count);
a125f5
+                            continue;
a125f5
+                        }
a125f5
+                    }
a125f5
+                    crm_notice("Tracking existing %s process (pid=%lld)",
a125f5
+                               pcmk_children[i].name,
a125f5
+                               (long long) PCMK__SPECIAL_PID_AS_0(
a125f5
+                                               pcmk_children[i].pid));
a125f5
+                    pcmk_children[i].respawn_count = -1;  /* 0~keep watching */
a125f5
+                    pcmk_children[i].active_before_startup = TRUE;
a125f5
+                    tracking++;
a125f5
+                    break;
a125f5
+                case 2:
a125f5
+                    if (pcmk_children[i].respawn_count == WAIT_TRIES) {
a125f5
+                        crm_crit("%s IPC end-point for existing authentic"
a125f5
+                                 " process %lld did not (re)appear",
a125f5
+                                 pcmk_children[i].endpoint,
a125f5
+                                 (long long) PCMK__SPECIAL_PID_AS_0(
a125f5
+                                                 pcmk_children[i].pid));
a125f5
+                        return -1;
a125f5
+                    }
a125f5
+                    wait_in_progress = true;
a125f5
+                    crm_warn("Cannot find %s IPC end-point for existing"
a125f5
+                             " authentic process %lld, can still (re)appear"
a125f5
+                             " in %d attempts (?)",
a125f5
+                             pcmk_children[i].endpoint,
a125f5
+                             (long long) PCMK__SPECIAL_PID_AS_0(
a125f5
+                                             pcmk_children[i].pid),
a125f5
+                             WAIT_TRIES - pcmk_children[i].respawn_count);
a125f5
+                    continue;
a125f5
+                case -1:
a125f5
+                case -2:
a125f5
+                    return cur;  /* messages already emitted */
a125f5
+                default:
a125f5
+                    crm_crit("Unexpected condition"CRM_XS"cur=%d", cur);
a125f5
+                    return -1;  /* unexpected condition */
a125f5
             }
a125f5
         }
a125f5
+        if (!wait_in_progress) {
a125f5
+            break;
a125f5
+        }
a125f5
+        (void) poll(NULL, 0, 250);  /* a bit for changes to possibly happen */
a125f5
+    }
a125f5
+    for (i = 0; i < SIZEOF(pcmk_children); i++) {
a125f5
+        pcmk_children[i].respawn_count = 0;  /* restore pristine state */
a125f5
     }
a125f5
 
a125f5
-    if (start_tracker) {
a125f5
-        g_timeout_add_seconds(PCMK_PROCESS_CHECK_INTERVAL, check_active_before_startup_processes,
a125f5
-                              NULL);
a125f5
+    if (tracking) {
a125f5
+        g_timeout_add_seconds(PCMK_PROCESS_CHECK_INTERVAL,
a125f5
+                              check_active_before_startup_processes, NULL);
a125f5
     }
a125f5
-    closedir(dp);
a125f5
-#else
a125f5
-    crm_notice("No procfs support, so skipping check for existing components");
a125f5
-#endif // SUPPORT_PROCFS
a125f5
+    return (tracking > INT_MAX) ? INT_MAX : tracking;
a125f5
 }
a125f5
 
a125f5
 static void
a125f5
@@ -1091,7 +1366,16 @@ main(int argc, char **argv)
a125f5
         setenv("PCMK_watchdog", "false", 1);
a125f5
     }
a125f5
 
a125f5
-    find_and_track_existing_processes();
a125f5
+    switch (find_and_track_existing_processes()) {
a125f5
+        case -1:
a125f5
+            crm_crit("Internal fatality, see the log");
a125f5
+            crm_exit(CRM_EX_FATAL);
a125f5
+        case -2:
a125f5
+            crm_crit("Blocked by foreign process, kill the offender");
a125f5
+            crm_exit(CRM_EX_CANTCREAT);
a125f5
+        default:
a125f5
+            break;
a125f5
+    };
a125f5
 
a125f5
     cluster.destroy = mcp_cpg_destroy;
a125f5
     cluster.cpg.cpg_deliver_fn = mcp_cpg_deliver;
a125f5
-- 
a125f5
1.8.3.1
a125f5
a125f5
a125f5
From 052e6045eea77685aabeed12c519c7c9eb9b5287 Mon Sep 17 00:00:00 2001
a125f5
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
a125f5
Date: Tue, 16 Apr 2019 00:13:31 +0200
a125f5
Subject: [PATCH 5/7] High: pacemakerd vs. IPC/procfs confused deputy
a125f5
 authenticity issue (3/4)
a125f5
a125f5
[3/4: other daemons to authenticate IPC servers of fellow processes]
a125f5
a125f5
Now that CVE-2018-16877 issue alone is still only partially covered
a125f5
based on the preceding commits in the set, put the server-by-client
a125f5
authentication (enabled and 1/3 and partially sported in 2/3) into
a125f5
practice widely amongst the communicating pacemaker child daemons and
a125f5
towards CPG API provided by 3rd party but principally using the same
a125f5
underlying IPC mechanism facilitated by libqb, and consequently close
a125f5
the remaining "big gap".
a125f5
a125f5
As a small justification to introducing yet another "return
a125f5
value" int variable, type-correctness is restored for those
a125f5
that shall be cs_error_t to begin with.
a125f5
---
a125f5
 daemons/pacemakerd/pcmkd_corosync.c |  61 +++++++++++-
a125f5
 lib/cluster/corosync.c              | 178 ++++++++++++++++++++++++++++++------
a125f5
 lib/cluster/cpg.c                   |  81 +++++++++++++---
a125f5
 lib/common/ipc.c                    |  43 ++++++++-
a125f5
 4 files changed, 317 insertions(+), 46 deletions(-)
a125f5
a125f5
diff --git a/daemons/pacemakerd/pcmkd_corosync.c b/daemons/pacemakerd/pcmkd_corosync.c
a125f5
index c73a1f5..65595fa 100644
a125f5
--- a/daemons/pacemakerd/pcmkd_corosync.c
a125f5
+++ b/daemons/pacemakerd/pcmkd_corosync.c
a125f5
@@ -1,5 +1,7 @@
a125f5
 /*
a125f5
- * Copyright 2010-2018 Andrew Beekhof <andrew@beekhof.net>
a125f5
+ * Copyright 2010-2019 the Pacemaker project contributors
a125f5
+ *
a125f5
+ * The version control history for this file may have further details.
a125f5
  *
a125f5
  * This source code is licensed under the GNU General Public License version 2
a125f5
  * or later (GPLv2+) WITHOUT ANY WARRANTY.
a125f5
@@ -21,8 +23,11 @@
a125f5
 #include <corosync/cmap.h>
a125f5
 
a125f5
 #include <crm/cluster/internal.h>
a125f5
+#include <crm/common/ipc.h>     /* for crm_ipc_is_authentic_process */
a125f5
 #include <crm/common/mainloop.h>
a125f5
 
a125f5
+#include <crm/common/ipc_internal.h>  /* PCMK__SPECIAL_PID* */
a125f5
+
a125f5
 enum cluster_type_e stack = pcmk_cluster_unknown;
a125f5
 static corosync_cfg_handle_t cfg_handle;
a125f5
 
a125f5
@@ -91,7 +96,10 @@ gboolean
a125f5
 cluster_connect_cfg(uint32_t * nodeid)
a125f5
 {
a125f5
     cs_error_t rc;
a125f5
-    int fd = 0, retries = 0;
a125f5
+    int fd = -1, retries = 0, rv;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
 
a125f5
     static struct mainloop_fd_callbacks cfg_fd_callbacks = {
a125f5
         .dispatch = pcmk_cfg_dispatch,
a125f5
@@ -101,13 +109,27 @@ cluster_connect_cfg(uint32_t * nodeid)
a125f5
     cs_repeat(retries, 30, rc = corosync_cfg_initialize(&cfg_handle, &cfg_callbacks));
a125f5
 
a125f5
     if (rc != CS_OK) {
a125f5
-        crm_err("corosync cfg init error %d", rc);
a125f5
+        crm_err("corosync cfg init: %s (%d)", cs_strerror(rc), rc);
a125f5
         return FALSE;
a125f5
     }
a125f5
 
a125f5
     rc = corosync_cfg_fd_get(cfg_handle, &fd;;
a125f5
     if (rc != CS_OK) {
a125f5
-        crm_err("corosync cfg fd_get error %d", rc);
a125f5
+        crm_err("corosync cfg fd_get: %s (%d)", cs_strerror(rc), rc);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
+    /* CFG provider run as root (in given user namespace, anyway)? */
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                            &found_uid, &found_gid))) {
a125f5
+        crm_err("CFG provider is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        goto bail;
a125f5
+    } else if (rv < 0) {
a125f5
+        crm_err("Could not verify authenticity of CFG provider: %s (%d)",
a125f5
+                strerror(-rv), -rv);
a125f5
         goto bail;
a125f5
     }
a125f5
 
a125f5
@@ -152,10 +174,15 @@ get_config_opt(uint64_t unused, cmap_handle_t object_handle, const char *key, ch
a125f5
 gboolean
a125f5
 mcp_read_config(void)
a125f5
 {
a125f5
-    int rc = CS_OK;
a125f5
+    cs_error_t rc = CS_OK;
a125f5
     int retries = 0;
a125f5
     cmap_handle_t local_handle;
a125f5
     uint64_t config = 0;
a125f5
+    int fd = -1;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     // There can be only one possibility
a125f5
     do {
a125f5
@@ -178,6 +205,30 @@ mcp_read_config(void)
a125f5
         return FALSE;
a125f5
     }
a125f5
 
a125f5
+    rc = cmap_fd_get(local_handle, &fd;;
a125f5
+    if (rc != CS_OK) {
a125f5
+        crm_err("Could not obtain the CMAP API connection: %s (%d)",
a125f5
+                cs_strerror(rc), rc);
a125f5
+        cmap_finalize(local_handle);
a125f5
+        return FALSE;
a125f5
+    }
a125f5
+
a125f5
+    /* CMAP provider run as root (in given user namespace, anyway)? */
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                            &found_uid, &found_gid))) {
a125f5
+        crm_err("CMAP provider is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        cmap_finalize(local_handle);
a125f5
+        return FALSE;
a125f5
+    } else if (rv < 0) {
a125f5
+        crm_err("Could not verify authenticity of CMAP provider: %s (%d)",
a125f5
+                strerror(-rv), -rv);
a125f5
+        cmap_finalize(local_handle);
a125f5
+        return FALSE;
a125f5
+    }
a125f5
+
a125f5
     stack = get_cluster_type();
a125f5
     crm_info("Reading configure for stack: %s", name_for_cluster_type(stack));
a125f5
 
a125f5
diff --git a/lib/cluster/corosync.c b/lib/cluster/corosync.c
a125f5
index fcdfc0e..648c0d5 100644
a125f5
--- a/lib/cluster/corosync.c
a125f5
+++ b/lib/cluster/corosync.c
a125f5
@@ -1,19 +1,10 @@
a125f5
 /*
a125f5
- * Copyright (C) 2004 Andrew Beekhof <andrew@beekhof.net>
a125f5
+ * Copyright 2004-2019 the Pacemaker project contributors
a125f5
  *
a125f5
- * This library is free software; you can redistribute it and/or
a125f5
- * modify it under the terms of the GNU Lesser General Public
a125f5
- * License as published by the Free Software Foundation; either
a125f5
- * version 2.1 of the License, or (at your option) any later version.
a125f5
+ * The version control history for this file may have further details.
a125f5
  *
a125f5
- * This library is distributed in the hope that it will be useful,
a125f5
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
a125f5
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
a125f5
- * Lesser General Public License for more details.
a125f5
- *
a125f5
- * You should have received a copy of the GNU Lesser General Public
a125f5
- * License along with this library; if not, write to the Free Software
a125f5
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
a125f5
+ * This source code is licensed under the GNU Lesser General Public License
a125f5
+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
a125f5
  */
a125f5
 
a125f5
 #include <crm_internal.h>
a125f5
@@ -43,6 +34,8 @@
a125f5
 
a125f5
 #include <crm/msg_xml.h>
a125f5
 
a125f5
+#include <crm/common/ipc_internal.h>  /* PCMK__SPECIAL_PID* */
a125f5
+
a125f5
 quorum_handle_t pcmk_quorum_handle = 0;
a125f5
 
a125f5
 gboolean(*quorum_app_callback) (unsigned long long seq, gboolean quorate) = NULL;
a125f5
@@ -68,10 +61,15 @@ char *
a125f5
 corosync_node_name(uint64_t /*cmap_handle_t */ cmap_handle, uint32_t nodeid)
a125f5
 {
a125f5
     int lpc = 0;
a125f5
-    int rc = CS_OK;
a125f5
+    cs_error_t rc = CS_OK;
a125f5
     int retries = 0;
a125f5
     char *name = NULL;
a125f5
     cmap_handle_t local_handle = 0;
a125f5
+    int fd = -1;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     if (nodeid == 0) {
a125f5
         nodeid = get_local_nodeid(0);
a125f5
@@ -100,6 +98,27 @@ corosync_node_name(uint64_t /*cmap_handle_t */ cmap_handle, uint32_t nodeid)
a125f5
 
a125f5
     if (cmap_handle == 0) {
a125f5
         cmap_handle = local_handle;
a125f5
+
a125f5
+        rc = cmap_fd_get(cmap_handle, &fd;;
a125f5
+        if (rc != CS_OK) {
a125f5
+            crm_err("Could not obtain the CMAP API connection: %s (%d)",
a125f5
+                    cs_strerror(rc), rc);
a125f5
+            goto bail;
a125f5
+        }
a125f5
+
a125f5
+        /* CMAP provider run as root (in given user namespace, anyway)? */
a125f5
+        if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                                &found_uid, &found_gid))) {
a125f5
+            crm_err("CMAP provider is not authentic:"
a125f5
+                    " process %lld (uid: %lld, gid: %lld)",
a125f5
+                    (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                    (long long) found_uid, (long long) found_gid);
a125f5
+            goto bail;
a125f5
+        } else if (rv < 0) {
a125f5
+            crm_err("Could not verify authenticity of CMAP provider: %s (%d)",
a125f5
+                    strerror(-rv), -rv);
a125f5
+            goto bail;
a125f5
+        }
a125f5
     }
a125f5
 
a125f5
     while (name == NULL && cmap_handle != 0) {
a125f5
@@ -140,6 +159,7 @@ corosync_node_name(uint64_t /*cmap_handle_t */ cmap_handle, uint32_t nodeid)
a125f5
         lpc++;
a125f5
     }
a125f5
 
a125f5
+bail:
a125f5
     if(local_handle) {
a125f5
         cmap_finalize(local_handle);
a125f5
     }
a125f5
@@ -251,11 +271,15 @@ gboolean
a125f5
 cluster_connect_quorum(gboolean(*dispatch) (unsigned long long, gboolean),
a125f5
                        void (*destroy) (gpointer))
a125f5
 {
a125f5
-    int rc = -1;
a125f5
+    cs_error_t rc;
a125f5
     int fd = 0;
a125f5
     int quorate = 0;
a125f5
     uint32_t quorum_type = 0;
a125f5
     struct mainloop_fd_callbacks quorum_fd_callbacks;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     quorum_fd_callbacks.dispatch = pcmk_quorum_dispatch;
a125f5
     quorum_fd_callbacks.destroy = destroy;
a125f5
@@ -264,7 +288,8 @@ cluster_connect_quorum(gboolean(*dispatch) (unsigned long long, gboolean),
a125f5
 
a125f5
     rc = quorum_initialize(&pcmk_quorum_handle, &quorum_callbacks, &quorum_type);
a125f5
     if (rc != CS_OK) {
a125f5
-        crm_err("Could not connect to the Quorum API: %d", rc);
a125f5
+        crm_err("Could not connect to the Quorum API: %s (%d)",
a125f5
+                cs_strerror(rc), rc);
a125f5
         goto bail;
a125f5
 
a125f5
     } else if (quorum_type != QUORUM_SET) {
a125f5
@@ -272,6 +297,29 @@ cluster_connect_quorum(gboolean(*dispatch) (unsigned long long, gboolean),
a125f5
         goto bail;
a125f5
     }
a125f5
 
a125f5
+    rc = quorum_fd_get(pcmk_quorum_handle, &fd;;
a125f5
+    if (rc != CS_OK) {
a125f5
+        crm_err("Could not obtain the Quorum API connection: %s (%d)",
a125f5
+                strerror(rc), rc);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
+    /* Quorum provider run as root (in given user namespace, anyway)? */
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                            &found_uid, &found_gid))) {
a125f5
+        crm_err("Quorum provider is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        rc = CS_ERR_ACCESS;
a125f5
+        goto bail;
a125f5
+    } else if (rv < 0) {
a125f5
+        crm_err("Could not verify authenticity of Quorum provider: %s (%d)",
a125f5
+                strerror(-rv), -rv);
a125f5
+        rc = CS_ERR_ACCESS;
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
     rc = quorum_getquorate(pcmk_quorum_handle, &quorate);
a125f5
     if (rc != CS_OK) {
a125f5
         crm_err("Could not obtain the current Quorum API state: %d", rc);
a125f5
@@ -292,12 +340,6 @@ cluster_connect_quorum(gboolean(*dispatch) (unsigned long long, gboolean),
a125f5
         goto bail;
a125f5
     }
a125f5
 
a125f5
-    rc = quorum_fd_get(pcmk_quorum_handle, &fd;;
a125f5
-    if (rc != CS_OK) {
a125f5
-        crm_err("Could not obtain the Quorum API connection: %d", rc);
a125f5
-        goto bail;
a125f5
-    }
a125f5
-
a125f5
     mainloop_add_fd("quorum", G_PRIORITY_HIGH, fd, dispatch, &quorum_fd_callbacks);
a125f5
 
a125f5
     corosync_initialize_nodelist(NULL, FALSE, NULL);
a125f5
@@ -488,10 +530,15 @@ gboolean
a125f5
 corosync_initialize_nodelist(void *cluster, gboolean force_member, xmlNode * xml_parent)
a125f5
 {
a125f5
     int lpc = 0;
a125f5
-    int rc = CS_OK;
a125f5
+    cs_error_t rc = CS_OK;
a125f5
     int retries = 0;
a125f5
     gboolean any = FALSE;
a125f5
     cmap_handle_t cmap_handle;
a125f5
+    int fd = -1;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     do {
a125f5
         rc = cmap_initialize(&cmap_handle);
a125f5
@@ -509,6 +556,27 @@ corosync_initialize_nodelist(void *cluster, gboolean force_member, xmlNode * xml
a125f5
         return FALSE;
a125f5
     }
a125f5
 
a125f5
+    rc = cmap_fd_get(cmap_handle, &fd;;
a125f5
+    if (rc != CS_OK) {
a125f5
+        crm_err("Could not obtain the CMAP API connection: %s (%d)",
a125f5
+                cs_strerror(rc), rc);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
+    /* CMAP provider run as root (in given user namespace, anyway)? */
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                            &found_uid, &found_gid))) {
a125f5
+        crm_err("CMAP provider is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        goto bail;
a125f5
+    } else if (rv < 0) {
a125f5
+        crm_err("Could not verify authenticity of CMAP provider: %s (%d)",
a125f5
+                strerror(-rv), -rv);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
     crm_peer_init();
a125f5
     crm_trace("Initializing corosync nodelist");
a125f5
     for (lpc = 0; TRUE; lpc++) {
a125f5
@@ -562,6 +630,7 @@ corosync_initialize_nodelist(void *cluster, gboolean force_member, xmlNode * xml
a125f5
 
a125f5
         free(name);
a125f5
     }
a125f5
+bail:
a125f5
     cmap_finalize(cmap_handle);
a125f5
     return any;
a125f5
 }
a125f5
@@ -571,36 +640,68 @@ corosync_cluster_name(void)
a125f5
 {
a125f5
     cmap_handle_t handle;
a125f5
     char *cluster_name = NULL;
a125f5
-    int rc = CS_OK;
a125f5
+    cs_error_t rc = CS_OK;
a125f5
+    int fd = -1;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     rc = cmap_initialize(&handle);
a125f5
     if (rc != CS_OK) {
a125f5
-        crm_info("Failed to initialize the cmap API: %s (%d)", ais_error2text(rc), rc);
a125f5
+        crm_info("Failed to initialize the cmap API: %s (%d)",
a125f5
+                 cs_strerror(rc), rc);
a125f5
         return NULL;
a125f5
     }
a125f5
 
a125f5
+    rc = cmap_fd_get(handle, &fd;;
a125f5
+    if (rc != CS_OK) {
a125f5
+        crm_err("Could not obtain the CMAP API connection: %s (%d)",
a125f5
+                cs_strerror(rc), rc);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
+    /* CMAP provider run as root (in given user namespace, anyway)? */
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                            &found_uid, &found_gid))) {
a125f5
+        crm_err("CMAP provider is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        goto bail;
a125f5
+    } else if (rv < 0) {
a125f5
+        crm_err("Could not verify authenticity of CMAP provider: %s (%d)",
a125f5
+                strerror(-rv), -rv);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
     rc = cmap_get_string(handle, "totem.cluster_name", &cluster_name);
a125f5
     if (rc != CS_OK) {
a125f5
-        crm_info("Cannot get totem.cluster_name: %s (%d)", ais_error2text(rc), rc);
a125f5
+        crm_info("Cannot get totem.cluster_name: %s (%d)", cs_strerror(rc), rc);
a125f5
 
a125f5
     } else {
a125f5
         crm_debug("cmap totem.cluster_name = '%s'", cluster_name);
a125f5
     }
a125f5
 
a125f5
+bail:
a125f5
     cmap_finalize(handle);
a125f5
-
a125f5
     return cluster_name;
a125f5
 }
a125f5
 
a125f5
 int
a125f5
 corosync_cmap_has_config(const char *prefix)
a125f5
 {
a125f5
-    int rc = CS_OK;
a125f5
+    cs_error_t rc = CS_OK;
a125f5
     int retries = 0;
a125f5
     static int found = -1;
a125f5
     cmap_handle_t cmap_handle;
a125f5
     cmap_iter_handle_t iter_handle;
a125f5
     char key_name[CMAP_KEYNAME_MAXLEN + 1];
a125f5
+    int fd = -1;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     if(found != -1) {
a125f5
         return found;
a125f5
@@ -623,6 +724,27 @@ corosync_cmap_has_config(const char *prefix)
a125f5
         return -1;
a125f5
     }
a125f5
 
a125f5
+    rc = cmap_fd_get(cmap_handle, &fd;;
a125f5
+    if (rc != CS_OK) {
a125f5
+        crm_err("Could not obtain the CMAP API connection: %s (%d)",
a125f5
+                cs_strerror(rc), rc);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
+    /* CMAP provider run as root (in given user namespace, anyway)? */
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                            &found_uid, &found_gid))) {
a125f5
+        crm_err("CMAP provider is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        goto bail;
a125f5
+    } else if (rv < 0) {
a125f5
+        crm_err("Could not verify authenticity of CMAP provider: %s (%d)",
a125f5
+                strerror(-rv), -rv);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
     rc = cmap_iter_init(cmap_handle, prefix, &iter_handle);
a125f5
     if (rc != CS_OK) {
a125f5
         crm_warn("Failed to initialize iteration for corosync cmap '%s': %s (rc=%d)",
a125f5
diff --git a/lib/cluster/cpg.c b/lib/cluster/cpg.c
a125f5
index 75af131..430195c 100644
a125f5
--- a/lib/cluster/cpg.c
a125f5
+++ b/lib/cluster/cpg.c
a125f5
@@ -1,5 +1,7 @@
a125f5
 /*
a125f5
- * Copyright 2004-2018 Andrew Beekhof <andrew@beekhof.net>
a125f5
+ * Copyright 2004-2019 the Pacemaker project contributors
a125f5
+ *
a125f5
+ * The version control history for this file may have further details.
a125f5
  *
a125f5
  * This source code is licensed under the GNU Lesser General Public License
a125f5
  * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
a125f5
@@ -27,6 +29,8 @@
a125f5
 
a125f5
 #include <crm/msg_xml.h>
a125f5
 
a125f5
+#include <crm/common/ipc_internal.h>  /* PCMK__SPECIAL_PID* */
a125f5
+
a125f5
 cpg_handle_t pcmk_cpg_handle = 0; /* TODO: Remove, use cluster.cpg_handle */
a125f5
 
a125f5
 static bool cpg_evicted = FALSE;
a125f5
@@ -60,11 +64,16 @@ cluster_disconnect_cpg(crm_cluster_t *cluster)
a125f5
 
a125f5
 uint32_t get_local_nodeid(cpg_handle_t handle)
a125f5
 {
a125f5
-    int rc = CS_OK;
a125f5
+    cs_error_t rc = CS_OK;
a125f5
     int retries = 0;
a125f5
     static uint32_t local_nodeid = 0;
a125f5
     cpg_handle_t local_handle = handle;
a125f5
     cpg_callbacks_t cb = { };
a125f5
+    int fd = -1;
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     if(local_nodeid != 0) {
a125f5
         return local_nodeid;
a125f5
@@ -73,6 +82,32 @@ uint32_t get_local_nodeid(cpg_handle_t handle)
a125f5
     if(handle == 0) {
a125f5
         crm_trace("Creating connection");
a125f5
         cs_repeat(retries, 5, rc = cpg_initialize(&local_handle, &cb));
a125f5
+        if (rc != CS_OK) {
a125f5
+            crm_err("Could not connect to the CPG API: %s (%d)",
a125f5
+                    cs_strerror(rc), rc);
a125f5
+            return 0;
a125f5
+        }
a125f5
+
a125f5
+        rc = cpg_fd_get(local_handle, &fd;;
a125f5
+        if (rc != CS_OK) {
a125f5
+            crm_err("Could not obtain the CPG API connection: %s (%d)",
a125f5
+                    cs_strerror(rc), rc);
a125f5
+            goto bail;
a125f5
+        }
a125f5
+
a125f5
+        /* CPG provider run as root (in given user namespace, anyway)? */
a125f5
+        if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                                &found_uid, &found_gid))) {
a125f5
+            crm_err("CPG provider is not authentic:"
a125f5
+                    " process %lld (uid: %lld, gid: %lld)",
a125f5
+                    (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                    (long long) found_uid, (long long) found_gid);
a125f5
+            goto bail;
a125f5
+        } else if (rv < 0) {
a125f5
+            crm_err("Could not verify authenticity of CPG provider: %s (%d)",
a125f5
+                    strerror(-rv), -rv);
a125f5
+            goto bail;
a125f5
+        }
a125f5
     }
a125f5
 
a125f5
     if (rc == CS_OK) {
a125f5
@@ -84,6 +119,8 @@ uint32_t get_local_nodeid(cpg_handle_t handle)
a125f5
     if (rc != CS_OK) {
a125f5
         crm_err("Could not get local node id from the CPG API: %s (%d)", ais_error2text(rc), rc);
a125f5
     }
a125f5
+
a125f5
+bail:
a125f5
     if(handle == 0) {
a125f5
         crm_trace("Closing connection");
a125f5
         cpg_finalize(local_handle);
a125f5
@@ -403,13 +440,17 @@ pcmk_cpg_membership(cpg_handle_t handle,
a125f5
 gboolean
a125f5
 cluster_connect_cpg(crm_cluster_t *cluster)
a125f5
 {
a125f5
-    int rc = -1;
a125f5
-    int fd = 0;
a125f5
+    cs_error_t rc;
a125f5
+    int fd = -1;
a125f5
     int retries = 0;
a125f5
     uint32_t id = 0;
a125f5
     crm_node_t *peer = NULL;
a125f5
     cpg_handle_t handle = 0;
a125f5
     const char *message_name = pcmk_message_name(crm_system_name);
a125f5
+    uid_t found_uid = 0;
a125f5
+    gid_t found_gid = 0;
a125f5
+    pid_t found_pid = 0;
a125f5
+    int rv;
a125f5
 
a125f5
     struct mainloop_fd_callbacks cpg_fd_callbacks = {
a125f5
         .dispatch = pcmk_cpg_dispatch,
a125f5
@@ -434,7 +475,31 @@ cluster_connect_cpg(crm_cluster_t *cluster)
a125f5
 
a125f5
     cs_repeat(retries, 30, rc = cpg_initialize(&handle, &cpg_callbacks));
a125f5
     if (rc != CS_OK) {
a125f5
-        crm_err("Could not connect to the Cluster Process Group API: %d", rc);
a125f5
+        crm_err("Could not connect to the CPG API: %s (%d)",
a125f5
+                cs_strerror(rc), rc);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
+    rc = cpg_fd_get(handle, &fd;;
a125f5
+    if (rc != CS_OK) {
a125f5
+        crm_err("Could not obtain the CPG API connection: %s (%d)",
a125f5
+                cs_strerror(rc), rc);
a125f5
+        goto bail;
a125f5
+    }
a125f5
+
a125f5
+    /* CPG provider run as root (in given user namespace, anyway)? */
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(fd, (uid_t) 0,(gid_t) 0, &found_pid,
a125f5
+                                            &found_uid, &found_gid))) {
a125f5
+        crm_err("CPG provider is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        rc = CS_ERR_ACCESS;
a125f5
+        goto bail;
a125f5
+    } else if (rv < 0) {
a125f5
+        crm_err("Could not verify authenticity of CPG provider: %s (%d)",
a125f5
+                strerror(-rv), -rv);
a125f5
+        rc = CS_ERR_ACCESS;
a125f5
         goto bail;
a125f5
     }
a125f5
 
a125f5
@@ -453,12 +518,6 @@ cluster_connect_cpg(crm_cluster_t *cluster)
a125f5
         goto bail;
a125f5
     }
a125f5
 
a125f5
-    rc = cpg_fd_get(handle, &fd;;
a125f5
-    if (rc != CS_OK) {
a125f5
-        crm_err("Could not obtain the CPG API connection: %d", rc);
a125f5
-        goto bail;
a125f5
-    }
a125f5
-
a125f5
     pcmk_cpg_handle = handle;
a125f5
     cluster->cpg_handle = handle;
a125f5
     mainloop_add_fd("corosync-cpg", G_PRIORITY_MEDIUM, fd, cluster, &cpg_fd_callbacks);
a125f5
diff --git a/lib/common/ipc.c b/lib/common/ipc.c
a125f5
index aa055d3..84d7cb9 100644
a125f5
--- a/lib/common/ipc.c
a125f5
+++ b/lib/common/ipc.c
a125f5
@@ -947,11 +947,18 @@ crm_ipc_new(const char *name, size_t max_size)
a125f5
  *
a125f5
  * \param[in] client  Connection instance obtained from crm_ipc_new()
a125f5
  *
a125f5
- * \return TRUE on success, FALSE otherwise (in which case errno will be set)
a125f5
+ * \return TRUE on success, FALSE otherwise (in which case errno will be set;
a125f5
+ *         specifically, in case of discovering the remote side is not
a125f5
+ *         authentic, its value is set to ECONNABORTED).
a125f5
  */
a125f5
 bool
a125f5
 crm_ipc_connect(crm_ipc_t * client)
a125f5
 {
a125f5
+    static uid_t cl_uid = 0;
a125f5
+    static gid_t cl_gid = 0;
a125f5
+    pid_t found_pid = 0; uid_t found_uid = 0; gid_t found_gid = 0;
a125f5
+    int rv;
a125f5
+
a125f5
     client->need_reply = FALSE;
a125f5
     client->ipc = qb_ipcc_connect(client->name, client->buf_size);
a125f5
 
a125f5
@@ -962,7 +969,39 @@ crm_ipc_connect(crm_ipc_t * client)
a125f5
 
a125f5
     client->pfd.fd = crm_ipc_get_fd(client);
a125f5
     if (client->pfd.fd < 0) {
a125f5
-        crm_debug("Could not obtain file descriptor for %s connection: %s (%d)", client->name, pcmk_strerror(errno), errno);
a125f5
+        rv = errno;
a125f5
+        /* message already omitted */
a125f5
+        crm_ipc_close(client);
a125f5
+        errno = rv;
a125f5
+        return FALSE;
a125f5
+    }
a125f5
+
a125f5
+    if (!cl_uid && !cl_gid
a125f5
+            && (rv = crm_user_lookup(CRM_DAEMON_USER, &cl_uid, &cl_gid)) < 0) {
a125f5
+        errno = -rv;
a125f5
+        /* message already omitted */
a125f5
+        crm_ipc_close(client);
a125f5
+        errno = -rv;
a125f5
+        return FALSE;
a125f5
+    }
a125f5
+
a125f5
+    if (!(rv = crm_ipc_is_authentic_process(client->pfd.fd, cl_uid, cl_gid,
a125f5
+                                            &found_pid, &found_uid,
a125f5
+                                            &found_gid))) {
a125f5
+        crm_err("Daemon (IPC %s) is not authentic:"
a125f5
+                " process %lld (uid: %lld, gid: %lld)",
a125f5
+                client->name,  (long long) PCMK__SPECIAL_PID_AS_0(found_pid),
a125f5
+                (long long) found_uid, (long long) found_gid);
a125f5
+        crm_ipc_close(client);
a125f5
+        errno = ECONNABORTED;
a125f5
+        return FALSE;
a125f5
+
a125f5
+    } else if (rv < 0) {
a125f5
+        errno = -rv;
a125f5
+        crm_perror(LOG_ERR, "Could not verify authenticity of daemon (IPC %s)",
a125f5
+                   client->name);
a125f5
+        crm_ipc_close(client);
a125f5
+        errno = -rv;
a125f5
         return FALSE;
a125f5
     }
a125f5
 
a125f5
-- 
a125f5
1.8.3.1
a125f5
a125f5
a125f5
From d324e407c0e2695f405974d567d79eb91d0ee69a Mon Sep 17 00:00:00 2001
a125f5
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
a125f5
Date: Tue, 16 Apr 2019 00:13:40 +0200
a125f5
Subject: [PATCH 6/7] High: pacemakerd vs. IPC/procfs confused deputy
a125f5
 authenticity issue (4/4)
a125f5
a125f5
[4/4: CPG users to be careful about now-more-probable rival processes]
a125f5
a125f5
In essence, this comes down to pacemaker confusing at-node CPG members
a125f5
with effectively the only plausible to co-exist at particular node,
a125f5
which doesn't hold and asks for a wider reconciliation of this
a125f5
reality-check.
a125f5
a125f5
However, in practical terms, since there are two factors lowering the
a125f5
priority of doing so:
a125f5
a125f5
1/ possibly the only non-self-inflicted scenario is either that
a125f5
   some of the cluster stack processes fail -- this the problem
a125f5
   that shall rather be deferred to arranged node disarming/fencing
a125f5
   to stay on the safe side with 100% certainty, at the cost of
a125f5
   possibly long-lasting failover process at other nodes
a125f5
   (for other possibility, someone running some of these by accident
a125f5
   so they effectively become rival processes, it's like getting
a125f5
   hands cut when playing with a lawnmower in an unintended way)
a125f5
a125f5
2/ for state tracking of the peer nodes, it may possibly cause troubles
a125f5
   in case the process observed as left wasn't the last for the
a125f5
   particular node, even if presumably just temporary, since the
a125f5
   situation may eventually resolve with imposed serialization of
a125f5
   the rival processes via API end-point singleton restriction (this
a125f5
   is also the most likely cause of why such non-final leave gets
a125f5
   observed in the first place), except in one case -- the legitimate
a125f5
   API end-point carrier won't possibly acknowledged as returned
a125f5
   by its peers, at least not immediately, unless it tries to join
a125f5
   anew, which verges on undefined behaviour (at least per corosync
a125f5
   documentation)
a125f5
a125f5
we make do just with a light code change so as to
a125f5
a125f5
* limit 1/ some more with in-daemon self-check for pre-existing
a125f5
  end-point existence (this is to complement the checks already made in
a125f5
  the parent daemon prior to spawning new instances, only some moments
a125f5
  later; note that we don't have any lock file etc. mechanisms to
a125f5
  prevent parallel runs of the same daemons, and people could run these
a125f5
  on their own deliberation), and to
a125f5
a125f5
* guard against the interferences from the rivals at the same node
a125f5
  per 2/ with ignoring their non-final leave messages altogether.
a125f5
a125f5
Note that CPG at this point is already expected to be authenticity-safe.
a125f5
a125f5
Regarding now-more-probable part, we actually traded the inherently racy
a125f5
procfs scanning for something (exactly that singleton mentioned above)
a125f5
rather firm (and unfakeable), but we admittedly got lost track of
a125f5
processes that are after CPG membership (that is, another form of
a125f5
a shared state) prior to (or in non-deterministic order allowing for
a125f5
the same) carring about publishing the end-point.
a125f5
a125f5
Big thanks is owed to Yan Gao of SUSE, for early discovery and reporting
a125f5
this discrepancy arising from the earlier commits in the set.
a125f5
---
a125f5
 daemons/attrd/pacemaker-attrd.c       | 15 ++++++
a125f5
 daemons/based/pacemaker-based.c       | 18 ++++++-
a125f5
 daemons/controld/pacemaker-controld.c | 18 ++++++-
a125f5
 daemons/fenced/pacemaker-fenced.c     | 15 ++++++
a125f5
 lib/cluster/cpg.c                     | 91 ++++++++++++++++++++++++++++++++---
a125f5
 5 files changed, 146 insertions(+), 11 deletions(-)
a125f5
a125f5
diff --git a/daemons/attrd/pacemaker-attrd.c b/daemons/attrd/pacemaker-attrd.c
a125f5
index 837459f..9ffe201 100644
a125f5
--- a/daemons/attrd/pacemaker-attrd.c
a125f5
+++ b/daemons/attrd/pacemaker-attrd.c
a125f5
@@ -344,6 +344,7 @@ main(int argc, char **argv)
a125f5
     int flag = 0;
a125f5
     int index = 0;
a125f5
     int argerr = 0;
a125f5
+    crm_ipc_t *old_instance = NULL;
a125f5
 
a125f5
     attrd_init_mainloop();
a125f5
     crm_log_preinit(NULL, argc, argv);
a125f5
@@ -380,6 +381,20 @@ main(int argc, char **argv)
a125f5
 
a125f5
     crm_log_init(T_ATTRD, LOG_INFO, TRUE, FALSE, argc, argv, FALSE);
a125f5
     crm_info("Starting up");
a125f5
+
a125f5
+    old_instance = crm_ipc_new(T_ATTRD, 0);
a125f5
+    if (crm_ipc_connect(old_instance)) {
a125f5
+        /* IPC end-point already up */
a125f5
+        crm_ipc_close(old_instance);
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        crm_err("pacemaker-attrdd is already active, aborting startup");
a125f5
+        crm_exit(CRM_EX_OK);
a125f5
+    } else {
a125f5
+        /* not up or not authentic, we'll proceed either way */
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        old_instance = NULL;
a125f5
+    }
a125f5
+
a125f5
     attributes = g_hash_table_new_full(crm_str_hash, g_str_equal, NULL, free_attribute);
a125f5
 
a125f5
     /* Connect to the CIB before connecting to the cluster or listening for IPC.
a125f5
diff --git a/daemons/based/pacemaker-based.c b/daemons/based/pacemaker-based.c
a125f5
index bd98ca8..f004f671 100644
a125f5
--- a/daemons/based/pacemaker-based.c
a125f5
+++ b/daemons/based/pacemaker-based.c
a125f5
@@ -90,13 +90,12 @@ main(int argc, char **argv)
a125f5
     int index = 0;
a125f5
     int argerr = 0;
a125f5
     struct passwd *pwentry = NULL;
a125f5
+    crm_ipc_t *old_instance = NULL;
a125f5
 
a125f5
     crm_log_preinit(NULL, argc, argv);
a125f5
     crm_set_options(NULL, "[options]",
a125f5
                     long_options, "Daemon for storing and replicating the cluster configuration");
a125f5
 
a125f5
-    crm_peer_init();
a125f5
-
a125f5
     mainloop_add_signal(SIGTERM, cib_shutdown);
a125f5
     mainloop_add_signal(SIGPIPE, cib_enable_writes);
a125f5
 
a125f5
@@ -171,6 +170,19 @@ main(int argc, char **argv)
a125f5
 
a125f5
     crm_log_init(NULL, LOG_INFO, TRUE, FALSE, argc, argv, FALSE);
a125f5
 
a125f5
+    old_instance = crm_ipc_new(CIB_CHANNEL_RO, 0);
a125f5
+    if (crm_ipc_connect(old_instance)) {
a125f5
+        /* IPC end-point already up */
a125f5
+        crm_ipc_close(old_instance);
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        crm_err("pacemaker-based is already active, aborting startup");
a125f5
+        crm_exit(CRM_EX_OK);
a125f5
+    } else {
a125f5
+        /* not up or not authentic, we'll proceed either way */
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        old_instance = NULL;
a125f5
+    }
a125f5
+
a125f5
     if (cib_root == NULL) {
a125f5
         cib_root = CRM_CONFIG_DIR;
a125f5
     } else {
a125f5
@@ -185,6 +197,8 @@ main(int argc, char **argv)
a125f5
         return CRM_EX_FATAL;
a125f5
     }
a125f5
 
a125f5
+    crm_peer_init();
a125f5
+
a125f5
     /* read local config file */
a125f5
     cib_init();
a125f5
 
a125f5
diff --git a/daemons/controld/pacemaker-controld.c b/daemons/controld/pacemaker-controld.c
a125f5
index 11f909e..38842fb 100644
a125f5
--- a/daemons/controld/pacemaker-controld.c
a125f5
+++ b/daemons/controld/pacemaker-controld.c
a125f5
@@ -1,5 +1,7 @@
a125f5
 /*
a125f5
- * Copyright 2004-2018 Andrew Beekhof <andrew@beekhof.net>
a125f5
+ * Copyright 2004-2019 the Pacemaker project contributors
a125f5
+ *
a125f5
+ * The version control history for this file may have further details.
a125f5
  *
a125f5
  * This source code is licensed under the GNU General Public License version 2
a125f5
  * or later (GPLv2+) WITHOUT ANY WARRANTY.
a125f5
@@ -50,6 +52,7 @@ main(int argc, char **argv)
a125f5
     int flag;
a125f5
     int index = 0;
a125f5
     int argerr = 0;
a125f5
+    crm_ipc_t *old_instance = NULL;
a125f5
 
a125f5
     crmd_mainloop = g_main_loop_new(NULL, FALSE);
a125f5
     crm_log_preinit(NULL, argc, argv);
a125f5
@@ -93,6 +96,19 @@ main(int argc, char **argv)
a125f5
         crm_help('?', CRM_EX_USAGE);
a125f5
     }
a125f5
 
a125f5
+    old_instance = crm_ipc_new(CRM_SYSTEM_CRMD, 0);
a125f5
+    if (crm_ipc_connect(old_instance)) {
a125f5
+        /* IPC end-point already up */
a125f5
+        crm_ipc_close(old_instance);
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        crm_err("pacemaker-controld is already active, aborting startup");
a125f5
+        crm_exit(CRM_EX_OK);
a125f5
+    } else {
a125f5
+        /* not up or not authentic, we'll proceed either way */
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        old_instance = NULL;
a125f5
+    }
a125f5
+
a125f5
     if (pcmk__daemon_can_write(PE_STATE_DIR, NULL) == FALSE) {
a125f5
         crm_err("Terminating due to bad permissions on " PE_STATE_DIR);
a125f5
         fprintf(stderr,
a125f5
diff --git a/daemons/fenced/pacemaker-fenced.c b/daemons/fenced/pacemaker-fenced.c
a125f5
index 105012b..da0dce9 100644
a125f5
--- a/daemons/fenced/pacemaker-fenced.c
a125f5
+++ b/daemons/fenced/pacemaker-fenced.c
a125f5
@@ -1262,6 +1262,7 @@ main(int argc, char **argv)
a125f5
     int option_index = 0;
a125f5
     crm_cluster_t cluster;
a125f5
     const char *actions[] = { "reboot", "off", "on", "list", "monitor", "status" };
a125f5
+    crm_ipc_t *old_instance = NULL;
a125f5
 
a125f5
     crm_log_preinit(NULL, argc, argv);
a125f5
     crm_set_options(NULL, "mode [options]", long_options,
a125f5
@@ -1432,6 +1433,20 @@ main(int argc, char **argv)
a125f5
     }
a125f5
 
a125f5
     crm_log_init(NULL, LOG_INFO, TRUE, FALSE, argc, argv, FALSE);
a125f5
+
a125f5
+    old_instance = crm_ipc_new("stonith-ng", 0);
a125f5
+    if (crm_ipc_connect(old_instance)) {
a125f5
+        /* IPC end-point already up */
a125f5
+        crm_ipc_close(old_instance);
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        crm_err("pacemaker-fenced is already active, aborting startup");
a125f5
+        crm_exit(CRM_EX_OK);
a125f5
+    } else {
a125f5
+        /* not up or not authentic, we'll proceed either way */
a125f5
+        crm_ipc_destroy(old_instance);
a125f5
+        old_instance = NULL;
a125f5
+    }
a125f5
+
a125f5
     mainloop_add_signal(SIGTERM, stonith_shutdown);
a125f5
 
a125f5
     crm_peer_init();
a125f5
diff --git a/lib/cluster/cpg.c b/lib/cluster/cpg.c
a125f5
index 430195c..2898c51 100644
a125f5
--- a/lib/cluster/cpg.c
a125f5
+++ b/lib/cluster/cpg.c
a125f5
@@ -362,6 +362,20 @@ pcmk_message_common_cs(cpg_handle_t handle, uint32_t nodeid, uint32_t pid, void
a125f5
 
a125f5
 #define PEER_NAME(peer) ((peer)? ((peer)->uname? (peer)->uname : "<unknown>") : "<none>")
a125f5
 
a125f5
+static int cmp_member_list_nodeid(const void *first,
a125f5
+                                  const void *second)
a125f5
+{
a125f5
+    const struct cpg_address *const a = *((const struct cpg_address **) first),
a125f5
+                             *const b = *((const struct cpg_address **) second);
a125f5
+    if (a->nodeid < b->nodeid) {
a125f5
+        return -1;
a125f5
+    } else if (a->nodeid > b->nodeid) {
a125f5
+        return 1;
a125f5
+    }
a125f5
+    /* don't bother with "reason" nor "pid" */
a125f5
+    return 0;
a125f5
+}
a125f5
+
a125f5
 void
a125f5
 pcmk_cpg_membership(cpg_handle_t handle,
a125f5
                     const struct cpg_name *groupName,
a125f5
@@ -373,29 +387,89 @@ pcmk_cpg_membership(cpg_handle_t handle,
a125f5
     gboolean found = FALSE;
a125f5
     static int counter = 0;
a125f5
     uint32_t local_nodeid = get_local_nodeid(handle);
a125f5
+    const struct cpg_address *key, **rival, **sorted;
a125f5
+
a125f5
+    sorted = malloc(member_list_entries * sizeof(const struct cpg_address *));
a125f5
+    CRM_ASSERT(sorted != NULL);
a125f5
+
a125f5
+    for (size_t iter = 0; iter < member_list_entries; iter++) {
a125f5
+        sorted[iter] = member_list + iter;
a125f5
+    }
a125f5
+    /* so that the cross-matching multiply-subscribed nodes is then cheap */
a125f5
+    qsort(sorted, member_list_entries, sizeof(const struct cpg_address *),
a125f5
+          cmp_member_list_nodeid);
a125f5
 
a125f5
     for (i = 0; i < left_list_entries; i++) {
a125f5
         crm_node_t *peer = crm_find_peer(left_list[i].nodeid, NULL);
a125f5
 
a125f5
-        crm_info("Group event %s.%d: node %u (%s) left",
a125f5
+        crm_info("Group event %s.%d: node %u (%s) left: %llu",
a125f5
                  groupName->value, counter, left_list[i].nodeid,
a125f5
-                 PEER_NAME(peer));
a125f5
+                 PEER_NAME(peer), (unsigned long long) left_list[i].pid);
a125f5
+
a125f5
+        /* in CPG world, NODE:PROCESS-IN-MEMBERSHIP-OF-G is an 1:N relation
a125f5
+           and not playing by this rule may go wild in case of multiple
a125f5
+           residual instances of the same pacemaker daemon at the same node
a125f5
+           -- we must ensure that the possible local rival(s) won't make us
a125f5
+           cry out and bail (e.g. when they quit themselves), since all the
a125f5
+           surrounding logic denies this simple fact that the full membership
a125f5
+           is discriminated also per the PID of the process beside mere node
a125f5
+           ID (and implicitly, group ID); practically, this will be sound in
a125f5
+           terms of not preventing progress, since all the CPG joiners are
a125f5
+           also API end-point carriers, and that's what matters locally
a125f5
+           (who's the winner);
a125f5
+           remotely, we will just compare leave_list and member_list and if
a125f5
+           the left process has it's node retained in member_list (under some
a125f5
+           other PID, anyway) we will just ignore it as well
a125f5
+           XXX: long-term fix is to establish in-out PID-aware tracking? */
a125f5
         if (peer) {
a125f5
-            crm_update_peer_proc(__FUNCTION__, peer, crm_proc_cpg, OFFLINESTATUS);
a125f5
+            key = &left_list[i];
a125f5
+            rival = bsearch(&key, sorted, member_list_entries,
a125f5
+                            sizeof(const struct cpg_address *),
a125f5
+                            cmp_member_list_nodeid);
a125f5
+            if (rival == NULL) {
a125f5
+                crm_update_peer_proc(__FUNCTION__, peer, crm_proc_cpg,
a125f5
+                                     OFFLINESTATUS);
a125f5
+            } else if (left_list[i].nodeid == local_nodeid) {
a125f5
+                crm_info("Ignoring the above event %s.%d, comes from a local"
a125f5
+                         " rival process (presumably not us): %llu",
a125f5
+                         groupName->value, counter,
a125f5
+                         (unsigned long long) left_list[i].pid);
a125f5
+            } else {
a125f5
+                crm_info("Ignoring the above event %s.%d, comes from"
a125f5
+                         " a rival-rich node: %llu (e.g. %llu process"
a125f5
+                         " carries on)",
a125f5
+                         groupName->value, counter,
a125f5
+                         (unsigned long long) left_list[i].pid,
a125f5
+                         (unsigned long long) (*rival)->pid);
a125f5
+            }
a125f5
         }
a125f5
     }
a125f5
+    free(sorted);
a125f5
+    sorted = NULL;
a125f5
 
a125f5
     for (i = 0; i < joined_list_entries; i++) {
a125f5
-        crm_info("Group event %s.%d: node %u joined",
a125f5
-                 groupName->value, counter, joined_list[i].nodeid);
a125f5
+        crm_info("Group event %s.%d: node %u joined: %llu"
a125f5
+                 " (unchecked for rivals)",
a125f5
+                 groupName->value, counter, joined_list[i].nodeid,
a125f5
+                 (unsigned long long) joined_list[i].pid);
a125f5
     }
a125f5
 
a125f5
     for (i = 0; i < member_list_entries; i++) {
a125f5
         crm_node_t *peer = crm_get_peer(member_list[i].nodeid, NULL);
a125f5
 
a125f5
-        crm_info("Group event %s.%d: node %u (%s) is member",
a125f5
+        crm_info("Group event %s.%d: node %u (%s) is member: %llu"
a125f5
+                 " (at least once)",
a125f5
                  groupName->value, counter, member_list[i].nodeid,
a125f5
-                 PEER_NAME(peer));
a125f5
+                 PEER_NAME(peer), member_list[i].pid);
a125f5
+
a125f5
+        if (member_list[i].nodeid == local_nodeid
a125f5
+                && member_list[i].pid != getpid()) {
a125f5
+            /* see the note above */
a125f5
+            crm_info("Ignoring the above event %s.%d, comes from a local rival"
a125f5
+                     " process: %llu", groupName->value, counter,
a125f5
+                     (unsigned long long) member_list[i].pid);
a125f5
+            continue;
a125f5
+        }
a125f5
 
a125f5
         /* If the caller left auto-reaping enabled, this will also update the
a125f5
          * state to member.
a125f5
@@ -418,7 +492,8 @@ pcmk_cpg_membership(cpg_handle_t handle,
a125f5
 
a125f5
             } else if (now > (peer->when_lost + 60)) {
a125f5
                 // If it persists for more than a minute, update the state
a125f5
-                crm_warn("Node %u member of group %s but believed offline",
a125f5
+                crm_warn("Node %u member of group %s but believed offline"
a125f5
+                         " (unchecked for rivals)",
a125f5
                          member_list[i].nodeid, groupName->value);
a125f5
                 crm_update_peer_state(__FUNCTION__, peer, CRM_NODE_MEMBER, 0);
a125f5
             }
a125f5
-- 
a125f5
1.8.3.1
a125f5
a125f5
a125f5
From 3ad7b2509d78f95b5dfc8fffc4d9a91be1da5113 Mon Sep 17 00:00:00 2001
a125f5
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
a125f5
Date: Wed, 17 Apr 2019 15:04:21 +0200
a125f5
Subject: [PATCH 7/7] Med: controld: fix possible NULL pointer dereference
a125f5
a125f5
This is now more likely triggerable once the problems related to
a125f5
CVE-2018-16878 are avoided.
a125f5
---
a125f5
 daemons/controld/controld_control.c | 15 +++++++++------
a125f5
 1 file changed, 9 insertions(+), 6 deletions(-)
a125f5
a125f5
diff --git a/daemons/controld/controld_control.c b/daemons/controld/controld_control.c
a125f5
index ee95698..0ac358c 100644
a125f5
--- a/daemons/controld/controld_control.c
a125f5
+++ b/daemons/controld/controld_control.c
a125f5
@@ -77,12 +77,15 @@ do_ha_control(long long action,
a125f5
             registered = crm_connect_corosync(cluster);
a125f5
 #endif
a125f5
         }
a125f5
-        controld_election_init(cluster->uname);
a125f5
-        fsa_our_uname = cluster->uname;
a125f5
-        fsa_our_uuid = cluster->uuid;
a125f5
-        if(cluster->uuid == NULL) {
a125f5
-            crm_err("Could not obtain local uuid");
a125f5
-            registered = FALSE;
a125f5
+
a125f5
+        if (registered == TRUE) {
a125f5
+            controld_election_init(cluster->uname);
a125f5
+            fsa_our_uname = cluster->uname;
a125f5
+            fsa_our_uuid = cluster->uuid;
a125f5
+            if(cluster->uuid == NULL) {
a125f5
+                crm_err("Could not obtain local uuid");
a125f5
+                registered = FALSE;
a125f5
+            }
a125f5
         }
a125f5
 
a125f5
         if (registered == FALSE) {
a125f5
-- 
a125f5
1.8.3.1
a125f5