Subject: Re: [vserver] NetApp filers and NFS junctions: Operation not permitted [RFC]
From:Cédric Dufour - Idiap Research Institute <cedric.dufour@idiap.ch>
Date: Thu, 23 Jul 2009 18:46:51 +0200
Thu, 23 Jul 2009 18:46:51 +0200
Hello again,

Here is what I had in mind (see attached patch - I hope it will make it
through the ML - and pasted relevant part below). Some rapid preliminary
tests seem to indicate it is working as expected.

Can some Kernel/VServer/security guru comment on those changes ?

Thank you very much!

Bests

Cédric Dufour @ Idiap Research Institute


===== MODIFIED PATCH =====

diff -NurpP --minimal linux-2.6.30.1/fs/super.c
linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/super.c
--- linux-2.6.30.1/fs/super.c   2009-06-11 17:13:08.000000000 +0200
+++ linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/super.c        2009-07-04
01:11:39.000000000 +0200
@@ -39,6 +39,7 @@
 #include <linux/mutex.h>
 #include <linux/file.h>
 #include <linux/async.h>
+#include <linux/vs context.h>
 #include <asm/uaccess.h>
 #include "internal.h"

@@ -1018,7 +1019,13 @@ vfs kern mount(struct file system type *
                goto out free secdata;
        BUG ON(!mnt->mnt sb);

-       error = security sb kern mount(mnt->mnt sb, flags, secdata);
+       error = -EPERM;
+       if (!vx capable(CAP SYS ADMIN, VXC BINARY MOUNT) &&
+               (type->fs flags & FS REQUIRES DEV) &&
+               !mnt->mnt sb->s bdev)
+               goto out sb;
+
+       error = security sb kern mount(mnt->mnt sb, flags, secdata);
        if (error)
                goto out sb;

@@ -1070,6 +1077,11 @@ do kern mount(const char *fstype, int fl
        struct vfsmount *mnt;
        if (!type)
                return ERR PTR(-ENODEV);
+
+       if ((type->fs flags & FS BINARY MOUNTDATA) &&
+               !vx capable(CAP SYS ADMIN, VXC BINARY MOUNT))
+               return ERR PTR(-EPERM);
+
        mnt = vfs kern mount(type, flags, name, data);
        if (!IS ERR(mnt) && (type->fs flags & FS HAS SUBTYPE) &&
            !mnt->mnt sb->s subtype)


On 23/07/09 15:22, Cédric Dufour - Idiap Research Institute wrote:
> Hello again,
>
> After some research in the kernel tree, it seems that the
> 'vfs kern mount' function gets called directly:
>  - to handle CIFS DFS (in function 'cifs dfs do refmount')
>  - to handle AFS automounts (in function 'afs mntpt do automount')
>  - to handle NFS "junctions" (in function 'nfs follow mountpoint')
> In other words, for all file systems that require some in-kernel
> "sub-mounting", and which should NOT be blocked in anyway (provided the
> "root" mount point has been successfully mounted, see below)
>
> Indirectly (via 'do kern mount'), to performs actual "out-of-kernel"
> mount requests.
>
> Indirectly (via 'kern mount data', with a MS KERNMOUNT flag that is used
> subsequently by 'selinux sb kern mount' to relax security) for:
>  - '/proc' sub-system
>  - IPC sub-system
>
> This suggests that the capability checks achieved in 'vfs kern mount'
> would - maybe - be better fitted and adapted in the calling
> functions/contexts:
>  - in 'do new mount' for actual file systems mount (where we find an
> existing 'capable('CAP SYS ADMIN')' test ;-) )
>  - ... I don't know about /proc and IPC ... (but aren't handling the
> former with the 'PROC SUPER MAGIC' flag ?)
>
> What do you think ?
>
> Cédric Dufour @ Idiap Research Institute
>
>
>
> On 23/07/09 13:19, Cédric Dufour - Idiap Research Institute wrote:
>   
>> Hello,
>>
>> We are currently putting our new NetApp ONTAP GX system into
>> production. From the NFS point of view, the particularity of this
>> product is that it allows to "assemble" multiple volumes within a
>> "unified" namespace, using so-called "junctions", and access this
>> "unified" namespace with a (apparently) single mount operation.
>>
>> Now, when using VServer - even from the host point of view - only the
>> root user is able to "roam" through the mounted hierarchy (as separate
>> volumes get automatically mounted in the background and appear in
>> '/proc/mounts'). Non-root user get a "Operation not permitted" message
>> as soon as they try to go pass the initially mounted volume.
>>
>> This problem seem absolutely similar to what was discussed in this
>> thread: http://www.paul.sladen.org/vserver/archives/200709/0046.html
>>
>> We have verified that removing the VServer-added checks in the
>> 'vfs kern mount' function (in '<kernel-source>/fs/super.c') "solves"
>> the problem (we used the latest 2.6.30.2 vanilla kernel and
>> 2.3.0.36.14pre4 VServer patches to perform this verification).
>>
>> Now to the questions:
>>
>>  1. What is the security impact of removing those checks (iow. is it a
>> "MUST NOT!!!" or a "well... that's being overkill anyway" ) ?
>>
>>  2. Can we imagine to perform those checks slightly differently, in
>> order to allow non-root users to use such "joined" NFS exports, both
>> from the host and guests (since we're no kernel gurus, we don't know
>> which function/capability to start looking for) ?
>>
>> Thank you very much for your help and time.
>>
>> Cheers
>>
>> -- 
>>
>> Cédric Dufour @ Idiap Research Institute
>>
>> Phone:  +41 27 721 77 40
>> Fax:    +41 27 721 77 12
>> Mail:   Idiap Research Institute
>>         Case postale 592
>>         Centre du Parc
>>         1920 Martigny (VS)
>>         Suisse (Switzerland)
>> Web:    www.idiap.ch <http://www.idiap.ch>
>>
>>     
>
>   


Hello again,

Here is what I had in mind (see attached patch - I hope it will make it through the ML - and pasted relevant part below). Some rapid preliminary tests seem to indicate it is working as expected.

Can some Kernel/VServer/security guru comment on those changes ?

Thank you very much!

Bests

Cédric Dufour @ Idiap Research Institute

Cédric Dufour @ Idiap Research Institute


===== MODIFIED PATCH =====

diff -NurpP --minimal linux-2.6.30.1/fs/super.c linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/super.c
--- linux-2.6.30.1/fs/super.c   2009-06-11 17:13:08.000000000 +0200
+++ linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/super.c        2009-07-04 01:11:39.000000000 +0200
@@ -39,6 +39,7 @@
 #include <linux/mutex.h>
 #include <linux/file.h>
 #include <linux/async.h>
+#include <linux/vs_context.h>
 #include <asm/uaccess.h>
 #include "internal.h"

@@ -1018,7 +1019,13 @@ vfs_kern_mount(struct file_system_type *
                goto out_free_secdata;
        BUG_ON(!mnt->mnt_sb);

-       error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
+       error = -EPERM;
+       if (!vx_capable(CAP_SYS_ADMIN, VXC_BINARY_MOUNT) &&
+               (type->fs_flags & FS_REQUIRES_DEV) &&
+               !mnt->mnt_sb->s_bdev)
+               goto out_sb;
+
+       error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
        if (error)
                goto out_sb;

@@ -1070,6 +1077,11 @@ do_kern_mount(const char *fstype, int fl
        struct vfsmount *mnt;
        if (!type)
                return ERR_PTR(-ENODEV);
+
+       if ((type->fs_flags & FS_BINARY_MOUNTDATA) &&
+               !vx_capable(CAP_SYS_ADMIN, VXC_BINARY_MOUNT))
+               return ERR_PTR(-EPERM);
+
        mnt = vfs_kern_mount(type, flags, name, data);
        if (!IS_ERR(mnt) && (type->fs_flags & FS_HAS_SUBTYPE) &&
            !mnt->mnt_sb->s_subtype)


On 23/07/09 15:22, Cédric Dufour - Idiap Research Institute wrote:
Hello again,

After some research in the kernel tree, it seems that the
'vfs_kern_mount' function gets called directly:
 - to handle CIFS DFS (in function 'cifs_dfs_do_refmount')
 - to handle AFS automounts (in function 'afs_mntpt_do_automount')
 - to handle NFS "junctions" (in function 'nfs_follow_mountpoint')
In other words, for all file systems that require some in-kernel
"sub-mounting", and which should NOT be blocked in anyway (provided the
"root" mount point has been successfully mounted, see below)

Indirectly (via 'do_kern_mount'), to performs actual "out-of-kernel"
mount requests.

Indirectly (via 'kern_mount_data', with a MS_KERNMOUNT flag that is used
subsequently by 'selinux_sb_kern_mount' to relax security) for:
 - '/proc' sub-system
 - IPC sub-system

This suggests that the capability checks achieved in 'vfs_kern_mount'
would - maybe - be better fitted and adapted in the calling
functions/contexts:
 - in 'do_new_mount' for actual file systems mount (where we find an
existing 'capable('CAP_SYS_ADMIN')' test ;-) )
 - ... I don't know about /proc and IPC ... (but aren't handling the
former with the 'PROC_SUPER_MAGIC' flag ?)

What do you think ?

Cédric Dufour @ Idiap Research Institute



On 23/07/09 13:19, Cédric Dufour - Idiap Research Institute wrote:
  
Hello,

We are currently putting our new NetApp ONTAP GX system into
production. From the NFS point of view, the particularity of this
product is that it allows to "assemble" multiple volumes within a
"unified" namespace, using so-called "junctions", and access this
"unified" namespace with a (apparently) single mount operation.

Now, when using VServer - even from the host point of view - only the
root user is able to "roam" through the mounted hierarchy (as separate
volumes get automatically mounted in the background and appear in
'/proc/mounts'). Non-root user get a "Operation not permitted" message
as soon as they try to go pass the initially mounted volume.

This problem seem absolutely similar to what was discussed in this
thread: http://www.paul.sladen.org/vserver/archives/200709/0046.html

We have verified that removing the VServer-added checks in the
'vfs_kern_mount' function (in '<kernel-source>/fs/super.c') "solves"
the problem (we used the latest 2.6.30.2 vanilla kernel and
2.3.0.36.14pre4 VServer patches to perform this verification).

Now to the questions:

 1. What is the security impact of removing those checks (iow. is it a
"MUST NOT!!!" or a "well... that's being overkill anyway" ) ?

 2. Can we imagine to perform those checks slightly differently, in
order to allow non-root users to use such "joined" NFS exports, both
from the host and guests (since we're no kernel gurus, we don't know
which function/capability to start looking for) ?

Thank you very much for your help and time.

Cheers

-- 

Cédric Dufour @ Idiap Research Institute

Phone:  +41 27 721 77 40
Fax:    +41 27 721 77 12
Mail:   Idiap Research Institute
        Case postale 592
        Centre du Parc
        1920 Martigny (VS)
        Suisse (Switzerland)
Web:    www.idiap.ch <http://www.idiap.ch>

    

  

--- patch-2.6.30.1-vs2.3.0.36.14-pre4.diff	2009-07-23 17:00:41.000000000 +0200
+++ patch-2.6.30-debian-vs2.3.0.36.14-idiap.diff	2009-07-23 17:44:55.000000000 +0200
@@ -6752,55 +6752,41 @@
 diff -NurpP --minimal linux-2.6.30.1/fs/super.c linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/super.c
 --- linux-2.6.30.1/fs/super.c	2009-06-11 17:13:08.000000000 +0200
 +++ linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/super.c	2009-07-04 01:11:39.000000000 +0200
-@@ -39,6 +39,9 @@
+@@ -39,6 +39,7 @@
  #include <linux/mutex.h>
  #include <linux/file.h>
  #include <linux/async.h>
-+#include <linux/devpts_fs.h>
-+#include <linux/proc_fs.h>
 +#include <linux/vs_context.h>
  #include <asm/uaccess.h>
  #include "internal.h"
  
-@@ -992,12 +995,18 @@ struct vfsmount *
- vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void *data)
- {
- 	struct vfsmount *mnt;
-+	struct super_block *sb;
- 	char *secdata = NULL;
- 	int error;
- 
- 	if (!type)
- 		return ERR_PTR(-ENODEV);
- 
-+	error = -EPERM;
-+	if ((type->fs_flags & FS_BINARY_MOUNTDATA) &&
-+		!vx_capable(CAP_SYS_ADMIN, VXC_BINARY_MOUNT))
-+		goto out;
-+
- 	error = -ENOMEM;
- 	mnt = alloc_vfsmnt(name);
- 	if (!mnt)
-@@ -1016,9 +1025,17 @@ vfs_kern_mount(struct file_system_type *
- 	error = type->get_sb(type, flags, name, data, mnt);
- 	if (error < 0)
+@@ -1018,7 +1019,13 @@ vfs_kern_mount(struct file_system_type *
  		goto out_free_secdata;
--	BUG_ON(!mnt->mnt_sb);
+ 	BUG_ON(!mnt->mnt_sb);
  
 - 	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
-+	sb = mnt->mnt_sb;
-+	BUG_ON(!sb);
-+
 +	error = -EPERM;
-+	if (!vx_capable(CAP_SYS_ADMIN, VXC_BINARY_MOUNT) && !sb->s_bdev &&
-+		(sb->s_magic != PROC_SUPER_MAGIC) &&
-+		(sb->s_magic != DEVPTS_SUPER_MAGIC))
++	if (!vx_capable(CAP_SYS_ADMIN, VXC_BINARY_MOUNT) &&
++		(type->fs_flags & FS_REQUIRES_DEV) && 
++		!mnt->mnt_sb->s_bdev)
 +		goto out_sb;
 +
-+	error = security_sb_kern_mount(sb, flags, secdata);
++	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
   	if (error)
   		goto out_sb;
  
+@@ -1070,6 +1077,11 @@ do_kern_mount(const char *fstype, int fl
+ 	struct vfsmount *mnt;
+ 	if (!type)
+ 		return ERR_PTR(-ENODEV);
++
++	if ((type->fs_flags & FS_BINARY_MOUNTDATA) &&
++		!vx_capable(CAP_SYS_ADMIN, VXC_BINARY_MOUNT))
++	        return ERR_PTR(-EPERM);
++
+ 	mnt = vfs_kern_mount(type, flags, name, data);
+ 	if (!IS_ERR(mnt) && (type->fs_flags & FS_HAS_SUBTYPE) &&
+ 	    !mnt->mnt_sb->s_subtype)
 diff -NurpP --minimal linux-2.6.30.1/fs/sysfs/mount.c linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/sysfs/mount.c
 --- linux-2.6.30.1/fs/sysfs/mount.c	2009-06-11 17:13:08.000000000 +0200
 +++ linux-2.6.30.1-vs2.3.0.36.14-pre4/fs/sysfs/mount.c	2009-07-04 01:11:39.000000000
+0200
@@ -24578,8 +24564,8 @@
  VERSION = 2
  PATCHLEVEL = 6
  SUBLEVEL = 30
--EXTRAVERSION = .1
-+EXTRAVERSION = .1-vs2.3.0.36.14-pre4
+-EXTRAVERSION =
++EXTRAVERSION = -vs2.3.0.36.14-idiap
  NAME = Man-Eating Seals of Antiquity
  
  # *DOCUMENTATION*


["application/x-pkcs7-signature" not shown]