From 8b17d390e8cf5fc21e42d62f62b8bbe58960ed8a Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Fri, 18 Mar 2022 08:47:57 -0400 Subject: [PATCH] Fix ACL checks for NFS kernel server This PR changes ZFS ACL checks to evaluate fsuid / fsgid rather than euid / egid to avoid accidentally granting elevated permissions to NFS clients. Reviewed-by: Serapheim Dimitropoulos Reviewed-by: Brian Behlendorf Co-authored-by: Andrew Walker Signed-off-by: Ryan Moeller Closes #13221 --- include/os/freebsd/spl/sys/Makefile.am | 1 - include/os/freebsd/spl/sys/cred.h | 136 ++----------------------- include/os/freebsd/spl/sys/kidmap.h | 41 -------- include/os/freebsd/spl/sys/sid.h | 25 ----- include/os/linux/spl/sys/cred.h | 5 - module/Makefile.bsd | 2 +- module/os/freebsd/zfs/zfs_acl.c | 6 +- module/os/freebsd/zfs/zfs_vnops_os.c | 19 +--- module/os/linux/spl/spl-cred.c | 42 +------- module/os/linux/zfs/policy.c | 10 +- module/os/linux/zfs/zpl_inode.c | 4 +- module/os/linux/zfs/zpl_xattr.c | 4 +- 12 files changed, 26 insertions(+), 269 deletions(-) delete mode 100644 include/os/freebsd/spl/sys/kidmap.h diff --git a/include/os/freebsd/spl/sys/Makefile.am b/include/os/freebsd/spl/sys/Makefile.am index 232aaf569fa2..7488e56c7649 100644 --- a/include/os/freebsd/spl/sys/Makefile.am +++ b/include/os/freebsd/spl/sys/Makefile.am @@ -22,7 +22,6 @@ KERNEL_H = \ inttypes.h \ isa_defs.h \ kmem_cache.h \ - kidmap.h \ kmem.h \ kstat.h \ list_impl.h \ diff --git a/include/os/freebsd/spl/sys/cred.h b/include/os/freebsd/spl/sys/cred.h index 86f79011d6da..db986af57bf5 100644 --- a/include/os/freebsd/spl/sys/cred.h +++ b/include/os/freebsd/spl/sys/cred.h @@ -48,138 +48,20 @@ extern "C" { typedef struct ucred cred_t; #define CRED() curthread->td_ucred -#define kcred (thread0.td_ucred) - -#define KUID_TO_SUID(x) (x) -#define KGID_TO_SGID(x) (x) -#define crgetuid(cred) ((cred)->cr_uid) -#define crgetruid(cred) ((cred)->cr_ruid) -#define crgetgid(cred) ((cred)->cr_gid) -#define crgetgroups(cred) ((cred)->cr_groups) -#define crgetngroups(cred) ((cred)->cr_ngroups) -#define crgetsid(cred, i) (NULL) -struct proc; /* cred.h is included in proc.h */ -struct prcred; -struct ksid; -struct ksidlist; -struct credklpd; -struct credgrp; - -struct auditinfo_addr; /* cred.h is included in audit.h */ - -extern int ngroups_max; /* * kcred is used when you need all privileges. */ +#define kcred (thread0.td_ucred) -extern void cred_init(void); -extern void crfree(cred_t *); -extern cred_t *cralloc(void); /* all but ref uninitialized */ -extern cred_t *cralloc_ksid(void); /* cralloc() + ksid alloc'ed */ -extern cred_t *crget(void); /* initialized */ -extern void crcopy_to(cred_t *, cred_t *); -extern cred_t *crdup(cred_t *); -extern void crdup_to(cred_t *, cred_t *); -extern cred_t *crgetcred(void); -extern void crset(struct proc *, cred_t *); -extern void crset_zone_privall(cred_t *); -extern int supgroupmember(gid_t, const cred_t *); -extern int hasprocperm(const cred_t *, const cred_t *); -extern int prochasprocperm(struct proc *, struct proc *, const cred_t *); -extern int crcmp(const cred_t *, const cred_t *); -extern cred_t *zone_kcred(void); - -extern gid_t crgetrgid(const cred_t *); -extern gid_t crgetsgid(const cred_t *); - -#define crgetzoneid(cr) ((cr)->cr_prison->pr_id) -extern projid_t crgetprojid(const cred_t *); - -extern cred_t *crgetmapped(const cred_t *); - - -extern const struct auditinfo_addr *crgetauinfo(const cred_t *); -extern struct auditinfo_addr *crgetauinfo_modifiable(cred_t *); - -extern uint_t crgetref(const cred_t *); - -extern const gid_t *crgetggroups(const struct credgrp *); - - -/* - * Sets real, effective and/or saved uid/gid; - * -1 argument accepted as "no change". - */ -extern int crsetresuid(cred_t *, uid_t, uid_t, uid_t); -extern int crsetresgid(cred_t *, gid_t, gid_t, gid_t); - -/* - * Sets real, effective and saved uids/gids all to the same - * values. Both values must be non-negative and <= MAXUID - */ -extern int crsetugid(cred_t *, uid_t, gid_t); - -/* - * Functions to handle the supplemental group list. - */ -extern struct credgrp *crgrpcopyin(int, gid_t *); -extern void crgrprele(struct credgrp *); -extern void crsetcredgrp(cred_t *, struct credgrp *); - -/* - * Private interface for setting zone association of credential. - */ -struct zone; -extern void crsetzone(cred_t *, struct zone *); -extern struct zone *crgetzone(const cred_t *); - -/* - * Private interface for setting project id in credential. - */ -extern void crsetprojid(cred_t *, projid_t); - -/* - * Private interface for nfs. - */ -extern cred_t *crnetadjust(cred_t *); - -/* - * Private interface for procfs. - */ -extern void cred2prcred(const cred_t *, struct prcred *); - -/* - * Private interfaces for Rampart Trusted Solaris. - */ -struct ts_label_s; -extern struct ts_label_s *crgetlabel(const cred_t *); -extern boolean_t crisremote(const cred_t *); - -/* - * Private interfaces for ephemeral uids. - */ -#define VALID_UID(id, zn) \ - ((id) <= MAXUID || valid_ephemeral_uid((zn), (id))) - -#define VALID_GID(id, zn) \ - ((id) <= MAXUID || valid_ephemeral_gid((zn), (id))) - -extern boolean_t valid_ephemeral_uid(struct zone *, uid_t); -extern boolean_t valid_ephemeral_gid(struct zone *, gid_t); - -extern int eph_uid_alloc(struct zone *, int, uid_t *, int); -extern int eph_gid_alloc(struct zone *, int, gid_t *, int); - -extern void crsetsid(cred_t *, struct ksid *, int); -extern void crsetsidlist(cred_t *, struct ksidlist *); - -extern struct ksidlist *crgetsidlist(const cred_t *); - -extern int crsetpriv(cred_t *, ...); - -extern struct credklpd *crgetcrklpd(const cred_t *); -extern void crsetcrklpd(cred_t *, struct credklpd *); +#define KUID_TO_SUID(x) (x) +#define KGID_TO_SGID(x) (x) +#define crgetuid(cr) ((cr)->cr_uid) +#define crgetruid(cr) ((cr)->cr_ruid) +#define crgetgid(cr) ((cr)->cr_gid) +#define crgetgroups(cr) ((cr)->cr_groups) +#define crgetngroups(cr) ((cr)->cr_ngroups) +#define crgetzoneid(cr) ((cr)->cr_prison->pr_id) #ifdef __cplusplus } diff --git a/include/os/freebsd/spl/sys/kidmap.h b/include/os/freebsd/spl/sys/kidmap.h deleted file mode 100644 index dc0cf5988a42..000000000000 --- a/include/os/freebsd/spl/sys/kidmap.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (c) 2007 Pawel Jakub Dawidek - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHORS AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - * - * $FreeBSD$ - */ - -#ifndef _OPENSOLARIS_SYS_KIDMAP_H_ -#define _OPENSOLARIS_SYS_KIDMAP_H_ - -#include - -typedef int32_t idmap_stat; -typedef void idmap_get_handle_t; - -#define kidmap_get_create() (NULL) -#define kidmap_get_destroy(hdl) do { } while (0) -#define kidmap_get_mappings(hdl) (NULL) - -#endif /* _OPENSOLARIS_SYS_KIDMAP_H_ */ diff --git a/include/os/freebsd/spl/sys/sid.h b/include/os/freebsd/spl/sys/sid.h index d3fab8b24744..f249d05d55a0 100644 --- a/include/os/freebsd/spl/sys/sid.h +++ b/include/os/freebsd/spl/sys/sid.h @@ -29,7 +29,6 @@ #ifndef _OPENSOLARIS_SYS_SID_H_ #define _OPENSOLARIS_SYS_SID_H_ #include -#include typedef struct ksiddomain { char *kd_name; /* Domain part of SID */ @@ -59,28 +58,4 @@ ksiddomain_rele(ksiddomain_t *kd) kmem_free(kd, sizeof (*kd)); } -static __inline uint_t -ksid_getid(ksid_t *ks) -{ - - panic("%s has been unexpectedly called", __func__); -} - -static __inline const char * -ksid_getdomain(ksid_t *ks) -{ - - panic("%s has been unexpectedly called", __func__); -} - -static __inline uint_t -ksid_getrid(ksid_t *ks) -{ - - panic("%s has been unexpectedly called", __func__); -} - -#define kidmap_getsidbyuid(zone, uid, sid_prefix, rid) (1) -#define kidmap_getsidbygid(zone, gid, sid_prefix, rid) (1) - #endif /* _OPENSOLARIS_SYS_SID_H_ */ diff --git a/include/os/linux/spl/sys/cred.h b/include/os/linux/spl/sys/cred.h index 9cc85deb5c8a..b7d3f38d70bb 100644 --- a/include/os/linux/spl/sys/cred.h +++ b/include/os/linux/spl/sys/cred.h @@ -49,12 +49,7 @@ extern void crhold(cred_t *cr); 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/Makefile.bsd b/module/Makefile.bsd index 8aa4ed22275e..dc6cc2b74243 100644 --- a/module/Makefile.bsd +++ b/module/Makefile.bsd @@ -32,7 +32,7 @@ CFLAGS+= -include ${INCDIR}/os/freebsd/spl/sys/ccompile.h CFLAGS+= -D__KERNEL__ -DFREEBSD_NAMECACHE -DBUILDING_ZFS -D__BSD_VISIBLE=1 \ -DHAVE_UIO_ZEROCOPY -DWITHOUT_NETDUMP -D__KERNEL -D_SYS_CONDVAR_H_ \ - -D_SYS_VMEM_H_ -DKDTRACE_HOOKS -DSMP -DHAVE_KSID -DCOMPAT_FREEBSD11 + -D_SYS_VMEM_H_ -DKDTRACE_HOOKS -DSMP -DCOMPAT_FREEBSD11 .if ${MACHINE_ARCH} == "amd64" CFLAGS+= -DHAVE_AVX2 -DHAVE_AVX -D__x86_64 -DHAVE_SSE2 -DHAVE_AVX512F -DHAVE_SSSE3 diff --git a/module/os/freebsd/zfs/zfs_acl.c b/module/os/freebsd/zfs/zfs_acl.c index ae758bcefe21..fe0f69132321 100644 --- a/module/os/freebsd/zfs/zfs_acl.c +++ b/module/os/freebsd/zfs/zfs_acl.c @@ -1653,8 +1653,10 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr, ZFS_GROUP, &acl_ids->z_fuidp); gid = vap->va_gid; } else { - acl_ids->z_fuid = zfs_fuid_create_cred(zfsvfs, ZFS_OWNER, - cr, &acl_ids->z_fuidp); + uid_t id = crgetuid(cr); + if (IS_EPHEMERAL(id)) + id = UID_NOBODY; + acl_ids->z_fuid = (uint64_t)id; acl_ids->z_fgid = 0; if (vap->va_mask & AT_GID) { acl_ids->z_fgid = zfs_fuid_create(zfsvfs, diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c index 7bcf80bf5a94..b2cc3d063f9c 100644 --- a/module/os/freebsd/zfs/zfs_vnops_os.c +++ b/module/os/freebsd/zfs/zfs_vnops_os.c @@ -1060,8 +1060,7 @@ zfs_create(znode_t *dzp, const char *name, vattr_t *vap, int excl, int mode, objset_t *os; dmu_tx_t *tx; int error; - ksid_t *ksid; - uid_t uid; + uid_t uid = crgetuid(cr); gid_t gid = crgetgid(cr); uint64_t projid = ZFS_DEFAULT_PROJID; zfs_acl_ids_t acl_ids; @@ -1075,13 +1074,6 @@ zfs_create(znode_t *dzp, const char *name, vattr_t *vap, int excl, int mode, * If we have an ephemeral id, ACL, or XVATTR then * make sure file system is at proper version */ - - ksid = crgetsid(cr, KSID_OWNER); - if (ksid) - uid = ksid_getid(ksid); - else - uid = crgetuid(cr); - if (zfsvfs->z_use_fuids == B_FALSE && (vsecp || (vap->va_mask & AT_XVATTR) || IS_EPHEMERAL(uid) || IS_EPHEMERAL(gid))) @@ -1415,8 +1407,7 @@ zfs_mkdir(znode_t *dzp, const char *dirname, vattr_t *vap, znode_t **zpp, uint64_t txtype; dmu_tx_t *tx; int error; - ksid_t *ksid; - uid_t uid; + uid_t uid = crgetuid(cr); gid_t gid = crgetgid(cr); zfs_acl_ids_t acl_ids; boolean_t fuid_dirtied; @@ -1427,12 +1418,6 @@ zfs_mkdir(znode_t *dzp, const char *dirname, vattr_t *vap, znode_t **zpp, * If we have an ephemeral id, ACL, or XVATTR then * make sure file system is at proper version */ - - ksid = crgetsid(cr, KSID_OWNER); - if (ksid) - uid = ksid_getid(ksid); - else - uid = crgetuid(cr); if (zfsvfs->z_use_fuids == B_FALSE && ((vap->va_mask & AT_XVATTR) || IS_EPHEMERAL(uid) || IS_EPHEMERAL(gid))) diff --git a/module/os/linux/spl/spl-cred.c b/module/os/linux/spl/spl-cred.c index 8fe1cc30ba99..f81b9540a639 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 */ @@ -138,44 +138,9 @@ crgetruid(const cred_t *cr) return (KUID_TO_SUID(cr->uid)); } -/* Return the saved user id */ -uid_t -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 the real group id */ -gid_t -crgetrgid(const cred_t *cr) -{ - return (KGID_TO_SGID(cr->gid)); -} - -/* Return the saved group id */ -gid_t -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)); } @@ -184,12 +149,7 @@ 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 928058ef677c..e088db5d29cf 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);