Subject: Re: [vserver] NetApp filers and NFS junctions: Operation not permitted [RFC]
From:Cédric Dufour - Idiap Research Institute <cedric.dufour@idiap.ch>
Date: Fri, 24 Jul 2009 10:55:55 +0200
Fri, 24 Jul 2009 10:55:55 +0200
Finally, here is the least "invasive" modification - from a security
point of view - we could come up with (see attached patch). NFS
junctions now work as expected. As for security, we'd definitively need
some feedback ;-)

Cheers

Cédric Dufour @ Idiap Research Institute



On 23/07/09 18:46, Cédric Dufour - Idiap Research Institute wrote:
> [STRIPPED]
>
> 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>
>>>
>>>     
>>
>>   


diff -NurpP --minimal linux-2.6.30-debian-vs2.3.0.36.14-pre4/fs/afs/super.c linux-2.6.30-debian-vs2.3.0.36.14-idiap/fs/afs/super.c
--- linux-2.6.30-debian-vs2.3.0.36.14-pre4/fs/afs/super.c	2009-06-10 05:05:27.000000000
+0200
+++ linux-2.6.30-debian-vs2.3.0.36.14-idiap/fs/afs/super.c	2009-07-24 10:02:23.000000000
+0200
@@ -23,10 +23,9 @@
 #include <linux/parser.h>
 #include <linux/statfs.h>
 #include <linux/sched.h>
+#include <linux/magic.h>
 #include "internal.h"
 
-#define AFS_FS_MAGIC 0x6B414653 /* 'kAFS' */
-
 static void afs_i_init_once(void *foo);
 static int afs_get_sb(struct file_system_type *fs_type,
 		      int flags, const char *dev_name,
@@ -308,7 +307,7 @@ static int afs_fill_super(struct super_b
 	/* fill in the superblock */
 	sb->s_blocksize		= PAGE_CACHE_SIZE;
 	sb->s_blocksize_bits	= PAGE_CACHE_SHIFT;
-	sb->s_magic		= AFS_FS_MAGIC;
+	sb->s_magic		= AFS_SUPER_MAGIC;
 	sb->s_op		= &afs_super_ops;
 	sb->s_fs_info		= as;
 
diff -NurpP --minimal linux-2.6.30-debian-vs2.3.0.36.14-pre4/fs/cifs/cifsfs.c linux-2.6.30-debian-vs2.3.0.36.14-idiap/fs/cifs/cifsfs.c
--- linux-2.6.30-debian-vs2.3.0.36.14-pre4/fs/cifs/cifsfs.c	2009-06-10 05:05:27.000000000
+0200
+++ linux-2.6.30-debian-vs2.3.0.36.14-idiap/fs/cifs/cifsfs.c	2009-07-24 10:02:23.000000000
+0200
@@ -47,7 +47,7 @@
 #include <linux/key-type.h>
 #include "dns_resolve.h"
 #include "cifs_spnego.h"
-#define CIFS_MAGIC_NUMBER 0xFF534D42	/* the first four bytes of SMB PDUs */
+#include <linux/magic.h>
 
 #ifdef CONFIG_CIFS_QUOTA
 static struct quotactl_ops cifs_quotactl_ops;
@@ -136,7 +136,7 @@ cifs_read_super(struct super_block *sb, 
 		goto out_mount_failed;
 	}
 
-	sb->s_magic = CIFS_MAGIC_NUMBER;
+	sb->s_magic = CIFS_SUPER_MAGIC;
 	sb->s_op = &cifs_super_ops;
 /*	if (cifs_sb->tcon->ses->server->maxBuf > MAX_CIFS_HDR_SIZE + 512)
 	    sb->s_blocksize =
@@ -230,7 +230,7 @@ cifs_statfs(struct dentry *dentry, struc
 
 	xid = GetXid();
 
-	buf->f_type = CIFS_MAGIC_NUMBER;
+	buf->f_type = CIFS_SUPER_MAGIC;
 
 	/*
 	 * PATH_MAX may be too long - it would presumably be total path,
diff -NurpP --minimal linux-2.6.30-debian-vs2.3.0.36.14-pre4/fs/super.c linux-2.6.30-debian-vs2.3.0.36.14-idiap/fs/super.c
--- linux-2.6.30-debian-vs2.3.0.36.14-pre4/fs/super.c	2009-07-24 08:58:26.000000000
+0200
+++ linux-2.6.30-debian-vs2.3.0.36.14-idiap/fs/super.c	2009-07-24 10:02:23.000000000
+0200
@@ -39,8 +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/magic.h>
 #include <linux/vs_context.h>
 #include <asm/uaccess.h>
 #include "internal.h"
@@ -1002,11 +1001,6 @@ vfs_kern_mount(struct file_system_type *
 	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)
@@ -1031,6 +1025,9 @@ vfs_kern_mount(struct file_system_type *
 
 	error = -EPERM;
 	if (!vx_capable(CAP_SYS_ADMIN, VXC_BINARY_MOUNT) && !sb->s_bdev &&
+		(sb->s_magic != NFS_SUPER_MAGIC) &&
+		(sb->s_magic != CIFS_SUPER_MAGIC) &&
+		(sb->s_magic != AFS_SUPER_MAGIC) &&
 		(sb->s_magic != PROC_SUPER_MAGIC) &&
 		(sb->s_magic != DEVPTS_SUPER_MAGIC))
 		goto out_sb;
@@ -1087,6 +1084,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-debian-vs2.3.0.36.14-pre4/include/linux/magic.h linux-2.6.30-debian-vs2.3.0.36.14-idiap/include/linux/magic.h
--- linux-2.6.30-debian-vs2.3.0.36.14-pre4/include/linux/magic.h	2009-07-24 08:58:26.000000000
+0200
+++ linux-2.6.30-debian-vs2.3.0.36.14-idiap/include/linux/magic.h	2009-07-24 10:02:23.000000000
+0200
@@ -21,6 +21,7 @@
 #define ISOFS_SUPER_MAGIC	0x9660
 #define JFFS2_SUPER_MAGIC	0x72b6
 #define ANON_INODE_FS_MAGIC	0x09041934
+#define CIFS_SUPER_MAGIC	0xFF534D42	/* the first four bytes of SMB PDUs */
 
 #define MINIX_SUPER_MAGIC	0x137F		/* original minix fs */
 #define MINIX_SUPER_MAGIC2	0x138F		/* minix fs, 30 char names */
diff -NurpP --minimal linux-2.6.30-debian-vs2.3.0.36.14-pre4/Makefile linux-2.6.30-debian-vs2.3.0.36.14-idiap/Makefile
--- linux-2.6.30-debian-vs2.3.0.36.14-pre4/Makefile	2009-07-24 08:58:26.000000000 +0200
+++ linux-2.6.30-debian-vs2.3.0.36.14-idiap/Makefile	2009-07-24 10:02:23.000000000 +0200
@@ -1,7 +1,7 @@
 VERSION = 2
 PATCHLEVEL = 6
 SUBLEVEL = 30
-EXTRAVERSION = -vs2.3.0.36.14-pre4
+EXTRAVERSION = -vs2.3.0.36.14-idiap
 NAME = Man-Eating Seals of Antiquity
 
 # *DOCUMENTATION*


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