kern: fix setgroups(2) and getgroups(2) to match other platforms

On most other platforms observed, including OpenBSD, NetBSD, and Linux,
these system calls have long since been converted to only touching the
supplementary groups of the process.  This poses both portability and
security concerns in porting software to and from FreeBSD, as this
subtle difference is a landmine waiting to happen.  Bugs have been
discovered even in FreeBSD-local sources, since this behavior is
somewhat unintuitive (see, e.g., fix 48fd05999b0f for chroot(8)).

Now that the egid is tracked outside of cr_groups in our ucred, convert
the syscalls to deal with only supplementary groups.  Some remaining
stragglers in base that had baked in assumptions about these syscalls
are fixed in the process to avoid heartburn in conversion.

For relnotes: application developers should audit their use of both
setgroups(2) and getgroups(2) for signs that they had assumed the
previous FreeBSD behavior of using the first element for the egid.  Any
calls to setgroups() to clear groups that used a single array of the
now or soon-to-be egid can be converted to setgroups(0, NULL) calls to
clear the supplementary groups entirely on all FreeBSD versions.

Co-authored-by:	olce (but bugs are likely mine)
Relnotes:	yes (see last paragraph)
Reviewed by:	kib
Differential Revision:	https://reviews.freebsd.org/D51648

1 files changed, 78 insertions, 48 deletions

diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 2cd5b7069023..beab30a9d157 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -310,6 +310,39 @@ sys_getegid(struct thread *td, struct getegid_args *uap)
 	return (0);
 }
 
+#ifdef COMPAT_FREEBSD14
+int
+freebsd14_getgroups(struct thread *td, struct freebsd14_getgroups_args *uap)
+{
+	struct ucred *cred;
+	int ngrp, error;
+
+	cred = td->td_ucred;
+
+	/*
+	 * For FreeBSD < 15.0, we account for the egid being placed at the
+	 * beginning of the group list prior to all supplementary groups.
+	 */
+	ngrp = cred->cr_ngroups + 1;
+	if (uap->gidsetsize == 0) {
+		error = 0;
+		goto out;
+	} else if (uap->gidsetsize < ngrp) {
+		return (EINVAL);
+	}
+
+	error = copyout(&cred->cr_gid, uap->gidset, sizeof(gid_t));
+	if (error != 0)
+		error = copyout(cred->cr_groups, uap->gidset + 1,
+		    (ngrp - 1) * sizeof(gid_t));
+
+out:
+	td->td_retval[0] = ngrp;
+	return (error);
+
+}
+#endif	/* COMPAT_FREEBSD14 */
+
 #ifndef _SYS_SYSPROTO_H_
 struct getgroups_args {
 	int	gidsetsize;
@@ -320,18 +353,11 @@ int
 sys_getgroups(struct thread *td, struct getgroups_args *uap)
 {
 	struct ucred *cred;
-	gid_t *ugidset;
 	int ngrp, error;
 
 	cred = td->td_ucred;
 
-	/*
-	 * cr_gid has been moved out of cr_groups, but we'll continue exporting
-	 * the egid as groups[0] for the time being until we audit userland for
-	 * any surprises.
-	 */
-	ngrp = cred->cr_ngroups + 1;
-
+	ngrp = cred->cr_ngroups;
 	if (uap->gidsetsize == 0) {
 		error = 0;
 		goto out;
@@ -339,14 +365,7 @@ sys_getgroups(struct thread *td, struct getgroups_args *uap)
 	if (uap->gidsetsize < ngrp)
 		return (EINVAL);
 
-	ugidset = uap->gidset;
-	error = copyout(&cred->cr_gid, ugidset, sizeof(*ugidset));
-	if (error != 0)
-		goto out;
-
-	if (ngrp > 1)
-		error = copyout(cred->cr_groups, ugidset + 1,
-		    (ngrp - 1) * sizeof(*ugidset));
+	error = copyout(cred->cr_groups, uap->gidset, ngrp * sizeof(gid_t));
 out:
 	td->td_retval[0] = ngrp;
 	return (error);
@@ -1186,6 +1205,44 @@ fail:
 	return (error);
 }
 
+#ifdef COMPAT_FREEBSD14
+int
+freebsd14_setgroups(struct thread *td, struct freebsd14_setgroups_args *uap)
+{
+	gid_t smallgroups[CRED_SMALLGROUPS_NB];
+	gid_t *groups;
+	int gidsetsize, error;
+
+	/*
+	 * Before FreeBSD 15.0, we allow one more group to be supplied to
+	 * account for the egid appearing before the supplementary groups.  This
+	 * may technically allow one more supplementary group for systems that
+	 * did use the default NGROUPS_MAX if we round it back up to 1024.
+	 */
+	gidsetsize = uap->gidsetsize;
+	if (gidsetsize > ngroups_max + 1 || gidsetsize < 0)
+		return (EINVAL);
+
+	if (gidsetsize > CRED_SMALLGROUPS_NB)
+		groups = malloc(gidsetsize * sizeof(gid_t), M_TEMP, M_WAITOK);
+	else
+		groups = smallgroups;
+
+	error = copyin(uap->gidset, groups, gidsetsize * sizeof(gid_t));
+	if (error == 0) {
+		int ngroups = gidsetsize > 0 ? gidsetsize - 1 /* egid */ : 0;
+
+		error = kern_setgroups(td, &ngroups, groups + 1);
+		if (error == 0 && gidsetsize > 0)
+			td->td_proc->p_ucred->cr_gid = groups[0];
+	}
+
+	if (groups != smallgroups)
+		free(groups, M_TEMP);
+	return (error);
+}
+#endif	/* COMPAT_FREEBSD14 */
+
 #ifndef _SYS_SYSPROTO_H_
 struct setgroups_args {
 	int	gidsetsize;
@@ -1210,8 +1267,7 @@ sys_setgroups(struct thread *td, struct setgroups_args *uap)
 	 * setgroups() differ.
 	 */
 	gidsetsize = uap->gidsetsize;
-	/* XXXKE Limit to ngroups_max when we change the userland interface. */
-	if (gidsetsize > ngroups_max + 1 || gidsetsize < 0)
+	if (gidsetsize > ngroups_max || gidsetsize < 0)
 		return (EINVAL);
 
 	if (gidsetsize > CRED_SMALLGROUPS_NB)
@@ -1238,35 +1294,17 @@ kern_setgroups(struct thread *td, int *ngrpp, gid_t *groups)
 	struct proc *p = td->td_proc;
 	struct ucred *newcred, *oldcred;
 	int ngrp, error;
-	gid_t egid;
 
 	ngrp = *ngrpp;
 	/* Sanity check size. */
-	/* XXXKE Limit to ngroups_max when we change the userland interface. */
-	if (ngrp < 0 || ngrp > ngroups_max + 1)
+	if (ngrp < 0 || ngrp > ngroups_max)
 		return (EINVAL);
 
 	AUDIT_ARG_GROUPSET(groups, ngrp);
-	/*
-	 * setgroups(0, NULL) is a legitimate way of clearing the groups vector
-	 * on non-BSD systems (which generally do not have the egid in the
-	 * groups[0]).  We risk security holes when running non-BSD software if
-	 * we do not do the same.  So we allow and treat 0 for 'ngrp' specially
-	 * below (twice).
-	 */
-	if (ngrp != 0) {
-		/*
-		 * To maintain userland compat for now, we use the first group
-		 * as our egid and we'll use the rest as our supplemental
-		 * groups.
-		 */
-		egid = groups[0];
-		ngrp--;
-		groups++;
 
-		groups_normalize(&ngrp, groups);
-		*ngrpp = ngrp;
-	}
+	groups_normalize(&ngrp, groups);
+	*ngrpp = ngrp;
+
 	newcred = crget();
 	crextend(newcred, ngrp);
 	PROC_LOCK(p);
@@ -1289,15 +1327,7 @@ kern_setgroups(struct thread *td, int *ngrpp, gid_t *groups)
 	if (error)
 		goto fail;
 
-	/*
-	 * If some groups were passed, the first one is currently the desired
-	 * egid.  This code is to be removed (along with some commented block
-	 * above) when setgroups() is changed to take only supplementary groups.
-	 */
-	if (ngrp != 0)
-		newcred->cr_gid = egid;
 	crsetgroups_internal(newcred, ngrp, groups);
-
 	setsugid(p);
 	proc_set_cred(p, newcred);
 	PROC_UNLOCK(p);