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 2/3] userns: Add a knob to disable setgroups on a per user namespace basis
Date: Tue, 02 Dec 2014 14:28:18 -0600 [thread overview]
Message-ID: <874mtdyexp.fsf_-_@x220.int.ebiederm.org> (raw)
In-Reply-To: <87fvcxyf28.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Tue, 02 Dec 2014 14:25:35 -0600")
- Expose the knob to user space through a proc file /proc/<pid>/setgroups
A value of 0 means the setgroups system call is disabled in the
current processes user namespace and can not be enabled in the
future in this user namespace.
A value of 1 means the segtoups system call is enabled.
- Descedent user namespaces inherit the value of setgroups from
their parents.
- A proc file is used (instead of a sysctl) as sysctls
currently do not pass in a struct file so file_ns_capable
is unusable.
- Update new_idmap_permitted to allow unprivileged users to
establish a mapping of their own gid, as such mappings
can no longer allow dropping of supplemental groups.
This set of changes restores as much as possible the functionality
that was lost when new_idmap_permitted was modified to not allow
mappinges to be established without privilege.
As this fixes a regression from: "userns: Avoid problems with negative groups"
it is probably a candidate for a backport.
Cc: stable@vger.kernel.org
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
This patch still needs a little bit of love.
I need to take a hard look at the interaction of barriers and atomic ops,
and it seems I have at least one comment fix that needs to move elsewhere.
But this should be enough to move the conversation forward.
arch/s390/kernel/compat_linux.c | 1 +
fs/proc/base.c | 31 ++++++++++----
include/linux/user_namespace.h | 3 ++
kernel/groups.c | 1 +
kernel/uid16.c | 1 +
kernel/user.c | 1 +
kernel/user_namespace.c | 95 ++++++++++++++++++++++++++++++++++++++++-
7 files changed, 124 insertions(+), 9 deletions(-)
diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linux.c
index 21c91feeca2d..6d0ee1b089fb 100644
--- a/arch/s390/kernel/compat_linux.c
+++ b/arch/s390/kernel/compat_linux.c
@@ -252,6 +252,7 @@ COMPAT_SYSCALL_DEFINE2(s390_setgroups16, int, gidsetsize, u16 __user *, grouplis
int retval;
if (!gid_mapping_possible(user_ns) ||
+ !atomic_read(&user_ns->setgroups_allowed) ||
!capable(CAP_SETGID))
return -EPERM;
if ((unsigned)gidsetsize > NGROUPS_MAX)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 772efa45a452..4ebed9f01d97 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2386,7 +2386,7 @@ static int proc_tgid_io_accounting(struct seq_file *m, struct pid_namespace *ns,
#endif /* CONFIG_TASK_IO_ACCOUNTING */
#ifdef CONFIG_USER_NS
-static int proc_id_map_open(struct inode *inode, struct file *file,
+static int proc_userns_open(struct inode *inode, struct file *file,
const struct seq_operations *seq_ops)
{
struct user_namespace *ns = NULL;
@@ -2418,7 +2418,7 @@ err:
return ret;
}
-static int proc_id_map_release(struct inode *inode, struct file *file)
+static int proc_userns_release(struct inode *inode, struct file *file)
{
struct seq_file *seq = file->private_data;
struct user_namespace *ns = seq->private;
@@ -2428,17 +2428,17 @@ static int proc_id_map_release(struct inode *inode, struct file *file)
static int proc_uid_map_open(struct inode *inode, struct file *file)
{
- return proc_id_map_open(inode, file, &proc_uid_seq_operations);
+ return proc_userns_open(inode, file, &proc_uid_seq_operations);
}
static int proc_gid_map_open(struct inode *inode, struct file *file)
{
- return proc_id_map_open(inode, file, &proc_gid_seq_operations);
+ return proc_userns_open(inode, file, &proc_gid_seq_operations);
}
static int proc_projid_map_open(struct inode *inode, struct file *file)
{
- return proc_id_map_open(inode, file, &proc_projid_seq_operations);
+ return proc_userns_open(inode, file, &proc_projid_seq_operations);
}
static const struct file_operations proc_uid_map_operations = {
@@ -2446,7 +2446,7 @@ static const struct file_operations proc_uid_map_operations = {
.write = proc_uid_map_write,
.read = seq_read,
.llseek = seq_lseek,
- .release = proc_id_map_release,
+ .release = proc_userns_release,
};
static const struct file_operations proc_gid_map_operations = {
@@ -2454,7 +2454,7 @@ static const struct file_operations proc_gid_map_operations = {
.write = proc_gid_map_write,
.read = seq_read,
.llseek = seq_lseek,
- .release = proc_id_map_release,
+ .release = proc_userns_release,
};
static const struct file_operations proc_projid_map_operations = {
@@ -2462,7 +2462,20 @@ static const struct file_operations proc_projid_map_operations = {
.write = proc_projid_map_write,
.read = seq_read,
.llseek = seq_lseek,
- .release = proc_id_map_release,
+ .release = proc_userns_release,
+};
+
+static int proc_setgroups_open(struct inode *inode, struct file *file)
+{
+ return proc_userns_open(inode, file, &proc_setgroups_seq_operations);
+}
+
+static const struct file_operations proc_setgroups_operations = {
+ .open = proc_setgroups_open,
+ .write = proc_setgroups_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = proc_userns_release,
};
#endif /* CONFIG_USER_NS */
@@ -2572,6 +2585,7 @@ static const struct pid_entry tgid_base_stuff[] = {
REG("uid_map", S_IRUGO|S_IWUSR, proc_uid_map_operations),
REG("gid_map", S_IRUGO|S_IWUSR, proc_gid_map_operations),
REG("projid_map", S_IRUGO|S_IWUSR, proc_projid_map_operations),
+ REG("setgroups", S_IRUGO|S_IWUSR, proc_setgroups_operations),
#endif
#ifdef CONFIG_CHECKPOINT_RESTORE
REG("timers", S_IRUGO, proc_timers_operations),
@@ -2913,6 +2927,7 @@ static const struct pid_entry tid_base_stuff[] = {
REG("uid_map", S_IRUGO|S_IWUSR, proc_uid_map_operations),
REG("gid_map", S_IRUGO|S_IWUSR, proc_gid_map_operations),
REG("projid_map", S_IRUGO|S_IWUSR, proc_projid_map_operations),
+ REG("setgroups", S_IRUGO|S_IWUSR, proc_setgroups_operations),
#endif
};
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 26d5e8f5db97..1e8cb168b1d0 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -27,6 +27,7 @@ struct user_namespace {
kuid_t owner;
kgid_t group;
unsigned int proc_inum;
+ atomic_t setgroups_allowed;
/* Register of per-UID persistent keyrings for this namespace */
#ifdef CONFIG_PERSISTENT_KEYRINGS
@@ -65,9 +66,11 @@ struct seq_operations;
extern const struct seq_operations proc_uid_seq_operations;
extern const struct seq_operations proc_gid_seq_operations;
extern const struct seq_operations proc_projid_seq_operations;
+extern const struct seq_operations proc_setgroups_seq_operations;
extern ssize_t proc_uid_map_write(struct file *, const char __user *, size_t, loff_t *);
extern ssize_t proc_gid_map_write(struct file *, const char __user *, size_t, loff_t *);
extern ssize_t proc_projid_map_write(struct file *, const char __user *, size_t, loff_t *);
+extern ssize_t proc_setgroups_write(struct file *, const char __user *, size_t, loff_t *);
#else
static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
diff --git a/kernel/groups.c b/kernel/groups.c
index b9a6a5c7e100..467ae954e859 100644
--- a/kernel/groups.c
+++ b/kernel/groups.c
@@ -226,6 +226,7 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist)
int retval;
if (!gid_mapping_possible(user_ns) ||
+ !atomic_read(&user_ns->setgroups_allowed) ||
!ns_capable(user_ns, CAP_SETGID))
return -EPERM;
if ((unsigned)gidsetsize > NGROUPS_MAX)
diff --git a/kernel/uid16.c b/kernel/uid16.c
index 602c7de2aa11..096962fa1975 100644
--- a/kernel/uid16.c
+++ b/kernel/uid16.c
@@ -179,6 +179,7 @@ SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t __user *, grouplist)
int retval;
if (!gid_mapping_possible(user_ns) ||
+ !atomic_read(&user_ns->setgroups_allowed) ||
!ns_capable(user_ns, CAP_SETGID))
return -EPERM;
if ((unsigned)gidsetsize > NGROUPS_MAX)
diff --git a/kernel/user.c b/kernel/user.c
index 4efa39350e44..0d78759f7dbe 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -51,6 +51,7 @@ struct user_namespace init_user_ns = {
.owner = GLOBAL_ROOT_UID,
.group = GLOBAL_ROOT_GID,
.proc_inum = PROC_USER_INIT_INO,
+ .setgroups_allowed = ATOMIC_INIT(1),
#ifdef CONFIG_PERSISTENT_KEYRINGS
.persistent_keyring_register_sem =
__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 51d65b444951..521c8d53ee17 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -98,6 +98,7 @@ int create_user_ns(struct cred *new)
ns->level = parent_ns->level + 1;
ns->owner = owner;
ns->group = group;
+ atomic_set(&ns->setgroups_allowed, atomic_read(&parent_ns->setgroups_allowed));
set_cred_user_ns(new, ns);
@@ -640,7 +641,7 @@ static ssize_t map_write(struct file *file, const char __user *buf,
if (!page)
goto out;
- /* Only allow <= page size writes at the beginning of the file */
+ /* Only allow < page size writes at the beginning of the file */
ret = -EINVAL;
if ((*ppos != 0) || (count >= PAGE_SIZE))
goto out;
@@ -826,6 +827,11 @@ static bool new_idmap_permitted(const struct file *file,
kuid_t uid = make_kuid(ns->parent, id);
if (uid_eq(uid, cred->euid))
return true;
+ } else if (cap_setid == CAP_SETGID) {
+ kgid_t gid = make_kgid(ns->parent, id);
+ if (!atomic_read(&ns->setgroups_allowed) &&
+ gid_eq(gid, cred->egid))
+ return true;
}
}
@@ -844,6 +850,93 @@ static bool new_idmap_permitted(const struct file *file,
return false;
}
+static void *setgroups_m_start(struct seq_file *seq, loff_t *ppos)
+{
+ struct user_namespace *ns = seq->private;
+
+ return (*ppos == 0) ? ns : NULL;
+}
+
+static void *setgroups_m_next(struct seq_file *seq, void *v, loff_t *ppos)
+{
+ ++*ppos;
+ return NULL;
+}
+
+static void setgroups_m_stop(struct seq_file *seq, void *v)
+{
+}
+
+static int setgroups_m_show(struct seq_file *seq, void *v)
+{
+ struct user_namespace *ns = seq->private;
+
+ seq_printf(seq, "%u\n", atomic_read(&ns->setgroups_allowed));
+ return 0;
+}
+
+const struct seq_operations proc_setgroups_seq_operations = {
+ .start = setgroups_m_start,
+ .stop = setgroups_m_stop,
+ .next = setgroups_m_next,
+ .show = setgroups_m_show,
+};
+
+ssize_t proc_setgroups_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct seq_file *seq = file->private_data;
+ struct user_namespace *ns = seq->private;
+ char kbuf[3];
+ int setgroups_allowed;
+ ssize_t ret;
+
+ ret = -EPERM;
+ if (!file_ns_capable(file, ns, CAP_SETGID))
+ goto out;
+
+ /* Only allow a very narrow range of strings to be written */
+ ret = -EINVAL;
+ if ((*ppos != 0) || (count >= sizeof(kbuf)) || (count < 1))
+ goto out;
+
+ /* What was written? */
+ ret = -EFAULT;
+ if (copy_from_user(kbuf, buf, count))
+ goto out;
+ kbuf[count] = '\0';
+
+ /* What is being requested? */
+ ret = -EINVAL;
+ if (kbuf[0] == '0')
+ setgroups_allowed = 0;
+ else if (kbuf[0] == '1')
+ setgroups_allowed = 1;
+ else
+ goto out;
+
+ /* Allow a trailing newline */
+ ret = -EINVAL;
+ if ((count == 2) && (kbuf[1] != '\n'))
+ goto out;
+
+
+ if (setgroups_allowed) {
+ ret = -EINVAL;
+ if (atomic_read(&ns->setgroups_allowed) == 0)
+ goto out;
+ } else {
+ atomic_set(&ns->setgroups_allowed, 0);
+ /* sigh memory barriers! */
+ }
+
+ /* Report a successful write */
+ *ppos = count;
+ ret = count;
+out:
+ return ret;
+}
+
static void *userns_get(struct task_struct *task)
{
struct user_namespace *user_ns;
--
1.9.1
next prev parent reply other threads:[~2014-12-02 20:28 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 ` [CFT][PATCH 1/3] userns: Avoid problems with negative groups Eric W. Biederman
2014-12-02 20:28 ` Eric W. Biederman [this message]
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=874mtdyexp.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).