From 39b4adf91d3f9f2b740cdfd44396aeb820c7c3a5 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Tue, 2 Nov 2021 16:54:30 -0400 Subject: [PATCH] Fix access check when cred allows override of ACL Properly evaluate edge cases where user credential may grant capability to override DAC in various situations. Switch to using ns-aware checks rather than capable(). Expand optimization allow bypass of zfs_zaccess() in case of trivial ACL if MAY_OPEN is included in requested mask. This will be evaluated in generic_permission() check, which is RCU walk safe. This means that in most cases evaluating permissions on boot volume with NFSv4 ACLs will follow the fast path on checking inode permissions. Additionally, CAP_SYS_ADMIN is granted to nfsd process, and so override for this capability in access2 policy check is removed in favor of a simple check for fsid == 0. Checks for CAP_DAC_OVERRIDE and other override capabilities are kept as-is. Signed-off-by: Andrew Walker --- module/os/linux/zfs/policy.c | 25 ++++++++++++++++--------- module/os/linux/zfs/zpl_xattr.c | 3 ++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/module/os/linux/zfs/policy.c b/module/os/linux/zfs/policy.c index f8a8086046..ebec1e93a4 100644 --- a/module/os/linux/zfs/policy.c +++ b/module/os/linux/zfs/policy.c @@ -114,17 +114,22 @@ secpolicy_vnode_access2(const cred_t *cr, struct inode *ip, uid_t owner, mode_t curmode, mode_t wantmode) { mode_t remainder = ~curmode & wantmode; + uid_t uid = crgetuid(cr); if ((ITOZSB(ip)->z_acl_type != ZFS_ACLTYPE_NFSV4) || (remainder == 0)) { return (0); } - /* - * short-circuit if root - */ - if (capable(CAP_SYS_ADMIN)) { + if ((uid == owner) || (uid == 0)) return (0); - } + + if (zpl_inode_owner_or_capable(kcred->user_ns, ip)) + return (0); + +#if defined(CONFIG_USER_NS) + if (!kuid_has_mapping(cr->user_ns, SUID_TO_KUID(owner))) + return (EPERM); +#endif /* * There are some situations in which capabilities @@ -132,20 +137,22 @@ secpolicy_vnode_access2(const cred_t *cr, struct inode *ip, uid_t owner, */ if (S_ISDIR(ip->i_mode)) { if (!(wantmode & S_IWUSR) && - capable(CAP_DAC_READ_SEARCH)) { + (priv_policy_user(cr, CAP_DAC_READ_SEARCH, EPERM) == 0)) { return (0); } - if (capable(CAP_DAC_OVERRIDE)) { + if (priv_policy_user(cr, CAP_DAC_OVERRIDE, EPERM) == 0) { return (0); } return (EACCES); } - if ((wantmode == S_IRUSR) && capable(CAP_DAC_READ_SEARCH)) { + if ((wantmode == S_IRUSR) && + (priv_policy_user(cr, CAP_DAC_READ_SEARCH, EPERM) == 0)) { return (0); } - if (!(remainder & S_IXUSR) && capable(CAP_DAC_OVERRIDE)) { + if (!(remainder & S_IXUSR) && + (priv_policy_user(cr, CAP_DAC_OVERRIDE, EPERM) == 0)) { return (0); } diff --git a/module/os/linux/zfs/zpl_xattr.c b/module/os/linux/zfs/zpl_xattr.c index 4005258728..332dc63970 100644 --- a/module/os/linux/zfs/zpl_xattr.c +++ b/module/os/linux/zfs/zpl_xattr.c @@ -107,7 +107,8 @@ static const struct { #endif }; -#define GENERIC_MASK(mask) ((mask & ~(MAY_READ | MAY_WRITE | MAY_EXEC)) == 0) +#define POSIX_MASKS (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_OPEN) +#define GENERIC_MASK(mask) ((mask & ~POSIX_MASKS) == 0) enum xattr_permission { XAPERM_DENY,