URL: https://github.com/SSSD/sssd/pull/616 Author: asheplyakov Title: #616: become_user: add supplementary groups so ad provider can access keytab Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/616/head:pr616 git checkout pr616
From 25ed58a1c020accf8955ba35b35026a95c1da28d Mon Sep 17 00:00:00 2001 From: Alexey Sheplyakov <asheplya...@altlinux.org> Date: Tue, 10 Jul 2018 15:42:31 +0000 Subject: [PATCH] become_user_ex: add supplementary groups so ad provider can access keytab For security reasons one might want to run providers as a non-privileged user (say, _sssd). However some providers (in particular ad) might need an access to restricted (non world-readable) files (for instance, /etc/krb5.keytab). One of the possible ways to solve the problem is to - add a special group (for instance, _keytab) - set the owner:group of the file in question to root:_keytab - set the permissions of the file in question to 640 - make the _sssd user a member of the _keytab group For this to work it's necessary to assign supplementary groups when switching to a non-privileged user. However in some contexts finding out supplementary groups might be too expensive/inappropriate (case in point: `krb5_child` switching to the user being authenticated). To solve the problem add a new function `become_user_ex`, and call it where appropriate, i.e. `server_setup` and `ldap_child`. Note: `ldap_child` can use the same approach to avoid being SUID (when sssd runs as a non-privileged user). However that's a subject of a separate patch. --- src/providers/ldap/ldap_child.c | 2 +- src/tests/cwrap/group | 1 + src/tests/cwrap/passwd | 1 + src/tests/cwrap/test_become_user.c | 88 ++++++++++++++++++++++++++++++++++++++ src/util/become_user.c | 42 ++++++++++++++---- src/util/server.c | 2 +- src/util/util.h | 1 + 7 files changed, 127 insertions(+), 10 deletions(-) diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c index 8c11d7896f..c65b8ed6a2 100644 --- a/src/providers/ldap/ldap_child.c +++ b/src/providers/ldap/ldap_child.c @@ -701,7 +701,7 @@ int main(int argc, const char *argv[]) } DEBUG(SSSDBG_TRACE_INTERNAL, "Kerberos context initialized\n"); - kerr = become_user(ibuf->uid, ibuf->gid); + kerr = become_user_ex(ibuf->uid, ibuf->gid, true); if (kerr != 0) { DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n"); goto fail; diff --git a/src/tests/cwrap/group b/src/tests/cwrap/group index d0cea659ea..e84e23ed49 100644 --- a/src/tests/cwrap/group +++ b/src/tests/cwrap/group @@ -1,2 +1,3 @@ sssd:x:123: +_keytab:x:321:_sssd foogroup:x:10001: diff --git a/src/tests/cwrap/passwd b/src/tests/cwrap/passwd index 862ccfe03e..4fe01c619b 100644 --- a/src/tests/cwrap/passwd +++ b/src/tests/cwrap/passwd @@ -1,2 +1,3 @@ sssd:x:123:456:sssd unprivileged user:/:/sbin/nologin +_sssd:x:987:654:sssd unprivileged user:/:/sbin/nologin foobar:x:10001:10001:User for SSSD testing:/home/foobar:/bin/bash diff --git a/src/tests/cwrap/test_become_user.c b/src/tests/cwrap/test_become_user.c index e63cde9d72..52a9123e5c 100644 --- a/src/tests/cwrap/test_become_user.c +++ b/src/tests/cwrap/test_become_user.c @@ -70,6 +70,93 @@ void test_become_user(void **state) assert_int_equal(WEXITSTATUS(status), 0); } +void test_become_user_ex(void **state) +{ + struct passwd *sssd; + TALLOC_CTX *tmp_ctx; + gid_t *groups, *actual_groups; + errno_t ret; + pid_t pid, wpid; + int status; + int group_count, actual_group_count; + + assert_true(leak_check_setup()); + + tmp_ctx = talloc_new(global_talloc_context); + assert_non_null(tmp_ctx); + check_leaks_push(tmp_ctx); + + /* Must root as root, real or fake */ + assert_int_equal(geteuid(), 0); + + sssd = getpwnam("_sssd"); + assert_non_null(sssd); + + getgrouplist(sssd->pw_name, sssd->pw_gid, NULL, &group_count); + assert_int_not_equal(group_count, 0); + groups = talloc_array(tmp_ctx, gid_t, group_count); + assert_non_null(groups); + actual_groups = talloc_array(tmp_ctx, gid_t, group_count); + assert_non_null(actual_groups); + status = getgrouplist(sssd->pw_name, sssd->pw_gid, groups, &group_count); + assert_int_equal(status, group_count); + + pid = fork(); + if (pid == 0) { + /* Change the UID in a child */ + ret = become_user_ex(sssd->pw_uid, sssd->pw_gid, true); + assert_int_equal(ret, EOK); + + /* Make sure we have the requested UID and GID now and there + * are no supplementary groups + */ + assert_int_equal(geteuid(), sssd->pw_uid); + assert_int_equal(getegid(), sssd->pw_gid); + assert_int_equal(getuid(), sssd->pw_uid); + assert_int_equal(getgid(), sssd->pw_gid); + + /* Another become_user is a no-op */ + ret = become_user(sssd->pw_uid, sssd->pw_gid); + assert_int_equal(ret, EOK); + + actual_group_count = getgroups(0, NULL); + assert_int_equal(actual_group_count, group_count); + status = getgroups(actual_group_count, actual_groups); + assert_int_not_equal(status, -1); + + for (int i = 0; i < group_count; i++) { + bool found_group = false; + if (actual_groups[i] == sssd->pw_gid) { + /* in general getgroups is not guaranteed to return effective gid */ + continue; + } + for (int j = 0; j < group_count; j++) { + if (groups[j] != actual_groups[i]) { + continue; + } else { + found_group = true; + break; + } + } + assert_true(found_group); + } + exit(0); + } + + assert_int_not_equal(pid, -1); + + wpid = waitpid(pid, &status, 0); + assert_int_equal(wpid, pid); + assert_true(WIFEXITED(status)); + assert_int_equal(WEXITSTATUS(status), 0); + + talloc_free(groups); + talloc_free(actual_groups); + assert_true(check_leaks_pop(tmp_ctx)); + talloc_free(tmp_ctx); + assert_true(leak_check_teardown()); +} + void test_switch_user(void **state) { errno_t ret; @@ -138,6 +225,7 @@ int main(int argc, const char *argv[]) const struct CMUnitTest tests[] = { cmocka_unit_test(test_become_user), cmocka_unit_test(test_switch_user), + cmocka_unit_test(test_become_user_ex), }; /* Set debug level to invalid value so we can decide if -d 0 was used. */ diff --git a/src/util/become_user.c b/src/util/become_user.c index c3f726d189..e2cad7e20f 100644 --- a/src/util/become_user.c +++ b/src/util/become_user.c @@ -24,11 +24,13 @@ #include "util/util.h" #include <grp.h> +#include <pwd.h> -errno_t become_user(uid_t uid, gid_t gid) +errno_t become_user_ex(uid_t uid, gid_t gid, bool suppl_groups) { uid_t cuid; int ret; + struct passwd *pwd; DEBUG(SSSDBG_FUNC_DATA, "Trying to become user [%"SPRIuid"][%"SPRIgid"].\n", uid, gid); @@ -40,13 +42,32 @@ errno_t become_user(uid_t uid, gid_t gid) return EOK; } - /* drop supplementary groups first */ - ret = setgroups(0, NULL); - if (ret == -1) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, - "setgroups failed [%d][%s].\n", ret, strerror(ret)); - return ret; + if (suppl_groups) { + /* init supplmentary groups */ + errno = 0; + pwd = getpwuid(uid); + if (pwd == NULL || pwd->pw_name == NULL) { + ret = errno ?: ENOENT; + DEBUG(SSSDBG_CRIT_FAILURE, + "getpwuid failed [%d][%s].\n", ret, strerror(ret)); + return ret; + } + ret = initgroups(pwd->pw_name, gid); + if (ret == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, + "initgroups failed [%d][%s].\n", ret, strerror(ret)); + return ret; + } + } else { + /* drop supplementary groups */ + ret = setgroups(0, NULL); + if (ret == -1) { + ret = errno; + DEBUG(SSSDBG_CRIT_FAILURE, + "setgroups failed [%d][%s].\n", ret, strerror(ret)); + return ret; + } } /* change GID so that root cannot be regained (changes saved GID too) */ @@ -71,6 +92,11 @@ errno_t become_user(uid_t uid, gid_t gid) return EOK; } +errno_t become_user(uid_t uid, gid_t gid) +{ + return become_user_ex(uid, gid, false); +} + struct sss_creds { uid_t uid; gid_t gid; diff --git a/src/util/server.c b/src/util/server.c index f34bf49f6b..7b544971e8 100644 --- a/src/util/server.c +++ b/src/util/server.c @@ -484,7 +484,7 @@ int server_setup(const char *name, int flags, "Cannot chown the debug files, debugging might not work!\n"); } - ret = become_user(uid, gid); + ret = become_user_ex(uid, gid, true); if (ret != EOK) { DEBUG(SSSDBG_FUNC_DATA, "Cannot become user [%"SPRIuid"][%"SPRIgid"].\n", uid, gid); diff --git a/src/util/util.h b/src/util/util.h index bc89ecbc25..61b42a1a85 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -641,6 +641,7 @@ char **concatenate_string_array(TALLOC_CTX *mem_ctx, char **arr2, size_t len2); /* from become_user.c */ +errno_t become_user_ex(uid_t uid, gid_t gid, bool suppl_groups); errno_t become_user(uid_t uid, gid_t gid); struct sss_creds; errno_t switch_creds(TALLOC_CTX *mem_ctx,
_______________________________________________ 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.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/KH4S5ZFMUBGB4KG25WNMUSIV7IA5AND2/