diff mbox

[02/15] user ns: setns: move capable checks into per-ns attach helper

Message ID 1314993400-6910-5-git-send-email-serge@hallyn.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Serge E. Hallyn Sept. 2, 2011, 7:56 p.m. UTC
From: "Serge E. Hallyn" <serge@hallyn.com>

Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
 ipc/namespace.c          |    7 +++++++
 kernel/fork.c            |    5 +++++
 kernel/nsproxy.c         |   11 ++++++++---
 kernel/utsname.c         |    7 +++++++
 net/core/net_namespace.c |    7 +++++++
 5 files changed, 34 insertions(+), 3 deletions(-)

Comments

Matt Helsley Sept. 4, 2011, 1:51 a.m. UTC | #1
On Fri, Sep 02, 2011 at 07:56:27PM +0000, Serge Hallyn wrote:
> From: "Serge E. Hallyn" <serge@hallyn.com>

I was confused about this patch until I realized that you're not
simply "moving" the capability checks but "distributing" them. Then
you're showing that you'll soon change some to nsown_capable() or
ns_capable() using the strange cpp pattern in the snippet below.

At least I think that's what you intended. A commit message would
help :).

Cheers,
	-Matt Helsley

> 
> Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  ipc/namespace.c          |    7 +++++++
>  kernel/fork.c            |    5 +++++
>  kernel/nsproxy.c         |   11 ++++++++---
>  kernel/utsname.c         |    7 +++++++
>  net/core/net_namespace.c |    7 +++++++
>  5 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/ipc/namespace.c b/ipc/namespace.c
> index ce0a647..a0a7609 100644
> --- a/ipc/namespace.c
> +++ b/ipc/namespace.c
> @@ -163,6 +163,13 @@ static void ipcns_put(void *ns)
> 
>  static int ipcns_install(struct nsproxy *nsproxy, void *ns)
>  {
> +#if 0
> +	struct ipc_namespace *newns = ns;
> +	if (!ns_capable(newns->user_ns, CAP_SYS_ADMIN))
> +#else
> +	if (!capable(CAP_SYS_ADMIN))
> +#endif
> +		return -1;
>  	/* Ditch state from the old ipc namespace */
>  	exit_sem(current);
>  	put_ipc_ns(nsproxy->ipc_ns);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Serge E. Hallyn Sept. 9, 2011, 2:56 p.m. UTC | #2
Quoting Matt Helsley (matthltc@us.ibm.com):
> On Fri, Sep 02, 2011 at 07:56:27PM +0000, Serge Hallyn wrote:
> > From: "Serge E. Hallyn" <serge@hallyn.com>
> 
> I was confused about this patch until I realized that you're not
> simply "moving" the capability checks but "distributing" them. Then
> you're showing that you'll soon change some to nsown_capable() or
> ns_capable() using the strange cpp pattern in the snippet below.
> 
> At least I think that's what you intended. A commit message would
> help :).

Yes, sorry - Eric convinced me several times to be more conservative in
the patch, and I failed to fix the commit msg when squashing the
resulting patches.  How about the following:

======

user ns: update capable calls when cloning and attaching namespaces

Distribute the capable() checks at ns attach into the namespace-specific
attach handler.

Note the fact that the capable() checks will be changed to targeted
checks at both namespace clone and attach methods, but don't actually
make that change yet.  Until that trigger is pulled, you must have
the capabilities targeted toward the initial user namespace in order to
do any of these actions, meaning that a task in a child user namespace
cannot do them.  Once we pull the trigger, a task in a child user
namespace will be able to clone new namespaces if it is privileged in
its own user namespace, and attach to existing namespaces to which it
has privilege.

======

Thanks for taking a look, Matt!

-serge
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/ipc/namespace.c b/ipc/namespace.c
index ce0a647..a0a7609 100644
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -163,6 +163,13 @@  static void ipcns_put(void *ns)
 
 static int ipcns_install(struct nsproxy *nsproxy, void *ns)
 {
+#if 0
+	struct ipc_namespace *newns = ns;
+	if (!ns_capable(newns->user_ns, CAP_SYS_ADMIN))
+#else
+	if (!capable(CAP_SYS_ADMIN))
+#endif
+		return -1;
 	/* Ditch state from the old ipc namespace */
 	exit_sem(current);
 	put_ipc_ns(nsproxy->ipc_ns);
diff --git a/kernel/fork.c b/kernel/fork.c
index 8e6b6f4..ca712f5 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1489,8 +1489,13 @@  long do_fork(unsigned long clone_flags,
 		/* hopefully this check will go away when userns support is
 		 * complete
 		 */
+#if 0
+		if (!nsown_capable(CAP_SYS_ADMIN) || !nsown_capable(CAP_SETUID) ||
+				!nsown_capable(CAP_SETGID))
+#else
 		if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SETUID) ||
 				!capable(CAP_SETGID))
+#endif
 			return -EPERM;
 	}
 
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index 9aeab4b..e274577 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -134,7 +134,11 @@  int copy_namespaces(unsigned long flags, struct task_struct *tsk)
 				CLONE_NEWPID | CLONE_NEWNET)))
 		return 0;
 
+#if 0
+	if (!nsown_capable(CAP_SYS_ADMIN)) {
+#else
 	if (!capable(CAP_SYS_ADMIN)) {
+#endif
 		err = -EPERM;
 		goto out;
 	}
@@ -191,7 +195,11 @@  int unshare_nsproxy_namespaces(unsigned long unshare_flags,
 			       CLONE_NEWNET)))
 		return 0;
 
+#if 0
+	if (!nsown_capable(CAP_SYS_ADMIN))
+#else
 	if (!capable(CAP_SYS_ADMIN))
+#endif
 		return -EPERM;
 
 	*new_nsp = create_new_namespaces(unshare_flags, current,
@@ -241,9 +249,6 @@  SYSCALL_DEFINE2(setns, int, fd, int, nstype)
 	struct file *file;
 	int err;
 
-	if (!capable(CAP_SYS_ADMIN))
-		return -EPERM;
-
 	file = proc_ns_fget(fd);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
diff --git a/kernel/utsname.c b/kernel/utsname.c
index bff131b..4638a54 100644
--- a/kernel/utsname.c
+++ b/kernel/utsname.c
@@ -104,6 +104,13 @@  static void utsns_put(void *ns)
 
 static int utsns_install(struct nsproxy *nsproxy, void *ns)
 {
+#if 0
+	struct uts_namespace *newns = ns;
+	if (!ns_capable(newns->user_ns, CAP_SYS_ADMIN))
+#else
+	if (!capable(CAP_SYS_ADMIN))
+#endif
+		return -1;
 	get_uts_ns(ns);
 	put_uts_ns(nsproxy->uts_ns);
 	nsproxy->uts_ns = ns;
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 5bbdbf0..6f6698d 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -620,6 +620,13 @@  static void netns_put(void *ns)
 
 static int netns_install(struct nsproxy *nsproxy, void *ns)
 {
+#if 0
+	struct net *net = ns;
+	if (!ns_capable(net->user_ns, CAP_SYS_ADMIN))
+#else
+	if (capable(CAP_SYS_ADMIN))
+#endif
+		return -1;
 	put_net(nsproxy->net_ns);
 	nsproxy->net_ns = get_net(ns);
 	return 0;