All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Serge E. Hallyn" <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>
To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Cc: Linux Containers
	<containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>,
	David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 2/4] userns: Require CAP_SYS_ADMIN for most uses of setns.
Date: Fri, 14 Dec 2012 23:35:24 +0000	[thread overview]
Message-ID: <20121214233524.GA13659@mail.hallyn.com> (raw)
In-Reply-To: <87hanoxpdh.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org):
> 
> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> found a nasty little bug in
> the permissions of setns.  With unprivileged user namespaces it
> became possible to create new namespaces without privilege.
> 
> However the setns calls were relaxed to only require CAP_SYS_ADMIN in
> the user nameapce of the targed namespace.
> 
> Which made the following nasty sequence possible.
> 
> pid = clone(CLONE_NEWUSER | CLONE_NEWNS);
> if (pid == 0) { /* child */
> 	system("mount --bind /home/me/passwd /etc/passwd");
> }
> else if (pid != 0) { /* parent */
> 	char path[PATH_MAX];
> 	snprintf(path, sizeof(path), "/proc/%u/ns/mnt");
> 	fd = open(path, O_RDONLY);
> 	setns(fd, 0);
> 	system("su -");
> }
>
> Prevent this possibility by requiring CAP_SYS_ADMIN
> in the current user namespace when joing all but the user namespace.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

