Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set supplementary groups by default, allow users to modify supplementary groups #68

Closed
stevendanna opened this issue Oct 23, 2014 · 6 comments
Labels
Type: Bug Does not work as expected.

Comments

@stevendanna
Copy link

In addition to the real and effective group ID of a process, every process has a list of supplementary groups IDs. From the credentials(7) man page on Linux:

Supplementary group IDs. This is a set of additional group IDs that are used for permission checks when accessing files and other shared resources. On Linux kernels before 2.6.4, a process can be a member of up to 32 supplementary groups; since kernel 2.6.4, a process can be a member of up to 65536 supplementary groups. The call sysconf(_SC_NGROUPS_MAX) can be used to determine the number of supplementary groups of which a process may be a member. A process can obtain its set of supplementary group IDs using getgroups(2), and can modify the set using setgroups(2).

See the related Chef bug here: https://tickets.opscode.com/browse/CHEF-3510

Programs such as sudo and su typically set the list of supplementary groups to the groups of which the EUID is a member.

In C, the relevant system calls to modify the group list are initgroups and setgroups

In Ruby, the group list can be modified with the Process.groups= method.

@stevendanna stevendanna changed the title Set supplmentary groups by default, allow users to modify supplemntary groups Set supplmentary groups by default, allow users to modify supplementary groups Oct 23, 2014
@maoueh
Copy link

maoueh commented Nov 3, 2014

@sersut My opinion would be to change this issue from Enhancement to Bug. This problem prevents valid use cases from functioning correctly.

@lamont-granquist
Copy link
Contributor

That's just a semantic argument, it won't make any difference for getting it fixed.

@sersut sersut added Bug and removed Enhancement labels Nov 4, 2014
@maoueh
Copy link

maoueh commented Nov 5, 2014

@lamont-granquist Indeed, sorry.

I started looking at this and it seems that Process.initgroups does exactly this, i.e. setting Process.groups with all groups user has access to.

Doing this implicitly is probably the right fix but may have unintended consequences. I will prepare a PR so we can discuss this.

Matt

@lamont-granquist
Copy link
Contributor

Yeah looks simple enough on the surface of it... I don't think there's any downside to it either. It gives the process more rights than it initially had so there should only be use case where it does not fail and now succeeds. And its not getting any rights that it shouldn't have access to, its just getting all the rights that it has a claim to. Seems like if anyone is broken by this that it'd be an xkcd 1172.

@sean-horn
Copy link

Affected customer in 3656

@sean-horn sean-horn changed the title Set supplmentary groups by default, allow users to modify supplementary groups Set supplementary groups by default, allow users to modify supplementary groups Apr 1, 2015
@lamont-granquist
Copy link
Contributor

with_login fixes are merged

@thommay thommay added Type: Bug Does not work as expected. and removed Bug labels Jan 24, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Does not work as expected.
Projects
None yet
Development

No branches or pull requests

6 participants