Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
Please login to access the resource
SUSE:SLE-15-SP1:GA
pacemaker.16898
bsc#1131353-bsc#1131356-0003-High-pacemakerd-vs...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File bsc#1131353-bsc#1131356-0003-High-pacemakerd-vs.-IPC-procfs-confused-deputy-authe.patch of Package pacemaker.16898
From 1148f45da977113dff588cdd1cfebb7a47760b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com> Date: Mon, 15 Apr 2019 17:10:07 +0200 Subject: [PATCH 3/7] High: pacemakerd vs. IPC/procfs confused deputy authenticity issue (1/4) [1/4: new helpers to allow IPC client side to authenticate the server] The title problem here could possibly lead to local privilege escalation up to the root's level (and implicitly unguarded by some additional protection layers like SELinux unless the defaults constrained further). Main problem is that the authenticity assumptions were built on two, seemingly mutually supporting legs leading to two CVEs assigned: * procfs (mere process existence and right path to binary check) used to verify (this part was assigned CVE-2018-16878), and * one-way only client-server authentication, putting the client here at the mercy of the server not necessarily cross-verified per the above point if at all (this part was assigned CVE-2018-16877) whereas these two were in fact orthogonal, tearing security assumptions about the "passive influencers" in the pacemaker's daemon resilience-friendly constellation (orchestrated by the main of them, pacemakerd) apart. Moreover, procfs-based approach is discouraged for other reasons. The layout of the basic fix is as follows: * 1/4: new helpers to allow IPC client side to authenticate the server (this commit, along with unifying subsequent solution for both CVEs) * 2/4: pacemakerd to trust pre-existing processes via new checks instead (along with unifying solution for both CVEs) * 3/4: other daemons to authenticate IPC servers of fellow processes (along with addressing CVE-2018-16877 alone, for parts of pacemaker not covered earlier) * 4/4: CPG users to be careful about now-more-probable rival processes (this is merely to mitigate corner case fallout from the new approaches taken to face CVE-2018-16878 in particular; courtesy of Yan Gao of SUSE for the reporting this) With "basic", it is meant that it constitutes a self-contained best effort solution with some compromises that can only be overcome with the assistance of IPC library, libqb, as is also elaborated in messages of remaining "fix" commits. Beside that, also conventional encapsulation of server-by-client authentication would be useful, but lack thereof is not an obstacle (more so should there by any security related neglectations on the IPC client side and its connection initiating arrangement within libqb that has a potential to strike as early as when the authenticity of the server side is yet to be examined). One extra kludge that's introduced for FreeBSD lacking Unix socket to remote peer PID mapping is masquerading such an unspecified PID with value of 1, since that shall always be around as "init" task and, deferring to proof by contradiction, cannot be pacemakerd-spawned child either even if PID 1 was pacemakerd (and running such a child alone is rather nonsensical). The code making decisions based on that value must acknowledge this craze and refrain from killing/signalling the underlying process on this platform (but shall in general follow the same elsewhere, keep in mind systemd socket-based activation for instance, which would end up in such a situation easily!). --- configure.ac | 43 +++++++++++ include/crm/common/Makefile.am | 3 +- include/crm/common/ipc.h | 40 ++++++++++- include/crm/common/ipc_internal.h | 69 ++++++++++++++++++ lib/common/ipc.c | 145 +++++++++++++++++++++++++++++++++++++- 5 files changed, 296 insertions(+), 4 deletions(-) create mode 100644 include/crm/common/ipc_internal.h Index: pacemaker-1.1.18+20180430.b12c320f5/configure.ac =================================================================== --- pacemaker-1.1.18+20180430.b12c320f5.orig/configure.ac +++ pacemaker-1.1.18+20180430.b12c320f5/configure.ac @@ -392,6 +392,48 @@ do fi done +us_auth= +AC_CHECK_HEADER([sys/socket.h], [ + AC_CHECK_DECL([SO_PEERCRED], [ + # Linux + AC_CHECK_TYPE([struct ucred], [ + us_auth=peercred_ucred; + AC_DEFINE([US_AUTH_PEERCRED_UCRED], [1], + [Define if Unix socket auth method is + getsockopt(s, SO_PEERCRED, &ucred, ...)]) + ], [ + # OpenBSD + AC_CHECK_TYPE([struct sockpeercred], [ + us_auth=localpeercred_sockepeercred; + AC_DEFINE([US_AUTH_PEERCRED_SOCKPEERCRED], [1], + [Define if Unix socket auth method is + getsockopt(s, SO_PEERCRED, &sockpeercred, ...)]) + ], [], [[#include <sys/socket.h>]]) + ], [[#define _GNU_SOURCE + #include <sys/socket.h>]]) + ], [], [[#include <sys/socket.h>]]) +]) + +if test -z "${us_auth}"; then + # FreeBSD + AC_CHECK_DECL([getpeereid], [ + us_auth=getpeereid; + AC_DEFINE([US_AUTH_GETPEEREID], [1], + [Define if Unix socket auth method is + getpeereid(s, &uid, &gid)]) + ], [ + # Solaris/OpenIndiana + AC_CHECK_DECL([getpeerucred], [ + us_auth=getpeerucred; + AC_DEFINE([US_AUTH_GETPEERUCRED], [1], + [Define if Unix socket auth method is + getpeercred(s, &ucred)]) + ], [ + AC_MSG_ERROR([No way to authenticate a Unix socket peer]) + ], [[#include <ucred.h>]]) + ]) +fi + dnl This OS-based decision-making is poor autotools practice; dnl feature-based mechanisms are strongly preferred. dnl @@ -1817,3 +1859,4 @@ AC_MSG_RESULT([ LDFLAGS_HARDENED_EXE AC_MSG_RESULT([ LDFLAGS_HARDENED_LIB = ${LDFLAGS_HARDENED_LIB}]) AC_MSG_RESULT([ Libraries = ${LIBS}]) AC_MSG_RESULT([ Stack Libraries = ${CLUSTERLIBS}]) +AC_MSG_RESULT([ Unix socket auth method = ${us_auth}]) Index: pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/Makefile.am =================================================================== --- pacemaker-1.1.18+20180430.b12c320f5.orig/include/crm/common/Makefile.am +++ pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/Makefile.am @@ -22,4 +22,5 @@ headerdir=$(pkgincludedir)/crm/common header_HEADERS = xml.h ipc.h util.h iso8601.h mainloop.h logging.h results.h noinst_HEADERS = cib_secrets.h ipcs.h internal.h alerts_internal.h \ - iso8601_internal.h xml_internal.h + iso8601_internal.h xml_internal.h \ + ipc_internal.h Index: pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/ipc.h =================================================================== --- pacemaker-1.1.18+20180430.b12c320f5.orig/include/crm/common/ipc.h +++ pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/ipc.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013 Andrew Beekhof <andrew@beekhof.net> + * Copyright 2004-2019 the Pacemaker project contributors * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -81,6 +81,44 @@ uint32_t crm_ipc_buffer_flags(crm_ipc_t const char *crm_ipc_name(crm_ipc_t * client); unsigned int crm_ipc_default_buffer_size(void); +/*! + * \brief Check the authenticity of the IPC socket peer process + * + * If everything goes well, peer's authenticity is verified by the means + * of comparing against provided referential UID and GID (either satisfies), + * and the result of this check can be deduced from the return value. + * As an exception, detected UID of 0 ("root") satisfies arbitrary + * provided referential daemon's credentials. + * + * \param[in] sock IPC related, connected Unix socket to check peer of + * \param[in] refuid referential UID to check against + * \param[in] refgid referential GID to check against + * \param[out] gotpid to optionally store obtained PID of the peer + * (not available on FreeBSD, special value of 1 + * used instead, and the caller is required to + * special case this value respectively) + * \param[out] gotuid to optionally store obtained UID of the peer + * \param[out] gotgid to optionally store obtained GID of the peer + * + * \return 0 if IPC related socket's peer is not authentic given the + * referential credentials (see above), 1 if it is, + * negative value on error (generally expressing -errno unless + * it was zero even on nonhappy path, -pcmk_err_generic is + * returned then; no message is directly emitted) + * + * \note While this function is tolerant on what constitutes authorized + * IPC daemon process (its effective user matches UID=0 or \p refuid, + * or at least its group matches \p refroup), either or both (in case + * of UID=0) mismatches on the expected credentials of such peer + * process \e shall be investigated at the caller when value of 1 + * gets returned there, since higher-than-expected privileges in + * respect to the expected/intended credentials possibly violate + * the least privilege principle and may pose an additional risk + * (i.e. such accidental inconsistency shall be eventually fixed). + */ +int crm_ipc_is_authentic_process(int sock, uid_t refuid, gid_t refgid, + pid_t *gotpid, uid_t *gotuid, gid_t *gotgid); + /* Utils */ xmlNode *create_hello_message(const char *uuid, const char *client_name, const char *major_version, const char *minor_version); Index: pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/ipc_internal.h =================================================================== --- /dev/null +++ pacemaker-1.1.18+20180430.b12c320f5/include/crm/common/ipc_internal.h @@ -0,0 +1,69 @@ +/* + * Copyright 2019 the Pacemaker project contributors + * + * The version control history for this file may have further details. + * + * This source code is licensed under the GNU Lesser General Public License + * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY. + */ + +#ifndef PCMK__IPC_INTERNAL_H +#define PCMK__IPC_INTERNAL_H + +#include <sys/types.h> + +#include <crm_config.h> /* US_AUTH_GETPEEREID */ + + +/* denotes "non yieldable PID" on FreeBSD, or actual PID1 in scenarios that + require a delicate handling anyway (socket-based activation with systemd); + we can be reasonably sure that this PID is never possessed by the actual + child daemon, as it gets taken either by the proper init, or by pacemakerd + itself (i.e. this precludes anything else); note that value of zero + is meant to carry "unset" meaning, and better not to bet on/conditionalize + over signedness of pid_t */ +#define PCMK__SPECIAL_PID 1 + +#if defined(US_AUTH_GETPEEREID) +/* on FreeBSD, we don't want to expose "non-yieldable PID" (leading to + "IPC liveness check only") as its nominal representation, which could + cause confusion -- this is unambiguous as long as there's no + socket-based activation like with systemd (very improbable) */ +#define PCMK__SPECIAL_PID_AS_0(p) (((p) == PCMK__SPECIAL_PID) ? 0 : (p)) +#else +#define PCMK__SPECIAL_PID_AS_0(p) (p) +#endif + +/*! + * \internal + * \brief Check the authenticity and liveness of the process via IPC end-point + * + * When IPC daemon under given IPC end-point (name) detected, its authenticity + * is verified by the means of comparing against provided referential UID and + * GID, and the result of this check can be deduced from the return value. + * As an exception, referential UID of 0 (~ root) satisfies arbitrary + * detected daemon's credentials. + * + * \param[in] name IPC name to base the search on + * \param[in] refuid referential UID to check against + * \param[in] refgid referential GID to check against + * \param[out] gotpid to optionally store obtained PID of the found process + * upon returning 1 or -2 + * (not available on FreeBSD, special value of 1, + * see PCMK__SPECIAL_PID, used instead, and the caller + * is required to special case this value respectively) + * + * \return 0 if no trace of IPC peer's liveness detected, 1 if it was, + * -1 on error, and -2 when the IPC blocked with unauthorized + * process (log message emitted in both latter cases) + * + * \note This function emits a log message also in case there isn't a perfect + * match in respect to \p reguid and/or \p refgid, for a possible + * least privilege principle violation. + * + * \see crm_ipc_is_authentic_process + */ +int pcmk__ipc_is_authentic_process_active(const char *name, uid_t refuid, + gid_t refgid, pid_t *gotpid); + +#endif Index: pacemaker-1.1.18+20180430.b12c320f5/lib/common/ipc.c =================================================================== --- pacemaker-1.1.18+20180430.b12c320f5.orig/lib/common/ipc.c +++ pacemaker-1.1.18+20180430.b12c320f5/lib/common/ipc.c @@ -1,5 +1,7 @@ /* - * Copyright 2004-2018 Andrew Beekhof <andrew@beekhof.net> + * Copyright 2004-2019 the Pacemaker project contributors + * + * The version control history for this file may have further details. * * This source code is licensed under the GNU Lesser General Public License * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY. @@ -7,6 +9,17 @@ #include <crm_internal.h> +#if defined(US_AUTH_PEERCRED_UCRED) || defined(US_AUTH_PEERCRED_SOCKPEERCRED) +# ifdef US_AUTH_PEERCRED_UCRED +# ifndef _GNU_SOURCE +# define _GNU_SOURCE +# endif +# endif +# include <sys/socket.h> +#elif defined(US_AUTH_GETPEERUCRED) +# include <ucred.h> +#endif + #include <sys/param.h> #include <stdio.h> @@ -19,11 +32,13 @@ #include <fcntl.h> #include <bzlib.h> -#include <crm/crm.h> +#include <crm/crm.h> /* indirectly: pcmk_err_generic */ #include <crm/msg_xml.h> #include <crm/common/ipc.h> #include <crm/common/ipcs.h> +#include <crm/common/ipc_internal.h> /* PCMK__SPECIAL_PID* */ + #define PCMK_IPC_VERSION 1 /* Evict clients whose event queue grows this large (by default) */ @@ -1338,6 +1353,132 @@ crm_ipc_send(crm_ipc_t * client, xmlNode return rc; } +int +crm_ipc_is_authentic_process(int sock, uid_t refuid, gid_t refgid, + pid_t *gotpid, uid_t *gotuid, gid_t *gotgid) { + int ret = 0; + pid_t found_pid = 0; uid_t found_uid = 0; gid_t found_gid = 0; +#if defined(US_AUTH_PEERCRED_UCRED) + struct ucred ucred; + socklen_t ucred_len = sizeof(ucred); + + if (!getsockopt(sock, SOL_SOCKET, SO_PEERCRED, + &ucred, &ucred_len) + && ucred_len == sizeof(ucred)) { + found_pid = ucred.pid; found_uid = ucred.uid; found_gid = ucred.gid; + +#elif defined(US_AUTH_PEERCRED_SOCKPEERCRED) + struct sockpeercred sockpeercred; + socklen_t sockpeercred_len = sizeof(sockpeercred); + + if (!getsockopt(sock, SOL_SOCKET, SO_PEERCRED, + &sockpeercred, &sockpeercred_len) + && sockpeercred_len == sizeof(sockpeercred_len)) { + found_pid = sockpeercred.pid; + found_uid = sockpeercred.uid; found_gid = sockpeercred.gid; + +#elif defined(US_AUTH_GETPEEREID) + if (!getpeereid(sock, &found_uid, &found_gid)) { + found_pid = PCMK__SPECIAL_PID; /* cannot obtain PID (FreeBSD) */ + +#elif defined(US_AUTH_GETPEERUCRED) + ucred_t *ucred; + if (!getpeerucred(sock, &ucred)) { + errno = 0; + found_pid = ucred_getpid(ucred); + found_uid = ucred_geteuid(ucred); found_gid = ucred_getegid(ucred); + ret = -errno; + ucred_free(ucred); + if (ret) { + return (ret < 0) ? ret : -pcmk_err_generic; + } + +#else +# error "No way to authenticate a Unix socket peer" + errno = 0; + if (0) { +#endif + if (gotpid != NULL) { + *gotpid = found_pid; + } + if (gotuid != NULL) { + *gotuid = found_uid; + } + if (gotgid != NULL) { + *gotgid = found_gid; + } + ret = (found_uid == 0 || found_uid == refuid || found_gid == refgid); + } else { + ret = (errno > 0) ? -errno : -pcmk_err_generic; + } + + return ret; +} + +int +pcmk__ipc_is_authentic_process_active(const char *name, uid_t refuid, + gid_t refgid, pid_t *gotpid) { + static char last_asked_name[PATH_MAX / 2] = ""; /* log spam prevention */ + int fd, ret = 0; + pid_t found_pid = 0; uid_t found_uid = 0; gid_t found_gid = 0; + qb_ipcc_connection_t *c; + + if ((c = qb_ipcc_connect(name, 0)) == NULL) { + crm_info("Could not connect to %s IPC: %s", name, strerror(errno)); + + } else if ((ret = qb_ipcc_fd_get(c, &fd))) { + crm_err("Could not get fd from %s IPC: %s (%d)", name, + strerror(-ret), -ret); + ret = -1; + + } else if ((ret = crm_ipc_is_authentic_process(fd, refuid, refgid, + &found_pid, &found_uid, + &found_gid)) < 0) { + if (ret == -pcmk_err_generic) { + crm_err("Could not get peer credentials from %s IPC", name); + } else { + crm_err("Could not get peer credentials from %s IPC: %s (%d)", + name, strerror(-ret), -ret); + } + ret = -1; + + } else { + if (gotpid != NULL) { + *gotpid = found_pid; + } + + if (!ret) { + crm_err("Daemon (IPC %s) effectively blocked with unauthorized" + " process %lld (uid: %lld, gid: %lld)", + name, (long long) PCMK__SPECIAL_PID_AS_0(found_pid), + (long long) found_uid, (long long) found_gid); + ret = -2; + } else if ((found_uid != refuid || found_gid != refgid) + && strncmp(last_asked_name, name, sizeof(last_asked_name))) { + if (!found_uid && refuid) { + crm_warn("Daemon (IPC %s) runs as root, whereas the expected" + " credentials are %lld:%lld, hazard of violating" + " the least privilege principle", + name, (long long) refuid, (long long) refgid); + } else { + crm_notice("Daemon (IPC %s) runs as %lld:%lld, whereas the" + " expected credentials are %lld:%lld, which may" + " mean a different set of privileges than expected", + name, (long long) found_uid, (long long) found_gid, + (long long) refuid, (long long) refgid); + } + memccpy(last_asked_name, name, '\0', sizeof(last_asked_name)); + } + } + + if (ret) { /* here, !ret only when we could not initially connect */ + qb_ipcc_disconnect(c); + } + + return ret; +} + + /* Utils */ xmlNode *
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
