Re: [PATCH] - avoid permission checks on EXCLUSIVE_CREATE replay

[Neil Brown <neilb@suse.de>]

On Thu, 22 Apr 2010 12:25:33 -0400
"J.Bruce Fields" <bfields@fieldses.org> wrote:

> On Thu, Apr 22, 2010 at 10:10:42AM +1000, Neil Brown wrote:
> > 
> > With NFSv4, if we create a file then open it we explicit avoid checking the
> > permissions on the file during the open because the fact that we created it
> > ensures we should be allow to open it (the create and the open should appear
> > to be a single operation).
> > 
> > However if the reply to an EXCLUSIVE create gets lots and the client resends
> > the create, the current code will perform the permission check - because it
> > doesn't realise that it did the open already..
> > 
> > This patch should fix this.
> 
> Thanks, but: hm, does this leave a loophole for a clever attacker?
> They'll still have to get past the initial
> 
> 	fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE)
> 
> but that just checks the parent directory; if the existing file is
> actually owned by someone else, do we allow an open that we shouldn't?
> 
> Maybe when "created" is set we should keep the permission check but add
> NFSD_ALLOW_OWNER_OVERRIDE?
> 

I think that is possibly a good idea.  However......

commit 81ac95c5569d7a60ab5db6c1ccec56c12b3ebcb5
Author: J. Bruce Fields <bfields@fieldses.org>
Date:   Wed Nov 8 17:44:40 2006 -0800

    [PATCH] nfsd4: fix open-create permissions
    
    In the case where an open creates the file, we shouldn't be rechecking
    permissions to open the file; the open succeeds regardless of what the new
    file's mode bits say.
    
    This patch fixes the problem, but only by introducing yet another parameter
    to nfsd_create_v3.  This is ugly.  This will be fixed by later patches.
    

I wouldn't want to get in the way of these 'later patches' that might be
going to remove the 'created' flag from nfsd_create_v3 :-)

NeilBrown