Re: [PATCH v19 0/4] overlayfs override_creds=off & nested get xattr fix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/16/2021 5:58 PM, David Anderson wrote:
Mark Salyzyn (3):
   Add flags option to get xattr method paired to __vfs_getxattr
   overlayfs: handle XATTR_NOSECURITY flag for get xattr method
   overlayfs: override_creds=off option bypass creator_cred

Mark Salyzyn + John Stultz (1):
   overlayfs: inode_owner_or_capable called during execv

The first three patches address fundamental security issues that should
be solved regardless of the override_creds=off feature.

The fourth 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.

This isn't very clear. Are you saying that the security attributes
of the upper, lower, and work directories are determined by the
attributes of the process that mounted the filesystem? What is an
"incoming access"? I'm sure that means something if you're steeped
in the lore of overlayfs, but it isn't obvious to me.

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.

I'm sorry, but I've tried pretty hard, and can't puzzle that one out.

   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.

DAC privileges are not hierarchical. This doesn't make any sense.

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.

I think I might have figured that one out, but in order to do so
I have to make way too many assumptions about the earlier paragraph.
Could you please try to explain what you're doing with more context?

   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>
Signed-off-by: David Anderson <dvander@xxxxxxxxxx>
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: John Stultz <john.stultz@xxxxxxxxxx>
Cc: linux-doc@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: linux-fsdevel@xxxxxxxxxxxxxxx
Cc: linux-unionfs@xxxxxxxxxxxxxxx
Cc: linux-security-module@xxxxxxxxxxxxxxx
Cc: kernel-team@xxxxxxxxxxx
Cc: selinux@xxxxxxxxxxxxxxx
Cc: paulmoore@xxxxxxxxxxxxx
Cc: Luca.Boccassi@xxxxxxxxxxxxx

---

v19
- rebase.

v18
- rebase + fix minor cut and paste error for inode argument in __vfs_getxattr

v17
- correct some zero-day build failures.
- fix up documentation

v16
- rebase and merge of two patches.
- add adjustment to deal with execv when overrides is off.

v15
- Revert back to v4 with fixes from on the way from v5-v14. The single
   structure argument passing to address the complaints about too many
   arguments was rejected by the community.
- Drop the udner discussion fix for an additional CAP_DAC_READ_SEARCH
   check. Can address that independently.
- ToDo: upstream test frame for thes security fixes (currently testing
   is all in Android).

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.

v12:
- Restore squished out patch 2 and 3 in the series,
   then change algorithm to add flags argument.
   Per-thread flag is a large security surface.

v11:
- Squish out v10 introduced patch 2 and 3 in the series,
   then and use per-thread flag instead for nesting.
- Switch name to ovl_do_vds_getxattr for __vds_getxattr wrapper.
- Add sb argument to ovl_revert_creds to match future work.

v10:
- Return NULL on CAP_DAC_READ_SEARCH
- Add __get xattr method to solve sepolicy logging issue
- Drop unnecessary sys_admin sepolicy checking for administrative
   driver internal xattr functions.

v6:
- Drop CONFIG_OVERLAY_FS_OVERRIDE_CREDS.
- Do better with the documentation, drop rationalizations.
- pr_warn message adjusted to report consequences.

v5:
- beefed up the caveats in the Documentation
- Is dependent on
   "overlayfs: check CAP_DAC_READ_SEARCH before issuing exportfs_decode_fh"
   "overlayfs: check CAP_MKNOD before issuing vfs_whiteout"
- Added prwarn when override_creds=off

v4:
- spelling and grammar errors in text

v3:
- Change name from caller_credentials / creator_credentials to the
   boolean override_creds.
- Changed from creator to mounter credentials.
- Updated and fortified the documentation.
- Added CONFIG_OVERLAY_FS_OVERRIDE_CREDS

v2:
- Forward port changed attr to stat, resulting in a build error.
- altered commit message.

