shadow-maint / shadow

Upstream shadow tree
Other
294 stars 229 forks source link

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

Open tycho-kirchner opened 5 years ago

tycho-kirchner commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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:

https://github.com/giuseppe/linux/commit/7e0701b389c497472d11fab8570c153a414050af https://github.com/giuseppe/linux/commit/1c5fe726346b216293a527719e64f34e6297f0c2

hallyn commented 3 years ago

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

giuseppe commented 3 years ago

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 commented 3 years ago

I've played with the kernel patches above and I think what @hallyn suggested here: https://github.com/shadow-maint/shadow/issues/135#issuecomment-827590264 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