* [PATCHv3 1/2] groups: Factor out a function to set a pre-sorted group list
@ 2014-11-15 23:49 Josh Triplett
[not found] ` <3ccec8a13019b5e8ce7b1d7889677b778b070dc8.1416095211.git.josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
2014-11-15 23:50 ` [PATCH] getgroups.2: Document unprivileged setgroups calls Josh Triplett
0 siblings, 2 replies; 4+ messages in thread
From: Josh Triplett @ 2014-11-15 23:49 UTC (permalink / raw)
To: Andrew Morton, Andy Lutomirski, Eric W. Biederman, Kees Cook,
mtk.manpages, linux-api, linux-man, linux-kernel
This way, functions that already need to sort the group list need not do
so twice.
The new set_groups_sorted is intentionally not exported.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v2, v3: No changes to patch 1/2.
kernel/groups.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/kernel/groups.c b/kernel/groups.c
index 451698f..f0667e7 100644
--- a/kernel/groups.c
+++ b/kernel/groups.c
@@ -154,16 +154,26 @@ int groups_search(const struct group_info *group_info, kgid_t grp)
}
/**
+ * set_groups_sorted - Change a group subscription in a set of credentials
+ * @new: The newly prepared set of credentials to alter
+ * @group_info: The group list to install; must be sorted
+ */
+static void set_groups_sorted(struct cred *new, struct group_info *group_info)
+{
+ put_group_info(new->group_info);
+ get_group_info(group_info);
+ new->group_info = group_info;
+}
+
+/**
* set_groups - Change a group subscription in a set of credentials
* @new: The newly prepared set of credentials to alter
* @group_info: The group list to install
*/
void set_groups(struct cred *new, struct group_info *group_info)
{
- put_group_info(new->group_info);
groups_sort(group_info);
- get_group_info(group_info);
- new->group_info = group_info;
+ set_groups_sorted(new, group_info);
}
EXPORT_SYMBOL(set_groups);
--
2.1.3
^ permalink raw reply related [flat|nested] 4+ messages in thread[parent not found: <3ccec8a13019b5e8ce7b1d7889677b778b070dc8.1416095211.git.josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>]
* [PATCHv3 2/2] groups: Allow unprivileged processes to use setgroups to drop groups 2014-11-15 23:49 [PATCHv3 1/2] groups: Factor out a function to set a pre-sorted group list Josh Triplett @ 2014-11-15 23:50 ` Josh Triplett 2014-11-15 23:50 ` [PATCH] getgroups.2: Document unprivileged setgroups calls Josh Triplett 1 sibling, 0 replies; 4+ messages in thread From: Josh Triplett @ 2014-11-15 23:50 UTC (permalink / raw) To: Andrew Morton, Andy Lutomirski, Eric W. Biederman, Kees Cook, mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-man-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Currently, unprivileged processes (without CAP_SETGID) cannot call setgroups at all. In particular, processes with a set of supplementary groups cannot further drop permissions without obtaining elevated permissions first. Allow unprivileged processes to call setgroups with a subset of their current groups; only require CAP_SETGID to add a group the process does not currently have (either as a supplementary group, or as its gid, egid, or sgid). Since some privileged programs (such as sudo) allow tests for non-membership in a group, require no_new_privs to drop group privileges. The kernel already maintains the list of supplementary group IDs in sorted order, and setgroups already needs to sort the new list, so this just requires a linear comparison of the two sorted lists. This moves the CAP_SETGID test from setgroups into set_current_groups. Tested via the following test program: void run_id(void) { pid_t p = fork(); switch (p) { case -1: err(1, "fork"); case 0: execl("/usr/bin/id", "id", NULL); err(1, "exec"); default: if (waitpid(p, NULL, 0) < 0) err(1, "waitpid"); } } int main(void) { gid_t list1[] = { 1, 2, 3, 4, 5 }; gid_t list2[] = { 2, 3, 4 }; run_id(); if (setgroups(5, list1) < 0) err(1, "setgroups 1"); run_id(); if (setresgid(1, 1, 1) < 0) err(1, "setresgid"); if (setresuid(1, 1, 1) < 0) err(1, "setresuid"); run_id(); if (setgroups(3, list2) < 0) err(1, "setgroups 2"); run_id(); if (setgroups(5, list1) < 0) err(1, "setgroups 3"); run_id(); return 0; } Without this patch, the test program gets EPERM from the second setgroups call, after dropping root privileges. With this patch, the test program successfully drops groups 1 and 5, but then gets EPERM from the third setgroups call, since that call attempts to add groups the process does not currently have. Signed-off-by: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org> --- v3: Allow gid, egid, or sgid. v2: Require no_new_privs. kernel/groups.c | 42 +++++++++++++++++++++++++++++++++++++++--- kernel/uid16.c | 2 -- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/kernel/groups.c b/kernel/groups.c index f0667e7..5114155 100644 --- a/kernel/groups.c +++ b/kernel/groups.c @@ -153,6 +153,37 @@ int groups_search(const struct group_info *group_info, kgid_t grp) return 0; } +/* Return true if the group_info is a subset of the group_info of the specified + * credentials. Also allow the first group_info to contain the gid, egid, or + * sgid of the credentials. + */ +static bool group_subset(const struct group_info *g1, + const struct cred *cred2) +{ + const struct group_info *g2 = cred2->group_info; + unsigned int i, j; + + for (i = 0, j = 0; i < g1->ngroups; i++) { + kgid_t gid1 = GROUP_AT(g1, i); + kgid_t gid2; + for (; j < g2->ngroups; j++) { + gid2 = GROUP_AT(g2, j); + if (gid_lte(gid1, gid2)) + break; + } + if (j >= g2->ngroups || !gid_eq(gid1, gid2)) { + if (!gid_eq(gid1, cred2->gid) + && !gid_eq(gid1, cred2->egid) + && !gid_eq(gid1, cred2->sgid)) + return false; + } else { + j++; + } + } + + return true; +} + /** * set_groups_sorted - Change a group subscription in a set of credentials * @new: The newly prepared set of credentials to alter @@ -189,11 +220,18 @@ int set_current_groups(struct group_info *group_info) { struct cred *new; + groups_sort(group_info); new = prepare_creds(); if (!new) return -ENOMEM; + if (!(ns_capable(current_user_ns(), CAP_SETGID) + || (task_no_new_privs(current) + && group_subset(group_info, new)))) { + abort_creds(new); + return -EPERM; + } - set_groups(new, group_info); + set_groups_sorted(new, group_info); return commit_creds(new); } @@ -233,8 +271,6 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist) struct group_info *group_info; int retval; - if (!ns_capable(current_user_ns(), CAP_SETGID)) - return -EPERM; if ((unsigned)gidsetsize > NGROUPS_MAX) return -EINVAL; diff --git a/kernel/uid16.c b/kernel/uid16.c index 602e5bb..b27e167 100644 --- a/kernel/uid16.c +++ b/kernel/uid16.c @@ -176,8 +176,6 @@ SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t __user *, grouplist) struct group_info *group_info; int retval; - if (!ns_capable(current_user_ns(), CAP_SETGID)) - return -EPERM; if ((unsigned)gidsetsize > NGROUPS_MAX) return -EINVAL; -- 2.1.3 -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCHv3 2/2] groups: Allow unprivileged processes to use setgroups to drop groups @ 2014-11-15 23:50 ` Josh Triplett 0 siblings, 0 replies; 4+ messages in thread From: Josh Triplett @ 2014-11-15 23:50 UTC (permalink / raw) To: Andrew Morton, Andy Lutomirski, Eric W. Biederman, Kees Cook, mtk.manpages, linux-api, linux-man, linux-kernel Currently, unprivileged processes (without CAP_SETGID) cannot call setgroups at all. In particular, processes with a set of supplementary groups cannot further drop permissions without obtaining elevated permissions first. Allow unprivileged processes to call setgroups with a subset of their current groups; only require CAP_SETGID to add a group the process does not currently have (either as a supplementary group, or as its gid, egid, or sgid). Since some privileged programs (such as sudo) allow tests for non-membership in a group, require no_new_privs to drop group privileges. The kernel already maintains the list of supplementary group IDs in sorted order, and setgroups already needs to sort the new list, so this just requires a linear comparison of the two sorted lists. This moves the CAP_SETGID test from setgroups into set_current_groups. Tested via the following test program: void run_id(void) { pid_t p = fork(); switch (p) { case -1: err(1, "fork"); case 0: execl("/usr/bin/id", "id", NULL); err(1, "exec"); default: if (waitpid(p, NULL, 0) < 0) err(1, "waitpid"); } } int main(void) { gid_t list1[] = { 1, 2, 3, 4, 5 }; gid_t list2[] = { 2, 3, 4 }; run_id(); if (setgroups(5, list1) < 0) err(1, "setgroups 1"); run_id(); if (setresgid(1, 1, 1) < 0) err(1, "setresgid"); if (setresuid(1, 1, 1) < 0) err(1, "setresuid"); run_id(); if (setgroups(3, list2) < 0) err(1, "setgroups 2"); run_id(); if (setgroups(5, list1) < 0) err(1, "setgroups 3"); run_id(); return 0; } Without this patch, the test program gets EPERM from the second setgroups call, after dropping root privileges. With this patch, the test program successfully drops groups 1 and 5, but then gets EPERM from the third setgroups call, since that call attempts to add groups the process does not currently have. Signed-off-by: Josh Triplett <josh@joshtriplett.org> --- v3: Allow gid, egid, or sgid. v2: Require no_new_privs. kernel/groups.c | 42 +++++++++++++++++++++++++++++++++++++++--- kernel/uid16.c | 2 -- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/kernel/groups.c b/kernel/groups.c index f0667e7..5114155 100644 --- a/kernel/groups.c +++ b/kernel/groups.c @@ -153,6 +153,37 @@ int groups_search(const struct group_info *group_info, kgid_t grp) return 0; } +/* Return true if the group_info is a subset of the group_info of the specified + * credentials. Also allow the first group_info to contain the gid, egid, or + * sgid of the credentials. + */ +static bool group_subset(const struct group_info *g1, + const struct cred *cred2) +{ + const struct group_info *g2 = cred2->group_info; + unsigned int i, j; + + for (i = 0, j = 0; i < g1->ngroups; i++) { + kgid_t gid1 = GROUP_AT(g1, i); + kgid_t gid2; + for (; j < g2->ngroups; j++) { + gid2 = GROUP_AT(g2, j); + if (gid_lte(gid1, gid2)) + break; + } + if (j >= g2->ngroups || !gid_eq(gid1, gid2)) { + if (!gid_eq(gid1, cred2->gid) + && !gid_eq(gid1, cred2->egid) + && !gid_eq(gid1, cred2->sgid)) + return false; + } else { + j++; + } + } + + return true; +} + /** * set_groups_sorted - Change a group subscription in a set of credentials * @new: The newly prepared set of credentials to alter @@ -189,11 +220,18 @@ int set_current_groups(struct group_info *group_info) { struct cred *new; + groups_sort(group_info); new = prepare_creds(); if (!new) return -ENOMEM; + if (!(ns_capable(current_user_ns(), CAP_SETGID) + || (task_no_new_privs(current) + && group_subset(group_info, new)))) { + abort_creds(new); + return -EPERM; + } - set_groups(new, group_info); + set_groups_sorted(new, group_info); return commit_creds(new); } @@ -233,8 +271,6 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist) struct group_info *group_info; int retval; - if (!ns_capable(current_user_ns(), CAP_SETGID)) - return -EPERM; if ((unsigned)gidsetsize > NGROUPS_MAX) return -EINVAL; diff --git a/kernel/uid16.c b/kernel/uid16.c index 602e5bb..b27e167 100644 --- a/kernel/uid16.c +++ b/kernel/uid16.c @@ -176,8 +176,6 @@ SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t __user *, grouplist) struct group_info *group_info; int retval; - if (!ns_capable(current_user_ns(), CAP_SETGID)) - return -EPERM; if ((unsigned)gidsetsize > NGROUPS_MAX) return -EINVAL; -- 2.1.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] getgroups.2: Document unprivileged setgroups calls 2014-11-15 23:49 [PATCHv3 1/2] groups: Factor out a function to set a pre-sorted group list Josh Triplett [not found] ` <3ccec8a13019b5e8ce7b1d7889677b778b070dc8.1416095211.git.josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org> @ 2014-11-15 23:50 ` Josh Triplett 1 sibling, 0 replies; 4+ messages in thread From: Josh Triplett @ 2014-11-15 23:50 UTC (permalink / raw) To: Andrew Morton, Andy Lutomirski, Eric W. Biederman, Kees Cook, mtk.manpages, linux-api, linux-man, linux-kernel Signed-off-by: Josh Triplett <josh@joshtriplett.org> --- v3: Document use of gid/egid/sgid. v2: Document requirement for no_new_privs. (If this doesn't end up going into 3.18, the version number in the patch will need updating.) man2/getgroups.2 | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/man2/getgroups.2 b/man2/getgroups.2 index 373c204..e2b834e 100644 --- a/man2/getgroups.2 +++ b/man2/getgroups.2 @@ -81,9 +81,16 @@ to be used in a further call to .PP .BR setgroups () sets the supplementary group IDs for the calling process. -Appropriate privileges (Linux: the +A process with the .B CAP_SETGID -capability) are required. +capability may change its supplementary group IDs arbitrarily. +As of Linux 3.18, any process that has enabled PR_SET_NO_NEW_PRIVS (see +.BR prctl (2)) +may drop supplementary groups, or add any of the current real UID, the current +effective UID, or the current saved set-user-ID; adding any other group ID +requires the +.B CAP_SETGID +capability. The .I size argument specifies the number of supplementary group IDs -- 2.1.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-11-15 23:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-15 23:49 [PATCHv3 1/2] groups: Factor out a function to set a pre-sorted group list Josh Triplett
[not found] ` <3ccec8a13019b5e8ce7b1d7889677b778b070dc8.1416095211.git.josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
2014-11-15 23:50 ` [PATCHv3 2/2] groups: Allow unprivileged processes to use setgroups to drop groups Josh Triplett
2014-11-15 23:50 ` Josh Triplett
2014-11-15 23:50 ` [PATCH] getgroups.2: Document unprivileged setgroups calls Josh Triplett
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.