diff --git a/src/util/become_user.c b/src/util/become_user.c index 28464d37b3..1d2af9c7d7 100644 --- a/src/util/become_user.c +++ b/src/util/become_user.c @@ -33,7 +33,6 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid) DEBUG(SSSDBG_FUNC_DATA, "Trying to become user [%"SPRIuid"][%"SPRIgid"].\n", uid, gid); - /* skip call if we already are the requested user */ cuid = geteuid(); if (uid == cuid) { DEBUG(SSSDBG_FUNC_DATA, "Already user [%"SPRIuid"].\n", uid); @@ -49,7 +48,6 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid) goto done; } - /* change GID so that root cannot be regained (changes saved GID too) */ ret = setresgid(gid, gid, gid); if (ret == -1) { ret = errno; @@ -58,8 +56,6 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid) goto done; } - /* change UID so that root cannot be regained (changes saved UID too) */ - /* this call also takes care of dropping CAP_SETUID, so this is a PNR */ ret = setresuid(uid, uid, (keep_set_uid ? -1 : uid)); if (ret == -1) { ret = errno; @@ -69,6 +65,9 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid) } done: + /* clears permitted set as well, so even if suid allows to set + * euid back to 0 this wouldn't regain any capabilities + */ sss_drop_all_caps(); return ret; @@ -134,55 +133,49 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx, ssc->gid = mygid; } - /* if we are regaining root, set EUID first so that we have CAP_SETUID back, - * and the other calls work too, otherwise call it last so that we can - * change groups before we loose CAP_SETUID */ - if (uid == 0) { - ret = setresuid(0, 0, 0); - if (ret == -1) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, - "setresuid failed [%d][%s].\n", ret, strerror(ret)); - goto done; - } - } - - /* TODO: use libcap-ng if we need to get/set capabilities too? */ - - - /* try to setgroups first should always work if CAP_SETUID is set, - * otherwise it will always fail, failure is not critical though as - * generally we only really care about UID and at most primary GID */ + /* 'man capabilities': + * - If the effective user ID is changed from 0 to nonzero, + * then all capabilities are cleared from the effective set. + * + * - If the effective user ID is changed from nonzero to 0, + * then the permitted set is copied to the effective set. + * + * The above means that behavior depends on if SSSD service user + * is 'root' or 'sssd'. + * Manage effective capabilities explicitly to avoid euid change + * effects. + */ + + sss_set_cap_effective(CAP_SETGID, true); /* if this fails then next op fails anyway */ ret = setgroups(num_gids, gids); if (ret == -1) { ret = errno; - DEBUG(SSSDBG_TRACE_FUNC, + DEBUG(SSSDBG_OP_FAILURE, "setgroups failed [%d][%s].\n", ret, strerror(ret)); + goto done; } - - /* change GID now, (leaves saved GID to current, so we can restore) */ ret = setresgid(-1, gid, -1); if (ret == -1) { ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, + DEBUG(SSSDBG_OP_FAILURE, "setresgid failed [%d][%s].\n", ret, strerror(ret)); goto done; } - if (uid != 0) { - /* change UID, (leaves saved UID to current, so we can restore) */ - ret = setresuid(-1, uid, -1); - if (ret == -1) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, - "setresuid failed [%d][%s].\n", ret, strerror(ret)); - goto done; - } + sss_set_cap_effective(CAP_SETUID, true); + ret = setresuid(-1, uid, -1); + if (ret == -1) { + ret = errno; + DEBUG(SSSDBG_OP_FAILURE, + "setresuid failed [%d][%s].\n", ret, strerror(ret)); + goto done; } ret = 0; done: + sss_set_cap_effective(CAP_SETGID, false); + sss_set_cap_effective(CAP_SETUID, false); /* might be no-op but don't bother */ if (ret) { /* attempt to restore creds first */ restore_creds(ssc);