linux-kernel - Re: [PATCH 1/3] VFS: introduce MAY_ACT_AS

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]

Date:   Thu, 04 Oct 2018 10:42:07 -0400
From:   Jan Harkes <jaharkes@...cmu.edu>
To:     David Howells <dhowells@...hat.com>, NeilBrown <neilb@...e.com>
CC:     dhowells@...hat.com, "J. Bruce Fields" <bfields@...ldses.org>,
        Anna Schumaker <anna.schumaker@...app.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        linux-nfs@...r.kernel.org, Miklos Szeredi <miklos@...redi.hu>,
        Jeff Layton <jlayton@...nel.org>, linux-kernel@...r.kernel.org,
        linux-afs@...ts.infradead.org, coda@...cmu.edu,
        linux-fsdevel@...r.kernel.org, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH 1/3] VFS: introduce MAY_ACT_AS_OWNER

Same for Coda.

uid/gid/mode don't mean anything, access is based on the directory ACL and the authentication token that is held by the userspace cache manager and ultimately decided by the servers.

Unless someone broke this recently and made permission checks uid based I would expect no change. If this is broken by a recent commit I expect something similar to what NFS is trying to do by allowing the actual check to be passed down.

Jan

On October 4, 2018 10:10:13 AM EDT, David Howells <dhowells@...hat.com> wrote:
>NeilBrown <neilb@...e.com> wrote:
>
>> diff --git a/fs/afs/security.c b/fs/afs/security.c
>> index 81dfedb7879f..ac2e39de8bff 100644
>> --- a/fs/afs/security.c
>> +++ b/fs/afs/security.c
>> @@ -349,6 +349,16 @@ int afs_permission(struct inode *inode, int
>mask)
>>  	if (mask & MAY_NOT_BLOCK)
>>  		return -ECHILD;
>>  
>> +	/* Short-circuit for owner */
>> +	if (mask & MAY_ACT_AS_OWNER) {
>> +		if (inode_owner_or_capable(inode))
>
>You don't know that inode->i_uid in meaningful.  You may have noticed
>that
>afs_permission() ignores i_uid and i_gid entirely.  It queries the
>server (if
>this information is not otherwise cached) to ask what permits the user
>is
>granted - where the user identity is defined by the key returned from
>afs_request_key()[*].
>
>So, NAK for the afs piece.
>
>David
>
>[*] If there's no appropriate key, anonymous permits will be used.