On 10/22/19 11:54 PM, Amir Goldstein wrote:
On Tue, Oct 22, 2019 at 11:45 PM Mark Salyzyn <salyzyn@xxxxxxxxxxx> 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@xxxxxxxxxxx>
Cc: Miklos Szeredi <miklos@xxxxxxxxxx>
Cc: Jonathan Corbet <corbet@xxxxxxx>
Cc: Vivek Goyal <vgoyal@xxxxxxxxxx>
Cc: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
Cc: Amir Goldstein <amir73il@xxxxxxxxx>
Cc: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
Cc: Stephen Smalley <sds@xxxxxxxxxxxxx>
Cc: linux-unionfs@xxxxxxxxxxxxxxx
Cc: linux-doc@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
---
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.
