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

newgidmap - allow mapping of all groups the user is a member of #135

Open
tycho-kirchner opened this issue Oct 22, 2018 · 8 comments
Open

Comments

@tycho-kirchner
Copy link

I need to unshare (only) the mountspace of an unprivileged process. To do so, I currently have to
unshare the user-namespace as well. Having done so, I then remap all groups, the user is a member of,
into that namespace by using newgidmap. For large servers with frequent user/group changes it is quite annoying, to keep /etc/subgid in sync to allow users to map their own groups into the new user-namespaces.
Therefore it seems desirable to add an option to newgidmap, which allows the mapping of
all groups, the user is a member of. I could write that myself, if you are interested. What do you think?

@hallyn
Copy link
Member

hallyn commented Oct 24, 2018

Hi,

groups can be funny. For instance, see the whole negative acls issue. So while offhand I have no problem with this, I think we'd best let this sit a bit for comments.

Please feel free to write a patch for it in the meantime, it might help push people to think about it.

@hallyn
Copy link
Member

hallyn commented Jun 13, 2019

Let's talk about how to do this. I think it needs to be optional. Can we add metadata to /etc/subgid ? I.e.

$ cat /etc/subgid
#OPTION allow-aux-groups
user:100000:65536
lxd:165536:65536

root:165536:65536

@rhatdan
Copy link

rhatdan commented Apr 26, 2021

@giuseppe opened up a PR for setgid system call in the kernel 5 years ago, to potentially allow us to do this in a different way.

Currently in Podman we can execute a podman --add-user=keep-id ... Which allows us to leak the supplemental groups into the rootless containers. The issue we have is we can not simultaneously add additional groups to the container from within the user namespace.

@hallyn
Copy link
Member

hallyn commented Apr 27, 2021

How about a login.defs option UNPRIV_DELEGATE_AUX_GROUPS ?

It can be on by default in new installations, but if unset then the default should be off.

@giuseppe
Copy link
Contributor

giuseppe commented May 3, 2021

How about a login.defs option UNPRIV_DELEGATE_AUX_GROUPS ?

It can be on by default in new installations, but if unset then the default should be off.

IMO, one advantage of fixing it in the kernel is that there won't be several GIDs mapped to the overflow ID inside the user namespace.

Also, currently newuidmap/newgidmap allow to circumvent the /proc/$PID/setgroups security check as it is possible to drop additional gids. I wonder if there should be a stricter mode in shadow-utils that blocks writes to /proc/$PID/gid_map unless /proc/$PID/setgroups is either set todeny or to (what I was proposing for the kernel) to shadow:

giuseppe/linux@7e0701b
giuseppe/linux@1c5fe72

@hallyn
Copy link
Member

hallyn commented May 7, 2021

@giuseppe have you sent / do you plan to send these to lkml?

@giuseppe
Copy link
Contributor

giuseppe commented May 7, 2021

I've not yet sent them to lkml. I will do that, thanks for the hint. I'll rebase them first as I've not touched them in almost 5 years :-)

@giuseppe
Copy link
Contributor

giuseppe commented May 8, 2021

I've played with the kernel patches above and I think what @hallyn suggested here: #135 (comment) is much a better and cleaner way to address the issue, especially now that we have libsubid and users of newuidmap/newgidmap can easily query the additional groups they have access to.

I'll think more about the "shadow" mode I was working on for the kernel and if it can still be useful with UNPRIV_DELEGATE_AUX_GROUPS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants