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

move docker user/group to extrausers #72

Open
anonymouse64 opened this issue Jun 12, 2020 · 16 comments · Fixed by #82
Open

move docker user/group to extrausers #72

anonymouse64 opened this issue Jun 12, 2020 · 16 comments · Fixed by #82
Labels

Comments

@anonymouse64
Copy link
Member

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
Copy link
Member Author

Also CC @tianon

@tianon
Copy link

tianon commented Jun 17, 2020

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? 😞

Edit: for reference, the PR that adjusted Docker's behavior is moby/moby#38126, which was included in 19.03+

@xnox
Copy link
Contributor

xnox commented Jul 27, 2020

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

@xnox xnox added the Blocked label Jul 27, 2020
@ogra1
Copy link

ogra1 commented Jul 27, 2020

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 ...

@anonymouse64
Copy link
Member Author

anonymouse64 commented Jul 27, 2020

@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
Copy link
Member Author

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.

@xnox
Copy link
Contributor

xnox commented Jul 27, 2020

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

@anonymouse64
Copy link
Member Author

This is no longer blocked for uc20

anonymouse64 added a commit to anonymouse64/core20 that referenced this issue Aug 24, 2020
This will allow users to add themselves to the docker group. This in combination
with an upgraded getent which reads from extrausers will allow the docker snap
to be used with non-root users.

Fixes: snapcore#72
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
@xnox xnox closed this as completed in #82 Sep 10, 2020
@jhenstridge
Copy link
Contributor

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
Copy link
Contributor

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.

@xnox
Copy link
Contributor

xnox commented Sep 14, 2020

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

@anonymouse64
Copy link
Member Author

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

@jhenstridge
Copy link
Contributor

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.

@anonymouse64
Copy link
Member Author

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.

@xnox
Copy link
Contributor

xnox commented Sep 16, 2020

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.

@jhenstridge
Copy link
Contributor

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.

@xnox xnox reopened this Sep 23, 2020
peat-psuwit added a commit to ubports/livecd-rootfs that referenced this issue Jul 22, 2021
Well, even though the phablet user is in extrausers db, the group it
needs to be in is still in /etc/group. Currently, we don't have a better
way. The alternative which is utilizing glibc's [SUCCESS=merge]
directive would require us patching libnss-extrausers which has been
orphaned for quite some time now.

Ubuntu Core based on snap also has the same problem.

https://forum.snapcraft.io/t/adding-users-to-system-groups-on-ubuntu-core/20109
snapcore/core20#72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants