mac_seeotheruids: allow specificgid to be a list of groups

The specificgid functionality has historically allowed only a single
group to be exempt, but in practice one might want a few services to
be exempt for reasons.  From a security perspective, we probably don't
want to encourage unrelated users to be grouped together solely for
this purpose, as that creates one point of shared access that could be
used for nefarious purposes.

Normalize the group list as we do cr_groups to allow for linear matching
rather than quadratic, we just need to account for the differences in
FreeBSD 15.0+ where cr_groups is entirely supplementary groups vs.
earlier versions, where cr_groups[0] is the egid and the rest is
sorted.

Reviewed by:	csjp, des (earlier version)
Sponsored by:	Klara, Inc.
Differential Revision:	https://reviews.freebsd.org/D56592

share/man/man4/mac_seeotheruids.4

@@ -28,7 +28,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd July 25, 2015
+.Dd Februrary 26, 2026
 .Dt MAC_SEEOTHERUIDS 4
 .Os
 .Sh NAME
@@ -80,7 +80,7 @@ set the sysctl OID
 .Va security.mac.seeotheruids.specificgid_enabled
 to 1, and
 .Va security.mac.seeotheruids.specificgid
-to the group ID to be exempted.
+to the list of group IDs to be exempted.
 .Ss Label Format
 No labels are defined for
 .Nm .

sys/security/mac_seeotheruids/mac_seeotheruids.c

@@ -45,9 +45,12 @@
 
 #include <sys/param.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
+#include <sys/malloc.h>
 #include <sys/module.h>
 #include <sys/priv.h>
 #include <sys/proc.h>
+#include <sys/rmlock.h>
 #include <sys/systm.h>
 #include <sys/socket.h>
 #include <sys/socketvar.h>
@@ -59,6 +62,9 @@
 
 #include <security/mac/mac_policy.h>
 
+static MALLOC_DEFINE(M_SEEOTHERUIDS, "mac_seeotheruids",
+    "mac_seeotheruids(4) security module");
+
 static SYSCTL_NODE(_security_mac, OID_AUTO, seeotheruids,
     CTLFLAG_RW | CTLFLAG_MPSAFE, 0,
     "TrustedBSD mac_seeotheruids policy controls");
@@ -94,13 +100,116 @@ SYSCTL_INT(_security_mac_seeotheruids, OID_AUTO, specificgid_enabled,
     CTLFLAG_RW, &specificgid_enabled, 0, "Make an exception for credentials "
     "with a specific gid as their real primary group id or group set");
 
-static gid_t	specificgid = 0;
-SYSCTL_UINT(_security_mac_seeotheruids, OID_AUTO, specificgid, CTLFLAG_RW,
-    &specificgid, 0, "Specific gid to be exempt from seeotheruids policy");
+static struct rmlock seeotheruids_rmlock;
+RM_SYSINIT_FLAGS(mac_seeotheruids_lock, &seeotheruids_rmlock,
+    "mac_seeotheruids_lock", RM_SLEEPABLE);
+
+static gid_t	*specificgids;
+static size_t	 specificgidcnt;
+
+static int
+gidp_cmp(const void *p1, const void *p2)
+{
+	const gid_t g1 = *(const gid_t *)p1;
+	const gid_t g2 = *(const gid_t *)p2;
+
+	return ((g1 > g2) - (g1 < g2));
+}
+
+static void
+specificgid_normalize(gid_t *gidlist, size_t *ngidp)
+{
+	int ins_idx;
+	gid_t prev_g;
+
+	if (*ngidp < 2)
+		return;
+
+	qsort(gidlist, *ngidp, sizeof(*gidlist), gidp_cmp);
+
+	prev_g = gidlist[0];
+	ins_idx = 1;
+	for (int i = ins_idx; i < *ngidp; ++i) {
+		const gid_t g = gidlist[i];
+
+		if (g != prev_g) {
+			if (i != ins_idx)
+				gidlist[ins_idx] = g;
+			++ins_idx;
+			prev_g = g;
+		}
+	}
+
+	*ngidp = ins_idx;
+}
+
+static int
+specificgid_sysctl(SYSCTL_HANDLER_ARGS)
+{
+	gid_t *newgids = NULL;
+	size_t ingidcnt, newgidcnt = 0;
+	int error;
+
+	/* Allocate our new gid array before we take our non-sleepable lock. */
+	if (req->newptr != NULL) {
+		if (req->newlen % sizeof(gid_t) != 0)
+			return (EINVAL);
+		ingidcnt = newgidcnt = howmany(req->newlen, sizeof(gid_t));
+		newgids = mallocarray(newgidcnt, sizeof(*newgids),
+		    M_SEEOTHERUIDS, M_WAITOK);
+
+		error = SYSCTL_IN(req, newgids, newgidcnt * sizeof(*newgids));
+		if (error != 0) {
+			free(newgids, M_SEEOTHERUIDS);
+			return (error);
+		}
+
+		specificgid_normalize(newgids, &newgidcnt);
+
+		/*
+		 * It might be debatable whether shrinking the allocation is
+		 * worth it, but we'll do it in the off-chance that someone is
+		 * generating specificgid entries from various configuration
+		 * sources that won't de-duplicate.
+		 */
+		if (newgidcnt < ingidcnt) {
+			newgids = realloc(newgids, newgidcnt * sizeof(*newgids),
+			    M_SEEOTHERUIDS, M_WAITOK);
+		}
+	}
+
+	rm_wlock(&seeotheruids_rmlock);
+
+	error = SYSCTL_OUT(req, specificgids,
+	    specificgidcnt * sizeof(*specificgids));
+	if (error == 0 && req->newptr != NULL) {
+		free(specificgids, M_SEEOTHERUIDS);
+
+		specificgids = newgids;
+		specificgidcnt = newgidcnt;
+	} else if (error != 0) {
+		free(newgids, M_SEEOTHERUIDS);
+	}
+
+	rm_wunlock(&seeotheruids_rmlock);
+	return (error);
+}
+SYSCTL_PROC(_security_mac_seeotheruids, OID_AUTO, specificgid,
+    CTLFLAG_RW | CTLTYPE_UINT | CTLFLAG_MPSAFE, 0, 0,
+    &specificgid_sysctl, "I",
+    "Specific gid(s) to be exempt from seeotheruids policy");
+
+static void
+seeotheruids_destroy(struct mac_policy_conf *mpc __unused)
+{
+	free(specificgids, M_SEEOTHERUIDS);
+}
 
 static int
 seeotheruids_check(struct ucred *cr1, struct ucred *cr2)
 {
+	struct rm_priotracker tracker;
+	int error = ESRCH;
 
 	if (!seeotheruids_enabled)
 		return (0);
@@ -110,12 +219,6 @@ seeotheruids_check(struct ucred *cr1, struct ucred *cr2)
 			return (0);
 	}
 
-	if (specificgid_enabled) {
-		if (cr1->cr_rgid == specificgid ||
-		    groupmember(specificgid, cr1))
-			return (0);
-	}
-
 	if (cr1->cr_ruid == cr2->cr_ruid)
 		return (0);
 
@@ -124,7 +227,57 @@ seeotheruids_check(struct ucred *cr1, struct ucred *cr2)
 			return (0);
 	}
 
-	return (ESRCH);
+	rm_rlock(&seeotheruids_rmlock, &tracker);
+	if (specificgid_enabled && specificgids != NULL) {
+		const gid_t *suppgroups = cr1->cr_groups;
+		size_t nsupp = cr1->cr_ngroups;
+
+#if __FreeBSD_version < 1500056
+		/*
+		 * FreeBSD 15.0 changed the cr_groups layout: earlier versions
+		 * used cr_groups[0] for the effective GID, but that's somewhat
+		 * error-prone when propagated throughout the various parts of
+		 * the system (e.g., setgroups/getgroups).  In older versions,
+		 * we want to hop over the egid.
+		 */
+		suppgroups++;
+		nsupp--;
+#endif
+
+		for (size_t i = 0, s = 0; i < specificgidcnt; i++) {
+			gid_t cgid;
+
+			cgid = specificgids[i];
+			if (cgid == cr1->cr_rgid) {
+				error = 0;
+				break;
+			}
+
+			/*
+			 * specificgids and suppgroups are both sorted
+			 * ascending, so advance past all of the supplemental
+			 * groups that are lower than the specificgid we're
+			 * currently at.
+			 */
+			while (s < nsupp && cgid > suppgroups[s])
+				s++;
+
+			/*
+			 * Out of supplementary groups, but we'll keep checking
+			 * for rgid matches.
+			 */
+			if (s == nsupp)
+				continue;
+
+			if (cgid == suppgroups[s]) {
+				error = 0;
+				break;
+			}
+		}
+	}
+
+	rm_runlock(&seeotheruids_rmlock, &tracker);
+	return (error);
 }
 
 static int
@@ -174,6 +327,7 @@ seeotheruids_socket_check_visible(struct ucred *cred, struct socket *so,
 
 static struct mac_policy_ops seeotheruids_ops =
 {
+	.mpo_destroy = seeotheruids_destroy,
 	.mpo_proc_check_debug = seeotheruids_proc_check_debug,
 	.mpo_proc_check_sched = seeotheruids_proc_check_sched,
 	.mpo_proc_check_signal = seeotheruids_proc_check_signal,