> ---
>  fs/namespace.c           |    3 ++-
>  ipc/namespace.c          |    3 ++-
>  kernel/pid_namespace.c   |    3 ++-
>  kernel/utsname.c         |    3 ++-
>  net/core/net_namespace.c |    3 ++-
>  5 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index c1bbe86..398a50f 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2781,7 +2781,8 @@ static int mntns_install(struct nsproxy *nsproxy, void *ns)
>  	struct path root;
>  
>  	if (!ns_capable(mnt_ns->user_ns, CAP_SYS_ADMIN) ||
> -	    !nsown_capable(CAP_SYS_CHROOT))
> +	    !nsown_capable(CAP_SYS_CHROOT) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	if (fs->users != 1)
> diff --git a/ipc/namespace.c b/ipc/namespace.c
> index cf3386a..7c1fa45 100644
> --- a/ipc/namespace.c
> +++ b/ipc/namespace.c
> @@ -170,7 +170,8 @@ static void ipcns_put(void *ns)
>  static int ipcns_install(struct nsproxy *nsproxy, void *new)
>  {
>  	struct ipc_namespace *ns = new;
> -	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* Ditch state from the old ipc namespace */
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index 560da0d..fdbd0cd 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -325,7 +325,8 @@ static int pidns_install(struct nsproxy *nsproxy, void *ns)
>  	struct pid_namespace *active = task_active_pid_ns(current);
>  	struct pid_namespace *ancestor, *new = ns;
>  
> -	if (!ns_capable(new->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(new->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/*
> diff --git a/kernel/utsname.c b/kernel/utsname.c
> index f6336d5..08b197e 100644
> --- a/kernel/utsname.c
> +++ b/kernel/utsname.c
> @@ -113,7 +113,8 @@ static int utsns_install(struct nsproxy *nsproxy, void *new)
>  {
>  	struct uts_namespace *ns = new;
>  
> -	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	get_uts_ns(ns);
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 2e9a313..8acce01 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -649,7 +649,8 @@ static int netns_install(struct nsproxy *nsproxy, void *ns)
>  {
>  	struct net *net = ns;
>  
> -	if (!ns_capable(net->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(net->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	put_net(nsproxy->net_ns);
> -- 
> 1.7.5.4

WARNING: multiple messages have this Message-ID (diff)
From: "Serge E. Hallyn" <serge@hallyn.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Linux Containers <containers@lists.linux-foundation.org>,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Andy Lutomirski <luto@amacapital.net>,
	David Howells <dhowells@redhat.com>
Subject: Re: [PATCH 2/4] userns: Require CAP_SYS_ADMIN for most uses of setns.
Date: Fri, 14 Dec 2012 23:35:24 +0000	[thread overview]
Message-ID: <20121214233524.GA13659@mail.hallyn.com> (raw)
In-Reply-To: <87hanoxpdh.fsf@xmission.com>

Quoting Eric W. Biederman (ebiederm@xmission.com):
> 
> Andy Lutomirski <luto@amacapital.net> found a nasty little bug in
> the permissions of setns.  With unprivileged user namespaces it
> became possible to create new namespaces without privilege.
> 
> However the setns calls were relaxed to only require CAP_SYS_ADMIN in
> the user nameapce of the targed namespace.
> 
> Which made the following nasty sequence possible.
> 
> pid = clone(CLONE_NEWUSER | CLONE_NEWNS);
> if (pid == 0) { /* child */
> 	system("mount --bind /home/me/passwd /etc/passwd");
> }
> else if (pid != 0) { /* parent */
> 	char path[PATH_MAX];
> 	snprintf(path, sizeof(path), "/proc/%u/ns/mnt");
> 	fd = open(path, O_RDONLY);
> 	setns(fd, 0);
> 	system("su -");
> }
>
> Prevent this possibility by requiring CAP_SYS_ADMIN
> in the current user namespace when joing all but the user namespace.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Acked-by: Serge Hallyn <serge.hallyn@canonical.com>

> ---
>  fs/namespace.c           |    3 ++-
>  ipc/namespace.c          |    3 ++-
>  kernel/pid_namespace.c   |    3 ++-
>  kernel/utsname.c         |    3 ++-
>  net/core/net_namespace.c |    3 ++-
>  5 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index c1bbe86..398a50f 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2781,7 +2781,8 @@ static int mntns_install(struct nsproxy *nsproxy, void *ns)
>  	struct path root;
>  
>  	if (!ns_capable(mnt_ns->user_ns, CAP_SYS_ADMIN) ||
> -	    !nsown_capable(CAP_SYS_CHROOT))
> +	    !nsown_capable(CAP_SYS_CHROOT) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	if (fs->users != 1)
> diff --git a/ipc/namespace.c b/ipc/namespace.c
> index cf3386a..7c1fa45 100644
> --- a/ipc/namespace.c
> +++ b/ipc/namespace.c
> @@ -170,7 +170,8 @@ static void ipcns_put(void *ns)
>  static int ipcns_install(struct nsproxy *nsproxy, void *new)
>  {
>  	struct ipc_namespace *ns = new;
> -	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* Ditch state from the old ipc namespace */
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index 560da0d..fdbd0cd 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -325,7 +325,8 @@ static int pidns_install(struct nsproxy *nsproxy, void *ns)
>  	struct pid_namespace *active = task_active_pid_ns(current);
>  	struct pid_namespace *ancestor, *new = ns;
>  
> -	if (!ns_capable(new->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(new->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/*
> diff --git a/kernel/utsname.c b/kernel/utsname.c
> index f6336d5..08b197e 100644
> --- a/kernel/utsname.c
> +++ b/kernel/utsname.c
> @@ -113,7 +113,8 @@ static int utsns_install(struct nsproxy *nsproxy, void *new)
>  {
>  	struct uts_namespace *ns = new;
>  
> -	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	get_uts_ns(ns);
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 2e9a313..8acce01 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -649,7 +649,8 @@ static int netns_install(struct nsproxy *nsproxy, void *ns)
>  {
>  	struct net *net = ns;
>  
> -	if (!ns_capable(net->user_ns, CAP_SYS_ADMIN))
> +	if (!ns_capable(net->user_ns, CAP_SYS_ADMIN) ||
> +	    !nsown_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	put_net(nsproxy->net_ns);
> -- 
> 1.7.5.4

  parent reply	other threads:[~2012-12-14 23:35 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-14 22:01 [PATCH 0/4] user namespace fixes Eric W. Biederman
2012-12-14 22:01 ` Eric W. Biederman
     [not found] ` <87txroxpgq.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-12-14 22:03   ` [PATCH 1/4] Fix cap_capable to only allow owners in the parent user namespace to have caps Eric W. Biederman
2012-12-14 22:03   ` [PATCH 2/4] userns: Require CAP_SYS_ADMIN for most uses of setns Eric W. Biederman
2012-12-14 22:04   ` [PATCH 3/4] userns: Add a more complete capability subset test to commit_creds Eric W. Biederman
2012-12-14 22:05   ` [PATCH 4/4] userns: Fix typo in description of the limitation of userns_install Eric W. Biederman
2012-12-14 22:05     ` Eric W. Biederman
2012-12-14 23:36     ` Serge E. Hallyn
     [not found]     ` <876244xpbj.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-12-14 23:36       ` Serge E. Hallyn
2012-12-17 19:08       ` Andy Lutomirski
2012-12-17 19:08         ` Andy Lutomirski
2012-12-17 19:03   ` [PATCH 0/4] user namespace fixes Andy Lutomirski
2012-12-17 19:03     ` Andy Lutomirski
     [not found]     ` <CALCETrX2Fa-DuM+wkgsij7oiJXzCD8W6Phkv-MjgCDg_Ma6CTg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-12-17 21:01       ` Eric W. Biederman
2012-12-17 21:01     ` Eric W. Biederman
2012-12-14 22:03 ` [PATCH 1/4] Fix cap_capable to only allow owners in the parent user namespace to have caps Eric W. Biederman
2012-12-14 22:03 ` [PATCH 2/4] userns: Require CAP_SYS_ADMIN for most uses of setns Eric W. Biederman
2012-12-17 19:03   ` Andy Lutomirski
     [not found]   ` <87hanoxpdh.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-12-14 23:35     ` Serge E. Hallyn [this message]
2012-12-14 23:35       ` Serge E. Hallyn
2012-12-17 19:03     ` Andy Lutomirski
2012-12-14 22:04 ` [PATCH 3/4] userns: Add a more complete capability subset test to commit_creds Eric W. Biederman
2012-12-15  0:03   ` Serge E. Hallyn
     [not found]     ` <20121215000338.GC13659-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
2012-12-15  0:11       ` Eric W. Biederman
2012-12-15  0:11     ` Eric W. Biederman
     [not found]       ` <87r4msrx6t.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-12-15  0:47         ` Serge E. Hallyn
2012-12-15  0:47           ` Serge E. Hallyn
     [not found]           ` <20121215004735.GA14295-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
2012-12-15  0:48             ` Eric W. Biederman
2012-12-15  0:48               ` Eric W. Biederman
2012-12-15  2:06               ` Serge E. Hallyn
     [not found]               ` <87lid0rvh9.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-12-15  2:06                 ` Serge E. Hallyn
2012-12-17 19:08                 ` Andy Lutomirski
2012-12-17 19:08               ` Andy Lutomirski
     [not found]   ` <87bodwxpcg.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-12-15  0:03     ` Serge E. Hallyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121214233524.GA13659@mail.hallyn.com \
    --to=serge-a9i7lubdfnhqt0dzr+alfa@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.