Skip to content

Commit

Permalink
Add effective group as the first element in the call to set groups (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
squell authored Jan 6, 2025
2 parents d92bc5a + 291b179 commit 33fd1c3
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/su/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl SuContext {

// last argument is the primary group
group = primary_group.clone();
user.groups.push(primary_group.gid);
user.groups.insert(0, primary_group.gid);
}

// add additional group if current user is root
Expand Down
13 changes: 10 additions & 3 deletions src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,16 @@ pub fn set_target_user(
) {
use std::os::unix::process::CommandExt;

// add target group to list of additional groups if not present
if !target_user.groups.contains(&target_group.gid) {
target_user.groups.push(target_group.gid);
if let Some(index) = target_user
.groups
.iter()
.position(|id| id == &target_group.gid)
{
// make sure the requested group id is the first in the list (necessary on FreeBSD)
target_user.groups.swap(0, index)
} else {
// add target group to list of additional groups if not present
target_user.groups.insert(0, target_group.gid);
}

// we need to do this in a `pre_exec` call since the `groups` method in `process::Command` is unstable
Expand Down
32 changes: 32 additions & 0 deletions test-framework/sudo-compliance-tests/src/sudo/flag_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,38 @@ fn adds_group_to_groups_output() -> Result<()> {
Ok(())
}

#[test]
fn supplementary_groups_can_be_made_primary() -> Result<()> {
let extra_group = "rustaceans";
let env = Env(SUDOERS_ALL_ALL_NOPASSWD)
.user(User(USERNAME).secondary_group(extra_group))
.group(Group(extra_group))
.group("secondary-group")
.build()?;

let stdout = Command::new("groups")
.as_user(USERNAME)
.output(&env)?
.stdout()?;
let mut groups_without_sudo = stdout.split_ascii_whitespace().collect::<Vec<_>>();

let stdout = Command::new("sudo")
.args(["-g", extra_group, "groups"])
.as_user(USERNAME)
.output(&env)?
.stdout()?;

let mut groups_with_sudo = stdout.split_ascii_whitespace().collect::<Vec<_>>();

assert_eq!(groups_with_sudo[0], extra_group);

groups_without_sudo.sort();
groups_with_sudo.sort();
assert_eq!(groups_with_sudo, groups_without_sudo);

Ok(())
}

#[test]
fn group_can_be_specified_by_id() -> Result<()> {
let expected_gid = 1234;
Expand Down

0 comments on commit 33fd1c3

Please sign in to comment.