Date: Thu, 23 Apr 2026 21:13:24 +0000 From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: b675ff8eedc9 - main - mac_seeotheruids: allow specificgid to be a list of groups Message-ID: <69ea8b74.30ff7.599b6a18@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=b675ff8eedc9ac93cdf1cfe33185b7a1a027df37 commit b675ff8eedc9ac93cdf1cfe33185b7a1a027df37 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2026-03-01 03:42:25 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2026-04-23 21:13:11 +0000 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 | 4 +- sys/security/mac_seeotheruids/mac_seeotheruids.c | 174 +++++++++++++++++++++-- 2 files changed, 166 insertions(+), 12 deletions(-) diff --git a/share/man/man4/mac_seeotheruids.4 b/share/man/man4/mac_seeotheruids.4 index 5b1718bf83d4..04f67ebb7ea3 100644 --- a/share/man/man4/mac_seeotheruids.4 +++ b/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 . diff --git a/sys/security/mac_seeotheruids/mac_seeotheruids.c b/sys/security/mac_seeotheruids/mac_seeotheruids.c index 9cd2e0f3c0fc..a112a904fa72 100644 --- a/sys/security/mac_seeotheruids/mac_seeotheruids.c +++ b/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,home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?69ea8b74.30ff7.599b6a18>