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 +Until +.Fx 15.0 , +the +.Fn getgroups +system call always returned the effective group-ID for the process as the first +element of the array. 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,16 @@ .Fn setgroups system call appeared in .Bx 4.2 . +.Pp +Until +.Fx 15.0 , +the +.Fn setgroups +system call would set the effective group-ID for the process to the first +element of +.Fa gidset , +and a later call to +.Xr setgid 2 +or +.Xr setegid 2 +would overwrite the first element provided. 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 supplemental 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,11 @@ 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 +332,7 @@ 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); @@ -1210,8 +1196,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 > ngroups_max || gidsetsize < 0) return (EINVAL); if (gidsetsize > CRED_SMALLGROUPS_NB) @@ -1238,35 +1223,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 +1256,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/usr.bin/newgrp/newgrp.c b/usr.bin/newgrp/newgrp.c --- a/usr.bin/newgrp/newgrp.c +++ b/usr.bin/newgrp/newgrp.c @@ -195,7 +195,7 @@ } /* Remove requested gid from supp. list if it exists. */ - if (grp->gr_gid != egid && inarray(grp->gr_gid, grps, ngrps)) { + if (inarray(grp->gr_gid, grps, ngrps)) { for (i = 0; i < ngrps; i++) if (grps[i] == grp->gr_gid) break; @@ -217,10 +217,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.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c --- a/usr.sbin/chroot/chroot.c +++ b/usr.sbin/chroot/chroot.c @@ -108,15 +108,10 @@ } } - 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);