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 <awalker@ixsystems.com>
This commit is contained in:
Andrew Walker 2021-11-02 16:54:30 -04:00 committed by Ameer Hamza
parent af2a8ddb0e
commit 39b4adf91d
2 changed files with 18 additions and 10 deletions

View File

@ -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 curmode, mode_t wantmode)
{ {
mode_t remainder = ~curmode & wantmode; mode_t remainder = ~curmode & wantmode;
uid_t uid = crgetuid(cr);
if ((ITOZSB(ip)->z_acl_type != ZFS_ACLTYPE_NFSV4) || if ((ITOZSB(ip)->z_acl_type != ZFS_ACLTYPE_NFSV4) ||
(remainder == 0)) { (remainder == 0)) {
return (0); return (0);
} }
/* if ((uid == owner) || (uid == 0))
* short-circuit if root
*/
if (capable(CAP_SYS_ADMIN)) {
return (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 * 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 (S_ISDIR(ip->i_mode)) {
if (!(wantmode & S_IWUSR) && if (!(wantmode & S_IWUSR) &&
capable(CAP_DAC_READ_SEARCH)) { (priv_policy_user(cr, CAP_DAC_READ_SEARCH, EPERM) == 0)) {
return (0); return (0);
} }
if (capable(CAP_DAC_OVERRIDE)) { if (priv_policy_user(cr, CAP_DAC_OVERRIDE, EPERM) == 0) {
return (0); return (0);
} }
return (EACCES); 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); 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); return (0);
} }

View File

@ -107,7 +107,8 @@ static const struct {
#endif #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 { enum xattr_permission {
XAPERM_DENY, XAPERM_DENY,