core20 icon indicating copy to clipboard operation
core20 copied to clipboard

move docker user/group to extrausers

Open anonymouse64 opened this issue 4 years ago • 16 comments

Currently the docker user/group is defined in /etc/{passwd,group},

$ cat /snap/core20/current/etc/passwd | grep docker
docker:x:107:113:Reserved:/nonexistent:/bin/false
$ cat /snap/core20/current/etc/group | grep docker
docker:x:113:

This is problematic because it means that users can never be added to the docker group and thus use the docker snap as non-root on UC20. We have this bug pre-existing on UC16/UC18 as well, but apparently moving/transitioning users from /etc/passwd to /var/lib/extrausers/passwd is very dangerous, so for UC20 we should just move this user/group before the release so we don't have to do the complicated transition for UC20.

What I tried which seemed to work is to replace the empty /var/lib/extrausers/{group,passwd} files in a built core20 snap with ones that contained the same definitions from /etc/{group,passwd} in the core20 snap right now. That unfortunately triggers https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1881588, but after working around that, I can then manually create a user that is in the docker group and this almost works but still fails because the docker snap is too naive and isn't properly checking the /var/lib/extrausers/group file when it creates it's socket, but if it did then we would be good to go.

Also note that this is originally from a customer request about being able to use the docker snap as non-root on UC18, which is a different beast, but I think since this is easy enough to fix for core20, we should do that before UC20 is released.

CC @slimjim777 @ogra1

anonymouse64 avatar Jun 12 '20 18:06 anonymouse64

Also CC @tianon

anonymouse64 avatar Jun 12 '20 18:06 anonymouse64

When using extrausers, doesn't getent inside the container return the "right" / smart result? As in, if I do getent group docker, it should return the extrausers group data, right?

The Docker snap used to have a patch to explicitly check extrausers, but it was dropped because Docker upstream added a more flexible/general solution in that if they fail to find docker in /etc/group, they'll shell out to getent, so it sounds like maybe that's not working as expected? :disappointed:

Edit: for reference, the PR that adjusted Docker's behavior is https://github.com/moby/moby/pull/38126, which was included in 19.03+

tianon avatar Jun 17 '20 23:06 tianon

Blocked on releasing https://bugs.launchpad.net/snapd/+bug/1881588 to stable

xnox avatar Jul 27 '20 14:07 xnox

be aware that the UID's/GID's must stay matching to not break on updates (there are user/group owned directories that are tied to the UID/GID on systems where docker has been used before).

there is a medium complex md5 check in the build scripts in livecd-rootfs that verifies if the actual /etc/group|passwd files have changed ...

ogra1 avatar Jul 27 '20 14:07 ogra1

@tianon

When using extrausers, doesn't getent inside the container return the "right" / smart result? As in, if I do getent group docker, it should return the extrausers group data, right?

When you say "container" here, do you mean the snap?

The Docker snap used to have a patch to explicitly check extrausers, but it was dropped because Docker upstream added a more flexible/general solution in that if they fail to find docker in /etc/group, they'll shell out to getent, so it sounds like maybe that's not working as expected?

Unfortunately, no it doesn't appear that getent works with the extrausers database, see my testing:

user@coreimg:~$ snap run --shell docker
user@coreimg:/home/user$ grep docker /var/lib/extrausers/group
docker:x:113:
user@coreimg:/home/user$ grep docker /etc/group
user@coreimg:/home/user$ getent group docker
user@coreimg:/home/user$ 

so either we need to fix getent in the base snaps, or the docker snap needs to be patched, however both of those things can be done orthogonally, since right now there is no way to add a user to the docker group anyways, so it's not a regression if the docker snap starts only listening on the socket as root.

@ogra1

be aware that the UID's/GID's must stay matching to not break on updates

we have not released uc20 yet so anyone we break with the docker snap here is not a problem to us, others can comment on the transition from /etc/group to /var/lib/extrausers/group, etc. for uc16/uc18. @xnox seemed to think it was not a problem as long as the same uid and gid were used in both places and the switch happened atomically.

