| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 15 Nov 2014 09:37:27 -0600
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Josh Triplett <josh@...htriplett.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Kees Cook <keescook@...omium.org>, mtk.manpages@...il.com,
linux-api@...r.kernel.org, linux-man@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] groups: Allow unprivileged processes to use setgroups to drop groups
Josh Triplett <josh@...htriplett.org> writes:
> 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.
A couple of questions.
- Is there precedence in other unix flavors for this?
- What motiviates this change?
- Have you looked to see if anything might for bug compatibilty
require applications not to be able to drop supplementary groups?
> 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:
>
> #include <err.h>
> #include <grp.h>
> #include <stdio.h>
> #include <sys/types.h>
> #include <unistd.h>
>
> 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@...htriplett.org>
> ---
> kernel/groups.c | 33 ++++++++++++++++++++++++++++++---
> kernel/uid16.c | 2 --
> 2 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/groups.c b/kernel/groups.c
> index f0667e7..fe7367d 100644
> --- a/kernel/groups.c
> +++ b/kernel/groups.c
> @@ -153,6 +153,29 @@ int groups_search(const struct group_info *group_info, kgid_t grp)
> return 0;
> }
>
> +/* Compare two sorted group lists; return true if the first is a subset of the
> + * second.
> + */
> +static bool is_subset(const struct group_info *g1, const struct group_info *g2)
> +{
> + 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))
> + return false;
> + 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 +212,17 @@ 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)
> + && !is_subset(group_info, new->group_info)) {
> + abort_creds(new);
> + return -EPERM;
> + }
>
> - set_groups(new, group_info);
> + set_groups_sorted(new, group_info);
> return commit_creds(new);
> }
>
> @@ -233,8 +262,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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists