Skip to content

Commit

Permalink
Fix access check when cred allows override of ACL
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
anodos325 authored and ixhamza committed Jun 20, 2023
1 parent 4d8b67b commit 8503a85
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
25 changes: 16 additions & 9 deletions module/os/linux/zfs/policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,38 +114,45 @@ 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
* may allow overriding the DACL.
*/
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);
}

Expand Down
3 changes: 2 additions & 1 deletion module/os/linux/zfs/zpl_xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 8503a85

Please sign in to comment.