linux-kernel - Re: [PATCH v2 2/2] nfs: NFSv3: fix SGID bit dropped when inheriting ACLs

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, 24 Jun 2021 13:13:50 +0000
From:   Trond Myklebust <trondmy@...merspace.com>
To:     "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "hsiangkao@...ux.alibaba.com" <hsiangkao@...ux.alibaba.com>
CC:     "joseph.qi@...ux.alibaba.com" <joseph.qi@...ux.alibaba.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "agruenba@...hat.com" <agruenba@...hat.com>,
        "anna.schumaker@...app.com" <anna.schumaker@...app.com>
Subject: Re: [PATCH v2 2/2] nfs: NFSv3: fix SGID bit dropped when inheriting
 ACLs

On Wed, 2021-06-23 at 14:38 +0800, Gao Xiang wrote:
> generic/444 fails with NFSv3 as shown above, "
>      QA output created by 444
>      drwxrwsr-x
>     -drwxrwsr-x
>     +drwxrwxr-x
> ", which tests "SGID inheritance with default ACLs" fs regression
> and looks after the following commits:
> 
> a3bb2d558752 ("ext4: Don't clear SGID when inheriting ACLs")
> 073931017b49 ("posix_acl: Clear SGID bit when setting file
> permissions")
> 
> commit 055ffbea0596 ("[PATCH] NFS: Fix handling of the umask when
> an NFSv3 default acl is present.") sets acls explicitly when
> when files are created in a directory that has a default ACL.
> However, after commit a3bb2d558752 and 073931017b49, SGID can be
> dropped if user is not member of the owning group with
> set_posix_acl() called.
> 
> Since underlayfs will handle ACL inheritance when creating
> files in a directory that has the default ACL and the umask is
> supposed to be ignored for such case. Therefore, I think no need
> to set acls explicitly (to avoid SGID bit cleared) but only apply
> client umask if the default ACL of the parent directory doesn't
> exist.

Hmm... Has this patch been tested with a Solaris server? Your assertion
above appears to be true for Linux servers, but this code needs to
interoperate with non-Linux draft posix acl compatible servers too.

I've already taken the other patch in this series, since that one
appears correct, and doesn't have any interoperability consequences.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@...merspace.com