Skip to content

Commit

Permalink
Fix ACL checks for NFS kernel server
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.

Signed-off-by: Andrew Walker <awalker@ixsystems.com>
  • Loading branch information
anodos325 authored and Ryan Moeller committed Mar 17, 2022
1 parent 6ef0019 commit a43f573
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 29 deletions.
2 changes: 0 additions & 2 deletions include/os/linux/spl/sys/cred.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,9 @@ extern void crfree(cred_t *cr);
extern uid_t crgetuid(const cred_t *cr);
extern uid_t crgetruid(const cred_t *cr);
extern uid_t crgetsuid(const cred_t *cr);
extern uid_t crgetfsuid(const cred_t *cr);
extern gid_t crgetgid(const cred_t *cr);
extern gid_t crgetrgid(const cred_t *cr);
extern gid_t crgetsgid(const cred_t *cr);
extern gid_t crgetfsgid(const cred_t *cr);
extern int crgetngroups(const cred_t *cr);
extern gid_t *crgetgroups(const cred_t *cr);
extern int groupmember(gid_t gid, const cred_t *cr);
Expand Down
20 changes: 2 additions & 18 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 All @@ -145,18 +145,11 @@ crgetsuid(const cred_t *cr)
return (KUID_TO_SUID(cr->suid));
}

/* Return the filesystem user id */
uid_t
crgetfsuid(const cred_t *cr)
{
return (KUID_TO_SUID(cr->fsuid));
}

/* Return the effective group id */
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 All @@ -173,23 +166,14 @@ crgetsgid(const cred_t *cr)
return (KGID_TO_SGID(cr->sgid));
}

/* Return the filesystem group id */
gid_t
crgetfsgid(const cred_t *cr)
{
return (KGID_TO_SGID(cr->fsgid));
}

EXPORT_SYMBOL(crhold);
EXPORT_SYMBOL(crfree);
EXPORT_SYMBOL(crgetuid);
EXPORT_SYMBOL(crgetruid);
EXPORT_SYMBOL(crgetsuid);
EXPORT_SYMBOL(crgetfsuid);
EXPORT_SYMBOL(crgetgid);
EXPORT_SYMBOL(crgetrgid);
EXPORT_SYMBOL(crgetsgid);
EXPORT_SYMBOL(crgetfsgid);
EXPORT_SYMBOL(crgetngroups);
EXPORT_SYMBOL(crgetgroups);
EXPORT_SYMBOL(groupmember);
10 changes: 5 additions & 5 deletions module/os/linux/zfs/policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ secpolicy_vnode_access2(const cred_t *cr, struct inode *ip, uid_t owner,
int
secpolicy_vnode_any_access(const cred_t *cr, struct inode *ip, uid_t owner)
{
if (crgetfsuid(cr) == owner)
if (crgetuid(cr) == owner)
return (0);

if (zpl_inode_owner_or_capable(kcred->user_ns, ip))
Expand All @@ -147,7 +147,7 @@ secpolicy_vnode_any_access(const cred_t *cr, struct inode *ip, uid_t owner)
int
secpolicy_vnode_chown(const cred_t *cr, uid_t owner)
{
if (crgetfsuid(cr) == owner)
if (crgetuid(cr) == owner)
return (0);

#if defined(CONFIG_USER_NS)
Expand Down Expand Up @@ -184,7 +184,7 @@ secpolicy_vnode_remove(const cred_t *cr)
int
secpolicy_vnode_setdac(const cred_t *cr, uid_t owner)
{
if (crgetfsuid(cr) == owner)
if (crgetuid(cr) == owner)
return (0);

#if defined(CONFIG_USER_NS)
Expand Down Expand Up @@ -220,7 +220,7 @@ secpolicy_vnode_setids_setgids(const cred_t *cr, gid_t gid)
if (!kgid_has_mapping(cr->user_ns, SGID_TO_KGID(gid)))
return (EPERM);
#endif
if (crgetfsgid(cr) != gid && !groupmember(gid, cr))
if (crgetgid(cr) != gid && !groupmember(gid, cr))
return (priv_policy_user(cr, CAP_FSETID, EPERM));

return (0);
Expand Down Expand Up @@ -286,7 +286,7 @@ secpolicy_setid_clear(vattr_t *vap, cred_t *cr)
static int
secpolicy_vnode_setid_modify(const cred_t *cr, uid_t owner)
{
if (crgetfsuid(cr) == owner)
if (crgetuid(cr) == owner)
return (0);

#if defined(CONFIG_USER_NS)
Expand Down
4 changes: 2 additions & 2 deletions module/os/linux/zfs/zpl_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@ zpl_vap_init(vattr_t *vap, struct inode *dir, umode_t mode, cred_t *cr)
{
vap->va_mask = ATTR_MODE;
vap->va_mode = mode;
vap->va_uid = crgetfsuid(cr);
vap->va_uid = crgetuid(cr);

if (dir && dir->i_mode & S_ISGID) {
vap->va_gid = KGID_TO_SGID(dir->i_gid);
if (S_ISDIR(mode))
vap->va_mode |= S_ISGID;
} else {
vap->va_gid = crgetfsgid(cr);
vap->va_gid = crgetgid(cr);
}
}

Expand Down
4 changes: 2 additions & 2 deletions module/os/linux/zfs/zpl_xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,8 @@ zpl_xattr_set_dir(struct inode *ip, const char *name, const void *value,
vap = kmem_zalloc(sizeof (vattr_t), KM_SLEEP);
vap->va_mode = xattr_mode;
vap->va_mask = ATTR_MODE;
vap->va_uid = crgetfsuid(cr);
vap->va_gid = crgetfsgid(cr);
vap->va_uid = crgetuid(cr);
vap->va_gid = crgetgid(cr);

error = -zfs_create(dxzp, (char *)name, vap, 0, 0644, &xzp,
cr, 0, NULL);
Expand Down

0 comments on commit a43f573

Please sign in to comment.