diff --git a/lib/libsys/getgroups.2 b/lib/libsys/getgroups.2 --- a/lib/libsys/getgroups.2 +++ b/lib/libsys/getgroups.2 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd January 21, 2011 +.Dd August 1, 2025 .Dt GETGROUPS 2 .Os .Sh NAME @@ -41,8 +41,8 @@ The .Fn getgroups system call -gets the current group access list of the user process -and stores it in the array +gets the current supplementary groups of the user process and stores it in the +array .Fa gidset . The .Fa gidsetlen @@ -54,7 +54,7 @@ system call returns the actual number of groups returned in .Fa gidset . -At least one and as many as {NGROUPS_MAX}+1 values may be returned. +As many as {NGROUPS_MAX} values may be returned. If .Fa gidsetlen is zero, @@ -102,3 +102,10 @@ .Fn getgroups system call appeared in .Bx 4.2 . +.Pp +Before +.Fx 15.0 , +the +.Fn getgroups +system call always returned the effective group ID for the process as the first +element of the array, before the supplementary groups. diff --git a/lib/libsys/setgroups.2 b/lib/libsys/setgroups.2 --- a/lib/libsys/setgroups.2 +++ b/lib/libsys/setgroups.2 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd January 19, 2018 +.Dd August 1, 2025 .Dt SETGROUPS 2 .Os .Sh NAME @@ -42,7 +42,7 @@ The .Fn setgroups system call -sets the group access list of the current user process +sets the supplementary group list of the current user process according to the array .Fa gidset . The @@ -50,26 +50,12 @@ argument indicates the number of entries in the array and must be no more than -.Dv {NGROUPS_MAX}+1 . -.Pp -Only the super-user may set a new group list. +.Dv {NGROUPS_MAX} . +The +.Fa ngroups +argument may be set to 0 to clear the supplementary group list. .Pp -The first entry of the group array -.Pq Va gidset[0] -is used as the effective group-ID for the process. -This entry is over-written when a setgid program is run. -To avoid losing access to the privileges of the -.Va gidset[0] -entry, it should be duplicated later in the group array. -By convention, -this happens because the group value indicated -in the password file also appears in -.Pa /etc/group . -The group value in the password file is placed in -.Va gidset[0] -and that value then gets added a second time when the -.Pa /etc/group -file is scanned to create the group set. +Only the super-user may set a new supplementary group list. .Sh RETURN VALUES .Rv -std setgroups .Sh ERRORS @@ -99,3 +85,11 @@ .Fn setgroups system call appeared in .Bx 4.2 . +.Pp +Before +.Fx 15.0 , +the +.Fn setgroups +system call would set the effective group ID for the process to the first +element of +.Fa gidset . diff --git a/sbin/hastd/subr.c b/sbin/hastd/subr.c --- a/sbin/hastd/subr.c +++ b/sbin/hastd/subr.c @@ -207,10 +207,8 @@ } } PJDLOG_VERIFY(chdir("/") == 0); - gidset[0] = pw->pw_gid; - if (setgroups(1, gidset) == -1) { - pjdlog_errno(LOG_ERR, "Unable to set groups to gid %u", - (unsigned int)pw->pw_gid); + if (setgroups(0, NULL) == -1) { + pjdlog_errno(LOG_ERR, "Unable to drop supplementary groups"); return (-1); } if (setgid(pw->pw_gid) == -1) { @@ -287,8 +285,7 @@ PJDLOG_VERIFY(egid == pw->pw_gid); PJDLOG_VERIFY(sgid == pw->pw_gid); PJDLOG_VERIFY(getgroups(0, NULL) == 1); - PJDLOG_VERIFY(getgroups(1, gidset) == 1); - PJDLOG_VERIFY(gidset[0] == pw->pw_gid); + PJDLOG_VERIFY(getgroups(1, gidset) == 0); pjdlog_debug(1, "Privileges successfully dropped using %s%s+setgid+setuid.", diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -320,18 +320,20 @@ sys_getgroups(struct thread *td, struct getgroups_args *uap) { struct ucred *cred; - gid_t *ugidset; int ngrp, error; +#ifdef COMPAT_FREEBSD14 + int osrel = td->td_proc->p_osrel; + bool getgroups_compat; +#endif 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; +#ifdef COMPAT_FREEBSD14 + getgroups_compat = osrel < P_OSREL_SETGROUPS_EGID; + if (getgroups_compat) + ngrp++; +#endif if (uap->gidsetsize == 0) { error = 0; goto out; @@ -339,14 +341,15 @@ 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)); +#ifdef COMPAT_FREEBSD14 + if (getgroups_compat) { + 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)); + } else +#endif + error = copyout(cred->cr_groups, uap->gidset, ngrp * sizeof(gid_t)); out: td->td_retval[0] = ngrp; return (error); @@ -1198,7 +1201,18 @@ { gid_t smallgroups[CRED_SMALLGROUPS_NB]; gid_t *groups; - int gidsetsize, error; + int gidsetsize, error, maxsize; +#ifdef COMPAT_FREEBSD14 + int osrel = td->td_proc->p_osrel; + bool setgroups_compat; +#endif + + maxsize = ngroups_max; +#ifdef COMPAT_FREEBSD14 + setgroups_compat = osrel < P_OSREL_SETGROUPS_EGID; + if (setgroups_compat) + maxsize++; /* One more to account for the egid */ +#endif /* * Sanity check size now to avoid passing too big a value to copyin(), @@ -1210,8 +1224,7 @@ * 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 > maxsize || gidsetsize < 0) return (EINVAL); if (gidsetsize > CRED_SMALLGROUPS_NB) @@ -1220,8 +1233,19 @@ groups = smallgroups; error = copyin(uap->gidset, groups, gidsetsize * sizeof(gid_t)); - if (error == 0) + if (error == 0) { +#ifdef COMPAT_FREEBSD14 + if (gidsetsize > 0 && setgroups_compat) { + gidsetsize--; + error = kern_setgroups(td, &gidsetsize, groups + 1); + } else +#endif error = kern_setgroups(td, &gidsetsize, groups); +#ifdef COMPAT_FREEBSD14 + if (uap->gidsetsize > 0 && error == 0 && setgroups_compat) + td->td_proc->p_ucred->cr_gid = groups[0]; +#endif + } if (groups != smallgroups) free(groups, M_TEMP); @@ -1238,35 +1262,17 @@ 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 +1295,7 @@ 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); diff --git a/sys/sys/param.h b/sys/sys/param.h --- a/sys/sys/param.h +++ b/sys/sys/param.h @@ -109,6 +109,7 @@ #define P_OSREL_ARM64_SPSR 1400084 #define P_OSREL_TLSBASE 1500044 #define P_OSREL_EXTERRCTL 1500045 +#define P_OSREL_SETGROUPS_EGID 1500056 #define P_OSREL_MAJOR(x) ((x) / 100000) #endif diff --git a/usr.bin/newgrp/newgrp.c b/usr.bin/newgrp/newgrp.c --- a/usr.bin/newgrp/newgrp.c +++ b/usr.bin/newgrp/newgrp.c @@ -186,7 +186,7 @@ } } - ngrps_max = sysconf(_SC_NGROUPS_MAX) + 1; + ngrps_max = sysconf(_SC_NGROUPS_MAX); if ((grps = malloc(sizeof(gid_t) * ngrps_max)) == NULL) err(1, "malloc"); if ((ngrps = getgroups(ngrps_max, (gid_t *)grps)) < 0) { @@ -194,7 +194,12 @@ goto end; } - /* Remove requested gid from supp. list if it exists. */ + /* + * Remove requested gid from supp. list if it exists and doesn't match + * our prior egid -- this exception is to avoid providing the user a + * means to get rid of a group that could be used for, e.g., negative + * permissions. + */ if (grp->gr_gid != egid && inarray(grp->gr_gid, grps, ngrps)) { for (i = 0; i < ngrps; i++) if (grps[i] == grp->gr_gid) @@ -217,10 +222,9 @@ goto end; } PRIV_END; - grps[0] = grp->gr_gid; /* Add old effective gid to supp. list if it does not exist. */ - if (egid != grp->gr_gid && !inarray(egid, grps, ngrps)) { + if (!inarray(egid, grps, ngrps)) { if (ngrps == ngrps_max) warnx("too many groups"); else { diff --git a/usr.bin/quota/quota.c b/usr.bin/quota/quota.c --- a/usr.bin/quota/quota.c +++ b/usr.bin/quota/quota.c @@ -100,8 +100,7 @@ int main(int argc, char *argv[]) { - int ngroups; - gid_t mygid, gidset[NGROUPS]; + int ngroups; int i, ch, gflag = 0, uflag = 0, errflag = 0; while ((ch = getopt(argc, argv, "f:ghlrquv")) != -1) { @@ -142,11 +141,15 @@ if (uflag) errflag += showuid(getuid()); if (gflag) { + gid_t mygid, myegid, gidset[NGROUPS_MAX]; + mygid = getgid(); - ngroups = getgroups(NGROUPS, gidset); + errflag += showgid(mygid); + myegid = getegid(); + errflag += showgid(myegid); + ngroups = getgroups(NGROUPS_MAX, gidset); if (ngroups < 0) err(1, "getgroups"); - errflag += showgid(mygid); for (i = 0; i < ngroups; i++) if (gidset[i] != mygid) errflag += showgid(gidset[i]); diff --git a/usr.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c --- a/usr.sbin/chroot/chroot.c +++ b/usr.sbin/chroot/chroot.c @@ -139,15 +139,10 @@ if (group != NULL) gid = resolve_group(group); - ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + ngroups_max = sysconf(_SC_NGROUPS_MAX); if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL) err(1, "malloc"); - /* Populate the egid slot in our groups to avoid accidents. */ - if (gid == 0) - gidlist[0] = getegid(); - else - gidlist[0] = gid; - for (gids = 1; + for (gids = 0; (p = strsep(&grouplist, ",")) != NULL && gids < ngroups_max; ) { if (*p == '\0') continue; diff --git a/usr.sbin/lpr/lpc/lpc.c b/usr.sbin/lpr/lpc/lpc.c --- a/usr.sbin/lpr/lpc/lpc.c +++ b/usr.sbin/lpr/lpc/lpc.c @@ -358,6 +358,8 @@ err(1, "getgroups"); } gid = gptr->gr_gid; + if (gid == getegid()) + return(1); for (i = 0; i < ngroups; i++) if (gid == groups[i]) return(1);