Skip to content

Commit

Permalink
Fix ACL checks for NFS kernel server (openzfs#40)
Browse files Browse the repository at this point in the history
For Linux NFS kernel server ops, fsuid and fsgid in
cred are populated with ids that operation is
being performed as, but euid and egid remain 0.

In Linux when setresuid(2) and setresgid(2) are
called, the fsuid and fsgid are set to the euid
and egid respectively.

This PR changes ZFS ACL checks to evaluate
fsuid / fsgid rather than euid / egid to avoid
accidentally granting elevated permissions to
NFS clients.

Additionally, CAP_SYS_ADMIN is granted to nfsd
process, and so override for this capability in
access2 policy check is removed in favor of
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 Feb 9, 2022
1 parent de69fa8 commit c48c936
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 9 deletions.
4 changes: 2 additions & 2 deletions module/os/linux/spl/spl-cred.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ groupmember(gid_t gid, const cred_t *cr)
uid_t
crgetuid(const cred_t *cr)
{
return (KUID_TO_SUID(cr->euid));
return (KUID_TO_SUID(cr->fsuid));
}

/* Return the real user id */
Expand Down Expand Up @@ -156,7 +156,7 @@ crgetfsuid(const cred_t *cr)
gid_t
crgetgid(const cred_t *cr)
{
return (KGID_TO_SGID(cr->egid));
return (KGID_TO_SGID(cr->fsgid));
}

/* Return the real group id */
Expand Down
9 changes: 2 additions & 7 deletions module/os/linux/zfs/policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@ 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 fsuid = crgetfsuid(cr);
if ((ITOZSB(ip)->z_acl_type != ZFS_ACLTYPE_NFSV4) ||
(remainder == 0)) {
return (0);
}

if (crgetfsuid(cr) == owner)
if ((fsuid == owner) || (fsuid == 0))
return (0);

if (zpl_inode_owner_or_capable(kcred->user_ns, ip))
Expand All @@ -129,12 +130,6 @@ secpolicy_vnode_access2(const cred_t *cr, struct inode *ip, uid_t owner,
if (!kuid_has_mapping(cr->user_ns, SUID_TO_KUID(owner)))
return (EPERM);
#endif
/*
* short-circuit if root
*/
if (priv_policy_user(cr, CAP_SYS_ADMIN, EPERM) == 0) {
return (0);
}

/*
* There are some situations in which capabilities
Expand Down

0 comments on commit c48c936

Please sign in to comment.