diff mbox

[RFC,4/5] invoke path_chroot() LSM hook on mntns_install()

Message ID 1469777680-3687-5-git-send-email-elena.reshetova@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Reshetova, Elena July 29, 2016, 7:34 a.m. UTC
This adds an additional invocation of the
security_path_chroot LSM hook inside mntns_install().
Currently only capabilities are checked at this point,
while process root actually changes.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
 fs/namespace.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Comments

Jann Horn July 29, 2016, 6:11 p.m. UTC | #1
On Fri, Jul 29, 2016 at 10:34:39AM +0300, Elena Reshetova wrote:
> This adds an additional invocation of the
> security_path_chroot LSM hook inside mntns_install().
> Currently only capabilities are checked at this point,
> while process root actually changes.

Are you aware that unprivileged user namespace creation doesn't work in
a chrooted process? See the invocation of current_chrooted() in
create_user_ns(). This means that for this new LSM hook to make any
sense, a namespace admin has to attempt to sandbox himself with chroot().

If the current namespace is the init namespace, the process has
CAP_SYS_ADMIN in the init namespace, meaning that filesystem sandboxing
is probably useless.

If the current namespace is not the init namespace, the process probably
used namespaces to sandbox itself, in which case it wouldn't be using
chroot in the first place, or it is running in a container with admin
privileges. In the latter case, this mitigation miiight make a
difference, I'm not sure exactly how powerful the APIs for namespace
admins are - but a mitigation that only makes a difference inside
containers would be weird anyway.

So: What is your specific usecase here?
Reshetova, Elena July 31, 2016, 10:39 a.m. UTC | #2
On Fri, Jul 29, 2016 at 10:34:39AM +0300, Elena Reshetova wrote:
> This adds an additional invocation of the security_path_chroot LSM 
> hook inside mntns_install().
> Currently only capabilities are checked at this point, while process 
> root actually changes.

>Are you aware that unprivileged user namespace creation doesn't work in a
chrooted process? See the invocation of current_chrooted() in
create_user_ns(). This means that for this new LSM hook to make any sense, a
namespace admin has to attempt >to sandbox himself with chroot().

I am not sure I understand you fully here. It is possible to create new
mount namespace without creating new user namespace, and when this happens,
if I understand the code right, there is no check like current_chrooted() or
smth like this. 
So, how does it relate to user namespace? 

>If the current namespace is the init namespace, the process has
CAP_SYS_ADMIN in the init namespace, meaning that filesystem sandboxing is
probably useless.
>If the current namespace is not the init namespace, the process probably
used namespaces to sandbox itself, in which case it wouldn't be using chroot
in the first place, 
Why? It is possible that we might be running in the namespace some daemon
which just uses chroot by default (legacy or whatever). I think you even
proposed that in another email. 

>r it is running in a container with admin privileges. 
>In the latter case, this mitigation miiight make a difference, I'm not sure
exactly how powerful the APIs for namespace admins are - but a mitigation
that only makes a difference inside containers would be weird anyway.
>So: What is your specific usecase here?

The usecase was similar to the unshare cases: we need to find places where
current process's root changes in the code and make sure we catch this
change in order to have the up-to-date information about each process's
root.  
So, if we have a process that was put into chroot, but then also creates a
mount ns inside? If we recorded the dentry of chrooted process , but then
not update it upon mount ns creation, we might end up with wrong entry and
all kinds of problems around it. 

But yes, the whole relationship with namespaces is tricky and there are sooo
many edge cases. I will try to create a list of possible combinations  and
use cases. It is true that this hardening LSM has a particular set of
use-cases in mind and there is no need to support all possible combinations.
I haven't even tested this at all with namespaces yet, which would be
separate fun on its own...
Jann Horn July 31, 2016, 11:29 a.m. UTC | #3
On Sun, Jul 31, 2016 at 10:39:04AM +0000, Reshetova, Elena wrote:
> On Fri, Jul 29, 2016 at 10:34:39AM +0300, Elena Reshetova wrote:
> > This adds an additional invocation of the security_path_chroot LSM 
> > hook inside mntns_install().
> > Currently only capabilities are checked at this point, while process 
> > root actually changes.
> 
> >Are you aware that unprivileged user namespace creation doesn't work in a
> chrooted process? See the invocation of current_chrooted() in
> create_user_ns(). This means that for this new LSM hook to make any sense, a
> namespace admin has to attempt >to sandbox himself with chroot().
> 
> I am not sure I understand you fully here. It is possible to create new
> mount namespace without creating new user namespace, and when this happens,
> if I understand the code right, there is no check like current_chrooted() or
> smth like this. 
> So, how does it relate to user namespace? 

You can only create a new mount namespace if you're privileged relative to
your current namespace. See the second and third invocation of ns_capable() in
mntns_install(). To get those privileges, you have to either be privileged
already or unshare the user namespace to get new namespaced privileges.
An unprivileged, chrooted process doesn't have existing privileges and can't
unshare the user namespace to get new ones, so it can't unshare other
namespaces (like the mount namespace) anymore.


> >If the current namespace is the init namespace, the process has
> CAP_SYS_ADMIN in the init namespace, meaning that filesystem sandboxing is
> probably useless.
> >If the current namespace is not the init namespace, the process probably
> used namespaces to sandbox itself, in which case it wouldn't be using chroot
> in the first place, 
> Why? It is possible that we might be running in the namespace some daemon
> which just uses chroot by default (legacy or whatever). I think you even
> proposed that in another email. 

True.
Reshetova, Elena Aug. 1, 2016, 9:26 a.m. UTC | #4
On Sun, Jul 31, 2016 at 10:39:04AM +0000, Reshetova, Elena wrote:
> On Fri, Jul 29, 2016 at 10:34:39AM +0300, Elena Reshetova wrote:
> > This adds an additional invocation of the security_path_chroot LSM 
> > hook inside mntns_install().
> > Currently only capabilities are checked at this point, while process 
> > root actually changes.
> 
> >Are you aware that unprivileged user namespace creation doesn't work 
> >in a
> chrooted process? See the invocation of current_chrooted() in 
> create_user_ns(). This means that for this new LSM hook to make any 
> sense, a namespace admin has to attempt >to sandbox himself with chroot().
> 
> I am not sure I understand you fully here. It is possible to create 
> new mount namespace without creating new user namespace, and when this 
> happens, if I understand the code right, there is no check like 
> current_chrooted() or smth like this.
> So, how does it relate to user namespace? 

>You can only create a new mount namespace if you're privileged relative to
your current namespace. See the second and third invocation of ns_capable()
in mntns_install(). To get those privileges, you have to either be
privileged already or unshare the user namespace to get new namespaced
privileges.
An unprivileged, chrooted process doesn't have existing privileges and can't
unshare the user namespace to get new ones, so it can't unshare other
namespaces (like the mount namespace) anymore.

Yes, all true, I just got confused that you said that checks for chroot are
done. So, yes, you need to have CAP_SYS_ADMIN in current namespace, and then
you can get your new mount ns. 
After thinking more on this, I think I will get rid of this altogether. Yes,
there is a case when a process first enters a new user ns, then enters
chroot and forgets to drop its caps. Then it runs for a while, gets
exploited and now it tries to get out of chroot. It can create a new mount
ns since it has CAP_SYS_ADMIN in his user ns, but what is the outcome?
Probably not much, it certainly doesn't help it much from chroot
perspective. And I can't think of a reason why a legitimate process would
want to create a new namespace inside the chroot....
diff mbox

Patch

diff --git a/fs/namespace.c b/fs/namespace.c
index 419f746..c5dcb09 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3325,6 +3325,7 @@  static int mntns_install(struct nsproxy *nsproxy, struct ns_common *ns)
 	struct fs_struct *fs = current->fs;
 	struct mnt_namespace *mnt_ns = to_mnt_ns(ns);
 	struct path root;
+	int retval;
 
 	if (!ns_capable(mnt_ns->user_ns, CAP_SYS_ADMIN) ||
 	    !ns_capable(current_user_ns(), CAP_SYS_CHROOT) ||
@@ -3334,10 +3335,6 @@  static int mntns_install(struct nsproxy *nsproxy, struct ns_common *ns)
 	if (fs->users != 1)
 		return -EINVAL;
 
-	get_mnt_ns(mnt_ns);
-	put_mnt_ns(nsproxy->mnt_ns);
-	nsproxy->mnt_ns = mnt_ns;
-
 	/* Find the root */
 	root.mnt    = &mnt_ns->root->mnt;
 	root.dentry = mnt_ns->root->mnt.mnt_root;
@@ -3345,6 +3342,16 @@  static int mntns_install(struct nsproxy *nsproxy, struct ns_common *ns)
 	while(d_mountpoint(root.dentry) && follow_down_one(&root))
 		;
 
+	retval = security_path_chroot(&root);
+	if (retval) {
+		path_put(&root);
+		return retval;
+	}
+
+	get_mnt_ns(mnt_ns);
+	put_mnt_ns(nsproxy->mnt_ns);
+	nsproxy->mnt_ns = mnt_ns;
+
 	/* Update the pwd and root */
 	set_fs_pwd(fs, &root);
 	set_fs_root(fs, &root);