Skip to content

Commit

Permalink
Fix ACL checks for NFS kernel server
Browse files Browse the repository at this point in the history
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 <serapheim@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Co-authored-by: Andrew Walker <awalker@ixsystems.com>
Signed-off-by: Ryan Moeller <freqlabs@FreeBSD.org>
Closes openzfs#13221
  • Loading branch information
Ryan Moeller authored and andrewc12 committed Sep 23, 2022
1 parent 3ac0f83 commit 5709c0c
Show file tree
Hide file tree
Showing 12 changed files with 26 additions and 269 deletions.
1 change: 0 additions & 1 deletion include/os/freebsd/spl/sys/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ KERNEL_H = \
inttypes.h \
isa_defs.h \
kmem_cache.h \
kidmap.h \
kmem.h \
kstat.h \
list_impl.h \
Expand Down
136 changes: 9 additions & 127 deletions include/os/freebsd/spl/sys/cred.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
41 changes: 0 additions & 41 deletions include/os/freebsd/spl/sys/kidmap.h

This file was deleted.

25 changes: 0 additions & 25 deletions include/os/freebsd/spl/sys/sid.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#ifndef _OPENSOLARIS_SYS_SID_H_
#define _OPENSOLARIS_SYS_SID_H_
#include <sys/idmap.h>
#include <sys/kidmap.h>

typedef struct ksiddomain {
char *kd_name; /* Domain part of SID */
Expand Down Expand Up @@ -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_ */
5 changes: 0 additions & 5 deletions include/os/linux/spl/sys/cred.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion module/Makefile.bsd
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,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
Expand Down
6 changes: 4 additions & 2 deletions module/os/freebsd/zfs/zfs_acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 2 additions & 17 deletions module/os/freebsd/zfs/zfs_vnops_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -1043,8 +1043,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;
Expand All @@ -1058,13 +1057,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)))
Expand Down Expand Up @@ -1396,8 +1388,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;
Expand All @@ -1408,12 +1399,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)))
Expand Down
42 changes: 1 addition & 41 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 @@ -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));
}
Expand All @@ -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);
Loading

0 comments on commit 5709c0c

Please sign in to comment.