David Anderson (4):
   Add flags option to get xattr method paired to __vfs_getxattr
   overlayfs: handle XATTR_NOSECURITY flag for get xattr method
   overlayfs: override_creds=off option bypass creator_cred
   overlayfs: inode_owner_or_capable called during execv

  Documentation/filesystems/locking.rst   |  2 +-
  Documentation/filesystems/overlayfs.rst | 26 ++++++++++++++-
  fs/9p/acl.c                             |  3 +-
  fs/9p/xattr.c                           |  3 +-
  fs/afs/xattr.c                          | 10 +++---
  fs/attr.c                               |  2 +-
  fs/btrfs/xattr.c                        |  3 +-
  fs/ceph/xattr.c                         |  3 +-
  fs/cifs/xattr.c                         |  2 +-
  fs/ecryptfs/inode.c                     |  6 ++--
  fs/ecryptfs/mmap.c                      |  5 +--
  fs/erofs/xattr.c                        |  3 +-
  fs/ext2/xattr_security.c                |  2 +-
  fs/ext2/xattr_trusted.c                 |  2 +-
  fs/ext2/xattr_user.c                    |  2 +-
  fs/ext4/xattr_hurd.c                    |  2 +-
  fs/ext4/xattr_security.c                |  2 +-
  fs/ext4/xattr_trusted.c                 |  2 +-
  fs/ext4/xattr_user.c                    |  2 +-
  fs/f2fs/xattr.c                         |  4 +--
  fs/fuse/xattr.c                         |  4 +--
  fs/gfs2/xattr.c                         |  3 +-
  fs/hfs/attr.c                           |  2 +-
  fs/hfsplus/xattr.c                      |  3 +-
  fs/hfsplus/xattr_security.c             |  3 +-
  fs/hfsplus/xattr_trusted.c              |  3 +-
  fs/hfsplus/xattr_user.c                 |  3 +-
  fs/inode.c                              |  7 +++--
  fs/internal.h                           |  3 +-
  fs/jffs2/security.c                     |  3 +-
  fs/jffs2/xattr_trusted.c                |  3 +-
  fs/jffs2/xattr_user.c                   |  3 +-
  fs/jfs/xattr.c                          |  5 +--
  fs/kernfs/inode.c                       |  3 +-
  fs/nfs/nfs4proc.c                       |  9 ++++--
  fs/ntfs3/xattr.c                        |  2 +-
  fs/ocfs2/xattr.c                        |  9 ++++--
  fs/open.c                               |  2 +-
  fs/orangefs/xattr.c                     |  3 +-
  fs/overlayfs/copy_up.c                  |  2 +-
  fs/overlayfs/dir.c                      | 17 +++++-----
  fs/overlayfs/file.c                     | 25 ++++++++-------
  fs/overlayfs/inode.c                    | 29 ++++++++---------
  fs/overlayfs/namei.c                    |  6 ++--
  fs/overlayfs/overlayfs.h                |  7 +++--
  fs/overlayfs/ovl_entry.h                |  1 +
  fs/overlayfs/readdir.c                  |  8 ++---
  fs/overlayfs/super.c                    | 34 ++++++++++++++++----
  fs/overlayfs/util.c                     | 13 ++++++--
  fs/posix_acl.c                          |  2 +-
  fs/reiserfs/xattr_security.c            |  3 +-
  fs/reiserfs/xattr_trusted.c             |  3 +-
  fs/reiserfs/xattr_user.c                |  3 +-
  fs/squashfs/xattr.c                     |  2 +-
  fs/ubifs/xattr.c                        |  3 +-
  fs/xattr.c                              | 42 +++++++++++++------------
  fs/xfs/xfs_xattr.c                      |  3 +-
  include/linux/lsm_hook_defs.h           |  3 +-
  include/linux/security.h                |  6 ++--
  include/linux/xattr.h                   |  6 ++--
  include/uapi/linux/xattr.h              |  7 +++--
  mm/shmem.c                              |  3 +-
  net/socket.c                            |  3 +-
  security/commoncap.c                    | 11 ++++---
  security/integrity/evm/evm_main.c       | 13 +++++---
  security/security.c                     |  5 +--
  security/selinux/hooks.c                | 19 ++++++-----
  security/smack/smack_lsm.c              | 18 ++++++-----
  68 files changed, 289 insertions(+), 167 deletions(-)




[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux