cred: Fix group_is_primary()

[markj]

Summary

This helper wasn't updated in commit be1f7435ef21, so in reality it was
testing whether "gid" is the first supplemental group. If a user
doesn't belong to a supplementary group, then it's testing an
uninitialized slot; since ucreds are allocated with M_ZERO, this
typically means that we're testing gid == 0.

group_is_primary() has exactly one use, in mac_do. There, it's used to
determine whether the requested primary GID can be used in a setcred(2)
call when the ruleset does not explicitly specify a target primary GID.

I believe this is mostly exploitable by daemons which have explicitly
dropped privileges and called setgroups(0, NULL); logged in users will
have a non-empty supplementary group list by virtue of having gone
through initgroups(3).

Fix group_is_primary(), and add a regression test.

Reported by: Hazley Samsudin of GovTech CSG
Fixes: be1f7435ef21 ("kern: start tracking cr_gid outside of cr_groups[]")

[kevans]

That's a bit embarrasing. =\ I have no idea how this one was overlooked, I did a lot of grepping around for cr_groups indexing...

[olce]

In D59051#1354343, @kevans wrote:

That's a bit embarrasing. =\ I have no idea how this one was overlooked, I did a lot of grepping around for cr_groups indexing...

Yeah, it's really strange that this was missed, as I also did a lot of grepping.

Thanks for noticing this. The fix in sys/sys/ucred.h is obviously correct.

I believe this is mostly exploitable by daemons which have explicitly
dropped privileges and called setgroups(0, NULL); logged in users will
have a non-empty supplementary group list by virtue of having gone
through initgroups(3).

Yes, but the bug itself extends beyond this case. For the precise exploitation described here, only rules with gid=0 as the "from part" are vulnerable (because, as you developed, cr_groups[0] is initialized to 0).

I haven't thoroughly reviewed the test case, as I'm still AFK, but it looks fine. Will be back in 2 days from now.

[markj]

In D59051#1356178, @olce wrote:

I haven't thoroughly reviewed the test case, as I'm still AFK, but it looks fine. Will be back in 2 days from now.

Thanks. We (secteam) would like to release the patch early this coming week, probably on tuesday. The test case can always be fixed up later if need be.

[olce]

In D59051, @markj wrote:

group_is_primary() has exactly one use, in mac_do. There, it's used to
determine whether to keep the caller's current primary groups. This
means that a rule such as gid=0>uid=0 will permit any credential with no
supplementary groups.

Sorry, after more examination, it appears that this description, as well as the one I added above, are both mostly wrong.

group_is_primary() is called only from rule_grant_primary_group(), whose purpose is to examine a candidate (in other words, target) for a "primary" group ID (real, effective or saved), and only with MDF_CURRENT set, which corresponds to a rule without gid or with an explicit gid=. in the target part.

Assuming that the initial primary groups are all set to the same value (this is the case when using mdo, and the expected usual use case), the bug allows the group_is_primary() test to pass on an original group or if the target primary group is the first supplementary group, or 0 when there is no supplementary groups (as you developed).

This is only one of many tests, so no arbitrary credentials can be set. The only unintended possibility is the ability to set a primary group to the first supplementary group or 0 in the target credentials, the rest still obeys the rules. The most problematic case from a security standpoint is the no supplementary groups case (as you identified), because basically that means you can request new credentials with a primary GID (such as the effective GID) of 0. This ability does not rely at all on the content of the "from part", so whether it is gid=0>... has no direct influence on whether the bug is triggered.

To wrap up, no arbitrary credentials can be set, and only an effective (and/or real, saved) GID of 0 can be requested if the target part of a rule has no gid at all (with or without flags) or an explicit gid=., provided the initial credentials have no supplementary groups.

You're certainly right that the addition of the gid=0>uid=0 rule to the bug above allows to become root (starting from the credentials matching the other rule, since gid=0>uid=0 is not "exploitable" as it allows a GID of 0 but only if already having it) once you've succeeded in setting credentials with a primary GID of 0. It's just that this rule alone does not trigger the bug, and its presence is necessary to be able to set arbitrary credentials (but that is in part because of the administrator's configuration). What is always true is that a credentials with a primary GID of 0 can be requested (with the conditions described above). I'd suggest separating these matters in the commit message for clarity.

In D59051#1356360, @markj wrote:

The test case can always be fixed up later if need be.

Sure. I'd at least remove the gid=0>uid=0 rule from it though, as it is not used and thus not necessary in the test.

Out of curiosity, any special reason for writing this test in C instead of shell script? In C, a dance with exec_paths is needed to be able to call setcred() and have it authorized by mac_do(4). Alternatively, mdo(1) could be leveraged directly.

[markj]

In D59051#1357232, @olce wrote:

In D59051, @markj wrote:

group_is_primary() has exactly one use, in mac_do. There, it's used to
determine whether to keep the caller's current primary groups. This
means that a rule such as gid=0>uid=0 will permit any credential with no
supplementary groups.

Sorry, after more examination, it appears that this description, as well as the one I added above, are both mostly wrong.

I've amended the commit log message a bit to try and correct this. It's too late to change the releng branches now, so the commit log message there will be inaccurate, but that's not the end of the world.

group_is_primary() is called only from rule_grant_primary_group(), whose purpose is to examine a candidate (in other words, target) for a "primary" group ID (real, effective or saved), and only with MDF_CURRENT set, which corresponds to a rule without gid or with an explicit gid=. in the target part.

Assuming that the initial primary groups are all set to the same value (this is the case when using mdo, and the expected usual use case), the bug allows the group_is_primary() test to pass on an original group or if the target primary group is the first supplementary group, or 0 when there is no supplementary groups (as you developed).

This is only one of many tests, so no arbitrary credentials can be set. The only unintended possibility is the ability to set a primary group to the first supplementary group or 0 in the target credentials, the rest still obeys the rules. The most problematic case from a security standpoint is the no supplementary groups case (as you identified), because basically that means you can request new credentials with a primary GID (such as the effective GID) of 0. This ability does not rely at all on the content of the "from part", so whether it is gid=0>... has no direct influence on whether the bug is triggered.

To wrap up, no arbitrary credentials can be set, and only an effective (and/or real, saved) GID of 0 can be requested if the target part of a rule has no gid at all (with or without flags) or an explicit gid=., provided the initial credentials have no supplementary groups.

You're certainly right that the addition of the gid=0>uid=0 rule to the bug above allows to become root (starting from the credentials matching the other rule, since gid=0>uid=0 is not "exploitable" as it allows a GID of 0 but only if already having it) once you've succeeded in setting credentials with a primary GID of 0. It's just that this rule alone does not trigger the bug, and its presence is necessary to be able to set arbitrary credentials (but that is in part because of the administrator's configuration). What is always true is that a credentials with a primary GID of 0 can be requested (with the conditions described above). I'd suggest separating these matters in the commit message for clarity.

I tried to make this clear in the security advisory text, I pasted the relevant sections below. Any feedback on that would be great, though please note that it will be published later today, so there is not much time.

In D59051#1356360, @markj wrote:

The test case can always be fixed up later if need be.

Sure. I'd at least remove the gid=0>uid=0 rule from it though, as it is not used and thus not necessary in the test.

Yeah. We're already building patches for a secteam release, so I prefer to make that change separately.

Out of curiosity, any special reason for writing this test in C instead of shell script? In C, a dance with exec_paths is needed to be able to call setcred() and have it authorized by mac_do(4). Alternatively, mdo(1) could be leveraged directly.

Not really, I thought the program would be simpler than it turned out, and I wanted to understand the setcred() interface. In particular I didn't know about the exec_paths restriction when I started.

For a standalone regression test it doesn't really matter. It'd be fine to rewrite this.

I.   Background                 

mac_do(4) is a MAC policy module that allows unprivileged processes to
switch credentials using setcred(2), subject to administrator-defined
rules.  Rules can restrict transitions based on the calling process'
user and group identities.                                                     
                                                                               
II.  Problem Description                
                                                                               
In FreeBSD 15.0, the kernel structure used to represent user credentials
changed: previously the primary group ID was stored in the first element
of the array containing the list of supplementary group IDs, whereas now
the primary group ID is stored in a dedicated field.  This change was
largely internal to the kernel and not user-visible.                  
                                                                               
One function, group_is_primary(), was not properly updated as a part of
this transition.  This function is used by mac_do to determine the             
primary group ID of the credential after applying a transition rule,     
used when the rule target does not explicitly specify a group.                 
                                                                               
As a result, with certain mac_do rules, it is possible for a credential  
switch to incorrectly set the primary group ID to the ID stored in the   
first element of the original credential's supplementary group array.
If the list of supplementary groups is empty, this value will be 0,
corresponding to the "wheel" group.  For example, a rule such as
"uid=1001>uid=1002" can be abused to set the primary group ID to 0 even
if the process did not originally belong to group 0.                        

III. Impact                                                                 
                                                                               
Certain mac_do rules can be abused to set a process' group ID to 0.
Note however, that the rule must apply to the caller in order for the
bug to be triggered, e.g., given the ruleset "uid=1001>uid=1002", the  
user must have user ID 1001 in order to trigger the bug.
                                                                               
Further, logged-in users will in general have a non-empty supplementary        
group list, in which case the bug can at worst be used to set the
credential's first supplementary group ID as its primary group ID.             
Processes must explicitly remove themselves from all supplementary 
groups, using the privileged setgroups(2) system call, in order to
exploit the bug to set 0 as the primary group ID.
Since membership in group 0 is often used to enable controlled privilege
escalation, the bug might be further exploitable to obtain root                
privileges, depending on the system configuration.  For instance, a      
ruleset such as the following could be exploited by a process running
as user 1001 and with an empty supplementary group list:
"uid=1001>uid=1002;gid=0>uid=0".

IV.  Workaround

Systems that do not configure a mac_do ruleset are not affected.

Rulesets which explicitly specify a target group ID are not subject to
the bug.

[olce]

The text of the advisory is really great! Thanks.