linux-kernel - Re: [NFS] [PATCH 2/7] NFS: if ATTR_KILL_S*ID bits are set, then skip mode change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20070914105838.efbfc45e.jlayton@redhat.com>
Date:	Fri, 14 Sep 2007 10:58:38 -0400
From:	Jeff Layton <jlayton@...hat.com>
To:	Greg Banks <gnb@....com>
Cc:	reiserfs-devel@...r.kernel.org, linux-kernel@...r.kernel.org,
	ecryptfs-devel@...ts.sourceforge.net, nfs@...ts.sourceforge.net,
	linux-fsdevel@...r.kernel.org, unionfs@...esystems.org,
	linux-cifs-client@...ts.samba.org
Subject: Re: [NFS] [PATCH 2/7] NFS: if ATTR_KILL_S*ID bits are set, then
 skip mode change

On Sat, 15 Sep 2007 00:40:33 +1000
Greg Banks <gnb@....com> wrote:

> On Fri, Sep 14, 2007 at 09:38:46AM -0400, Jeff Layton wrote:
> > On Fri, 14 Sep 2007 23:09:24 +1000
> > Greg Banks <gnb@....com> wrote:
> > 
> > > On Fri, Sep 14, 2007 at 07:02:58AM -0400, Jeff Layton wrote:
> > > > On Fri, 14 Sep 2007 20:25:45 +1000
> > > > Greg Banks <gnb@....com> wrote:
> > > > 
> > > > > I'm curious about the reasons behind this change.  You mention
> > > > > credential issues; how exactly is it that you have the correct creds
> > > > > to perform a WRITE rpc but not a SETATTR rpc?
> > > > > 
> > > > 
> > > > Consider this case. user1 and user2 are both members of group
> > > > "allusers":
> > > > 
> > > > user1$ echo foo > foo
> > > > user1$ chgrp allusers foo
> > > > user1$ chmod 04770 foo
> > > > user2$ echo bar >> foo
> > > > 
> > > > On most local filesystems, this would work correctly. The end result
> > > > would be a file with mode 0770 and the expected contents. On NFS
> > > > though, the write by user2 fails. When the write is attempted, the
> > > > kernel tries to squash the setuid bit using the credentials of user2,
> > > > who's not allowed to change the mode. The write then fails because the
> > > > setattr fails.
> > > 
> > > Ok, I ran an experiment and I see this failure mode.
> > > 
> > > So the SETATTR rpc is really a side effect of the client kernel's
> > > behaviour and not an operation directly requested by the user process
> > > on the client.  Is there any reason why that rpc needs to have user2's
> > > creds?  Why not do the rpc with a fake set of creds with uid and gid
> > > set to the uid and gid of the file, in this case user1/allusers ?
> > > That way the rpc will most likely pass the server's permission check.
> > > 
> > 
> > That might work in some cases, but there are many where it wouldn't...
> > 
> > Suppose user1 here is root and all of the user1 operations are being
> > done on the server. If the server has root squashing enabled, then
> > user2's operation would still fail.
> 
> In that case, user1's operations would also fail, which is even more
> serious a problem.  Also arguably you actually *want* writes by a
> nonroot user to a setuid root executable to fail ;-)
> 

Well, user1's operations would fail if done from the client, which is
why I mentioned that they would have to be done on the server.

The second point is a good one, but POSIX says that it should be allowed
if the permissions allow for it. The whole situation is somewhat
contrived anyway, I can't think of a place where this is something
you'd really want to do, but I think we need to try to follow the spec
as best as possible...

> > Another problem:
> > 
> > Suppose we're using gssapi. There's no guarantee that the client will
> > have the proper credentials to fake up a call as user1 (you might need
> > user1 krb5 tickets, etc).
> 
> Yes, good point.  You could use the root creds, except for root squashing.
> Ok, you convinced me.
> 

Right. When I was first looking at this, I considered some similar
approaches, but hit roadblocks with all of them. The only real option
seems to be to leave this to the server, but that does assume that the
server handles this properly.

Servers that don't are broken, IMO. If Irix isn't clearing these bits
on a write then it might be good to see if they can fix that...

-- 
Jeff Layton <jlayton@...hat.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/