diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 985a669a1b..571578c4e4 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -404,11 +404,16 @@ char* gid_to_name(gid_t gid) { return ret; } +static bool gid_list_has(const gid_t *list, size_t size, gid_t val) { + for (size_t i = 0; i < size; i++) + if (list[i] == val) + return true; + return false; +} + int in_gid(gid_t gid) { - _cleanup_free_ gid_t *allocated = NULL; - gid_t local[16], *p = local; - int ngroups = ELEMENTSOF(local); - unsigned attempt = 0; + _cleanup_free_ gid_t *gids = NULL; + int ngroups; if (getgid() == gid) return 1; @@ -419,6 +424,57 @@ int in_gid(gid_t gid) { if (!gid_is_valid(gid)) return -EINVAL; + ngroups = getgroups_alloc(&gids); + if (ngroups < 0) + return ngroups; + + return gid_list_has(gids, ngroups, gid); +} + +int in_group(const char *name) { + int r; + gid_t gid; + + r = get_group_creds(&name, &gid, 0); + if (r < 0) + return r; + + return in_gid(gid); +} + +int merge_gid_lists(const gid_t *list1, size_t size1, const gid_t *list2, size_t size2, gid_t **ret) { + size_t nresult = 0; + assert(ret); + + if (size2 > INT_MAX - size1) + return -ENOBUFS; + + gid_t *buf = new(gid_t, size1 + size2); + if (!buf) + return -ENOMEM; + + /* Duplicates need to be skipped on merging, otherwise they'll be passed on and stored in the kernel. */ + for (size_t i = 0; i < size1; i++) + if (!gid_list_has(buf, nresult, list1[i])) + buf[nresult++] = list1[i]; + for (size_t i = 0; i < size2; i++) + if (!gid_list_has(buf, nresult, list2[i])) + buf[nresult++] = list2[i]; + *ret = buf; + return (int)nresult; +} + +int getgroups_alloc(gid_t** gids) { + gid_t *allocated; + _cleanup_free_ gid_t *p = NULL; + int ngroups = 8; + unsigned attempt = 0; + + allocated = new(gid_t, ngroups); + if (!allocated) + return -ENOMEM; + p = allocated; + for (;;) { ngroups = getgroups(ngroups, p); if (ngroups >= 0) @@ -447,22 +503,8 @@ int in_gid(gid_t gid) { p = allocated; } - for (int i = 0; i < ngroups; i++) - if (p[i] == gid) - return true; - - return false; -} - -int in_group(const char *name) { - int r; - gid_t gid; - - r = get_group_creds(&name, &gid, 0); - if (r < 0) - return r; - - return in_gid(gid); + *gids = TAKE_PTR(p); + return ngroups; } int get_home_dir(char **_h) { diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 7488071086..e11dd9b379 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -42,6 +42,9 @@ char* gid_to_name(gid_t gid); int in_gid(gid_t gid); int in_group(const char *name); +int merge_gid_lists(const gid_t *list1, size_t size1, const gid_t *list2, size_t size2, gid_t **result); +int getgroups_alloc(gid_t** gids); + int get_home_dir(char **ret); int get_shell(char **_ret); diff --git a/src/core/execute.c b/src/core/execute.c index 11372958a6..5dc111f714 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1193,6 +1193,10 @@ static int setup_pam( if (pam_code != PAM_SUCCESS) goto fail; + pam_code = pam_setcred(handle, PAM_ESTABLISH_CRED | flags); + if (pam_code != PAM_SUCCESS) + goto fail; + pam_code = pam_open_session(handle, flags); if (pam_code != PAM_SUCCESS) goto fail; @@ -1277,6 +1281,10 @@ static int setup_pam( } } + pam_code = pam_setcred(handle, PAM_DELETE_CRED | flags); + if (pam_code != PAM_SUCCESS) + goto child_finish; + /* If our parent died we'll end the session */ if (getppid() != parent_pid) { pam_code = pam_close_session(handle, flags); @@ -3010,6 +3018,8 @@ static int exec_child( size_t n_fds; ExecDirectoryType dt; int secure_bits; + _cleanup_free_ gid_t *gids_after_pam = NULL; + int ngids_after_pam = 0; assert(unit); assert(command); @@ -3423,6 +3433,12 @@ static int exec_child( *exit_status = EXIT_PAM; return log_unit_error_errno(unit, r, "Failed to set up PAM session: %m"); } + + ngids_after_pam = getgroups_alloc(&gids_after_pam); + if (ngids_after_pam < 0) { + *exit_status = EXIT_MEMORY; + return log_unit_error_errno(unit, ngids_after_pam, "Failed to obtain groups after setting up PAM: %m"); + } } } @@ -3502,7 +3518,22 @@ static int exec_child( * This needs to be done after PrivateDevices=y setup as device nodes should be owned by the host's root. * For non-root in a userns, devices will be owned by the user/group before the group change, and nobody. */ if (needs_setuid) { - r = enforce_groups(gid, supplementary_gids, ngids); + _cleanup_free_ gid_t *gids_to_enforce = NULL; + int ngids_to_enforce = 0; + + ngids_to_enforce = merge_gid_lists(supplementary_gids, + ngids, + gids_after_pam, + ngids_after_pam, + &gids_to_enforce); + if (ngids_to_enforce < 0) { + *exit_status = EXIT_MEMORY; + return log_unit_error_errno(unit, + ngids_to_enforce, + "Failed to merge group lists. Group membership might be incorrect: %m"); + } + + r = enforce_groups(gid, gids_to_enforce, ngids_to_enforce); if (r < 0) { *exit_status = EXIT_GROUP; return log_unit_error_errno(unit, r, "Changing group credentials failed: %m"); diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c index 47baacb518..fde8336bf9 100644 --- a/src/test/test-user-util.c +++ b/src/test/test-user-util.c @@ -4,6 +4,7 @@ #include "format-util.h" #include "log.h" #include "macro.h" +#include "memory-util.h" #include "path-util.h" #include "string-util.h" #include "user-util.h" @@ -287,12 +288,41 @@ static void test_make_salt(void) { } static void test_in_gid(void) { - assert(in_gid(getgid()) >= 0); - assert(in_gid(getegid()) >= 0); + assert(in_gid(getegid()) >= 0); assert(in_gid(TTY_GID) == 0); /* The TTY gid is for owning ttys, it would be really really weird if we were in it. */ +} - assert(in_gid(GID_INVALID) < 0); - assert(in_gid(TTY_GID) == 0); /* The TTY gid is for owning ttys, it would be really really weird if we were in it. */ +static void test_gid_lists_ops(void) { + static const gid_t l1[] = { 5, 10, 15, 20, 25}; + static const gid_t l2[] = { 1, 2, 3, 15, 20, 25}; + static const gid_t l3[] = { 5, 10, 15, 20, 25, 26, 27}; + static const gid_t l4[] = { 25, 26, 20, 15, 5, 27, 10}; + + static const gid_t result1[] = {1, 2, 3, 5, 10, 15, 20, 25, 26, 27}; + static const gid_t result2[] = {5, 10, 15, 20, 25, 26, 27}; + + _cleanup_free_ gid_t *gids = NULL; + _cleanup_free_ gid_t *res1 = NULL; + _cleanup_free_ gid_t *res2 = NULL; + _cleanup_free_ gid_t *res3 = NULL; + _cleanup_free_ gid_t *res4 = NULL; + int nresult; + + nresult = merge_gid_lists(l2, ELEMENTSOF(l2), l3, ELEMENTSOF(l3), &res1); + assert_se(memcmp_nn(res1, nresult, result1, ELEMENTSOF(result1)) == 0); + + nresult = merge_gid_lists(NULL, 0, l2, ELEMENTSOF(l2), &res2); + assert_se(memcmp_nn(res2, nresult, l2, ELEMENTSOF(l2)) == 0); + + nresult = merge_gid_lists(l1, ELEMENTSOF(l1), l1, ELEMENTSOF(l1), &res3); + assert_se(memcmp_nn(l1, ELEMENTSOF(l1), res3, nresult) == 0); + + nresult = merge_gid_lists(l1, ELEMENTSOF(l1), l4, ELEMENTSOF(l4), &res4); + assert_se(memcmp_nn(result2, ELEMENTSOF(result2), res4, nresult) == 0); + + nresult = getgroups_alloc(&gids); + assert_se(nresult >= 0 || nresult == -EINVAL || nresult == -ENOMEM); + assert_se(gids); } int main(int argc, char *argv[]) { @@ -330,6 +360,7 @@ int main(int argc, char *argv[]) { test_make_salt(); test_in_gid(); + test_gid_lists_ops(); return 0; }