anonymouse64 avatar Jul 27 '20 14:07 anonymouse64

I filed https://bugs.launchpad.net/snapd/+bug/1889092 for getent not supporting extrausers, not sure if there is a simple switch or something to enable getent to support extrausers.

anonymouse64 avatar Jul 27 '20 14:07 anonymouse64

@ogra1 livecd-rootfs is not used to build neither Core20 nor Core18 snaps.

xnox avatar Jul 27 '20 15:07 xnox

This is no longer blocked for uc20

anonymouse64 avatar Aug 24 '20 12:08 anonymouse64

The merged change does not result in a usable docker user account. Entries in /var/lib/extrausers/passwd with a uid less than 500 are ignored. The same goes for groups with gid less than 500:

https://git.launchpad.net/ubuntu/+source/libnss-extrausers/tree/s_config.h

I can definitely see the docker entry in the extrausers databases, but it doesn't show up in getent passwd or getent group output.

jhenstridge avatar Sep 14 '20 07:09 jhenstridge

A real solution would probably be to use the [SUCCESS=merge] directive in nsswitch.conf, with a second source that can provide additional members for the group. The nss-extrausers module in it's current state is not necessarily that service, given the uid/gid restrictions. It could be patched to remove the uid/gid restrictions, but that still leaves open the question of management.

jhenstridge avatar Sep 14 '20 07:09 jhenstridge

A real solution would probably be to use the [SUCCESS=merge] directive in nsswitch.conf, with a second source that can provide additional members for the group. The nss-extrausers module in it's current state is not necessarily that service, given the uid/gid restrictions. It could be patched to remove the uid/gid restrictions, but that still leaves open the question of management.

I wouldn't want to do that. Many of the system groups have special meaning, and if it is existing restriction to partition uid/gid by read-only, read-write I would want to keep that.

The best course of action is to either remove all the references to docker, or to bump the uid/gid to 998

xnox avatar Sep 14 '20 14:09 xnox

We will be having a meeting to discuss what to do for uc20 1.0 this week and will update this issue with the result of that meeting

anonymouse64 avatar Sep 14 '20 14:09 anonymouse64

It would be nice to have a solution for adding users to the system groups though. In particular, it would be nice to be able to add users to the sudo group.

While snap create-user handles the specific case of the sudo command line tool via sudoers.d config fragments, that doesn't help with other tools checking for admin access like polkitd.

jhenstridge avatar Sep 15 '20 03:09 jhenstridge

It would be nice to have a solution for adding users to the system groups though.

Yes this will be part of the discussion we have, I will update here what we decide. The main discussion is what to do before uc20 1.0 is released as we can make various "breaking" changes before then but not afterwards, but obviously how to add users to a group on ubuntu core is a key part of this discussion.

anonymouse64 avatar Sep 15 '20 14:09 anonymouse64

It would be nice to have a solution for adding users to the system groups though. In particular, it would be nice to be able to add users to the sudo group.

While snap create-user handles the specific case of the sudo command line tool via sudoers.d config fragments, that doesn't help with other tools checking for admin access like polkitd.

hm, indeed. adm, sudo, dialout groups come to mind. I thought extrausers allowed doing that, or at least did in Ubuntu Touch days, unless I am completely misremembering things.

xnox avatar Sep 16 '20 08:09 xnox

We could kind of get that with the [SUCCESS=merge] flag, and a version of nss-extrausers with the MINGID restriction lifted. If the same group was defined in both /etc/group and /var/lib/extrausers/group, then the reported membership would be the union of the two.

It would probably be wise to include checks that the group name and ID match when such a system group is found. Of course nss-extrausers has been unmaintained for the last 4 years, so it isn't clear where to send such patches.

jhenstridge avatar Sep 16 '20 09:09 jhenstridge