From: ebiederm@xmission.com (Eric W. Biederman)
To: Andy Lutomirski <luto@amacapital.net>
Cc: Linux Containers <containers@lists.linux-foundation.org>,
Josh Triplett <josh@joshtriplett.org>,
Andrew Morton <akpm@linux-foundation.org>,
Kees Cook <keescook@chromium.org>,
Michael Kerrisk-manpages <mtk.manpages@gmail.com>,
Linux API <linux-api@vger.kernel.org>,
linux-man <linux-man@vger.kernel.org>,
"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
LSM <linux-security-module@vger.kernel.org>,
Casey Schaufler <casey@schaufler-ca.com>,
"Serge E. Hallyn" <serge@hallyn.com>,
Richard Weinberger <richard@nod.at>,
Kenton Varda <kenton@sandstorm.io>,
stable <stable@vger.kernel.org>
Subject: [CFT][PATCH 1/3] userns: Avoid problems with negative groups
Date: Tue, 02 Dec 2014 14:25:35 -0600 [thread overview]
Message-ID: <87fvcxyf28.fsf_-_@x220.int.ebiederm.org> (raw)
In-Reply-To: <CALCETrVfO4sBdZcQiZXsofPZMj7pqKeVbX+4g3dAj6WjUca+1w@mail.gmail.com> (Andy Lutomirski's message of "Tue, 2 Dec 2014 12:13:32 -0800")
Classic unix permission checks have an interesting feature, the group
permissions for a file can be set to less than the other permissions
on a file. Occassionally this is used deliberately to give a certain
group of users fewer permissions than the default.
Overlooking negative groups has resulted in the permission checks for
setting up a group mapping in a user namespace to be too lax. Tighten
the permission checks in new_idmap_permitted to ensure that mapping
uids and gids into user namespaces without privilege will not result
in new combinations of credentials being available to the users.
When setting mappings without privilege only the creator of the user
namespace is interesting as all other users that have CAP_SETUID over
the user namespace will also have CAP_SETUID over the user namespaces
parent. So the scope of the unprivileged check is reduced to just
the case where cred->euid is the namespace creator.
For setting a uid mapping without privilege only euid is considered as
setresuid can set uid, suid and fsuid from euid without privielege
making any combination of uids possible with user namespaces already
possible without them.
For now seeting a gid mapping without privilege is removed. The only
possible set of credentials that would be safe without a gid mapping
(egid without any supplementary groups) just doesn't happen in practice
so would simply lead to unused untested code.
setgroups is modified to fail not only when the group ids do not
map but also when there are no gid mappings at all, preventing
setgroups(0, NULL) from succeeding when gid mappings have not been
established.
For a small class of applications this change breaks userspace
and removes useful functionality. This small class of applications
includes tools/testing/selftests/mount/unprivileged-remount-test.c
Most of the removed functionality will be added back with the
addition of a one way knob to disable setgroups. Once setgroups
is disabled setting the gid_map becomes as safe as setting the uid_map.
For more common applications that set the uid_map and the gid_map with
privilege this change will have no effect on them.
This should fix CVE-2014-8989.
Cc: stable@vger.kernel.org
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
arch/s390/kernel/compat_linux.c | 5 ++++-
include/linux/user_namespace.h | 10 ++++++++++
kernel/groups.c | 5 ++++-
kernel/uid16.c | 5 ++++-
kernel/user_namespace.c | 17 ++++++++++-------
5 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linux.c
index ca38139423ae..21c91feeca2d 100644
--- a/arch/s390/kernel/compat_linux.c
+++ b/arch/s390/kernel/compat_linux.c
@@ -49,6 +49,7 @@
#include <linux/fadvise.h>
#include <linux/ipc.h>
#include <linux/slab.h>
+#include <linux/user_namespace.h>
#include <asm/types.h>
#include <asm/uaccess.h>
@@ -246,10 +247,12 @@ out:
COMPAT_SYSCALL_DEFINE2(s390_setgroups16, int, gidsetsize, u16 __user *, grouplist)
{
+ struct user_namespace *user_ns = current_user_ns();
struct group_info *group_info;
int retval;
- if (!capable(CAP_SETGID))
+ if (!gid_mapping_possible(user_ns) ||
+ !capable(CAP_SETGID))
return -EPERM;
if ((unsigned)gidsetsize > NGROUPS_MAX)
return -EINVAL;
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index e95372654f09..26d5e8f5db97 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -46,6 +46,11 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
return ns;
}
+static inline bool gid_mapping_possible(const struct user_namespace *ns)
+{
+ return ns->gid_map.nr_extents != 0;
+}
+
extern int create_user_ns(struct cred *new);
extern int unshare_userns(unsigned long unshare_flags, struct cred **new_cred);
extern void free_user_ns(struct user_namespace *ns);
@@ -70,6 +75,11 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
return &init_user_ns;
}
+static inline bool gid_mapping_possible(const struct user_namespace *ns)
+{
+ return true;
+}
+
static inline int create_user_ns(struct cred *new)
{
return -EINVAL;
diff --git a/kernel/groups.c b/kernel/groups.c
index 451698f86cfa..b9a6a5c7e100 100644
--- a/kernel/groups.c
+++ b/kernel/groups.c
@@ -6,6 +6,7 @@
#include <linux/slab.h>
#include <linux/security.h>
#include <linux/syscalls.h>
+#include <linux/user_namespace.h>
#include <asm/uaccess.h>
/* init to 2 - one for init_task, one to ensure it is never freed */
@@ -220,10 +221,12 @@ out:
SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist)
{
+ struct user_namespace *user_ns = current_user_ns();
struct group_info *group_info;
int retval;
- if (!ns_capable(current_user_ns(), CAP_SETGID))
+ if (!gid_mapping_possible(user_ns) ||
+ !ns_capable(user_ns, CAP_SETGID))
return -EPERM;
if ((unsigned)gidsetsize > NGROUPS_MAX)
return -EINVAL;
diff --git a/kernel/uid16.c b/kernel/uid16.c
index 602e5bbbceff..602c7de2aa11 100644
--- a/kernel/uid16.c
+++ b/kernel/uid16.c
@@ -13,6 +13,7 @@
#include <linux/highuid.h>
#include <linux/security.h>
#include <linux/syscalls.h>
+#include <linux/user_namespace.h>
#include <asm/uaccess.h>
@@ -173,10 +174,12 @@ out:
SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t __user *, grouplist)
{
+ struct user_namespace *user_ns = current_user_ns();
struct group_info *group_info;
int retval;
- if (!ns_capable(current_user_ns(), CAP_SETGID))
+ if (!gid_mapping_possible(user_ns) ||
+ !ns_capable(user_ns, CAP_SETGID))
return -EPERM;
if ((unsigned)gidsetsize > NGROUPS_MAX)
return -EINVAL;
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index aa312b0dc3ec..51d65b444951 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -812,16 +812,19 @@ static bool new_idmap_permitted(const struct file *file,
struct user_namespace *ns, int cap_setid,
struct uid_gid_map *new_map)
{
- /* Allow mapping to your own filesystem ids */
- if ((new_map->nr_extents == 1) && (new_map->extent[0].count == 1)) {
+ const struct cred *cred = file->f_cred;
+
+ /* Allow a mapping without capabilities when allowing the root
+ * of the user namespace capabilities restricted to that id
+ * will not change the set of credentials available to that
+ * user.
+ */
+ if ((new_map->nr_extents == 1) && (new_map->extent[0].count == 1) &&
+ uid_eq(ns->owner, cred->euid)) {
u32 id = new_map->extent[0].lower_first;
if (cap_setid == CAP_SETUID) {
kuid_t uid = make_kuid(ns->parent, id);
- if (uid_eq(uid, file->f_cred->fsuid))
- return true;
- } else if (cap_setid == CAP_SETGID) {
- kgid_t gid = make_kgid(ns->parent, id);
- if (gid_eq(gid, file->f_cred->fsgid))
+ if (uid_eq(uid, cred->euid))
return true;
}
}
--
1.9.1
next prev parent reply other threads:[~2014-12-02 20:25 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-29 17:26 [PATCH v2] userns: Disallow setgroups unless the gid_map writer is privileged Andy Lutomirski
2014-12-02 12:09 ` Eric W. Biederman
2014-12-02 18:53 ` Andy Lutomirski
2014-12-02 19:45 ` Eric W. Biederman
2014-12-02 20:13 ` Andy Lutomirski
2014-12-02 20:25 ` Eric W. Biederman [this message]
2014-12-02 20:28 ` [CFT][PATCH 2/3] userns: Add a knob to disable setgroups on a per user namespace basis Eric W. Biederman
2014-12-02 20:30 ` [CFT][PATCH 3/3] userns: Unbreak the unprivileged remount tests Eric W. Biederman
2014-12-02 21:05 ` [CFT][PATCH 2/3] userns: Add a knob to disable setgroups on a per user namespace basis Andy Lutomirski
2014-12-02 21:45 ` Eric W. Biederman
2014-12-02 22:17 ` Andy Lutomirski
2014-12-02 23:07 ` Eric W. Biederman
2014-12-02 23:17 ` Andy Lutomirski
2014-12-08 22:06 ` [CFT][PATCH 1/7] userns: Document what the invariant required for safe unprivileged mappings Eric W. Biederman
2014-12-08 22:07 ` [CFT][PATCH 2/7] userns: Don't allow setgroups until a gid mapping has been setablished Eric W. Biederman
2014-12-08 22:11 ` Andy Lutomirski
[not found] ` <87h9x5ok0h.fsf@x220.int.ebiederm.org>
2014-12-08 22:33 ` Andy Lutomirski
2014-12-08 22:17 ` Richard Weinberger
2014-12-08 22:25 ` Andy Lutomirski
2014-12-08 22:27 ` Richard Weinberger
[not found] ` <874mt5ojfh.fsf@x220.int.ebiederm.org>
2014-12-08 22:47 ` Andy Lutomirski
2014-12-08 22:07 ` [CFT][PATCH 3/7] userns: Don't allow unprivileged creation of gid mappings Eric W. Biederman
2014-12-08 22:08 ` [CFT][PATCH 4/7] userns: Check euid no fsuid when establishing an unprivileged uid mapping Eric W. Biederman
2014-12-08 22:12 ` Andy Lutomirski
2014-12-08 22:10 ` [CFT][PATCH 5/7] userns: Only allow the creator of the userns unprivileged mappings Eric W. Biederman
2014-12-08 22:15 ` Andy Lutomirski
2014-12-08 22:11 ` [CFT][PATCH 6/7] userns: Add a knob to disable setgroups on a per user namespace basis Eric W. Biederman
2014-12-08 22:21 ` Andy Lutomirski
2014-12-08 22:44 ` Eric W. Biederman
2014-12-08 22:48 ` Andy Lutomirski
2014-12-08 23:30 ` Eric W. Biederman
2014-12-09 19:31 ` Eric W. Biederman
2014-12-09 20:36 ` [CFT][PATCH 1/8] userns: Document what the invariant required for safe unprivileged mappings Eric W. Biederman
2014-12-09 20:38 ` [CFT][PATCH 2/8] userns: Don't allow setgroups until a gid mapping has been setablished Eric W. Biederman
2014-12-09 22:49 ` Andy Lutomirski
2014-12-09 20:39 ` [CFT][PATCH 3/8] userns: Don't allow unprivileged creation of gid mappings Eric W. Biederman
2014-12-09 23:00 ` Andy Lutomirski
2014-12-09 20:39 ` [CFT][PATCH 4/8] userns: Check euid no fsuid when establishing an unprivileged uid mapping Eric W. Biederman
2014-12-09 20:41 ` [CFT][PATCH 5/8] userns: Only allow the creator of the userns unprivileged mappings Eric W. Biederman
2014-12-09 20:41 ` [CFT][PATCH 6/8] userns: Rename id_map_mutex to userns_state_mutex Eric W. Biederman
2014-12-09 22:49 ` Andy Lutomirski
2014-12-09 20:42 ` [CFT][PATCH 7/8] userns: Add a knob to disable setgroups on a per user namespace basis Eric W. Biederman
2014-12-09 22:28 ` Andy Lutomirski
[not found] ` <971ad3f6-90fd-4e3f-916c-8988af3c826d@email.android.com>
2014-12-10 0:21 ` Andy Lutomirski
[not found] ` <87wq5zf83t.fsf@x220.int.ebiederm.org>
[not found] ` <87iohh3c9c.fsf@x220.int.ebiederm.org>
2014-12-12 1:30 ` Andy Lutomirski
[not found] ` <8761dh3b7k.fsf_-_@x220.int.ebiederm.org>
[not found] ` <878uicy1r9.fsf_-_@x220.int.ebiederm.org>
2014-12-12 21:54 ` [PATCH 1/2] proc.5: Document /proc/[pid]/setgroups Eric W. Biederman
2015-02-02 15:36 ` Michael Kerrisk (man-pages)
2015-02-11 8:01 ` Michael Kerrisk (man-pages)
2015-02-11 13:51 ` Eric W. Biederman
2015-02-12 13:53 ` Michael Kerrisk (man-pages)
2015-02-21 7:57 ` Michael Kerrisk (man-pages)
2015-03-03 11:39 ` Michael Kerrisk (man-pages)
2014-12-12 21:54 ` [PATCH 2/2] user_namespaces.7: Update the documention to reflect the fixes for negative groups Eric W. Biederman
2015-02-02 15:37 ` Michael Kerrisk (man-pages)
2015-02-11 8:02 ` Michael Kerrisk (man-pages)
2015-02-11 14:01 ` Eric W. Biederman
2015-02-12 10:11 ` Michael Kerrisk (man-pages)
2015-02-02 21:31 ` Alban Crequy
2015-03-04 14:00 ` Michael Kerrisk (man-pages)
2014-12-09 20:43 ` [CFT][PATCH 8/8] userns: Allow setting gid_maps without privilege when setgroups is disabled Eric W. Biederman
2014-12-10 16:39 ` [CFT] Can I get some Tested-By's on this series? Eric W. Biederman
2014-12-10 22:48 ` Serge Hallyn
2014-12-10 22:50 ` Richard Weinberger
2014-12-10 23:19 ` Eric W. Biederman
2014-12-11 19:27 ` Richard Weinberger
2014-12-12 6:56 ` Chen, Hanxiao
2014-12-13 22:31 ` serge
[not found] ` <87lhmcy2et.fsf@x220.int.ebiederm.org>
[not found] ` <20141212220840.GF22091@castiana.ipv6.teksavvy.com>
[not found] ` <8761dgze56.fsf@x220.int.ebiederm.org>
2014-12-15 19:38 ` Serge Hallyn
2014-12-15 20:11 ` Eric W. Biederman
2014-12-15 20:49 ` Serge Hallyn
2014-12-16 2:05 ` Andy Lutomirski
2014-12-16 9:23 ` Richard Weinberger
2014-12-08 22:14 ` [CFT][PATCH 7/7] userns: Allow setting gid_maps without privilege when setgroups is disabled Eric W. Biederman
2014-12-08 22:26 ` Andy Lutomirski
2014-12-02 20:58 ` [CFT][PATCH 1/3] userns: Avoid problems with negative groups Andy Lutomirski
2014-12-02 21:26 ` Eric W. Biederman
2014-12-02 22:09 ` Andy Lutomirski
2014-12-02 22:48 ` Eric W. Biederman
2014-12-02 22:56 ` Andy Lutomirski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87fvcxyf28.fsf_-_@x220.int.ebiederm.org \
--to=ebiederm@xmission.com \
--cc=akpm@linux-foundation.org \
--cc=casey@schaufler-ca.com \
--cc=containers@lists.linux-foundation.org \
--cc=josh@joshtriplett.org \
--cc=keescook@chromium.org \
--cc=kenton@sandstorm.io \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-man@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mtk.manpages@gmail.com \
--cc=richard@nod.at \
--cc=serge@hallyn.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).