From a43f573337490003ae274beda8ee1252bdbc8b37 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 9 Feb 2022 11:27:00 -0600 Subject: [PATCH] Fix ACL checks for NFS kernel server 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 --- include/os/linux/spl/sys/cred.h | 2 -- module/os/linux/spl/spl-cred.c | 20 ++------------------ module/os/linux/zfs/policy.c | 10 +++++----- module/os/linux/zfs/zpl_inode.c | 4 ++-- module/os/linux/zfs/zpl_xattr.c | 4 ++-- 5 files changed, 11 insertions(+), 29 deletions(-) diff --git a/include/os/linux/spl/sys/cred.h b/include/os/linux/spl/sys/cred.h index 9cc85deb5c8a..d88648acd643 100644 --- a/include/os/linux/spl/sys/cred.h +++ b/include/os/linux/spl/sys/cred.h @@ -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); diff --git a/module/os/linux/spl/spl-cred.c b/module/os/linux/spl/spl-cred.c index 8fe1cc30ba99..9479f6746692 100644 --- a/module/os/linux/spl/spl-cred.c +++ b/module/os/linux/spl/spl-cred.c @@ -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 */ @@ -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 */ @@ -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); diff --git a/module/os/linux/zfs/policy.c b/module/os/linux/zfs/policy.c index bbccb2e572d9..5a52092bb90a 100644 --- a/module/os/linux/zfs/policy.c +++ b/module/os/linux/zfs/policy.c @@ -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)) @@ -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) @@ -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) @@ -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); @@ -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) diff --git a/module/os/linux/zfs/zpl_inode.c b/module/os/linux/zfs/zpl_inode.c index 24a8b036bf0f..4f79265a0856 100644 --- a/module/os/linux/zfs/zpl_inode.c +++ b/module/os/linux/zfs/zpl_inode.c @@ -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); } } diff --git a/module/os/linux/zfs/zpl_xattr.c b/module/os/linux/zfs/zpl_xattr.c index 3b8ac517ada9..c53bf3c2ab25 100644 --- a/module/os/linux/zfs/zpl_xattr.c +++ b/module/os/linux/zfs/zpl_xattr.c @@ -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);