URL: https://github.com/SSSD/sssd/pull/702 Author: jhrozek Title: #702: NSS: Avoid changing the memory cache ownership away from the SSSD user Action: opened
PR body: """ Resolves: https://pagure.io/SSSD/sssd/issue/3890 In case SSSD is compiled --with-sssd-user but run as root (which is the default on RHEL and derivatives), then the memory cache will be owned by the user that sssd_nss runs as, so root. This conflicts with the packaging which specifies sssd.sssd as the owner. And in turn, this means that users can't reliably assess the package integrity using rpm -V. This patch makes sure that the memory cache files are chowned to sssd.sssd even if the nss responder runs as root. Also, this patch changes the sssd_nss responder so that is becomes a member of the supplementary sssd group. Even though in traditional UNIX sense, a process running as root could write to a file owned by sssd:sssd, with SELinux enforcing mode this becomes problematic as SELinux emits an error such as: type=AVC msg=audit(1543524888.125:1495): avc: denied { fsetid } for pid=7706 comm="sssd_nss" capability=4 scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:system_r:sssd_t:s0 tclass=capability To make it possible for the sssd_nss process to write to the files, the files are also made group-writable. The 'others' permission is still set to read only. """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/702/head:pr702 git checkout pr702
From ed33e33df552ed53130135a925678c8e25f2e0d2 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Thu, 29 Nov 2018 09:18:32 +0100 Subject: [PATCH] NSS: Avoid changing the memory cache ownership away from the SSSD user Resolves: https://pagure.io/SSSD/sssd/issue/3890 In case SSSD is compiled --with-sssd-user but run as root (which is the default on RHEL and derivatives), then the memory cache will be owned by the user that sssd_nss runs as, so root. This conflicts with the packaging which specifies sssd.sssd as the owner. And in turn, this means that users can't reliably assess the package integrity using rpm -V. This patch makes sure that the memory cache files are chowned to sssd.sssd even if the nss responder runs as root. Also, this patch changes the sssd_nss responder so that is becomes a member of the supplementary sssd group. Even though in traditional UNIX sense, a process running as root could write to a file owned by sssd:sssd, with SELinux enforcing mode this becomes problematic as SELinux emits an error such as: type=AVC msg=audit(1543524888.125:1495): avc: denied { fsetid } for pid=7706 comm="sssd_nss" capability=4 scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:system_r:sssd_t:s0 tclass=capability To make it possible for the sssd_nss process to write to the files, the files are also made group-writable. The 'others' permission is still set to read only. --- contrib/sssd.spec.in | 8 +- src/responder/nss/nsssrv.c | 111 +++++++++++++++++++++++++- src/responder/nss/nsssrv_mmap_cache.c | 43 +++++++++- src/responder/nss/nsssrv_mmap_cache.h | 1 + 4 files changed, 155 insertions(+), 8 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 26fae6d68..22a1063b2 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -1039,11 +1039,11 @@ done %dir %{sssdstatedir} %dir %{_localstatedir}/cache/krb5rcache %attr(700,sssd,sssd) %dir %{dbpath} -%attr(755,sssd,sssd) %dir %{mcpath} +%attr(775,sssd,sssd) %dir %{mcpath} %attr(751,sssd,sssd) %dir %{deskprofilepath} -%ghost %attr(0644,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/passwd -%ghost %attr(0644,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/group -%ghost %attr(0644,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/initgroups +%ghost %attr(0664,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/passwd +%ghost %attr(0664,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/group +%ghost %attr(0664,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/initgroups %attr(755,sssd,sssd) %dir %{pipepath} %attr(750,sssd,root) %dir %{pipepath}/private %attr(755,sssd,sssd) %dir %{pubconfpath} diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c index fb7326a02..808b96108 100644 --- a/src/responder/nss/nsssrv.c +++ b/src/responder/nss/nsssrv.c @@ -209,6 +209,8 @@ static int setup_memcaches(struct nss_ctx *nctx) { int ret; int memcache_timeout; + uid_t sssd_uid; + gid_t sssd_gid; /* Remove the CLEAR_MC_FLAG file if exists. */ ret = unlink(SSS_NSS_MCACHE_DIR"/"CLEAR_MC_FLAG); @@ -236,22 +238,40 @@ static int setup_memcaches(struct nss_ctx *nctx) return EOK; } + /* + * We explicitly read the IDs of the SSSD user even though the server + * receives --uid and --gid by parameters to account for the case where + * the SSSD is compiled --with-sssd-user=sssd but the default of the + * user option is root (this is what RHEL does) + */ + ret = sss_user_by_name_or_uid(SSSD_USER, &sssd_uid, &sssd_gid); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, "Cannot get info on "SSSD_USER); + return ret; + } + /* TODO: read cache sizes from configuration */ - ret = sss_mmap_cache_init(nctx, "passwd", SSS_MC_PASSWD, + ret = sss_mmap_cache_init(nctx, "passwd", + sssd_uid, sssd_gid, + SSS_MC_PASSWD, SSS_MC_CACHE_ELEMENTS, (time_t)memcache_timeout, &nctx->pwd_mc_ctx); if (ret) { DEBUG(SSSDBG_CRIT_FAILURE, "passwd mmap cache is DISABLED\n"); } - ret = sss_mmap_cache_init(nctx, "group", SSS_MC_GROUP, + ret = sss_mmap_cache_init(nctx, "group", + sssd_uid, sssd_gid, + SSS_MC_GROUP, SSS_MC_CACHE_ELEMENTS, (time_t)memcache_timeout, &nctx->grp_mc_ctx); if (ret) { DEBUG(SSSDBG_CRIT_FAILURE, "group mmap cache is DISABLED\n"); } - ret = sss_mmap_cache_init(nctx, "initgroups", SSS_MC_INITGROUPS, + ret = sss_mmap_cache_init(nctx, "initgroups", + sssd_uid, sssd_gid, + SSS_MC_INITGROUPS, SSS_MC_CACHE_ELEMENTS, (time_t)memcache_timeout, &nctx->initgr_mc_ctx); if (ret) { @@ -421,6 +441,78 @@ int nss_process_init(TALLOC_CTX *mem_ctx, return ret; } +static int sssd_supplementary_group(void) +{ + gid_t sssd_gid; + errno_t ret; + int size; + gid_t *supp_gids = NULL; + + /* + * We explicitly read the IDs of the SSSD user even though the server + * receives --uid and --gid by parameters to account for the case where + * the SSSD is compiled --with-sssd-user=sssd but the default of the + * user option is root (this is what RHEL does) + */ + ret = sss_user_by_name_or_uid(SSSD_USER, NULL, &sssd_gid); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, "Cannot get info on "SSSD_USER); + return ret; + } + + if (getgid() == sssd_gid) { + DEBUG(SSSDBG_TRACE_FUNC, "Already running as the sssd group\n"); + return EOK; + } + + size = getgroups(0, NULL); + if (size == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, "Getgroups failed! (%d, %s)\n", + ret, sss_strerror(ret)); + return ret; + } + + if (size > 0) { + supp_gids = talloc_zero_array(NULL, gid_t, size); + if (supp_gids == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Allocation failed!\n"); + ret = ENOMEM; + goto done; + } + } + + size = getgroups(size, supp_gids); + if (size == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, "Getgroups failed! (%d, %s)\n", + ret, sss_strerror(ret)); + goto done; + } + + for (int i = 0; i < size; i++) { + if (supp_gids[i] == sssd_gid) { + DEBUG(SSSDBG_TRACE_FUNC, + "Already assigned to the SSSD supplementary group\n"); + ret = EOK; + goto done; + } + } + + ret = setgroups(1, &sssd_gid); + if (ret != EOK) { + ret = errno; + DEBUG(SSSDBG_OP_FAILURE, + "Cannot setgroups [%d]: %s\n", ret, sss_strerror(ret)); + goto done; + } + + ret = EOK; +done: + talloc_free(supp_gids); + return ret; +} + int main(int argc, const char *argv[]) { int opt; @@ -469,6 +561,19 @@ int main(int argc, const char *argv[]) &main_ctx); if (ret != EOK) return 2; + /* + * Adding the NSS process to the SSSD supplementary group avoids + * dac_override AVC messages from SELinux in case sssd_nss runs + * as root and tries to write to memcache owned by sssd:sssd + */ + ret = sssd_supplementary_group(); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Cannot add process to the sssd supplementary group [%d]: %s\n", + ret, sss_strerror(ret)); + return 4; + } + ret = die_if_parent_died(); if (ret != EOK) { /* This is not fatal, don't return */ diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index de9e67513..ac99caa19 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -57,6 +57,9 @@ struct sss_mc_ctx { char *file; /* mmap cache file name */ int fd; /* file descriptor */ + uid_t uid; /* User ID of owner */ + gid_t gid; /* Group ID of owner */ + uint32_t seed; /* pseudo-random seed to avoid collision attacks */ time_t valid_time_slot; /* maximum time the entry is valid in seconds */ @@ -1144,6 +1147,20 @@ static errno_t sss_mc_create_file(struct sss_mc_ctx *mc_ctx) return ret; } + ret = fchown(mc_ctx->fd, mc_ctx->uid, mc_ctx->gid); + if (ret != 0) { + ret = errno; + return ret; + } + + ret = fchmod(mc_ctx->fd, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH); + if (ret == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to chmod mmap file %s: %d(%s)\n", + mc_ctx->file, ret, strerror(ret)); + return ret; + } + ret = sss_br_lock_file(mc_ctx->fd, 0, 1, retries, t); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, @@ -1224,6 +1241,7 @@ static int mc_ctx_destructor(struct sss_mc_ctx *mc_ctx) } errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, + uid_t uid, gid_t gid, enum sss_mc_type type, size_t n_elem, time_t timeout, struct sss_mc_ctx **mcc) { @@ -1259,6 +1277,9 @@ errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, goto done; } + mc_ctx->uid = uid; + mc_ctx->gid = gid; + mc_ctx->type = type; mc_ctx->valid_time_slot = timeout; @@ -1359,6 +1380,8 @@ errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, size_t n_elem, TALLOC_CTX* tmp_ctx = NULL; char *name; enum sss_mc_type type; + uid_t sssd_uid; + gid_t sssd_gid; if (mc_ctx == NULL || (*mc_ctx) == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -1394,7 +1417,25 @@ errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, size_t n_elem, /* make sure we do not leave a potentially freed pointer around */ *mc_ctx = NULL; - ret = sss_mmap_cache_init(mem_ctx, name, type, n_elem, timeout, mc_ctx); + /* + * We explicitly read the IDs of the SSSD user even though the server + * receives --uid and --gid by parameters to account for the case where + * the SSSD is compiled --with-sssd-user=sssd but the default of the + * user option is root (this is what RHEL does) + */ + ret = sss_user_by_name_or_uid(SSSD_USER, &sssd_uid, &sssd_gid); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, "Cannot get info on "SSSD_USER); + goto done; + } + + ret = sss_mmap_cache_init(mem_ctx, + name, + sssd_uid, sssd_gid, + type, + n_elem, + timeout, + mc_ctx); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to re-initialize mmap cache.\n"); goto done; diff --git a/src/responder/nss/nsssrv_mmap_cache.h b/src/responder/nss/nsssrv_mmap_cache.h index b84fbc8ed..b1820f718 100644 --- a/src/responder/nss/nsssrv_mmap_cache.h +++ b/src/responder/nss/nsssrv_mmap_cache.h @@ -34,6 +34,7 @@ enum sss_mc_type { }; errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, + uid_t uid, gid_t gid, enum sss_mc_type type, size_t n_elem, time_t valid_time, struct sss_mc_ctx **mcc);
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
