Skip to content

Commit

Permalink
NSS: Avoid changing the memory cache ownership away from the sssd user
Browse files Browse the repository at this point in the history
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.

Reviewed-by: Michal Židek <mzidek@redhat.com>
  • Loading branch information
jhrozek committed Dec 13, 2018
1 parent e49e9f7 commit 61e4ba5
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 14 deletions.
8 changes: 4 additions & 4 deletions contrib/sssd.spec.in
Expand Up @@ -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}
Expand Down
2 changes: 2 additions & 0 deletions src/responder/nss/nss_private.h
Expand Up @@ -87,6 +87,8 @@ struct nss_ctx {
struct sss_mc_ctx *pwd_mc_ctx;
struct sss_mc_ctx *grp_mc_ctx;
struct sss_mc_ctx *initgr_mc_ctx;
uid_t mc_uid;
gid_t mc_gid;
};

struct sss_cmd_table *get_nss_cmds(void);
Expand Down
106 changes: 100 additions & 6 deletions src/responder/nss/nsssrv.c
Expand Up @@ -85,7 +85,8 @@ nss_clear_memcache(TALLOC_CTX *mem_ctx,

/* TODO: read cache sizes from configuration */
DEBUG(SSSDBG_TRACE_FUNC, "Clearing memory caches.\n");
ret = sss_mmap_cache_reinit(nctx, SSS_MC_CACHE_ELEMENTS,
ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid,
SSS_MC_CACHE_ELEMENTS,
(time_t) memcache_timeout,
&nctx->pwd_mc_ctx);
if (ret != EOK) {
Expand All @@ -94,7 +95,8 @@ nss_clear_memcache(TALLOC_CTX *mem_ctx,
return ret;
}

ret = sss_mmap_cache_reinit(nctx, SSS_MC_CACHE_ELEMENTS,
ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid,
SSS_MC_CACHE_ELEMENTS,
(time_t) memcache_timeout,
&nctx->grp_mc_ctx);
if (ret != EOK) {
Expand All @@ -103,7 +105,8 @@ nss_clear_memcache(TALLOC_CTX *mem_ctx,
return ret;
}

ret = sss_mmap_cache_reinit(nctx, SSS_MC_CACHE_ELEMENTS,
ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid,
SSS_MC_CACHE_ELEMENTS,
(time_t)memcache_timeout,
&nctx->initgr_mc_ctx);
if (ret != EOK) {
Expand Down Expand Up @@ -237,21 +240,27 @@ static int setup_memcaches(struct nss_ctx *nctx)
}

/* TODO: read cache sizes from configuration */
ret = sss_mmap_cache_init(nctx, "passwd", SSS_MC_PASSWD,
ret = sss_mmap_cache_init(nctx, "passwd",
nctx->mc_uid, nctx->mc_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",
nctx->mc_uid, nctx->mc_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",
nctx->mc_uid, nctx->mc_gid,
SSS_MC_INITGROUPS,
SSS_MC_CACHE_ELEMENTS, (time_t)memcache_timeout,
&nctx->initgr_mc_ctx);
if (ret) {
Expand Down Expand Up @@ -288,6 +297,79 @@ nss_register_service_iface(struct nss_ctx *nss_ctx,
return ret;
}

static int sssd_supplementary_group(struct nss_ctx *nss_ctx)
{
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,
&nss_ctx->mc_uid,
&nss_ctx->mc_gid);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE, "Cannot get info on "SSSD_USER);
return ret;
}

if (getgid() == nss_ctx->mc_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] == nss_ctx->mc_gid) {
DEBUG(SSSDBG_TRACE_FUNC,
"Already assigned to the SSSD supplementary group\n");
ret = EOK;
goto done;
}
}

ret = setgroups(1, &nss_ctx->mc_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 nss_process_init(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct confdb_ctx *cdb)
Expand Down Expand Up @@ -372,6 +454,18 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
ret = EFAULT;
goto fail;
}
/*
* 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(nctx);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE,
"Cannot add process to the sssd supplementary group [%d]: %s\n",
ret, sss_strerror(ret));
goto fail;
}

ret = setup_memcaches(nctx);
if (ret != EOK) {
Expand Down
51 changes: 48 additions & 3 deletions src/responder/nss/nsssrv_mmap_cache.c
Expand Up @@ -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 */

Expand Down Expand Up @@ -623,7 +626,9 @@ static errno_t sss_mc_get_record(struct sss_mc_ctx **_mcc,
if (ret == EFAULT) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Fatal internal mmap cache error, invalidating cache!\n");
(void)sss_mmap_cache_reinit(talloc_parent(mcc), -1, -1, _mcc);
(void)sss_mmap_cache_reinit(talloc_parent(mcc),
-1, -1, -1, -1,
_mcc);
}
return ret;
}
Expand Down Expand Up @@ -1144,6 +1149,26 @@ static errno_t sss_mc_create_file(struct sss_mc_ctx *mc_ctx)
return ret;
}

/* Make sure that the memory cache files are chowned to sssd.sssd even
* if the nss responder runs as root. This is because the specfile
* has the ownership recorded as sssd.sssd
*/
ret = fchown(mc_ctx->fd, mc_ctx->uid, mc_ctx->gid);
if (ret != 0) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to chown mmap file %s: %d(%s)\n",
mc_ctx->file, ret, strerror(ret));
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,
Expand Down Expand Up @@ -1224,6 +1249,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)
{
Expand Down Expand Up @@ -1259,6 +1285,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;
Expand Down Expand Up @@ -1352,7 +1381,9 @@ errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name,
return ret;
}

errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, size_t n_elem,
errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx,
uid_t uid, gid_t gid,
size_t n_elem,
time_t timeout, struct sss_mc_ctx **mc_ctx)
{
errno_t ret;
Expand Down Expand Up @@ -1389,12 +1420,26 @@ errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, size_t n_elem,
timeout = (*mc_ctx)->valid_time_slot;
}

if (uid == (uid_t)-1) {
uid = (*mc_ctx)->uid;
}

if (gid == (gid_t)-1) {
gid = (*mc_ctx)->gid;
}

talloc_free(*mc_ctx);

/* 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);
ret = sss_mmap_cache_init(mem_ctx,
name,
uid, gid,
type,
n_elem,
timeout,
mc_ctx);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to re-initialize mmap cache.\n");
goto done;
Expand Down
5 changes: 4 additions & 1 deletion src/responder/nss/nsssrv_mmap_cache.h
Expand Up @@ -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);

Expand Down Expand Up @@ -70,7 +71,9 @@ errno_t sss_mmap_cache_gr_invalidate_gid(struct sss_mc_ctx *mcc, gid_t gid);
errno_t sss_mmap_cache_initgr_invalidate(struct sss_mc_ctx *mcc,
struct sized_string *name);

errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, size_t n_elem,
errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx,
uid_t uid, gid_t gid,
size_t n_elem,
time_t timeout, struct sss_mc_ctx **mc_ctx);

void sss_mmap_cache_reset(struct sss_mc_ctx *mc_ctx);
Expand Down

0 comments on commit 61e4ba5

Please sign in to comment.