| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20110804220115.GA28300@sergelap>
Date: Thu, 4 Aug 2011 17:01:15 -0500
From: "Serge E. Hallyn" <serge.hallyn@...onical.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: "Serge E. Hallyn" <serge@...lyn.com>, dhowells@...hat.com,
netdev@...r.kernel.org, containers@...ts.linux-foundation.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/14] allow root in container to copy namespaces (v3)
Quoting Eric W. Biederman (ebiederm@...ssion.com):
> "Serge E. Hallyn" <serge.hallyn@...onical.com> writes:
>
> > Quoting Eric W. Biederman (ebiederm@...ssion.com):
> >> The dangers of changing the namespace of a process remain the same,
> >> confused suid programs. I don't believe there are any unique new
> >> dangers.
> >>
> >> Not allowing joining namespaces you already have a copy of is just
> >> a matter of making it hard to get things wrong.
> >>
> >> I would feel more a bit more comfortable if the way we did this was
> >> to move all of the capable calls into the per namespace methods
> >> and then changed them one namespace at a time. I don't think
> >
> > The patch belows moves them into the per namespace methods, for
> > what it's worth. If you like I can change them, for now, to
> > 'capable(CAP_SYS_ADMIN)' targeted at init_user_ns, but if we're
> > targetting at the userns owning the destination namespace, it
> > seems this must be sufficient...
>
> I like the was this was done. I was mostly thinking of the non
> setns case when I was talking about moving the calls.
Oh, you mean unshare and copy namespaces?
(The flow on those paths is scary to touch :)
> >> there are any fundmanetal dangers of allowing unshare without
> >> the global CAP_SYS_ADMIN, but it would be good to be able to audit
> >
> > If you have suspicions that there may in fact be dangers, then
> > perhaps this whole patch should be delayed, and copy_namespaces()
> > and unshare_nsproxy_namespaces() should continue to check global
> > CAP_SYS_ADMIN? The only part which would remain would be the
> > moving of the setns capable check into the per-ns ->install
> > method, but it would check the global CAP_SYS_ADMIN?
>
> Yes. I am in favor of delaying this and making the changes one
> namespace at a time. I don't think there are real dangers but I do
> think we should try and think through the possible dangers.
Ok, so for now here is a patch to fold into the previous one
which I think sets us at a reasonable point.
>From 78e1a4efa464086e8df95fc3ffd35c385e363957 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn@...onical.com>
Date: Thu, 4 Aug 2011 22:10:12 +0100
Subject: [PATCH 1/2] fold up - dont yet target the capable checks for
namespace manipulation
Signed-off-by: Serge Hallyn <serge.hallyn@...onical.com>
---
ipc/namespace.c | 4 ++++
kernel/fork.c | 5 +++++
kernel/nsproxy.c | 8 ++++++++
kernel/utsname.c | 4 ++++
net/core/net_namespace.c | 4 ++++
5 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/ipc/namespace.c b/ipc/namespace.c
index f527e49..a0a7609 100644
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -163,8 +163,12 @@ 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);
diff --git a/kernel/fork.c b/kernel/fork.c
index f9fac70..a25343c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1488,8 +1488,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 62a995d..752b477 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -136,7 +136,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;
}
@@ -193,7 +197,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,
diff --git a/kernel/utsname.c b/kernel/utsname.c
index 8f648cc..4638a54 100644
--- a/kernel/utsname.c
+++ b/kernel/utsname.c
@@ -104,8 +104,12 @@ 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);
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 8778a0a..5ca95cc 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -623,8 +623,12 @@ 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);
--
1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists