Pass "allow owner override" flags from NFSD down to actual FS.

Pass "allow owner override" flags from NFSD down to actual FS.

[Oleg Drokin]

Hello!

   NFSD does its own internal checks to possibly override restrictive file mode
   for file owner already, to allow writing into (opened) file with some
   restrictive mode (like 0000). But it does not pass this info down to
   actual filesystems, and if that filesystem is also doing permission checks
   in open, such an open would fail at FS-level.
   (I thought of making an example with NFS exported with NFS, but this appears
    to be not allowed, so I choose different example).
   For example Lustre is contacting its metadata server for every open, and
   metadata server does permission checks for open, obviously.

   I wonder if something like the patch below can be useful for any other
   distributed FS now in use and ultimately to end up accepted into vanilla
   tree?


--- linux-2.6.16/include/asm-generic/fcntl.h.orig	2006-04-22 23:09:57.000000000 +0300
+++ linux-2.6.16/include/asm-generic/fcntl.h	2006-04-22 23:10:58.000000000 +0300
@@ -52,6 +52,9 @@
 #ifndef O_NDELAY
 #define O_NDELAY	O_NONBLOCK
 #endif
+#ifndef O_OWNER_OVERRIDE
+#define O_OWNER_OVERRIDE 02000000
+#endif
 
 #define F_DUPFD		0	/* dup */
 #define F_GETFD		1	/* get close_on_exec */
--- linux-2.6.16/include/linux/fs.h.orig	2006-04-22 23:02:22.000000000 +0300
+++ linux-2.6.16/include/linux/fs.h	2006-04-22 23:06:22.000000000 +0300
@@ -265,6 +265,7 @@ typedef void (dio_iodone_t)(struct kiocb
 #define ATTR_KILL_SUID	2048
 #define ATTR_KILL_SGID	4096
 #define ATTR_FILE	8192
+#define ATTR_OWNER_OVERRIDE	16384
 
 /*
  * This is the Inode Attributes structure, used for notify_change().  It
--- linux-2.6.16/fs/nfsd/vfs.c.orig	2006-04-22 23:03:49.000000000 +0300
+++ linux-2.6.16/fs/nfsd/vfs.c	2006-04-22 23:11:34.000000000 +0300
@@ -341,9 +341,10 @@ nfsd_setattr(struct svc_rqst *rqstp, str
 	if ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid)
 		iap->ia_valid |= ATTR_KILL_SGID;
 
-	/* Change the attributes. */
+	/* Change the attributes. Allow owner of file to change attributes
+	 * even if mode does not permit so. */
 
-	iap->ia_valid |= ATTR_CTIME;
+	iap->ia_valid |= ATTR_CTIME | ATTR_OWNER_OVERRIDE;
 
 	err = nfserr_notsync;
 	if (!check_guard || guardtime == inode->i_ctime.tv_sec) {
@@ -640,7 +641,7 @@ nfsd_open(struct svc_rqst *rqstp, struct
 {
 	struct dentry	*dentry;
 	struct inode	*inode;
-	int		flags = O_RDONLY|O_LARGEFILE, err;
+	int		flags = O_RDONLY|O_LARGEFILE|O_OWNER_OVERRIDE, err;
 
 	/*
 	 * If we get here, then the client has already done an "open",

Re: Pass "allow owner override" flags from NFSD down to actual FS.

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

On Sat, 22 Apr 2006 23:29:35 +0300 Oleg Drokin <green@linuxhacker.ru> wrote:
>
> --- linux-2.6.16/include/asm-generic/fcntl.h.orig	2006-04-22 23:09:57.000000000 +0300
> +++ linux-2.6.16/include/asm-generic/fcntl.h	2006-04-22 23:10:58.000000000 +0300
> @@ -52,6 +52,9 @@
>  #ifndef O_NDELAY
>  #define O_NDELAY	O_NONBLOCK
>  #endif
> +#ifndef O_OWNER_OVERRIDE
> +#define O_OWNER_OVERRIDE 02000000
                            ^^^^^^^^
Choose again, this clashes with O_DIRECT on alpha and O_RSYNC on parisc.
(Or define O_OWNER_OVERRIDE for them.)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Pass "allow owner override" flags from NFSD down to actual FS.

[Neil Brown]

On Saturday April 22, green@linuxhacker.ru wrote:
> Hello!
> 
>    NFSD does its own internal checks to possibly override restrictive file mode
>    for file owner already, to allow writing into (opened) file with some
>    restrictive mode (like 0000). But it does not pass this info down to
>    actual filesystems, and if that filesystem is also doing permission checks
>    in open, such an open would fail at FS-level.
>    (I thought of making an example with NFS exported with NFS, but this appears
>     to be not allowed, so I choose different example).
>    For example Lustre is contacting its metadata server for every open, and
>    metadata server does permission checks for open, obviously.
> 
>    I wonder if something like the patch below can be useful for any other
>    distributed FS now in use and ultimately to end up accepted into vanilla
>    tree?
> 

Would it be acceptable for the lowlevel filesystem to simply always
assume OWNER_OVERRIDE??

There are no security concerns in allowing read or write access to an
owner.  The IRUSR bit is almost entirely pointless, and the IWUSR bit
is at most hint for user-level processes.  Given that you know the
syscall interface will be checking these, and given that they have no
security implication, is there really any value in having the lowlevel
filesystem check them as well?

Other than that, my only concern is that it seems to allow a
user-space application to open a file with O_OWNER_OVERRIDE and you
would want that sort of change to get wide visibility.  I don't think
it is a problem, but there might be those who would.

NeilBrown

Re: Pass "allow owner override" flags from NFSD down to actual FS.

[Oleg Drokin]

Hello!

On Thu, Apr 27, 2006 at 10:28:16AM +1000, Neil Brown wrote:
> >    NFSD does its own internal checks to possibly override restrictive file mode
> >    for file owner already, to allow writing into (opened) file with some
> >    restrictive mode (like 0000). But it does not pass this info down to
> >    actual filesystems, and if that filesystem is also doing permission checks
> >    in open, such an open would fail at FS-level.
> >    (I thought of making an example with NFS exported with NFS, but this appears
> >     to be not allowed, so I choose different example).
> >    For example Lustre is contacting its metadata server for every open, and
> >    metadata server does permission checks for open, obviously.
> > 
> >    I wonder if something like the patch below can be useful for any other
> >    distributed FS now in use and ultimately to end up accepted into vanilla
> >    tree?
> Would it be acceptable for the lowlevel filesystem to simply always
> assume OWNER_OVERRIDE??

No. E.g. Lustre is usually bypassing local permission checks since server would
do those anyway. In this case we need to explicitly tell server that
client did the check and server should skip it.

> There are no security concerns in allowing read or write access to an
> owner.  The IRUSR bit is almost entirely pointless, and the IWUSR bit
> is at most hint for user-level processes.  Given that you know the
> syscall interface will be checking these, and given that they have no
> security implication, is there really any value in having the lowlevel
> filesystem check them as well?

If non-root process would be able to read/write files they own with 0 permission
bits set, then various posix/whatever conformance testing suites regard this
as a bug.

> Other than that, my only concern is that it seems to allow a
> user-space application to open a file with O_OWNER_OVERRIDE and you
> would want that sort of change to get wide visibility.  I don't think
> it is a problem, but there might be those who would.

I also do not think this is all that big problem, as owner can do
chmod as well.
I just do not want to add any extra arguments to ->open() methods or something
equally ugly to hide this flag from userspace.

Bye,
    Oleg