From 6708075f104c3c9b04b23336bb0366ca30c3931b Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 14 Apr 2013 13:47:02 -0700 Subject: userns: Don't let unprivileged users trick privileged users into setting the id_map When we require privilege for setting /proc//uid_map or /proc//gid_map no longer allow an unprivileged user to open the file and pass it to a privileged program to write to the file. Instead when privilege is required require both the opener and the writer to have the necessary capabilities. I have tested this code and verified that setting /proc//uid_map fails when an unprivileged user opens the file and a privielged user attempts to set the mapping, that unprivileged users can still map their own id, and that a privileged users can still setup an arbitrary mapping. Reported-by: Andy Lutomirski Signed-off-by: "Eric W. Biederman" Signed-off-by: Andy Lutomirski --- kernel/user_namespace.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'kernel/user_namespace.c') diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index a54f26f82eb..e2d4ace4481 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -25,7 +25,8 @@ static struct kmem_cache *user_ns_cachep __read_mostly; -static bool new_idmap_permitted(struct user_namespace *ns, int cap_setid, +static bool new_idmap_permitted(const struct file *file, + struct user_namespace *ns, int cap_setid, struct uid_gid_map *map); static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns) @@ -700,7 +701,7 @@ static ssize_t map_write(struct file *file, const char __user *buf, ret = -EPERM; /* Validate the user is allowed to use user id's mapped to. */ - if (!new_idmap_permitted(ns, cap_setid, &new_map)) + if (!new_idmap_permitted(file, ns, cap_setid, &new_map)) goto out; /* Map the lower ids from the parent user namespace to the @@ -787,7 +788,8 @@ ssize_t proc_projid_map_write(struct file *file, const char __user *buf, size_t &ns->projid_map, &ns->parent->projid_map); } -static bool new_idmap_permitted(struct user_namespace *ns, int cap_setid, +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 */ @@ -811,8 +813,10 @@ static bool new_idmap_permitted(struct user_namespace *ns, int cap_setid, /* Allow the specified ids if we have the appropriate capability * (CAP_SETUID or CAP_SETGID) over the parent user namespace. + * And the opener of the id file also had the approprpiate capability. */ - if (ns_capable(ns->parent, cap_setid)) + if (ns_capable(ns->parent, cap_setid) && + file_ns_capable(file, ns->parent, cap_setid)) return true; return false; -- cgit v1.2.3-70-g09d2 From e3211c120a85b792978bcb4be7b2886df18d27f0 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Sun, 14 Apr 2013 16:28:19 -0700 Subject: userns: Check uid_map's opener's fsuid, not the current fsuid Signed-off-by: Andy Lutomirski --- kernel/user_namespace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'kernel/user_namespace.c') diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index e2d4ace4481..5c16f3aa757 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -797,12 +797,12 @@ static bool new_idmap_permitted(const struct file *file, 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, current_fsuid())) + 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, current_fsgid())) + if (gid_eq(gid, file->f_cred->fsgid)) return true; } } -- cgit v1.2.3-70-g09d2 From 41c21e351e79004dbb4efa4bc14a53a7e0af38c5 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Sun, 14 Apr 2013 11:44:04 -0700 Subject: userns: Changing any namespace id mappings should require privileges Changing uid/gid/projid mappings doesn't change your id within the namespace; it reconfigures the namespace. Unprivileged programs should *not* be able to write these files. (We're also checking the privileges on the wrong task.) Given the write-once nature of these files and the other security checks, this is likely impossible to usefully exploit. Signed-off-by: Andy Lutomirski --- kernel/user_namespace.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'kernel/user_namespace.c') diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index 5c16f3aa757..e134d8f365d 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -613,10 +613,10 @@ static ssize_t map_write(struct file *file, const char __user *buf, if (map->nr_extents != 0) goto out; - /* Require the appropriate privilege CAP_SETUID or CAP_SETGID - * over the user namespace in order to set the id mapping. + /* + * Adjusting namespace settings requires capabilities on the target. */ - if (cap_valid(cap_setid) && !ns_capable(ns, cap_setid)) + if (cap_valid(cap_setid) && !file_ns_capable(file, ns, CAP_SYS_ADMIN)) goto out; /* Get a buffer */ -- cgit v1.2.3-70-g09d2 From 0bb80f240520c4148b623161e7856858c021696d Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 12 Apr 2013 01:50:06 +0100 Subject: proc: Split the namespace stuff out into linux/proc_ns.h Split the proc namespace stuff out into linux/proc_ns.h. Signed-off-by: David Howells cc: netdev@vger.kernel.org cc: Serge E. Hallyn cc: Eric W. Biederman Signed-off-by: Al Viro --- fs/namespace.c | 6 ++-- fs/proc/inode.c | 8 +++--- fs/proc/namespaces.c | 17 +++++++---- include/linux/proc_fs.h | 68 ++------------------------------------------ include/linux/proc_ns.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ init/version.c | 2 +- ipc/msgutil.c | 2 +- ipc/namespace.c | 2 +- kernel/nsproxy.c | 6 ++-- kernel/pid.c | 1 + kernel/pid_namespace.c | 2 +- kernel/user.c | 2 +- kernel/user_namespace.c | 2 +- kernel/utsname.c | 2 +- net/core/net_namespace.c | 7 +++-- 15 files changed, 109 insertions(+), 92 deletions(-) create mode 100644 include/linux/proc_ns.h (limited to 'kernel/user_namespace.c') diff --git a/fs/namespace.c b/fs/namespace.c index ed0708f2415..0f0cf9379c9 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -21,7 +21,7 @@ #include /* get_fs_root et.al. */ #include /* fsnotify_vfsmount_delete */ #include -#include +#include #include "pnode.h" #include "internal.h" @@ -1350,13 +1350,13 @@ static bool mnt_ns_loop(struct path *path) * mount namespace loop? */ struct inode *inode = path->dentry->d_inode; - struct proc_inode *ei; + struct proc_ns *ei; struct mnt_namespace *mnt_ns; if (!proc_ns_inode(inode)) return false; - ei = PROC_I(inode); + ei = get_proc_ns(inode); if (ei->ns_ops != &mntns_operations) return false; diff --git a/fs/proc/inode.c b/fs/proc/inode.c index bd2f76427fe..073aea60cf8 100644 --- a/fs/proc/inode.c +++ b/fs/proc/inode.c @@ -51,8 +51,8 @@ static void proc_evict_inode(struct inode *inode) sysctl_head_put(head); } /* Release any associated namespace */ - ns_ops = PROC_I(inode)->ns_ops; - ns = PROC_I(inode)->ns; + ns_ops = PROC_I(inode)->ns.ns_ops; + ns = PROC_I(inode)->ns.ns; if (ns_ops && ns) ns_ops->put(ns); } @@ -73,8 +73,8 @@ static struct inode *proc_alloc_inode(struct super_block *sb) ei->pde = NULL; ei->sysctl = NULL; ei->sysctl_entry = NULL; - ei->ns = NULL; - ei->ns_ops = NULL; + ei->ns.ns = NULL; + ei->ns.ns_ops = NULL; inode = &ei->vfs_inode; inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME; return inode; diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c index 66b51c0383d..54bdc6701e9 100644 --- a/fs/proc/namespaces.c +++ b/fs/proc/namespaces.c @@ -51,7 +51,7 @@ static int ns_delete_dentry(const struct dentry *dentry) static char *ns_dname(struct dentry *dentry, char *buffer, int buflen) { struct inode *inode = dentry->d_inode; - const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops; + const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns.ns_ops; return dynamic_dname(dentry, buffer, buflen, "%s:[%lu]", ns_ops->name, inode->i_ino); @@ -95,8 +95,8 @@ static struct dentry *proc_ns_get_dentry(struct super_block *sb, inode->i_op = &ns_inode_operations; inode->i_mode = S_IFREG | S_IRUGO; inode->i_fop = &ns_file_operations; - ei->ns_ops = ns_ops; - ei->ns = ns; + ei->ns.ns_ops = ns_ops; + ei->ns.ns = ns; unlock_new_inode(inode); } else { ns_ops->put(ns); @@ -128,7 +128,7 @@ static void *proc_ns_follow_link(struct dentry *dentry, struct nameidata *nd) if (!ptrace_may_access(task, PTRACE_MODE_READ)) goto out_put_task; - ns_path.dentry = proc_ns_get_dentry(sb, task, ei->ns_ops); + ns_path.dentry = proc_ns_get_dentry(sb, task, ei->ns.ns_ops); if (IS_ERR(ns_path.dentry)) { error = ERR_CAST(ns_path.dentry); goto out_put_task; @@ -148,7 +148,7 @@ static int proc_ns_readlink(struct dentry *dentry, char __user *buffer, int bufl { struct inode *inode = dentry->d_inode; struct proc_inode *ei = PROC_I(inode); - const struct proc_ns_operations *ns_ops = ei->ns_ops; + const struct proc_ns_operations *ns_ops = ei->ns.ns_ops; struct task_struct *task; void *ns; char name[50]; @@ -202,7 +202,7 @@ static struct dentry *proc_ns_instantiate(struct inode *dir, ei = PROC_I(inode); inode->i_mode = S_IFLNK|S_IRWXUGO; inode->i_op = &proc_ns_link_inode_operations; - ei->ns_ops = ns_ops; + ei->ns.ns_ops = ns_ops; d_set_d_op(dentry, &pid_dentry_operations); d_add(dentry, inode); @@ -337,6 +337,11 @@ out_invalid: return ERR_PTR(-EINVAL); } +struct proc_ns *get_proc_ns(struct inode *inode) +{ + return &PROC_I(inode)->ns; +} + bool proc_ns_inode(struct inode *inode) { return inode->i_fop == &ns_file_operations; diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 28a4d7e7880..8f7d8f24141 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -6,6 +6,7 @@ #include #include #include +#include struct net; struct completion; @@ -23,18 +24,6 @@ struct mm_struct; /* Worst case buffer size needed for holding an integer. */ #define PROC_NUMBUF 13 -/* - * We always define these enumerators - */ - -enum { - PROC_ROOT_INO = 1, - PROC_IPC_INIT_INO = 0xEFFFFFFFU, - PROC_UTS_INIT_INO = 0xEFFFFFFEU, - PROC_USER_INIT_INO = 0xEFFFFFFDU, - PROC_PID_INIT_INO = 0xEFFFFFFCU, -}; - /* * This is not completely implemented yet. The idea is to * create an in-memory tree (like the actual /proc filesystem @@ -81,10 +70,6 @@ struct proc_dir_entry *proc_create_data(const char *name, umode_t mode, extern void remove_proc_entry(const char *name, struct proc_dir_entry *parent); extern int remove_proc_subtree(const char *name, struct proc_dir_entry *parent); -struct pid_namespace; - -extern int pid_ns_prepare_proc(struct pid_namespace *ns); -extern void pid_ns_release_proc(struct pid_namespace *ns); /* * proc_tty.c @@ -132,12 +117,6 @@ extern struct proc_dir_entry *proc_net_mkdir(struct net *net, const char *name, extern void proc_set_size(struct proc_dir_entry *, loff_t); extern void proc_set_user(struct proc_dir_entry *, kuid_t, kgid_t); - -extern struct file *proc_ns_fget(int fd); -extern bool proc_ns_inode(struct inode *inode); - -extern int proc_alloc_inum(unsigned int *pino); -extern void proc_free_inum(unsigned int inum); #else static inline void proc_flush_task(struct task_struct *task) @@ -168,50 +147,8 @@ struct tty_driver; static inline void proc_tty_register_driver(struct tty_driver *driver) {}; static inline void proc_tty_unregister_driver(struct tty_driver *driver) {}; -static inline int pid_ns_prepare_proc(struct pid_namespace *ns) -{ - return 0; -} - -static inline void pid_ns_release_proc(struct pid_namespace *ns) -{ -} - -static inline struct file *proc_ns_fget(int fd) -{ - return ERR_PTR(-EINVAL); -} - -static inline bool proc_ns_inode(struct inode *inode) -{ - return false; -} - -static inline int proc_alloc_inum(unsigned int *inum) -{ - *inum = 1; - return 0; -} -static inline void proc_free_inum(unsigned int inum) -{ -} #endif /* CONFIG_PROC_FS */ -struct nsproxy; -struct proc_ns_operations { - const char *name; - int type; - void *(*get)(struct task_struct *task); - void (*put)(void *ns); - int (*install)(struct nsproxy *nsproxy, void *ns); - unsigned int (*inum)(void *ns); -}; -extern const struct proc_ns_operations netns_operations; -extern const struct proc_ns_operations utsns_operations; -extern const struct proc_ns_operations ipcns_operations; -extern const struct proc_ns_operations pidns_operations; -extern const struct proc_ns_operations userns_operations; -extern const struct proc_ns_operations mntns_operations; union proc_op { int (*proc_get_link)(struct dentry *, struct path *); @@ -231,8 +168,7 @@ struct proc_inode { struct proc_dir_entry *pde; struct ctl_table_header *sysctl; struct ctl_table *sysctl_entry; - void *ns; - const struct proc_ns_operations *ns_ops; + struct proc_ns ns; struct inode vfs_inode; }; diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h new file mode 100644 index 00000000000..34a1e105bef --- /dev/null +++ b/include/linux/proc_ns.h @@ -0,0 +1,74 @@ +/* + * procfs namespace bits + */ +#ifndef _LINUX_PROC_NS_H +#define _LINUX_PROC_NS_H + +struct pid_namespace; +struct nsproxy; + +struct proc_ns_operations { + const char *name; + int type; + void *(*get)(struct task_struct *task); + void (*put)(void *ns); + int (*install)(struct nsproxy *nsproxy, void *ns); + unsigned int (*inum)(void *ns); +}; + +struct proc_ns { + void *ns; + const struct proc_ns_operations *ns_ops; +}; + +extern const struct proc_ns_operations netns_operations; +extern const struct proc_ns_operations utsns_operations; +extern const struct proc_ns_operations ipcns_operations; +extern const struct proc_ns_operations pidns_operations; +extern const struct proc_ns_operations userns_operations; +extern const struct proc_ns_operations mntns_operations; + +/* + * We always define these enumerators + */ +enum { + PROC_ROOT_INO = 1, + PROC_IPC_INIT_INO = 0xEFFFFFFFU, + PROC_UTS_INIT_INO = 0xEFFFFFFEU, + PROC_USER_INIT_INO = 0xEFFFFFFDU, + PROC_PID_INIT_INO = 0xEFFFFFFCU, +}; + +#ifdef CONFIG_PROC_FS + +extern int pid_ns_prepare_proc(struct pid_namespace *ns); +extern void pid_ns_release_proc(struct pid_namespace *ns); +extern struct file *proc_ns_fget(int fd); +extern struct proc_ns *get_proc_ns(struct inode *); +extern int proc_alloc_inum(unsigned int *pino); +extern void proc_free_inum(unsigned int inum); +extern bool proc_ns_inode(struct inode *inode); + +#else /* CONFIG_PROC_FS */ + +static inline int pid_ns_prepare_proc(struct pid_namespace *ns) { return 0; } +static inline void pid_ns_release_proc(struct pid_namespace *ns) {} + +static inline struct file *proc_ns_fget(int fd) +{ + return ERR_PTR(-EINVAL); +} + +static inline struct proc_ns *get_proc_ns(struct inode *inode) { return NULL; } + +static inline int proc_alloc_inum(unsigned int *inum) +{ + *inum = 1; + return 0; +} +static inline void proc_free_inum(unsigned int inum) {} +static inline bool proc_ns_inode(struct inode *inode) { return false; } + +#endif /* CONFIG_PROC_FS */ + +#endif /* _LINUX_PROC_NS_H */ diff --git a/init/version.c b/init/version.c index 58170f18912..1a4718e500f 100644 --- a/init/version.c +++ b/init/version.c @@ -12,7 +12,7 @@ #include #include #include -#include +#include #ifndef CONFIG_KALLSYMS #define version(a) Version_ ## a diff --git a/ipc/msgutil.c b/ipc/msgutil.c index 5df8e4bf1db..8f0201735f1 100644 --- a/ipc/msgutil.c +++ b/ipc/msgutil.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include "util.h" diff --git a/ipc/namespace.c b/ipc/namespace.c index 7c1fa451b0b..7ee61bf4493 100644 --- a/ipc/namespace.c +++ b/ipc/namespace.c @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include "util.h" diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index afc0456f227..364ceab15f0 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -22,7 +22,7 @@ #include #include #include -#include +#include #include #include @@ -241,7 +241,7 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) const struct proc_ns_operations *ops; struct task_struct *tsk = current; struct nsproxy *new_nsproxy; - struct proc_inode *ei; + struct proc_ns *ei; struct file *file; int err; @@ -250,7 +250,7 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) return PTR_ERR(file); err = -EINVAL; - ei = PROC_I(file_inode(file)); + ei = get_proc_ns(file_inode(file)); ops = ei->ns_ops; if (nstype && (ops->type != nstype)) goto out; diff --git a/kernel/pid.c b/kernel/pid.c index 047dc626463..686255e2c39 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #define pid_hashfn(nr, ns) \ diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index c1c3dc1c602..4af28a84906 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include diff --git a/kernel/user.c b/kernel/user.c index e81978e8c03..5bbb91988e6 100644 --- a/kernel/user.c +++ b/kernel/user.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include /* * userns count is 1 for root user, 1 for init_uts_ns, diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index b14f4d34204..51855f5f631 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/kernel/utsname.c b/kernel/utsname.c index a47fc5de311..2fc8576efaa 100644 --- a/kernel/utsname.c +++ b/kernel/utsname.c @@ -15,7 +15,7 @@ #include #include #include -#include +#include static struct uts_namespace *create_uts_ns(void) { diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 80e271d9e64..f9765203675 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -10,7 +10,8 @@ #include #include #include -#include +#include +#include #include #include #include @@ -336,7 +337,7 @@ EXPORT_SYMBOL_GPL(__put_net); struct net *get_net_ns_by_fd(int fd) { - struct proc_inode *ei; + struct proc_ns *ei; struct file *file; struct net *net; @@ -344,7 +345,7 @@ struct net *get_net_ns_by_fd(int fd) if (IS_ERR(file)) return ERR_CAST(file); - ei = PROC_I(file_inode(file)); + ei = get_proc_ns(file_inode(file)); if (ei->ns_ops == &netns_operations) net = get_net(ei->ns); else -- cgit v1.2.3-70-g09d2