Bug 220779 - getgroups result is affected by setegid
Summary: getgroups result is affected by setegid
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: standards (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-standards (Nobody)
Depends on:
Reported: 2017-07-16 17:49 UTC by Michael Zuo
Modified: 2017-10-04 20:33 UTC (History)
4 users (show)

See Also:

test case demonstrating the behaviour (546 bytes, text/x-csrc)
2017-07-16 17:52 UTC, Michael Zuo
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Zuo 2017-07-16 17:49:43 UTC
On FreeBSD, the groups applicable to a process are stored in an array with the egid in [0] and and zero or more (not necessarily distinct) additional gids in sorted order.

The setgid, setegid, and setregid functions change the egid of a process, but the process's supplementary group ids are to remain unchanged. Since getgroups, which claims to conform to IEEE Std 1003.1-2008 (“POSIX.1”), is defined to fill its second argument with calling process's supplementary group ids, so its output must not include the egid, which is changed by setgid &c.. The current implementation fills the second argument with the entire groups array, which does include the egid in [0].

- "The setgid() function shall not affect the supplementary group list in any way. Any supplementary group IDs of the calling process shall remain unchanged."
- "The setegid() function shall not affect the supplementary group list in any way."
- (ditto for setregid)
- "The getgroups() function shall fill in the array grouplist with the current supplementary group IDs of the calling process. It is implementation-defined whether getgroups() also returns the effective group ID in the grouplist array."
- "A process has up to {NGROUPS_MAX} supplementary group IDs in addition to the effective group ID."
Comment 1 Michael Zuo 2017-07-16 17:52:16 UTC
Created attachment 184407 [details]
test case demonstrating the behaviour
Comment 2 Michael Zuo 2017-07-16 17:55:18 UTC
(In reply to Michael Zuo from comment #1)
Expected output:
0 5 egid=0
0 5 egid=1

Actual output:
0 5 egid=0
1 5 egid=1
Comment 3 Konstantin Belousov freebsd_committer 2017-10-04 06:54:26 UTC
The SUSv4TC2 is quite explicit about it.  It allows either behavior, even accepting changing behavior on the same system:

As implied by the definition of supplementary groups, the
effective group ID may appear in the array returned by
getgroups() or it may be returned only by getegid( ). Duplication
may exist, but the application needs to call getegid( ) to be
sure of getting all of the information. Various implementation
variations and administrative sequences cause the set of groups
appearing in the result of getgroups( ) to vary in order and as
to whether the effective group ID is included, even when the set
of groups is the same (in the mathematical sense of
``set''). (The history of a process and its parents could affect
the details of the result.)

So as far as no group ids are dropped by an operation, it is fine to have
egid to not appear in the secondary group list, or to appear after further
changes.  Can you demonstrate a case where we loose a group from the list ?
Comment 4 Michael Zuo 2017-10-04 10:52:32 UTC
The supplementary group list contains the egid.

If we set the supplementary group list with setgroups, then change the egid with setegid, the previous egid is lost from the list of supplementary groups.
Comment 5 paulm 2017-10-04 20:33:16 UTC
The effective GID is stored in cr_groups[0] in the ucred struct.

kern_setgroups() writes to cr_groups[0] in the ucred struct.

Therefore, the syscall setgroups(int ngroups, const gid_t *gidset) overwrites the effective GID at cr_groups[0] with the first GID in *gidset.

(Because setgroups() writes to cr_groups[0], a subsequent call to setegid() will overwrite the supplementary GID at that location.  I think this is the symptom Michael Zuo is calling attention to.)      

Unless I'm confused about something, the implementation of setgroups() should write the supplementary group list starting at an offset of +1 in the cr_groups array in struct ucred in order to preserve the effective GID at cr_groups[0].