[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c0eb1b6e-65f6-9d38-64b9-333f3e82905a@android.com>
Date: Wed, 23 Oct 2019 07:13:03 -0700
From: Mark Salyzyn <salyzyn@...roid.com>
To: Amir Goldstein <amir73il@...il.com>
Cc: linux-kernel <linux-kernel@...r.kernel.org>,
kernel-team@...roid.com, Miklos Szeredi <miklos@...redi.hu>,
Jonathan Corbet <corbet@....net>,
Vivek Goyal <vgoyal@...hat.com>,
"Eric W . Biederman" <ebiederm@...ssion.com>,
Randy Dunlap <rdunlap@...radead.org>,
Stephen Smalley <sds@...ho.nsa.gov>,
overlayfs <linux-unionfs@...r.kernel.org>,
linux-doc@...r.kernel.org, Christoph Hellwig <hch@...radead.org>,
Greg KH <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v14 0/5] overlayfs override_creds=off & nested get xattr
fix
On 10/22/19 11:54 PM, Amir Goldstein wrote:
> On Tue, Oct 22, 2019 at 11:45 PM Mark Salyzyn <salyzyn@...roid.com> wrote:
>> Patch series:
>>
>> Mark Salyzyn (5):
>> Add flags option to get xattr method paired to __vfs_getxattr
>> overlayfs: check CAP_DAC_READ_SEARCH before issuing exportfs_decode_fh
>> overlayfs: handle XATTR_NOSECURITY flag for get xattr method
>> overlayfs: internal getxattr operations without sepolicy checking
>> overlayfs: override_creds=off option bypass creator_cred
>>
>> The first four patches address fundamental security issues that should
>> be solved regardless of the override_creds=off feature.
>>
>> The fifth adds the feature depends on these other fixes.
>>
>> By default, all access to the upper, lower and work directories is the
>> recorded mounter's MAC and DAC credentials. The incoming accesses are
>> checked against the caller's credentials.
>>
>> If the principles of least privilege are applied for sepolicy, the
>> mounter's credentials might not overlap the credentials of the caller's
>> when accessing the overlayfs filesystem. For example, a file that a
>> lower DAC privileged caller can execute, is MAC denied to the
>> generally higher DAC privileged mounter, to prevent an attack vector.
>>
>> We add the option to turn off override_creds in the mount options; all
>> subsequent operations after mount on the filesystem will be only the
>> caller's credentials. The module boolean parameter and mount option
>> override_creds is also added as a presence check for this "feature",
>> existence of /sys/module/overlay/parameters/overlay_creds
>>
>> Signed-off-by: Mark Salyzyn <salyzyn@...roid.com>
>> Cc: Miklos Szeredi <miklos@...redi.hu>
>> Cc: Jonathan Corbet <corbet@....net>
>> Cc: Vivek Goyal <vgoyal@...hat.com>
>> Cc: Eric W. Biederman <ebiederm@...ssion.com>
>> Cc: Amir Goldstein <amir73il@...il.com>
>> Cc: Randy Dunlap <rdunlap@...radead.org>
>> Cc: Stephen Smalley <sds@...ho.nsa.gov>
>> Cc: linux-unionfs@...r.kernel.org
>> Cc: linux-doc@...r.kernel.org
>> Cc: linux-kernel@...r.kernel.org
>>
>> ---
>> v14:
>> - Rejoin, rebase and a few adjustments.
>>
>> v13:
>> - Pull out first patch and try to get it in alone feedback, some
>> Acks, and then <crickets> because people forgot why we were doing i.
> Mark,
>
> I do not see the first patch on fsdevel
> and I am confused from all the suggested APIs
> I recall Christoph's comment on v8 for not using xattr_gs_args
> and just adding flags to existing get() method.
> I agree to that comment.
As already responded, third (?) patch version was like that, gregkh@
said it passed the limit for number of arguments, is looking a bit silly
(my paraphrase), and that it should be passed as a structure. Two others
agreed. We gained because both set and get use the same structure after
this change (this allows a simplified read-modify-write cycle).
We will need a quorum on this, 3 (structure) to 2 (flag) now (but really
basically between Greg and Christoph?). Coding style issue: Add a flag,
or switch to a common xattr argument structure?
> I remember asking - don't remember the answer -
> do you have any testing for this feature?
Yes, on an unnamed 4.19-based and mainline-based Android and virtual
cuttlefish product ... which was critically unworkable without this
patch series.
> I have a WIP branch to run unionmount-testsuite not as root,
> which is a start, but I didn't get to finish the work.
> Let me know if you want to take up this work.
Please refer it in private email to me, no guarantees, my cycles are so
sparse right now that it took a month to respin this patch series to
upstream. If I can make it test on Android with overlayfs activated, big
gain.
>
> Thanks,
> Amir.
Powered by blists - more mailing lists