OpenVZ Forum - RDF feed
type: channel
items: userns: targeted capabilities v5
items: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
items: [PATCH 2/9] security: Make capabilities relative to the user namespace.
items: [PATCH 3/9] allow sethostname in a container
items: [PATCH 4/9] allow killing tasks in your own or child userns
items: [PATCH 5/9] Allow ptrace from non-init user namespaces
items: [PATCH 6/9] user namespaces: convert all capable checks in kernel/sys.c
items: [PATCH 7/9] add a user namespace owner of ipc ns
items: [PATCH 8/9] user namespaces: convert several capable() calls
items: [PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
items: Re: userns: targeted capabilities v5
items: Re: [PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
items: Re: [PATCH 8/9] user namespaces: convert several capable() calls
items: Re: [PATCH 6/9] user namespaces: convert all capable checks in kernel/sys.c
items: Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
items: Re: [PATCH 4/9] allow killing tasks in your own or child userns
items: Re: [PATCH 3/9] allow sethostname in a container
items: Re: [PATCH 7/9] add a user namespace owner of ipc ns
items: Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
items: Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
items: User namespaces and keys
items: Re: User namespaces and keys
items: [PATCH] userns: ptrace: incorporate feedback from Eric
items: Re: [PATCH] userns: ptrace: incorporate feedback from Eric

userns: targeted capabilities v5
type: item
description: Here is a repost of my previous user namespace patch, ported onto last night's git head. It fixes several things I was doing wrong in the last (v4) posting, in particular: 1. don't set uts_ns->user_ns to current's when !CLONE_NEWUTS 2. add a ipc_ns->user_ns which owns ipc_ns, and use that to decide CAP_IPC_OWNER 3. fix logic flaw caused by bad parantheses 4. allow do_prlimit to current 5. don't always give root full privs to init_user_ns The expected course of development for user namespaces is laid out at https://wiki.ubuntu.com/UserNamespace. Bugs aside, this patchset is supposed to not at all affect systems which are not actively using user namespaces, and only restrict what tasks in child user namespace can do. They begin to limit privilege to a user namespace, so that root in a container cannot kill or ptrace tasks in the parent user namespace, and can only get world access rights to files. Since all files currently belong to the initila user namespace, that means that child user namespaces can only get world access rights to *all* files. While this temporarily makes user namespaces bad for system containers, it starts to get useful for some sandboxing. I've run the 'runltplite.sh' with and without this patchset and found no difference. So all in all, this is the first version of this patchset for which I feel comfortable asking: please consider applying. thanks, -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: serge
date: 2011-02-17T15:02:24-00:00

[PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: copy_process() handles CLONE_NEWUSER before the rest of the namespaces. So in the case of clone(CLONE_NEWUSER|CLONE_NEWUTS) the new uts namespace will have the new user namespace as its owner. That is what we want, since we want root in that new userns to be able to have privilege over it. Changelog: Feb 15: don't set uts_ns->user_ns if we didn't create a new uts_ns. Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- include/linux/utsname.h | 3 +++ init/version.c | 2 ++ kernel/nsproxy.c | 5 +++++ kernel/user.c | 8 ++++++-- kernel/utsname.c | 4 ++++ 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/include/linux/utsname.h b/include/linux/utsname.h index 69f3997..85171be 100644 --- a/include/linux/utsname.h +++ b/include/linux/utsname.h @@ -37,9 +37,12 @@ struct new_utsname { #include <linux/nsproxy.h> #include <linux/err.h> +struct user_namespace; + struct uts_namespace { struct kref kref; struct new_utsname name; + struct user_namespace *user_ns; }; extern struct uts_namespace init_uts_ns; diff --git a/init/version.c b/init/version.c index adff586..97bb86f 100644 --- a/init/version.c +++ b/init/version.c @@ -21,6 +21,7 @@ extern int version_string(LINUX_VERSION_CODE); int version_string(LINUX_VERSION_CODE); #endif +extern struct user_namespace init_user_ns; struct uts_namespace init_uts_ns = { .kref = { .refcount = ATOMIC_INIT(2), @@ -33,6 +34,7 @@ struct uts_namespace init_uts_ns = { .machine = UTS_MACHINE, .domainname = UTS_DOMAINNAME, }, + .user_ns = &init_user_ns, }; EXPORT_SYMBOL_GPL(init_uts_ns); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index f74e6c0..034dc2e 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -74,6 +74,11 @@ static struct nsproxy *create_new_namespaces(unsigned long flags, err = PTR_ERR(new_nsp->uts_ns); goto out_uts; } + if (new_nsp->uts_ns != tsk->nsproxy->uts_ns) { + put_user_ns(new_nsp->uts_ns->user_ns); + new_nsp->uts_ns->user_ns = task_cred_xxx(tsk, user)->user_ns; + get_user_ns(new_nsp->uts_ns->use…
creator: serge
date: 2011-02-17T15:02:57-00:00

[PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: - Introduce ns_capable to test for a capability in a non-default user namespace. - Teach cap_capable to handle capabilities in a non-default user namespace. The motivation is to get to the unprivileged creation of new namespaces. It looks like this gets us 90% of the way there, with only potential uid confusion issues left. I still need to handle getting all caps after creation but otherwise I think I have a good starter patch that achieves all of your goals. Changelog: 11/05/2010: [serge] add apparmor 12/14/2010: [serge] fix capabilities to created user namespaces Without this, if user serge creates a user_ns, he won't have capabilities to the user_ns he created. THis is because we were first checking whether his effective caps had the caps he needed and returning -EPERM if not, and THEN checking whether he was the creator. Reverse those checks. 12/16/2010: [serge] security_real_capable needs ns argument in !security case 01/11/2011: [serge] add task_ns_capable helper 01/11/2011: [serge] add nsown_capable() helper per Bastian Blank suggestion 02/16/2011: [serge] fix a logic bug: the root user is always creator of init_user_ns, but should not always have capabilities to it! Fix the check in cap_capable(). Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- include/linux/capability.h | 10 ++++++++-- include/linux/security.h | 25 ++++++++++++++----------- kernel/capability.c | 32 ++++++++++++++++++++++++++++++-- security/apparmor/lsm.c | 5 +++-- security/commoncap.c | 40 +++++++++++++++++++++++++++++++++------- security/security.c | 16 ++++++++++------ security/selinux/hooks.c | 14 +++++++++----- 7 files changed, 107 insertions(+), 35 deletions(-) diff --git a/include/linux/capability.h b/include/linux/capability.h index fb16a36..cb3d2d9 100644 --- a/include/linux/capability.h +++ b/include/linux/capability.h @@ -544,7 +544,7 @@ extern const kernel_cap_t __cap_init_eff_set; * * Note that this does no…
creator: serge
date: 2011-02-17T15:03:06-00:00

[PATCH 3/9] allow sethostname in a container
type: item
description: Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- kernel/sys.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/kernel/sys.c b/kernel/sys.c index 18da702..7a1bbad 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -1177,7 +1177,7 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len) int errno; char tmp[__NEW_UTS_LEN]; - if (!capable(CAP_SYS_ADMIN)) + if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) return -EPERM; if (len < 0 || len > __NEW_UTS_LEN) return -EINVAL; -- 1.7.0.4 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: serge
date: 2011-02-17T15:03:16-00:00

[PATCH 4/9] allow killing tasks in your own or child userns
type: item
description: Changelog: Dec 8: Fixed bug in my check_kill_permission pointed out by Eric Biederman. Dec 13: Apply Eric's suggestion to pass target task into kill_ok_by_cred() for clarity Dec 31: address comment by Eric Biederman: don't need cred/tcred in check_kill_permission. Jan 1: use const cred struct. Jan 11: Per Bastian Blank's advice, clean up kill_ok_by_cred(). Feb 16: kill_ok_by_cred: fix bad parentheses Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- kernel/signal.c | 30 ++++++++++++++++++++++-------- 1 files changed, 22 insertions(+), 8 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index 4e3cff1..ffe4bdf 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -636,13 +636,33 @@ static inline bool si_fromuser(const struct siginfo *info) } /* + * called with RCU read lock from check_kill_permission() + */ +static inline int kill_ok_by_cred(struct task_struct *t) +{ + const struct cred *cred = current_cred(); + const struct cred *tcred = __task_cred(t); + + if (cred->user->user_ns == tcred->user->user_ns && + (cred->euid == tcred->suid || + cred->euid == tcred->uid || + cred->uid == tcred->suid || + cred->uid == tcred->uid)) + return 1; + + if (ns_capable(tcred->user->user_ns, CAP_KILL)) + return 1; + + return 0; +} + +/* * Bad permissions for sending the signal * - the caller must hold the RCU read lock */ static int check_kill_permission(int sig, struct siginfo *info, struct task_struct *t) { - const struct cred *cred, *tcred; struct pid *sid; int error; @@ -656,14 +676,8 @@ static int check_kill_permission(int sig, struct siginfo *info, if (error) return error; - cred = current_cred(); - tcred = __task_cred(t); if (!same_thread_group(current, t) && - (cred->euid ^ tcred->suid) && - (cred->euid ^ tcred->uid) && - (cred->uid ^ tcred->suid) && - (cred->uid ^ tcred->uid) && - !capable(CAP_KILL)) { + !kill_ok_by_cred(t)) { switch (sig) { case SIGCONT: sid = task_session(t); -- 1.7.0.4 _______________________________________________ Containers ma…
creator: serge
date: 2011-02-17T15:03:25-00:00

[PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: ptrace is allowed to tasks in the same user namespace according to the usual rules (i.e. the same rules as for two tasks in the init user namespace). ptrace is also allowed to a user namespace to which the current task the has CAP_SYS_PTRACE capability. Changelog: Dec 31: Address feedback by Eric: . Correct ptrace uid check . Rename may_ptrace_ns to ptrace_capable . Also fix the cap_ptrace checks. Jan 1: Use const cred struct Jan 11: use task_ns_capable() in place of ptrace_capable(). Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- include/linux/capability.h | 2 + include/linux/user_namespace.h | 9 +++++++ kernel/ptrace.c | 27 ++++++++++++---------- kernel/user_namespace.c | 16 +++++++++++++ security/commoncap.c | 48 +++++++++++++++++++++++++++++++++------ 5 files changed, 82 insertions(+), 20 deletions(-) diff --git a/include/linux/capability.h b/include/linux/capability.h index cb3d2d9..bc0f262 100644 --- a/include/linux/capability.h +++ b/include/linux/capability.h @@ -546,6 +546,8 @@ extern const kernel_cap_t __cap_init_eff_set; */ #define has_capability(t, cap) (security_real_capable((t), &init_user_ns, (cap)) == 0) +#define has_ns_capability(t, ns, cap) (security_real_capable((t), (ns), (cap)) == 0) + /** * has_capability_noaudit - Determine if a task has a superior capability available (unaudited) * @t: The task in question diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h index faf4679..862fc59 100644 --- a/include/linux/user_namespace.h +++ b/include/linux/user_namespace.h @@ -39,6 +39,9 @@ static inline void put_user_ns(struct user_namespace *ns) uid_t user_ns_map_uid(struct user_namespace *to, const struct cred *cred, uid_t uid); gid_t user_ns_map_gid(struct user_namespace *to, const struct cred *cred, gid_t gid); +int same_or_ancestor_user_ns(struct task_struct *task, + struct task_struct *victim); + #else static inline struct user_namespace *get_user_ns(struct user_namespace *ns) @@ -66,6 +69,12 @@ static inlin…
creator: serge
date: 2011-02-17T15:03:33-00:00

[PATCH 6/9] user namespaces: convert all capable checks in kernel/sys.c
type: item
description: This allows setuid/setgid in containers. It also fixes some corner cases where kernel logic foregoes capability checks when uids are equivalent. The latter will need to be done throughout the whole kernel. Changelog: Jan 11: Use nsown_capable() as suggested by Bastian Blank. Jan 11: Fix logic errors in uid checks pointed out by Bastian. Feb 15: allow prlimit to current (was regression in previous version) Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- kernel/sys.c | 74 ++++++++++++++++++++++++++++++++++++--------------------- 1 files changed, 47 insertions(+), 27 deletions(-) diff --git a/kernel/sys.c b/kernel/sys.c index 7a1bbad..075370d 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -118,17 +118,29 @@ EXPORT_SYMBOL(cad_pid); void (*pm_power_off_prepare)(void); +/* called with rcu_read_lock, creds are safe */ +static inline int set_one_prio_perm(struct task_struct *p) +{ + const struct cred *cred = current_cred(), *pcred = __task_cred(p); + + if (pcred->user->user_ns == cred->user->user_ns && + (pcred->uid == cred->euid || + pcred->euid == cred->euid)) + return 1; + if (ns_capable(pcred->user->user_ns, CAP_SYS_NICE)) + return 1; + return 0; +} + /* * set the priority of a task * - the caller must hold the RCU read lock */ static int set_one_prio(struct task_struct *p, int niceval, int error) { - const struct cred *cred = current_cred(), *pcred = __task_cred(p); int no_nice; - if (pcred->uid != cred->euid && - pcred->euid != cred->euid && !capable(CAP_SYS_NICE)) { + if (!set_one_prio_perm(p)) { error = -EPERM; goto out; } @@ -502,7 +514,7 @@ SYSCALL_DEFINE2(setregid, gid_t, rgid, gid_t, egid) if (rgid != (gid_t) -1) { if (old->gid == rgid || old->egid == rgid || - capable(CAP_SETGID)) + nsown_capable(CAP_SETGID)) new->gid = rgid; else goto error; @@ -511,7 +523,7 @@ SYSCALL_DEFINE2(setregid, gid_t, rgid, gid_t, egid) if (old->gid == egid || old->egid == egid || old->sgid == egid || - capable(CAP_SETGID)) + nsown_capable(CAP_SETGID)) new->egid = egi…
creator: serge
date: 2011-02-17T15:03:42-00:00

[PATCH 7/9] add a user namespace owner of ipc ns
type: item
description: Changelog: Feb 15: Don't set new ipc->user_ns if we didn't create a new ipc_ns. Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- include/linux/ipc_namespace.h | 3 +++ ipc/msgutil.c | 3 +++ ipc/namespace.c | 9 +++++++-- kernel/nsproxy.c | 5 +++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h index 5195298..46d2eb4 100644 --- a/include/linux/ipc_namespace.h +++ b/include/linux/ipc_namespace.h @@ -24,6 +24,7 @@ struct ipc_ids { struct idr ipcs_idr; }; +struct user_namespace; struct ipc_namespace { atomic_t count; struct ipc_ids ids[3]; @@ -56,6 +57,8 @@ struct ipc_namespace { unsigned int mq_msg_max; /* initialized to DFLT_MSGMAX */ unsigned int mq_msgsize_max; /* initialized to DFLT_MSGSIZEMAX */ + /* user_ns which owns the ipc ns */ + struct user_namespace *user_ns; }; extern struct ipc_namespace init_ipc_ns; diff --git a/ipc/msgutil.c b/ipc/msgutil.c index f095ee2..d91ff4b 100644 --- a/ipc/msgutil.c +++ b/ipc/msgutil.c @@ -20,6 +20,8 @@ DEFINE_SPINLOCK(mq_lock); +extern struct user_namespace init_user_ns; + /* * The next 2 defines are here bc this is the only file * compiled when either CONFIG_SYSVIPC and CONFIG_POSIX_MQUEUE @@ -32,6 +34,7 @@ struct ipc_namespace init_ipc_ns = { .mq_msg_max = DFLT_MSGMAX, .mq_msgsize_max = DFLT_MSGSIZEMAX, #endif + .user_ns = &init_user_ns, }; atomic_t nr_ipc_ns = ATOMIC_INIT(1); diff --git a/ipc/namespace.c b/ipc/namespace.c index a1094ff..aa18899 100644 --- a/ipc/namespace.c +++ b/ipc/namespace.c @@ -11,10 +11,11 @@ #include <linux/slab.h> #include <linux/fs.h> #include <linux/mount.h> +#include <linux/user_namespace.h> #include "util.h" -static struct ipc_namespace *create_ipc_ns(void) +static struct ipc_namespace *create_ipc_ns(struct ipc_namespace *old_ns) { struct ipc_namespace *ns; int err; @@ -43,6 +44,9 @@ static struct ipc_namespace *create_ipc_ns(void) ipcns_notify(IPCNS_CREATED); register_ipcns_notifier(ns); + ns->user_ns = old_…
creator: serge
date: 2011-02-17T15:03:49-00:00

[PATCH 8/9] user namespaces: convert several capable() calls
type: item
description: CAP_IPC_OWNER and CAP_IPC_LOCK can be checked against current_user_ns(), because the resource comes from current's own ipc namespace. setuid/setgid are to uids in own namespace, so again checks can be against current_user_ns(). Changelog: Jan 11: Use task_ns_capable() in place of sched_capable(). Jan 11: Use nsown_capable() as suggested by Bastian Blank. Jan 11: Clarify (hopefully) some logic in futex and sched.c Feb 15: use ns_capable for ipc, not nsown_capable Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- ipc/shm.c | 2 +- ipc/util.c | 5 +++-- kernel/futex.c | 11 ++++++++++- kernel/futex_compat.c | 11 ++++++++++- kernel/groups.c | 2 +- kernel/sched.c | 9 ++++++--- kernel/uid16.c | 2 +- 7 files changed, 32 insertions(+), 10 deletions(-) diff --git a/ipc/shm.c b/ipc/shm.c index 7d3bb22..e91e2e9 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -773,7 +773,7 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf) audit_ipc_obj(&(shp->shm_perm)); - if (!capable(CAP_IPC_LOCK)) { + if (!ns_capable(ns->user_ns, CAP_IPC_LOCK)) { uid_t euid = current_euid(); err = -EPERM; if (euid != shp->shm_perm.uid && diff --git a/ipc/util.c b/ipc/util.c index 69a0cc1..8e7ec6a 100644 --- a/ipc/util.c +++ b/ipc/util.c @@ -627,7 +627,7 @@ int ipcperms (struct kern_ipc_perm *ipcp, short flag) granted_mode >>= 3; /* is there some bit set in requested_mode but not in granted_mode? */ if ((requested_mode & ~granted_mode & 0007) && - !capable(CAP_IPC_OWNER)) + !ns_capable(current->nsproxy->ipc_ns->user_ns, CAP_IPC_OWNER)) return -1; return security_ipc_permission(ipcp, flag); @@ -800,7 +800,8 @@ struct kern_ipc_perm *ipcctl_pre_down(struct ipc_ids *ids, int id, int cmd, euid = current_euid(); if (euid == ipcp->cuid || - euid == ipcp->uid || capable(CAP_SYS_ADMIN)) + euid == ipcp->uid || + ns_capable(current->nsproxy->ipc_ns->user_ns, CAP_SYS_ADMIN)) return ipcp; err = -EPERM; diff --git a/kernel/futex.c b/kernel/futex.c index b766d28..1e876f1 100644 --- a/kerne…
creator: serge
date: 2011-02-17T15:03:56-00:00

[PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
type: item
description: Cheat for now and say all files belong to init_user_ns. Next step will be to let superblocks belong to a user_ns, and derive inode_userns(inode) from inode->i_sb->s_user_ns. Finally we'll introduce more flexible arrangements. Changelog: Feb 15: make is_owner_or_cap take const struct inode Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- fs/inode.c | 17 +++++++++++++++++ fs/namei.c | 20 +++++++++++++++----- include/linux/fs.h | 9 +++++++-- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index da85e56..1930b45 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -25,6 +25,7 @@ #include <linux/async.h> #include <linux/posix_acl.h> #include <linux/ima.h> +#include <linux/cred.h> /* * This is needed for the following functions: @@ -1722,3 +1723,19 @@ void inode_init_owner(struct inode *inode, const struct inode *dir, inode->i_mode = mode; } EXPORT_SYMBOL(inode_init_owner); + +/* + * return 1 if current either has CAP_FOWNER to the + * file, or owns the file. + */ +int is_owner_or_cap(const struct inode *inode) +{ + struct user_namespace *ns = inode_userns(inode); + + if (current_user_ns() == ns && current_fsuid() == inode->i_uid) + return 1; + if (ns_capable(ns, CAP_FOWNER)) + return 1; + return 0; +} +EXPORT_SYMBOL(is_owner_or_cap); diff --git a/fs/namei.c b/fs/namei.c index 9e701e2..cfac5b4 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -176,6 +176,9 @@ static int acl_permission_check(struct inode *inode, int mask, unsigned int flag mask &= MAY_READ | MAY_WRITE | MAY_EXEC; + if (current_user_ns() != inode_userns(inode)) + goto other_perms; + if (current_fsuid() == inode->i_uid) mode >>= 6; else { @@ -189,6 +192,7 @@ static int acl_permission_check(struct inode *inode, int mask, unsigned int flag mode >>= 3; } +other_perms: /* * If the DACs are ok we don't need any capability check. */ @@ -230,7 +234,7 @@ int generic_permission(struct inode *inode, int mask, unsigned int flags, * Executable DACs are overridable if at least one e…
creator: serge
date: 2011-02-17T15:04:07-00:00

Re: userns: targeted capabilities v5
type: item
description: On Thu, 17 Feb 2011 15:02:24 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > Here is a repost of my previous user namespace patch, ported onto > last night's git head. > > It fixes several things I was doing wrong in the last (v4) > posting, in particular: > > 1. don't set uts_ns->user_ns to current's when !CLONE_NEWUTS > 2. add a ipc_ns->user_ns which owns ipc_ns, and use that to > decide CAP_IPC_OWNER > 3. fix logic flaw caused by bad parantheses > 4. allow do_prlimit to current > 5. don't always give root full privs to init_user_ns > > The expected course of development for user namespaces is laid out > at https://wiki.ubuntu.com/UserNamespace. Seems like a nice feature to be developing. I worry about the maturity of it all at this stage. How far along is it *really*? Is anyone else working with you on developing and reviewing this work? _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-18T00:21:46-00:00

Re: [PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > Cheat for now and say all files belong to init_user_ns. Next > step will be to let superblocks belong to a user_ns, and derive > inode_userns(inode) from inode->i_sb->s_user_ns. Finally we'll > introduce more flexible arrangements. This looks good. I am a little worried that a concept like inode_user_ns will imply that there is only ever one. However this looks like a good place to start and it will only be strange filesystems that implement a notion of permissions that is namespace aware so I don't expect the generic code needs to handle that case other than allowing the permission checks to be overridden. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Changelog: > Feb 15: make is_owner_or_cap take const struct inode > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > fs/inode.c | 17 +++++++++++++++++ > fs/namei.c | 20 +++++++++++++++----- > include/linux/fs.h | 9 +++++++-- > 3 files changed, 39 insertions(+), 7 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index da85e56..1930b45 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -25,6 +25,7 @@ > #include <linux/async.h> > #include <linux/posix_acl.h> > #include <linux/ima.h> > +#include <linux/cred.h> > > /* > * This is needed for the following functions: > @@ -1722,3 +1723,19 @@ void inode_init_owner(struct inode *inode, const struct inode *dir, > inode->i_mode = mode; > } > EXPORT_SYMBOL(inode_init_owner); > + > +/* > + * return 1 if current either has CAP_FOWNER to the > + * file, or owns the file. > + */ > +int is_owner_or_cap(const struct inode *inode) > +{ > + struct user_namespace *ns = inode_userns(inode); > + > + if (current_user_ns() == ns && current_fsuid() == inode->i_uid) > + return 1; > + if (ns_capable(ns, CAP_FOWNER)) > + return 1; > + return 0; > +} > +EXPORT_SYMBOL(is_owner_or_cap); > diff --git a/fs/namei.c b/fs/namei.c > index 9e701e2..cfac5b4 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -176,6 +176,9 @@…
creator: ebiederm
date: 2011-02-18T01:29:32-00:00

Re: [PATCH 8/9] user namespaces: convert several capable() calls
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > CAP_IPC_OWNER and CAP_IPC_LOCK can be checked against current_user_ns(), > because the resource comes from current's own ipc namespace. > > setuid/setgid are to uids in own namespace, so again checks can be > against current_user_ns(). Some nits below. But this generally looks good if a little bit all over the map for a single patch. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > Changelog: > Jan 11: Use task_ns_capable() in place of sched_capable(). > Jan 11: Use nsown_capable() as suggested by Bastian Blank. > Jan 11: Clarify (hopefully) some logic in futex and sched.c > Feb 15: use ns_capable for ipc, not nsown_capable > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > ipc/shm.c | 2 +- > ipc/util.c | 5 +++-- > kernel/futex.c | 11 ++++++++++- > kernel/futex_compat.c | 11 ++++++++++- > kernel/groups.c | 2 +- > kernel/sched.c | 9 ++++++--- > kernel/uid16.c | 2 +- > 7 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/ipc/shm.c b/ipc/shm.c > index 7d3bb22..e91e2e9 100644 > --- a/ipc/shm.c > +++ b/ipc/shm.c > @@ -773,7 +773,7 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf) > > audit_ipc_obj(&(shp->shm_perm)); > > - if (!capable(CAP_IPC_LOCK)) { > + if (!ns_capable(ns->user_ns, CAP_IPC_LOCK)) { > uid_t euid = current_euid(); > err = -EPERM; > if (euid != shp->shm_perm.uid && > diff --git a/ipc/util.c b/ipc/util.c > index 69a0cc1..8e7ec6a 100644 > --- a/ipc/util.c > +++ b/ipc/util.c > @@ -627,7 +627,7 @@ int ipcperms (struct kern_ipc_perm *ipcp, short flag) > granted_mode >>= 3; > /* is there some bit set in requested_mode but not in granted_mode? */ > if ((requested_mode & ~granted_mode & 0007) && > - !capable(CAP_IPC_OWNER)) > + !ns_capable(current->nsproxy->ipc_ns->user_ns, CAP_IPC_OWNER)) > return -1; Serge can we please modify the code to pass the ns down from ipcget_public to ipcperms. It is passed in and dropping the value and going back to …
creator: ebiederm
date: 2011-02-18T01:51:25-00:00

Re: [PATCH 6/9] user namespaces: convert all capable checks in kernel/sys.c
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > This allows setuid/setgid in containers. It also fixes some > corner cases where kernel logic foregoes capability checks when > uids are equivalent. The latter will need to be done throughout > the whole kernel. Except for the extra printk in sethostname this looks good. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Changelog: > Jan 11: Use nsown_capable() as suggested by Bastian Blank. > Jan 11: Fix logic errors in uid checks pointed out by Bastian. > Feb 15: allow prlimit to current (was regression in previous version) > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > kernel/sys.c | 74 ++++++++++++++++++++++++++++++++++++--------------------- > 1 files changed, 47 insertions(+), 27 deletions(-) > > diff --git a/kernel/sys.c b/kernel/sys.c > index 7a1bbad..075370d 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -118,17 +118,29 @@ EXPORT_SYMBOL(cad_pid); > > void (*pm_power_off_prepare)(void); > > +/* called with rcu_read_lock, creds are safe */ > +static inline int set_one_prio_perm(struct task_struct *p) > +{ > + const struct cred *cred = current_cred(), *pcred = __task_cred(p); > + > + if (pcred->user->user_ns == cred->user->user_ns && > + (pcred->uid == cred->euid || > + pcred->euid == cred->euid)) > + return 1; > + if (ns_capable(pcred->user->user_ns, CAP_SYS_NICE)) > + return 1; > + return 0; > +} > + > /* > * set the priority of a task > * - the caller must hold the RCU read lock > */ > static int set_one_prio(struct task_struct *p, int niceval, int error) > { > - const struct cred *cred = current_cred(), *pcred = __task_cred(p); > int no_nice; > > - if (pcred->uid != cred->euid && > - pcred->euid != cred->euid && !capable(CAP_SYS_NICE)) { > + if (!set_one_prio_perm(p)) { > error = -EPERM; > goto out; > } > @@ -502,7 +514,7 @@ SYSCALL_DEFINE2(setregid, gid_t, rgid, gid_t, egid) > if (rgid != (gid_t) -1) { > if (old->gid == rgid || > old->egid == rgid || > - capable(CAP_SETG…
creator: ebiederm
date: 2011-02-18T01:57:15-00:00

Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > ptrace is allowed to tasks in the same user namespace according to > the usual rules (i.e. the same rules as for two tasks in the init > user namespace). ptrace is also allowed to a user namespace to > which the current task the has CAP_SYS_PTRACE capability. I don't see how it can go wrong at the moment but same_or_ancestore_user_ns is too permissive and potentially inefficient. Can you please replace it with a simple user namespace equality check. Eric > Changelog: > Dec 31: Address feedback by Eric: > . Correct ptrace uid check > . Rename may_ptrace_ns to ptrace_capable > . Also fix the cap_ptrace checks. > Jan 1: Use const cred struct > Jan 11: use task_ns_capable() in place of ptrace_capable(). > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > include/linux/capability.h | 2 + > include/linux/user_namespace.h | 9 +++++++ > kernel/ptrace.c | 27 ++++++++++++---------- > kernel/user_namespace.c | 16 +++++++++++++ > security/commoncap.c | 48 +++++++++++++++++++++++++++++++++------ > 5 files changed, 82 insertions(+), 20 deletions(-) > > diff --git a/include/linux/capability.h b/include/linux/capability.h > index cb3d2d9..bc0f262 100644 > --- a/include/linux/capability.h > +++ b/include/linux/capability.h > @@ -546,6 +546,8 @@ extern const kernel_cap_t __cap_init_eff_set; > */ > #define has_capability(t, cap) (security_real_capable((t), &init_user_ns, (cap)) == 0) > > +#define has_ns_capability(t, ns, cap) (security_real_capable((t), (ns), (cap)) == 0) > + > /** > * has_capability_noaudit - Determine if a task has a superior capability available (unaudited) > * @t: The task in question > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h > index faf4679..862fc59 100644 > --- a/include/linux/user_namespace.h > +++ b/include/linux/user_namespace.h > @@ -39,6 +39,9 @@ static inline void put_user_ns(struct user_namespace *ns) > uid_t user_ns_map_uid(struct user_namespace *to, c…
creator: ebiederm
date: 2011-02-18T02:59:57-00:00

Re: [PATCH 4/9] allow killing tasks in your own or child userns
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > Changelog: > Dec 8: Fixed bug in my check_kill_permission pointed out by > Eric Biederman. > Dec 13: Apply Eric's suggestion to pass target task into kill_ok_by_cred() > for clarity > Dec 31: address comment by Eric Biederman: > don't need cred/tcred in check_kill_permission. > Jan 1: use const cred struct. > Jan 11: Per Bastian Blank's advice, clean up kill_ok_by_cred(). > Feb 16: kill_ok_by_cred: fix bad parentheses Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > kernel/signal.c | 30 ++++++++++++++++++++++-------- > 1 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/kernel/signal.c b/kernel/signal.c > index 4e3cff1..ffe4bdf 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -636,13 +636,33 @@ static inline bool si_fromuser(const struct siginfo *info) > } > > /* > + * called with RCU read lock from check_kill_permission() > + */ > +static inline int kill_ok_by_cred(struct task_struct *t) > +{ > + const struct cred *cred = current_cred(); > + const struct cred *tcred = __task_cred(t); > + > + if (cred->user->user_ns == tcred->user->user_ns && > + (cred->euid == tcred->suid || > + cred->euid == tcred->uid || > + cred->uid == tcred->suid || > + cred->uid == tcred->uid)) > + return 1; > + > + if (ns_capable(tcred->user->user_ns, CAP_KILL)) > + return 1; > + > + return 0; > +} > + > +/* > * Bad permissions for sending the signal > * - the caller must hold the RCU read lock > */ > static int check_kill_permission(int sig, struct siginfo *info, > struct task_struct *t) > { > - const struct cred *cred, *tcred; > struct pid *sid; > int error; > > @@ -656,14 +676,8 @@ static int check_kill_permission(int sig, struct siginfo *info, > if (error) > return error; > > - cred = current_cred(); > - tcred = __task_cred(t); > if (!same_thread_group(current, t) && > - (cred->euid ^ tcred->suid) && > - (cred->euid ^ tcred->uid) && > - (cr…
creator: ebiederm
date: 2011-02-18T03:00:44-00:00

Re: [PATCH 3/9] allow sethostname in a container
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > kernel/sys.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/kernel/sys.c b/kernel/sys.c > index 18da702..7a1bbad 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -1177,7 +1177,7 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len) > int errno; > char tmp[__NEW_UTS_LEN]; > > - if (!capable(CAP_SYS_ADMIN)) > + if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) > return -EPERM; > if (len < 0 || len > __NEW_UTS_LEN) > return -EINVAL; _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: ebiederm
date: 2011-02-18T03:05:21-00:00

Re: [PATCH 7/9] add a user namespace owner of ipc ns
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > Changelog: > Feb 15: Don't set new ipc->user_ns if we didn't create a new > ipc_ns. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > include/linux/ipc_namespace.h | 3 +++ > ipc/msgutil.c | 3 +++ > ipc/namespace.c | 9 +++++++-- > kernel/nsproxy.c | 5 +++++ > 4 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h > index 5195298..46d2eb4 100644 > --- a/include/linux/ipc_namespace.h > +++ b/include/linux/ipc_namespace.h > @@ -24,6 +24,7 @@ struct ipc_ids { > struct idr ipcs_idr; > }; > > +struct user_namespace; > struct ipc_namespace { > atomic_t count; > struct ipc_ids ids[3]; > @@ -56,6 +57,8 @@ struct ipc_namespace { > unsigned int mq_msg_max; /* initialized to DFLT_MSGMAX */ > unsigned int mq_msgsize_max; /* initialized to DFLT_MSGSIZEMAX */ > > + /* user_ns which owns the ipc ns */ > + struct user_namespace *user_ns; > }; > > extern struct ipc_namespace init_ipc_ns; > diff --git a/ipc/msgutil.c b/ipc/msgutil.c > index f095ee2..d91ff4b 100644 > --- a/ipc/msgutil.c > +++ b/ipc/msgutil.c > @@ -20,6 +20,8 @@ > > DEFINE_SPINLOCK(mq_lock); > > +extern struct user_namespace init_user_ns; > + > /* > * The next 2 defines are here bc this is the only file > * compiled when either CONFIG_SYSVIPC and CONFIG_POSIX_MQUEUE > @@ -32,6 +34,7 @@ struct ipc_namespace init_ipc_ns = { > .mq_msg_max = DFLT_MSGMAX, > .mq_msgsize_max = DFLT_MSGSIZEMAX, > #endif > + .user_ns = &init_user_ns, > }; > > atomic_t nr_ipc_ns = ATOMIC_INIT(1); > diff --git a/ipc/namespace.c b/ipc/namespace.c > index a1094ff..aa18899 100644 > --- a/ipc/namespace.c > +++ b/ipc/namespace.c > @@ -11,10 +11,11 @@ > #include <linux/slab.h> > #include <linux/fs.h> > #include <linux/mount.h> > +#include <linux/user_namespace.h> > > #include "util.h" > > -static struct ipc_namespace *create_ipc_ns(void) > +static struct ipc_na…
creator: ebiederm
date: 2011-02-18T03:19:58-00:00

Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > copy_process() handles CLONE_NEWUSER before the rest of the > namespaces. So in the case of clone(CLONE_NEWUSER|CLONE_NEWUTS) > the new uts namespace will have the new user namespace as its > owner. That is what we want, since we want root in that new > userns to be able to have privilege over it. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Changelog: > Feb 15: don't set uts_ns->user_ns if we didn't create > a new uts_ns. > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > include/linux/utsname.h | 3 +++ > init/version.c | 2 ++ > kernel/nsproxy.c | 5 +++++ > kernel/user.c | 8 ++++++-- > kernel/utsname.c | 4 ++++ > 5 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/include/linux/utsname.h b/include/linux/utsname.h > index 69f3997..85171be 100644 > --- a/include/linux/utsname.h > +++ b/include/linux/utsname.h > @@ -37,9 +37,12 @@ struct new_utsname { > #include <linux/nsproxy.h> > #include <linux/err.h> > > +struct user_namespace; > + > struct uts_namespace { > struct kref kref; > struct new_utsname name; > + struct user_namespace *user_ns; > }; > extern struct uts_namespace init_uts_ns; > > diff --git a/init/version.c b/init/version.c > index adff586..97bb86f 100644 > --- a/init/version.c > +++ b/init/version.c > @@ -21,6 +21,7 @@ extern int version_string(LINUX_VERSION_CODE); > int version_string(LINUX_VERSION_CODE); > #endif > > +extern struct user_namespace init_user_ns; > struct uts_namespace init_uts_ns = { > .kref = { > .refcount = ATOMIC_INIT(2), > @@ -33,6 +34,7 @@ struct uts_namespace init_uts_ns = { > .machine = UTS_MACHINE, > .domainname = UTS_DOMAINNAME, > }, > + .user_ns = &init_user_ns, > }; > EXPORT_SYMBOL_GPL(init_uts_ns); > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c > index f74e6c0..034dc2e 100644 > --- a/kernel/nsproxy.c > +++ b/kernel/nsproxy.c > @@ -74,6 +74,11 @@ static struct nsproxy *create_new_namespaces(unsigned long flags, > err = PTR_ERR…
creator: ebiederm
date: 2011-02-18T03:31:04-00:00

Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > - Introduce ns_capable to test for a capability in a non-default > user namespace. > - Teach cap_capable to handle capabilities in a non-default > user namespace. > > The motivation is to get to the unprivileged creation of new > namespaces. It looks like this gets us 90% of the way there, with > only potential uid confusion issues left. > > I still need to handle getting all caps after creation but otherwise I > think I have a good starter patch that achieves all of your goals. Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > Changelog: > 11/05/2010: [serge] add apparmor > 12/14/2010: [serge] fix capabilities to created user namespaces > Without this, if user serge creates a user_ns, he won't have > capabilities to the user_ns he created. THis is because we > were first checking whether his effective caps had the caps > he needed and returning -EPERM if not, and THEN checking whether > he was the creator. Reverse those checks. > 12/16/2010: [serge] security_real_capable needs ns argument in !security case > 01/11/2011: [serge] add task_ns_capable helper > 01/11/2011: [serge] add nsown_capable() helper per Bastian Blank suggestion > 02/16/2011: [serge] fix a logic bug: the root user is always creator of > init_user_ns, but should not always have capabilities to > it! Fix the check in cap_capable(). > > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > --- > include/linux/capability.h | 10 ++++++++-- > include/linux/security.h | 25 ++++++++++++++----------- > kernel/capability.c | 32 ++++++++++++++++++++++++++++++-- > security/apparmor/lsm.c | 5 +++-- > security/commoncap.c | 40 +++++++++++++++++++++++++++++++++------- > security/security.c | 16 ++++++++++------ > security/selinux/hooks.c | 14 +++++++++----- > 7 files changed, 107 insertions(+), 35 deletions(-) > > diff --git a/include/linux/capability.h b/include/linux/capability.h > index f…
creator: ebiederm
date: 2011-02-18T03:46:09-00:00

Re: userns: targeted capabilities v5
type: item
description: Andrew Morton <akpm@linux-foundation.org> writes: > On Thu, 17 Feb 2011 15:02:24 +0000 > "Serge E. Hallyn" <serge@hallyn.com> wrote: > >> Here is a repost of my previous user namespace patch, ported onto >> last night's git head. >> >> It fixes several things I was doing wrong in the last (v4) >> posting, in particular: >> >> 1. don't set uts_ns->user_ns to current's when !CLONE_NEWUTS >> 2. add a ipc_ns->user_ns which owns ipc_ns, and use that to >> decide CAP_IPC_OWNER >> 3. fix logic flaw caused by bad parantheses >> 4. allow do_prlimit to current >> 5. don't always give root full privs to init_user_ns >> >> The expected course of development for user namespaces is laid out >> at https://wiki.ubuntu.com/UserNamespace. > > Seems like a nice feature to be developing. > > I worry about the maturity of it all at this stage. How far along is > it *really*? > > Is anyone else working with you on developing and reviewing this work? I did a lot of the initial design and I have been reviewing as I have time. Andrew at a practical level we have to merge this incrementally. Anything much bigger than Serge's current patchset will be too big to review. The first really bit step is making capabilities local to the user namespace, and that is what this patchset does along with using that localness in some good places. Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: ebiederm
date: 2011-02-18T03:53:37-00:00

Re: userns: targeted capabilities v5
type: item
description: Quoting Andrew Morton (akpm@linux-foundation.org): > On Thu, 17 Feb 2011 15:02:24 +0000 > "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > Here is a repost of my previous user namespace patch, ported onto > > last night's git head. > > > > It fixes several things I was doing wrong in the last (v4) > > posting, in particular: > > > > 1. don't set uts_ns->user_ns to current's when !CLONE_NEWUTS > > 2. add a ipc_ns->user_ns which owns ipc_ns, and use that to > > decide CAP_IPC_OWNER > > 3. fix logic flaw caused by bad parantheses > > 4. allow do_prlimit to current > > 5. don't always give root full privs to init_user_ns > > > > The expected course of development for user namespaces is laid out > > at https://wiki.ubuntu.com/UserNamespace. > > Seems like a nice feature to be developing. > > I worry about the maturity of it all at this stage. How far along is > it *really*? > > Is anyone else working with you on developing and reviewing this work? Thanks, Andrew. I'm not sure what definition of 'maturity' you were looking for here. If you meant completeness of the feature, it's definately not there. Of the goals for user namespaces sandboxing will be the quickest to mature. Completing that will largely be an exercise of running the breadth of the kernel looking for simple uid/gid comparisons and making them namespace aware. The design has been meshed around (publicly) on and off for many years by eric and I. This particular patchset has gotten some great reviews by Eric Biederman and Bastian Blank (to who, unfortunately, to this day I cannot send a direct email - they're always bounced). As Eric said, this feature will have to go in incrementally. Furthermore, each piece touches scary code so it's likely to go pretty slowly. My hope is less than a year for sandboxing, and two years for containers. It might go way faster, but experience tells me that's unlikely :) thanks, -serge _______________________________________________ Containers mailing list Containers@lists.linu…
creator: serge
date: 2011-02-18T04:28:15-00:00

Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: Quoting Eric W. Biederman (ebiederm@xmission.com): > "Serge E. Hallyn" <serge@hallyn.com> writes: > > > ptrace is allowed to tasks in the same user namespace according to > > the usual rules (i.e. the same rules as for two tasks in the init > > user namespace). ptrace is also allowed to a user namespace to > > which the current task the has CAP_SYS_PTRACE capability. > > > I don't see how it can go wrong at the moment but > same_or_ancestore_user_ns is too permissive and potentially inefficient. > Can you please replace it with a simple user namespace equality check. > > Eric > > > > Changelog: > > Dec 31: Address feedback by Eric: > > . Correct ptrace uid check > > . Rename may_ptrace_ns to ptrace_capable > > . Also fix the cap_ptrace checks. > > Jan 1: Use const cred struct > > Jan 11: use task_ns_capable() in place of ptrace_capable(). > > > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > > --- > > include/linux/capability.h | 2 + > > include/linux/user_namespace.h | 9 +++++++ > > kernel/ptrace.c | 27 ++++++++++++---------- > > kernel/user_namespace.c | 16 +++++++++++++ > > security/commoncap.c | 48 +++++++++++++++++++++++++++++++++------ > > 5 files changed, 82 insertions(+), 20 deletions(-) > > > > diff --git a/include/linux/capability.h b/include/linux/capability.h > > index cb3d2d9..bc0f262 100644 > > --- a/include/linux/capability.h > > +++ b/include/linux/capability.h > > @@ -546,6 +546,8 @@ extern const kernel_cap_t __cap_init_eff_set; > > */ > > #define has_capability(t, cap) (security_real_capable((t), &init_user_ns, (cap)) == 0) > > > > +#define has_ns_capability(t, ns, cap) (security_real_capable((t), (ns), (cap)) == 0) > > + > > /** > > * has_capability_noaudit - Determine if a task has a superior capability available (unaudited) > > * @t: The task in question > > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h > > index faf4679..862fc59 100644 > > --- a/include/linux/user_namespace.h > > +++ b/include/linu…
creator: serge
date: 2011-02-18T04:36:02-00:00

Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: On 02/17/2011 04:02 PM, Serge E. Hallyn wrote: > copy_process() handles CLONE_NEWUSER before the rest of the > namespaces. So in the case of clone(CLONE_NEWUSER|CLONE_NEWUTS) > the new uts namespace will have the new user namespace as its > owner. That is what we want, since we want root in that new > userns to be able to have privilege over it. > > Changelog: > Feb 15: don't set uts_ns->user_ns if we didn't create > a new uts_ns. > > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> A couple of comments. > --- > include/linux/utsname.h | 3 +++ > init/version.c | 2 ++ > kernel/nsproxy.c | 5 +++++ > kernel/user.c | 8 ++++++-- > kernel/utsname.c | 4 ++++ > 5 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/include/linux/utsname.h b/include/linux/utsname.h > index 69f3997..85171be 100644 > --- a/include/linux/utsname.h > +++ b/include/linux/utsname.h > @@ -37,9 +37,12 @@ struct new_utsname { > #include<linux/nsproxy.h> > #include<linux/err.h> > > +struct user_namespace; > + > struct uts_namespace { > struct kref kref; > struct new_utsname name; > + struct user_namespace *user_ns; > }; > extern struct uts_namespace init_uts_ns; > > diff --git a/init/version.c b/init/version.c > index adff586..97bb86f 100644 > --- a/init/version.c > +++ b/init/version.c > @@ -21,6 +21,7 @@ extern int version_string(LINUX_VERSION_CODE); > int version_string(LINUX_VERSION_CODE); > #endif > > +extern struct user_namespace init_user_ns; > struct uts_namespace init_uts_ns = { > .kref = { > .refcount = ATOMIC_INIT(2), > @@ -33,6 +34,7 @@ struct uts_namespace init_uts_ns = { > .machine = UTS_MACHINE, > .domainname = UTS_DOMAINNAME, > }, > + .user_ns =&init_user_ns, > }; > EXPORT_SYMBOL_GPL(init_uts_ns); > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c > index f74e6c0..034dc2e 100644 > --- a/kernel/nsproxy.c > +++ b/kernel/nsproxy.c > @@ -74,6 +74,11 @@ static struct nsproxy *create_new_namespaces(unsigned long flags,…
creator: Daniel Lezcano
date: 2011-02-18T16:57:41-00:00

Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: On 02/17/2011 04:03 PM, Serge E. Hallyn wrote: > - Introduce ns_capable to test for a capability in a non-default > user namespace. > - Teach cap_capable to handle capabilities in a non-default > user namespace. > > The motivation is to get to the unprivileged creation of new > namespaces. It looks like this gets us 90% of the way there, with > only potential uid confusion issues left. > > I still need to handle getting all caps after creation but otherwise I > think I have a good starter patch that achieves all of your goals. > > Changelog: > 11/05/2010: [serge] add apparmor > 12/14/2010: [serge] fix capabilities to created user namespaces > Without this, if user serge creates a user_ns, he won't have > capabilities to the user_ns he created. THis is because we > were first checking whether his effective caps had the caps > he needed and returning -EPERM if not, and THEN checking whether > he was the creator. Reverse those checks. > 12/16/2010: [serge] security_real_capable needs ns argument in !security case > 01/11/2011: [serge] add task_ns_capable helper > 01/11/2011: [serge] add nsown_capable() helper per Bastian Blank suggestion > 02/16/2011: [serge] fix a logic bug: the root user is always creator of > init_user_ns, but should not always have capabilities to > it! Fix the check in cap_capable(). > > Signed-off-by: Eric W. Biederman<ebiederm@xmission.com> > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> > --- Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-18T23:44:50-00:00

Re: [PATCH 3/9] allow sethostname in a container
type: item
description: On 02/17/2011 04:03 PM, Serge E. Hallyn wrote: > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> > --- Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-18T23:46:15-00:00

Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: On Thu, 17 Feb 2011 15:02:57 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > +/* > + * userns count is 1 for root user, 1 for init_uts_ns, > + * and 1 for... ? > + */ ? _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-18T23:59:12-00:00

Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: On Thu, 17 Feb 2011 15:03:06 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > - Introduce ns_capable to test for a capability in a non-default > user namespace. > - Teach cap_capable to handle capabilities in a non-default > user namespace. > > The motivation is to get to the unprivileged creation of new > namespaces. It looks like this gets us 90% of the way there, with > only potential uid confusion issues left. > > I still need to handle getting all caps after creation but otherwise I > think I have a good starter patch that achieves all of your goals. > > > ... > > --- a/include/linux/capability.h > +++ b/include/linux/capability.h > @@ -544,7 +544,7 @@ extern const kernel_cap_t __cap_init_eff_set; > * > * Note that this does not set PF_SUPERPRIV on the task. > */ > -#define has_capability(t, cap) (security_real_capable((t), (cap)) == 0) > +#define has_capability(t, cap) (security_real_capable((t), &init_user_ns, (cap)) == 0) > > /** > * has_capability_noaudit - Determine if a task has a superior capability available (unaudited) > @@ -558,9 +558,15 @@ extern const kernel_cap_t __cap_init_eff_set; > * Note that this does not set PF_SUPERPRIV on the task. > */ > #define has_capability_noaudit(t, cap) \ > - (security_real_capable_noaudit((t), (cap)) == 0) > + (security_real_capable_noaudit((t), &init_user_ns, (cap)) == 0) > > +struct user_namespace; > +extern struct user_namespace init_user_ns; Two icky-should-be-written-in-C macros which reference init_user_ns, followed by the declaration of init_user_ns and its type. Declarations which duplicate those in other header files. It's ripe for some upcleaning, methinks? Also, please ensure that the forward struct declarations are all at top-of-file (as in include/linux/security.h). Otherwise we can end up accumulating multiple forward declarations of the same thing in the one file. > extern int capable(int cap); > +extern int ns_capable(struct user_namespace *ns, int cap); > +extern int task_ns_capable(struct task_s…
creator: akpm
date: 2011-02-18T23:59:18-00:00

Re: [PATCH 4/9] allow killing tasks in your own or child userns
type: item
description: On Thu, 17 Feb 2011 15:03:25 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > /* > + * called with RCU read lock from check_kill_permission() > + */ > +static inline int kill_ok_by_cred(struct task_struct *t) > +{ > + const struct cred *cred = current_cred(); > + const struct cred *tcred = __task_cred(t); > + > + if (cred->user->user_ns == tcred->user->user_ns && > + (cred->euid == tcred->suid || > + cred->euid == tcred->uid || > + cred->uid == tcred->suid || > + cred->uid == tcred->uid)) > + return 1; > + > + if (ns_capable(tcred->user->user_ns, CAP_KILL)) > + return 1; > + > + return 0; > +} The compiler will inline this for us. > +/* > * Bad permissions for sending the signal > * - the caller must hold the RCU read lock > */ > static int check_kill_permission(int sig, struct siginfo *info, > struct task_struct *t) > { > - const struct cred *cred, *tcred; > struct pid *sid; > int error; > > @@ -656,14 +676,8 @@ static int check_kill_permission(int sig, struct siginfo *info, > if (error) > return error; > > - cred = current_cred(); > - tcred = __task_cred(t); > if (!same_thread_group(current, t) && > - (cred->euid ^ tcred->suid) && > - (cred->euid ^ tcred->uid) && > - (cred->uid ^ tcred->suid) && > - (cred->uid ^ tcred->uid) && > - !capable(CAP_KILL)) { > + !kill_ok_by_cred(t)) { > switch (sig) { > case SIGCONT: > sid = task_session(t); _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-18T23:59:21-00:00

Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: On Thu, 17 Feb 2011 15:03:33 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > ptrace is allowed to tasks in the same user namespace according to > the usual rules (i.e. the same rules as for two tasks in the init > user namespace). ptrace is also allowed to a user namespace to > which the current task the has CAP_SYS_PTRACE capability. > > > ... > > --- a/include/linux/capability.h > +++ b/include/linux/capability.h > @@ -546,6 +546,8 @@ extern const kernel_cap_t __cap_init_eff_set; > */ > #define has_capability(t, cap) (security_real_capable((t), &init_user_ns, (cap)) == 0) > > +#define has_ns_capability(t, ns, cap) (security_real_capable((t), (ns), (cap)) == 0) macroitis. > /** > * has_capability_noaudit - Determine if a task has a superior capability available (unaudited) > * @t: The task in question > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h > index faf4679..862fc59 100644 > --- a/include/linux/user_namespace.h > +++ b/include/linux/user_namespace.h > @@ -39,6 +39,9 @@ static inline void put_user_ns(struct user_namespace *ns) > uid_t user_ns_map_uid(struct user_namespace *to, const struct cred *cred, uid_t uid); > gid_t user_ns_map_gid(struct user_namespace *to, const struct cred *cred, gid_t gid); > > +int same_or_ancestor_user_ns(struct task_struct *task, > + struct task_struct *victim); bool. > #else > > static inline struct user_namespace *get_user_ns(struct user_namespace *ns) > > ... > > --- a/kernel/user_namespace.c > +++ b/kernel/user_namespace.c > @@ -129,6 +129,22 @@ gid_t user_ns_map_gid(struct user_namespace *to, const struct cred *cred, gid_t > return overflowgid; > } > > +int same_or_ancestor_user_ns(struct task_struct *task, > + struct task_struct *victim) > +{ > + struct user_namespace *u1 = task_cred_xxx(task, user)->user_ns; > + struct user_namespace *u2 = task_cred_xxx(victim, user)->user_ns; > + for (;;) { > + if (u1 == u2) > + return 1; > + if (u1 == &init_user_ns) > + return 0; > + u1 = u1->creator->us…
creator: akpm
date: 2011-02-18T23:59:25-00:00

Re: [PATCH 6/9] user namespaces: convert all capable checks in kernel/sys.c
type: item
description: On Thu, 17 Feb 2011 15:03:42 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > This allows setuid/setgid in containers. It also fixes some > corner cases where kernel logic foregoes capability checks when > uids are equivalent. The latter will need to be done throughout > the whole kernel. > > > ... > > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -118,17 +118,29 @@ EXPORT_SYMBOL(cad_pid); > > void (*pm_power_off_prepare)(void); > > +/* called with rcu_read_lock, creds are safe */ > +static inline int set_one_prio_perm(struct task_struct *p) > +{ > + const struct cred *cred = current_cred(), *pcred = __task_cred(p); > + > + if (pcred->user->user_ns == cred->user->user_ns && > + (pcred->uid == cred->euid || > + pcred->euid == cred->euid)) > + return 1; > + if (ns_capable(pcred->user->user_ns, CAP_SYS_NICE)) > + return 1; > + return 0; > +} uninline. Document return value? > > ... > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-18T23:59:28-00:00

Re: [PATCH 7/9] add a user namespace owner of ipc ns
type: item
description: On Thu, 17 Feb 2011 15:03:49 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > > ... > > --- a/include/linux/ipc_namespace.h > +++ b/include/linux/ipc_namespace.h > @@ -24,6 +24,7 @@ struct ipc_ids { > struct idr ipcs_idr; > }; > > +struct user_namespace; Move to top of file. > struct ipc_namespace { > atomic_t count; > struct ipc_ids ids[3]; > @@ -56,6 +57,8 @@ struct ipc_namespace { > unsigned int mq_msg_max; /* initialized to DFLT_MSGMAX */ > unsigned int mq_msgsize_max; /* initialized to DFLT_MSGSIZEMAX */ > > + /* user_ns which owns the ipc ns */ > + struct user_namespace *user_ns; > }; > > extern struct ipc_namespace init_ipc_ns; > diff --git a/ipc/msgutil.c b/ipc/msgutil.c > index f095ee2..d91ff4b 100644 > --- a/ipc/msgutil.c > +++ b/ipc/msgutil.c > @@ -20,6 +20,8 @@ > > DEFINE_SPINLOCK(mq_lock); > > +extern struct user_namespace init_user_ns; Should be declared in .h, not in .c. > /* > * The next 2 defines are here bc this is the only file > * compiled when either CONFIG_SYSVIPC and CONFIG_POSIX_MQUEUE > > ... > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-18T23:59:31-00:00

Re: [PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
type: item
description: On Thu, 17 Feb 2011 15:04:07 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > Cheat for now and say all files belong to init_user_ns. Next > step will be to let superblocks belong to a user_ns, and derive > inode_userns(inode) from inode->i_sb->s_user_ns. Finally we'll > introduce more flexible arrangements. > > > ... > > + > +/* > + * return 1 if current either has CAP_FOWNER to the > + * file, or owns the file. > + */ > +int is_owner_or_cap(const struct inode *inode) > +{ > + struct user_namespace *ns = inode_userns(inode); > + > + if (current_user_ns() == ns && current_fsuid() == inode->i_uid) > + return 1; > + if (ns_capable(ns, CAP_FOWNER)) > + return 1; > + return 0; > +} bool? > +EXPORT_SYMBOL(is_owner_or_cap); There's a fairly well adhered to convention that global symbols (and often static symbols) have a prefix which identifies the subsystem to which they belong. This patchset rather scorns that convention. Most of these identifiers are pretty obviously from the capability subsystem, but still... > > ... > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-18T23:59:35-00:00

Re: [PATCH 6/9] user namespaces: convert all capable checks in kernel/sys.c
type: item
description: On Thu, 17 Feb 2011 15:03:42 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > @@ -1177,8 +1189,11 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len) > int errno; > char tmp[__NEW_UTS_LEN]; > > - if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) > + if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) { > + printk(KERN_NOTICE "%s: did not have CAP_SYS_ADMIN\n", __func__); > return -EPERM; > + } > + printk(KERN_NOTICE "%s: did have CAP_SYS_ADMIN\n", __func__); > if (len < 0 || len > __NEW_UTS_LEN) > return -EINVAL; > down_write(&uts_sem); Left over debugging printks? _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-19T00:01:24-00:00

Re: [PATCH 4/9] allow killing tasks in your own or child userns
type: item
description: On 02/17/2011 04:03 PM, Serge E. Hallyn wrote: > Changelog: > Dec 8: Fixed bug in my check_kill_permission pointed out by > Eric Biederman. > Dec 13: Apply Eric's suggestion to pass target task into kill_ok_by_cred() > for clarity > Dec 31: address comment by Eric Biederman: > don't need cred/tcred in check_kill_permission. > Jan 1: use const cred struct. > Jan 11: Per Bastian Blank's advice, clean up kill_ok_by_cred(). > Feb 16: kill_ok_by_cred: fix bad parentheses > > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-19T10:55:01-00:00

Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: On 02/17/2011 04:03 PM, Serge E. Hallyn wrote: > ptrace is allowed to tasks in the same user namespace according to > the usual rules (i.e. the same rules as for two tasks in the init > user namespace). ptrace is also allowed to a user namespace to > which the current task the has CAP_SYS_PTRACE capability. > > Changelog: > Dec 31: Address feedback by Eric: > . Correct ptrace uid check > . Rename may_ptrace_ns to ptrace_capable > . Also fix the cap_ptrace checks. > Jan 1: Use const cred struct > Jan 11: use task_ns_capable() in place of ptrace_capable(). > > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-19T17:49:47-00:00

Re: [PATCH 6/9] user namespaces: convert all capable checks in kernel/sys.c
type: item
description: On 02/17/2011 04:03 PM, Serge E. Hallyn wrote: > This allows setuid/setgid in containers. It also fixes some > corner cases where kernel logic foregoes capability checks when > uids are equivalent. The latter will need to be done throughout > the whole kernel. > > Changelog: > Jan 11: Use nsown_capable() as suggested by Bastian Blank. > Jan 11: Fix logic errors in uid checks pointed out by Bastian. > Feb 15: allow prlimit to current (was regression in previous version) > > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> > > - if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) > + if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) { > + printk(KERN_NOTICE "%s: did not have CAP_SYS_ADMIN\n", __func__); > return -EPERM; > + } > + printk(KERN_NOTICE "%s: did have CAP_SYS_ADMIN\n", __func__); A couple of printk left here. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-19T17:52:46-00:00

Re: [PATCH 7/9] add a user namespace owner of ipc ns
type: item
description: On 02/17/2011 04:03 PM, Serge E. Hallyn wrote: > Changelog: > Feb 15: Don't set new ipc->user_ns if we didn't create a new > ipc_ns. > > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> > --- Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> [ ... ] > + ns->user_ns = old_ns->user_ns; > + get_user_ns(ns->user_ns); A mindless change. ns->user_ns = get_user_ns(old_ns->user_ns); _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-19T17:57:25-00:00

Re: [PATCH 8/9] user namespaces: convert several capable() calls
type: item
description: On 02/17/2011 04:03 PM, Serge E. Hallyn wrote: > CAP_IPC_OWNER and CAP_IPC_LOCK can be checked against current_user_ns(), > because the resource comes from current's own ipc namespace. > > setuid/setgid are to uids in own namespace, so again checks can be > against current_user_ns(). > > Changelog: > Jan 11: Use task_ns_capable() in place of sched_capable(). > Jan 11: Use nsown_capable() as suggested by Bastian Blank. > Jan 11: Clarify (hopefully) some logic in futex and sched.c > Feb 15: use ns_capable for ipc, not nsown_capable > > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> > --- Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-19T19:07:56-00:00

Re: [PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
type: item
description: On 02/17/2011 04:04 PM, Serge E. Hallyn wrote: > Cheat for now and say all files belong to init_user_ns. Next > step will be to let superblocks belong to a user_ns, and derive > inode_userns(inode) from inode->i_sb->s_user_ns. Finally we'll > introduce more flexible arrangements. > > Changelog: > Feb 15: make is_owner_or_cap take const struct inode > > Signed-off-by: Serge E. Hallyn<serge.hallyn@canonical.com> > --- Acked-by: Daniel Lezcano <daniel.lezcano@free.fr> _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Daniel Lezcano
date: 2011-02-19T19:22:39-00:00

Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: Serge E. Hallyn <serge@hallyn.com> wrote: > int (*capable) (struct task_struct *tsk, const struct cred *cred, > - int cap, int audit); > + struct user_namespace *ns, int cap, int audit); Hmmm... A chunk of the contents of the cred struct are user-namespaced. Could you add the user_namespace pointer to the cred struct and thus avoid passing it as an argument to other things. In fact, I think you probably have to do this so that cachefiles, for example, can override the user namespace when it operates on behalf of a process that's in another namespace. In fact, looking later on in your patch, I see: > - || (cap_capable(current, current_cred(), CAP_SETPCAP, > + || (cap_capable(current, current_cred(), > + current_cred()->user->user_ns, CAP_SETPCAP, so the user_ns _is_ already available through the creds. So is there really a need to pass it as an argument to anything that already takes a cred? > * @cred contains the credentials to use. > + * @ns contains the user namespace we want the capability in > * @cap contains the capability <include/linux/capability.h>. That should be tabbed to match the lines either side. David _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: David Howells
date: 2011-02-23T11:40:09-00:00

Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: David Howells <dhowells@redhat.com> wrote: > > int (*capable) (struct task_struct *tsk, const struct cred *cred, > > - int cap, int audit); > > + struct user_namespace *ns, int cap, int audit); > > Hmmm... A chunk of the contents of the cred struct are user-namespaced. > Could you add the user_namespace pointer to the cred struct and thus avoid > passing it as an argument to other things. Ah, no... Ignore that, I think I see that you do need it. > +int cap_capable(struct task_struct *tsk, const struct cred *cred, > + struct user_namespace *targ_ns, int cap, int audit) > { > - return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM; > + for (;;) { > + /* The creator of the user namespace has all caps. */ > + if (targ_ns != &init_user_ns && targ_ns->creator == cred->user) > + return 0; Why is that last comment so? Why should the creating namespace sport all possible capabilities? Do you have to have all capabilities available to you to be permitted create a new user namespace? Also, would it be worth having a separate cap_ns_capable()? Wouldn't most calls to cap_capable() only be checking the caps granted in the current user namespace? David _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: David Howells
date: 2011-02-23T12:01:57-00:00

User namespaces and keys
type: item
description: I guess we need to look at how to mix keys and namespaces again. Possibly the trickiest problem with keys is how to upcall key construction to /sbin/request-key when the keys may be of a different user namespace. David _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: David Howells
date: 2011-02-23T12:05:29-00:00

Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: Quoting David Howells (dhowells@redhat.com): > David Howells <dhowells@redhat.com> wrote: > > > > int (*capable) (struct task_struct *tsk, const struct cred *cred, > > > - int cap, int audit); > > > + struct user_namespace *ns, int cap, int audit); > > > > Hmmm... A chunk of the contents of the cred struct are user-namespaced. > > Could you add the user_namespace pointer to the cred struct and thus avoid > > passing it as an argument to other things. > > Ah, no... Ignore that, I think I see that you do need it. Thanks for reviewing, David. > > +int cap_capable(struct task_struct *tsk, const struct cred *cred, > > + struct user_namespace *targ_ns, int cap, int audit) > > { > > - return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM; > > + for (;;) { > > + /* The creator of the user namespace has all caps. */ > > + if (targ_ns != &init_user_ns && targ_ns->creator == cred->user) > > + return 0; > > Why is that last comment so? Why should the creating namespace sport all > possible capabilities? Do you have to have all capabilities available to you > to be permitted create a new user namespace? It's not the creating namespace, but the creating user, which has all caps. So for instance, if uid 500 in init_user_ns creates a namespace, then: a. uid 500 in init_user_ns has all caps to the child user_ns, so it can kill the tasks in the userns, clean up, etc. b. uid X in any other child user_ns has no caps to the child user_ns. c. root in init_user_ns has whatever capabilities are in his pE to the child user_ns. Again, this is so that the admin in any user_ns can clean up any messes made by users in his user_ns. One of the goals of the user namespaces it to make it safe to allow unprivileged users to create child user namespaces in which they have targeted privilege. Anything which happens in that child user namespace should be: a. constrained to resources which the user can control anyway b. able to be cleaned up by the user c. (especially) able to be cleaned up by the pr…
creator: serge
date: 2011-02-23T13:43:52-00:00

Re: User namespaces and keys
type: item
description: Quoting David Howells (dhowells@redhat.com): > > I guess we need to look at how to mix keys and namespaces again. >From strictly kernel pov, at the moment, keys are strictly usable only by the user in your own user namespace. We may want to look at this again, but for now I think that would be a safe enough default. Later, we'll probably want the user creating a child_user_ns to allow his keys to be inherited by the child user_ns. Though, as I type that, it seems to me that that'll just become a maintenance pain, and it's just plain safer to have the user re-enter his keys, sharing them over a file if needed. I'm going to not consider the TPM at the moment :) > Possibly the trickiest problem with keys is how to upcall key construction to > /sbin/request-key when the keys may be of a different user namespace. Hm, jinkeys, yes. -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: serge
date: 2011-02-23T13:58:14-00:00

Re: User namespaces and keys
type: item
description: "Serge E. Hallyn" <serge@hallyn.com> writes: > Quoting David Howells (dhowells@redhat.com): >> >> I guess we need to look at how to mix keys and namespaces again. > >>From strictly kernel pov, at the moment, keys are strictly usable only > by the user in your own user namespace. > > We may want to look at this again, but for now I think that would be a > safe enough default. Later, we'll probably want the user creating a > child_user_ns to allow his keys to be inherited by the child user_ns. > Though, as I type that, it seems to me that that'll just become a > maintenance pain, and it's just plain safer to have the user re-enter > his keys, sharing them over a file if needed. > > I'm going to not consider the TPM at the moment :) > >> Possibly the trickiest problem with keys is how to upcall key construction to >> /sbin/request-key when the keys may be of a different user namespace. > > Hm, jinkeys, yes. Serge short term this is where I think we need to add a check or two so that keys only work in the init_user_ns. When the rest of the details are sorted out we can open up the use of keys some more. As the first stage of converting the network stack we added patches that turned off everything in non init namespaces. Those patches were trivial and easy to review, and it made the conversion process a lot easier. I suspect for keys and possibly security modules and anything else that does is related we want to turn off by default. Then once the core of user namespaces is safe to unshare without privilege we can come back and get the more difficult bits. Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: ebiederm
date: 2011-02-23T14:46:00-00:00

Re: User namespaces and keys
type: item
description: Serge E. Hallyn <serge@hallyn.com> wrote: > > I guess we need to look at how to mix keys and namespaces again. > > From strictly kernel pov, at the moment, keys are strictly usable only > by the user in your own user namespace. I'm not sure that's currently completely true. Key quota maintenance is namespaced, and the key's owner UID/GID belong to that namespace, so that's okay, but: (*) key_task_permission() does not distinguish UIDs and GIDs from different namespaces. (*) A key can be referred to by its serial number, no matter whose namespace it is in, and will yield up its given UID/GID, even if these aren't actually meaningful in your namespace. This means request_key() can successfully upcall at the moment. I wonder if I should make the following changes: (1) If the key and the accessor are in different user namespaces, then skip the UID and GID comparisons in key_task_permission(). That means that to be able to access the key you'd have to possess the key and the key would have to grant you Possessor access, or the key would have to grant you Other access. (2) If the key and someone viewing the key description are in different namespaces, then indicate that the UID and the GID are -1, irrespective of the actual values. (3) When an upcall is attempting to instantiate a key, it is allowed to access the keys of requestor using the requestor's credentials (UID, GID, groups, security label). Ensure that this will be done in the requestor's user namespace. Nothing should need to be done here, since search_process_keyrings() switches to the requestor's creds. Oh, and are security labels user-namespaced? > We may want to look at this again, but for now I think that would be a > safe enough default. Later, we'll probably want the user creating a > child_user_ns to allow his keys to be inherited by the child user_ns. That depends what you mean by 'allow his keys to be inherited'. Do you mean copying all the creator's keys en mass? You may find all you need to do is to …
creator: David Howells
date: 2011-02-23T15:06:14-00:00

Re: User namespaces and keys
type: item
description: David Howells <dhowells@redhat.com> writes: > Serge E. Hallyn <serge@hallyn.com> wrote: > >> > I guess we need to look at how to mix keys and namespaces again. >> >> From strictly kernel pov, at the moment, keys are strictly usable only >> by the user in your own user namespace. > > I'm not sure that's currently completely true. Key quota maintenance is > namespaced, and the key's owner UID/GID belong to that namespace, so that's > okay, but: > > (*) key_task_permission() does not distinguish UIDs and GIDs from different > namespaces. > > (*) A key can be referred to by its serial number, no matter whose namespace > it is in, and will yield up its given UID/GID, even if these aren't > actually meaningful in your namespace. > > This means request_key() can successfully upcall at the moment. > > I wonder if I should make the following changes: > > (1) If the key and the accessor are in different user namespaces, then skip > the UID and GID comparisons in key_task_permission(). That means that to > be able to access the key you'd have to possess the key and the key would > have to grant you Possessor access, or the key would have to grant you > Other access. > > (2) If the key and someone viewing the key description are in different > namespaces, then indicate that the UID and the GID are -1, irrespective of > the actual values. > > (3) When an upcall is attempting to instantiate a key, it is allowed to access > the keys of requestor using the requestor's credentials (UID, GID, groups, > security label). Ensure that this will be done in the requestor's user > namespace. > > Nothing should need to be done here, since search_process_keyrings() > switches to the requestor's creds. > > Oh, and are security labels user-namespaced? Not at this time. The user namespace as currently merged is little more than a place holder for a proper implementation. Serge is busily fleshing out that proper implementation. Until we reach the point where all checks that have historically been…
creator: ebiederm
date: 2011-02-23T15:45:49-00:00

Re: User namespaces and keys
type: item
description: Quoting Eric W. Biederman (ebiederm@xmission.com): > David Howells <dhowells@redhat.com> writes: > > > Serge E. Hallyn <serge@hallyn.com> wrote: > > > >> > I guess we need to look at how to mix keys and namespaces again. > >> > >> From strictly kernel pov, at the moment, keys are strictly usable only > >> by the user in your own user namespace. > > > > I'm not sure that's currently completely true. Key quota maintenance is > > namespaced, and the key's owner UID/GID belong to that namespace, so that's > > okay, but: > > > > (*) key_task_permission() does not distinguish UIDs and GIDs from different > > namespaces. > > > > (*) A key can be referred to by its serial number, no matter whose namespace > > it is in, and will yield up its given UID/GID, even if these aren't > > actually meaningful in your namespace. > > > > This means request_key() can successfully upcall at the moment. > > > > I wonder if I should make the following changes: > > > > (1) If the key and the accessor are in different user namespaces, then skip > > the UID and GID comparisons in key_task_permission(). That means that to > > be able to access the key you'd have to possess the key and the key would > > have to grant you Possessor access, or the key would have to grant you > > Other access. > > > > (2) If the key and someone viewing the key description are in different > > namespaces, then indicate that the UID and the GID are -1, irrespective of > > the actual values. > > > > (3) When an upcall is attempting to instantiate a key, it is allowed to access > > the keys of requestor using the requestor's credentials (UID, GID, groups, > > security label). Ensure that this will be done in the requestor's user > > namespace. > > > > Nothing should need to be done here, since search_process_keyrings() > > switches to the requestor's creds. > > > > Oh, and are security labels user-namespaced? > > Not at this time. The user namespace as currently merged is little more > than a place holder for a proper…
creator: Serge E. Hallyn
date: 2011-02-23T15:53:29-00:00

Re: [PATCH 2/9] security: Make capabilities relative to the user namespace.
type: item
description: Serge E. Hallyn <serge@hallyn.com> wrote: > + /* If you have the capability in a parent user ns you have it > + * in the over all children user namespaces as well, so see > + * if this process has the capability in the parent user > + * namespace. > + */ "... in the over all children user namespaces ..." I think need a couple of words dropping. David _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: David Howells
date: 2011-02-23T16:59:19-00:00

Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: Serge E. Hallyn <serge@hallyn.com> wrote: > ptrace is allowed to tasks in the same user namespace according to There's a verb missing after 'allowed to' - presumably 'trace'. David _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: David Howells
date: 2011-02-23T17:05:41-00:00

Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: Serge E. Hallyn <serge@hallyn.com> wrote: > +int same_or_ancestor_user_ns(struct task_struct *task, > + struct task_struct *victim) > +{ > + struct user_namespace *u1 = task_cred_xxx(task, user)->user_ns; > + struct user_namespace *u2 = task_cred_xxx(victim, user)->user_ns; Hmmm. task_cred_xxx() uses task->real_cred, which is correct for victim (the object), but normally you'd use task->cred for task (the subject). However, in this case, I think it's probably okay. David _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: David Howells
date: 2011-02-23T17:11:01-00:00

Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: Serge E. Hallyn <serge@hallyn.com> wrote: > struct uts_namespace { > struct kref kref; > struct new_utsname name; > + struct user_namespace *user_ns; > }; If a uts_namespace belongs to a user_namespace, should CLONE_NEWUSER then imply CLONE_NEWUTS? Or is uts_namespace::user_ns more an implication that the set of users in user_namespace are the only ones authorised to alter the uts data. I presume that the uts_namespace of a process must be owned by one of the user_namespaces in the alternating inheritance chain of namespaces and their creators leading from current_user_ns() to init_user_ns. With that in mind, looking at patch 3: - if (!capable(CAP_SYS_ADMIN)) + if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) what is it you're actually asking? I presume it's 'does this user have CAP_SYS_ADMIN capability over objects belonging to the uts_namespace's user_namespace?' So, to look at the important bit of patch 2: -int cap_capable(struct task_struct *tsk, const struct cred *cred, int cap, - int audit) +int cap_capable(struct task_struct *tsk, const struct cred *cred, + struct user_namespace *targ_ns, int cap, int audit) { - return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM; + for (;;) { + /* The creator of the user namespace has all caps. */ + if (targ_ns != &init_user_ns && targ_ns->creator == cred->user) + return 0; + + /* Do we have the necessary capabilities? */ + if (targ_ns == cred->user->user_ns) + return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM; + + /* Have we tried all of the parent namespaces? */ + if (targ_ns == &init_user_ns) + return -EPERM; + + /* If you have the capability in a parent user ns you have it + * in the over all children user namespaces as well, so see + * if this process has the capability in the parent user + * namespace. + */ + targ_ns = targ_ns->creator->user_ns; + } + + /* We never get here */ + return -EPERM; } On entry, as we're called from ns_capable(), cred->user is the user that the current process is…
creator: David Howells
date: 2011-02-23T17:16:33-00:00

Re: User namespaces and keys
type: item
description: On 2/23/2011 7:53 AM, Serge E. Hallyn wrote: > Quoting Eric W. Biederman (ebiederm@xmission.com): >> David Howells <dhowells@redhat.com> writes: >> >>> Serge E. Hallyn <serge@hallyn.com> wrote: >>> >>>>> I guess we need to look at how to mix keys and namespaces again. >>>> From strictly kernel pov, at the moment, keys are strictly usable only >>>> by the user in your own user namespace. >>> I'm not sure that's currently completely true. Key quota maintenance is >>> namespaced, and the key's owner UID/GID belong to that namespace, so that's >>> okay, but: >>> >>> (*) key_task_permission() does not distinguish UIDs and GIDs from different >>> namespaces. >>> >>> (*) A key can be referred to by its serial number, no matter whose namespace >>> it is in, and will yield up its given UID/GID, even if these aren't >>> actually meaningful in your namespace. >>> >>> This means request_key() can successfully upcall at the moment. >>> >>> I wonder if I should make the following changes: >>> >>> (1) If the key and the accessor are in different user namespaces, then skip >>> the UID and GID comparisons in key_task_permission(). That means that to >>> be able to access the key you'd have to possess the key and the key would >>> have to grant you Possessor access, or the key would have to grant you >>> Other access. >>> >>> (2) If the key and someone viewing the key description are in different >>> namespaces, then indicate that the UID and the GID are -1, irrespective of >>> the actual values. >>> >>> (3) When an upcall is attempting to instantiate a key, it is allowed to access >>> the keys of requestor using the requestor's credentials (UID, GID, groups, >>> security label). Ensure that this will be done in the requestor's user >>> namespace. >>> >>> Nothing should need to be done here, since search_process_keyrings() >>> switches to the requestor's creds. >>> >>> Oh, and are security labels user-namespaced? >> Not at this time. The user namespace as currently merged is little m…
creator: Casey Schaufler
date: 2011-02-23T19:24:20-00:00

Re: User namespaces and keys
type: item
description: Casey Schaufler <casey@schaufler-ca.com> writes: > I confess that I remain less well educated on namespaces than > I probably should be, but with what I do know it seems that the > relationships between user namespaces and LSMs are bound to be > strained from the beginning. Some LSMs (SELinux and Smack) are > providing similar sandbox capabilities to what you get from user > namespaces, but from different directions and with different > use cases. Casey I won't argue about the possibility of things being strained, but I think if we focus on the semantics and not on the end goal of exactly how the pieces are to be used there can be some reasonable dialog. >From 10,000 feet an user namespace represents one administrative domain. uids, gids and other external tokens are all controlled by a single group with a single security policy. In that single administrative domain things like nfs are expected to work without translating uids and gids. Before the implementation of user namespaces a single kernel could not even express the ability of dealing with multiple user namespaces simultaneously (nfs has usually punted and said despite being multiple machines you still have to be in the same administrative domain so the same user identifiers can be used throughout). >From that principle we have the concept that use assigned/visible identifiers uid, gid, capabilities, security keys?, security labels? logically belong to the user namespace. Which means in implementing there are two pieces of work in implementing the user namespace. - Find all of the interesting comparisons and change them from "if (id == id)" to "if (userns == userns) && (id == id)". - Potentially define and handle what happens when you mix user namespaces. I think for the first round of this we simply want to define lsms and the user namespace as not mixing or the lsm interfaces only being visible if you are in the initial user namespace. Thus reserving the definition of what happens when you have lsms and mul…
creator: ebiederm
date: 2011-02-23T20:55:01-00:00

Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: David Howells <dhowells@redhat.com> writes: > Serge E. Hallyn <serge@hallyn.com> wrote: > >> struct uts_namespace { >> struct kref kref; >> struct new_utsname name; >> + struct user_namespace *user_ns; >> }; > > If a uts_namespace belongs to a user_namespace, should CLONE_NEWUSER then > imply CLONE_NEWUTS? > > Or is uts_namespace::user_ns more an implication that the set of users in > user_namespace are the only ones authorised to alter the uts data. The later. > I presume that the uts_namespace of a process must be owned by one of the > user_namespaces in the alternating inheritance chain of namespaces and their > creators leading from current_user_ns() to init_user_ns. > > With that in mind, looking at patch 3: > > - if (!capable(CAP_SYS_ADMIN)) > + if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) > > what is it you're actually asking? I presume it's 'does this user have > CAP_SYS_ADMIN capability over objects belonging to the uts_namespace's > user_namespace?' Yes. > So, to look at the important bit of patch 2: > > -int cap_capable(struct task_struct *tsk, const struct cred *cred, int cap, > - int audit) > +int cap_capable(struct task_struct *tsk, const struct cred *cred, > + struct user_namespace *targ_ns, int cap, int audit) > { > - return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM; > + for (;;) { > + /* The creator of the user namespace has all caps. */ > + if (targ_ns != &init_user_ns && targ_ns->creator == cred->user) > + return 0; > + > + /* Do we have the necessary capabilities? */ > + if (targ_ns == cred->user->user_ns) > + return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM; > + > + /* Have we tried all of the parent namespaces? */ > + if (targ_ns == &init_user_ns) > + return -EPERM; > + > + /* If you have the capability in a parent user ns you have it > + * in the over all children user namespaces as well, so see > + * if this process has the capability in the parent user > + * namespace. > + */ > + targ_ns = targ_ns->creat…
creator: ebiederm
date: 2011-02-23T21:21:28-00:00

Re: User namespaces and keys
type: item
description: On 2/23/2011 12:55 PM, Eric W. Biederman wrote: > Casey Schaufler <casey@schaufler-ca.com> writes: > >> I confess that I remain less well educated on namespaces than >> I probably should be, but with what I do know it seems that the >> relationships between user namespaces and LSMs are bound to be >> strained from the beginning. Some LSMs (SELinux and Smack) are >> providing similar sandbox capabilities to what you get from user >> namespaces, but from different directions and with different >> use cases. > Casey I won't argue about the possibility of things being strained, but > I think if we focus on the semantics and not on the end goal of exactly > how the pieces are to be used there can be some reasonable dialog. I'm sure that there will be cases where they will work together like horses in a troika. Making sensible semantics for the interactions is key, and it is entirely possible that in some cases a comparison of semantics and behaviors will lead an end user to chose either an LSM or namespaces over the combination. Just like I expect that even when we allow multiple LSMs the SELinux and Smack combination will be rare among the sane. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Casey Schaufler
date: 2011-02-23T21:37:16-00:00

Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: Eric W. Biederman <ebiederm@xmission.com> wrote: > > Which means that unless the uts_namespace belongs to our user_namespace, we > > cannot change it. Is that correct? > > No. If you are root in a parent namespace you can also change it. But surely, by definition, if you're a user in this namespace, you can't also be root in a parent namespace... For the case I worked through current_user() is a member of current_user_ns() and can't also be a member of its parent, grandparent, etc. - or can it? David _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: David Howells
date: 2011-02-23T23:19:08-00:00

Re: [PATCH 1/9] Add a user_namespace as creator/owner of uts_namespace
type: item
description: David Howells <dhowells@redhat.com> writes: > Eric W. Biederman <ebiederm@xmission.com> wrote: > >> > Which means that unless the uts_namespace belongs to our user_namespace, we >> > cannot change it. Is that correct? >> >> No. If you are root in a parent namespace you can also change it. > > But surely, by definition, if you're a user in this namespace, you can't also > be root in a parent namespace... To be clear the case you looked at was: > - if (!capable(CAP_SYS_ADMIN)) > + if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN)) > > what is it you're actually asking? I presume it's 'does this user have > CAP_SYS_ADMIN capability over objects belonging to the uts_namespace's > user_namespace?' Here "current->nsproxy->uts_ns->user_ns" (the target_ns value) is only refers to the uts_ns we are talking about. The user itself comes from current_user(). > For the case I worked through current_user() is a member of current_user_ns() > and can't also be a member of its parent, grandparent, etc. - or can > it? Right now if you are looking at current_user() because of limitations in the creation ordering I think you are correct. However in the near term pile of changes to merge, are the syscalls for joining an existing namespace. At which point there is no reason in general to suppose the current limitations of creation apply. Although it is conceivable that unshare of namespaces can also get you to someplace similar to joining prexisting namespaces. Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: ebiederm
date: 2011-02-23T23:54:12-00:00

Re: [PATCH 5/9] Allow ptrace from non-init user namespaces
type: item
description: Quoting Andrew Morton (akpm@linux-foundation.org): > On Thu, 17 Feb 2011 15:03:33 +0000 > "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > ptrace is allowed to tasks in the same user namespace according to > > the usual rules (i.e. the same rules as for two tasks in the init > > user namespace). ptrace is also allowed to a user namespace to > > which the current task the has CAP_SYS_PTRACE capability. > > > > > > ... > > > > --- a/include/linux/capability.h > > +++ b/include/linux/capability.h > > @@ -546,6 +546,8 @@ extern const kernel_cap_t __cap_init_eff_set; > > */ > > #define has_capability(t, cap) (security_real_capable((t), &init_user_ns, (cap)) == 0) > > > > +#define has_ns_capability(t, ns, cap) (security_real_capable((t), (ns), (cap)) == 0) > > macroitis. Thanks for the review, Andrew. Unfortunately this one is hard to turn into a function beecause it uses security_real_capable(), which is sometimes defined in security/security.c as a real function, and other times as a static inline in include/linux/security.h. So I'd have to #include security.h in capability.h, but security.h already #includes capability.h. All the other comments affect same_or_ancestor_user_ns(), which following Eric's feedback is going away. > > /** > > * has_capability_noaudit - Determine if a task has a superior capability available (unaudited) > > * @t: The task in question > > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h > > index faf4679..862fc59 100644 > > --- a/include/linux/user_namespace.h > > +++ b/include/linux/user_namespace.h > > @@ -39,6 +39,9 @@ static inline void put_user_ns(struct user_namespace *ns) > > uid_t user_ns_map_uid(struct user_namespace *to, const struct cred *cred, uid_t uid); > > gid_t user_ns_map_gid(struct user_namespace *to, const struct cred *cred, gid_t gid); > > > > +int same_or_ancestor_user_ns(struct task_struct *task, > > + struct task_struct *victim); > > bool. > > > #else > > > > static inline struct user_namespa…
creator: serge
date: 2011-02-24T00:43:37-00:00

Re: [PATCH 4/9] allow killing tasks in your own or child userns
type: item
description: Quoting Andrew Morton (akpm@linux-foundation.org): > On Thu, 17 Feb 2011 15:03:25 +0000 > "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > /* > > + * called with RCU read lock from check_kill_permission() > > + */ > > +static inline int kill_ok_by_cred(struct task_struct *t) > > +{ > > + const struct cred *cred = current_cred(); > > + const struct cred *tcred = __task_cred(t); > > + > > + if (cred->user->user_ns == tcred->user->user_ns && > > + (cred->euid == tcred->suid || > > + cred->euid == tcred->uid || > > + cred->uid == tcred->suid || > > + cred->uid == tcred->uid)) > > + return 1; > > + > > + if (ns_capable(tcred->user->user_ns, CAP_KILL)) > > + return 1; > > + > > + return 0; > > +} > > The compiler will inline this for us. Is that simply true with everything (worth inlining) nowadays, or is there a particular implicit hint to the compiler that'll make that happen? Not that I guess it's even particularly important in this case. From: Serge E. Hallyn <serge.hallyn@canonical.com> Date: Thu, 24 Feb 2011 00:26:02 +0000 Subject: [PATCH 1/2] userns: let compiler inline kill_ok_by_cred (per akpm) Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- kernel/signal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index ffe4bdf..12702b4 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -638,7 +638,7 @@ static inline bool si_fromuser(const struct siginfo *info) /* * called with RCU read lock from check_kill_permission() */ -static inline int kill_ok_by_cred(struct task_struct *t) +static int kill_ok_by_cred(struct task_struct *t) { const struct cred *cred = current_cred(); const struct cred *tcred = __task_cred(t); -- 1.7.0.4 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: serge
date: 2011-02-24T00:48:18-00:00

[PATCH] userns: ptrace: incorporate feedback from Eric
type: item
description: same_or_ancestore_user_ns() was not an appropriate check to constrain cap_issubset. Rather, cap_issubset() only is meaningful when both capsets are in the same user_ns. Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> --- include/linux/user_namespace.h | 9 --------- kernel/user_namespace.c | 16 ---------------- security/commoncap.c | 28 ++++++++++------------------ 3 files changed, 10 insertions(+), 43 deletions(-) diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h index 862fc59..faf4679 100644 --- a/include/linux/user_namespace.h +++ b/include/linux/user_namespace.h @@ -39,9 +39,6 @@ static inline void put_user_ns(struct user_namespace *ns) uid_t user_ns_map_uid(struct user_namespace *to, const struct cred *cred, uid_t uid); gid_t user_ns_map_gid(struct user_namespace *to, const struct cred *cred, gid_t gid); -int same_or_ancestor_user_ns(struct task_struct *task, - struct task_struct *victim); - #else static inline struct user_namespace *get_user_ns(struct user_namespace *ns) @@ -69,12 +66,6 @@ static inline gid_t user_ns_map_gid(struct user_namespace *to, return gid; } -static inline int same_or_ancestor_user_ns(struct task_struct *task, - struct task_struct *victim) -{ - return 1; -} - #endif #endif /* _LINUX_USER_H */ diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index 0ef2258..9da289c 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -129,22 +129,6 @@ gid_t user_ns_map_gid(struct user_namespace *to, const struct cred *cred, gid_t return overflowgid; } -int same_or_ancestor_user_ns(struct task_struct *task, - struct task_struct *victim) -{ - struct user_namespace *u1 = task_cred_xxx(task, user)->user_ns; - struct user_namespace *u2 = task_cred_xxx(victim, user)->user_ns; - for (;;) { - if (u1 == u2) - return 1; - if (u1 == &init_user_ns) - return 0; - u1 = u1->creator->user_ns; - } - /* We never get here */ - return 0; -} - static __init int user_namespaces_init(void) { user_ns_cachep = K…
creator: serge
date: 2011-02-24T00:49:01-00:00

Re: [PATCH 4/9] allow killing tasks in your own or child userns
type: item
description: On Thu, 24 Feb 2011 00:48:18 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > Quoting Andrew Morton (akpm@linux-foundation.org): > > On Thu, 17 Feb 2011 15:03:25 +0000 > > "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > > > /* > > > + * called with RCU read lock from check_kill_permission() > > > + */ > > > +static inline int kill_ok_by_cred(struct task_struct *t) > > > +{ > > > + const struct cred *cred = current_cred(); > > > + const struct cred *tcred = __task_cred(t); > > > + > > > + if (cred->user->user_ns == tcred->user->user_ns && > > > + (cred->euid == tcred->suid || > > > + cred->euid == tcred->uid || > > > + cred->uid == tcred->suid || > > > + cred->uid == tcred->uid)) > > > + return 1; > > > + > > > + if (ns_capable(tcred->user->user_ns, CAP_KILL)) > > > + return 1; > > > + > > > + return 0; > > > +} > > > > The compiler will inline this for us. > > Is that simply true with everything (worth inlining) nowadays, or is > there a particular implicit hint to the compiler that'll make that > happen? We've basically stopped inlining things nowadays. gcc inlines aggressively and sometimes we have to use noinline to stop it. Also, modern gcc's like to ignore the inline directive anwyay, so we have to resort to __always_inline when we disagree. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-24T00:54:30-00:00

Re: [PATCH] userns: ptrace: incorporate feedback from Eric
type: item
description: On Thu, 24 Feb 2011 00:49:01 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > same_or_ancestore_user_ns() was not an appropriate check to > constrain cap_issubset. Rather, cap_issubset() only is > meaningful when both capsets are in the same user_ns. I queued this as a fix against userns-allow-ptrace-from-non-init-user-namespaces.patch, but I get the feeling that it would be better to just drop everything and start again? _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-24T00:56:51-00:00

Re: [PATCH] userns: ptrace: incorporate feedback from Eric
type: item
description: Quoting Andrew Morton (akpm@linux-foundation.org): > On Thu, 24 Feb 2011 00:49:01 +0000 > "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > same_or_ancestore_user_ns() was not an appropriate check to > > constrain cap_issubset. Rather, cap_issubset() only is > > meaningful when both capsets are in the same user_ns. > > I queued this as a fix against > userns-allow-ptrace-from-non-init-user-namespaces.patch, but I get the > feeling that it would be better to just drop everything and start > again? Sure, I'll rebase and resend. I wonder if I should trim the Cc list for the next round. thanks, -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: serge
date: 2011-02-24T03:15:18-00:00

Re: [PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
type: item
description: Quoting Andrew Morton (akpm@linux-foundation.org): > On Thu, 17 Feb 2011 15:04:07 +0000 > "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > Cheat for now and say all files belong to init_user_ns. Next > > step will be to let superblocks belong to a user_ns, and derive > > inode_userns(inode) from inode->i_sb->s_user_ns. Finally we'll > > introduce more flexible arrangements. > > > > > > ... > > > > + > > +/* > > + * return 1 if current either has CAP_FOWNER to the > > + * file, or owns the file. > > + */ > > +int is_owner_or_cap(const struct inode *inode) > > +{ > > + struct user_namespace *ns = inode_userns(inode); > > + > > + if (current_user_ns() == ns && current_fsuid() == inode->i_uid) > > + return 1; > > + if (ns_capable(ns, CAP_FOWNER)) > > + return 1; > > + return 0; > > +} > > bool? > > > +EXPORT_SYMBOL(is_owner_or_cap); > > There's a fairly well adhered to convention that global symbols (and > often static symbols) have a prefix which identifies the subsystem to > which they belong. This patchset rather scorns that convention. > > Most of these identifiers are pretty obviously from the capability > subsystem, but still... Would 'inode_owner_or_capable' be better and and make sense? -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: serge
date: 2011-02-24T03:24:16-00:00

Re: [PATCH 9/9] userns: check user namespace for task-&gt;file uid equivalence checks
type: item
description: On Thu, 24 Feb 2011 03:24:16 +0000 "Serge E. Hallyn" <serge@hallyn.com> wrote: > Quoting Andrew Morton (akpm@linux-foundation.org): > > On Thu, 17 Feb 2011 15:04:07 +0000 > > "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > > There's a fairly well adhered to convention that global symbols (and > > often static symbols) have a prefix which identifies the subsystem to > > which they belong. This patchset rather scorns that convention. > > > > Most of these identifiers are pretty obviously from the capability > > subsystem, but still... > > Would 'inode_owner_or_capable' be better and and make sense? > I suppose so. We've totally screwed that pooch in the VFS (grep EXPORT fs/inode.c). But it's never to late to start. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: akpm
date: 2011-02-24T05:08:08-00:00

Re: User namespaces and keys
type: item
description: Casey Schaufler <casey@schaufler-ca.com> writes: > On 2/23/2011 12:55 PM, Eric W. Biederman wrote: >> Casey Schaufler <casey@schaufler-ca.com> writes: >> >>> I confess that I remain less well educated on namespaces than >>> I probably should be, but with what I do know it seems that the >>> relationships between user namespaces and LSMs are bound to be >>> strained from the beginning. Some LSMs (SELinux and Smack) are >>> providing similar sandbox capabilities to what you get from user >>> namespaces, but from different directions and with different >>> use cases. >> Casey I won't argue about the possibility of things being strained, but >> I think if we focus on the semantics and not on the end goal of exactly >> how the pieces are to be used there can be some reasonable dialog. > > I'm sure that there will be cases where they will work together > like horses in a troika. Making sensible semantics for the interactions > is key, and it is entirely possible that in some cases a comparison > of semantics and behaviors will lead an end user to chose either an > LSM or namespaces over the combination. Just like I expect that even > when we allow multiple LSMs the SELinux and Smack combination will be > rare among the sane. That sounds about right. Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: ebiederm
date: 2011-02-24T06:56:01-00:00