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

SupplementaryGroups= inherits from /etc/group (or: "test-execute" unit test fails on Arch) #9881

Closed
LukeShu opened this issue Aug 17, 2018 · 3 comments
Labels
bug 🐛 Programming errors, that need preferential fixing tests

Comments

@LukeShu
Copy link
Contributor

LukeShu commented Aug 17, 2018

systemd version the issue has been seen with

239

Used distribution

Parabola GNU/Linux-libre (like Arch Linux)

Expected behaviour you didn't see

I expected the "test-execute" unit test to pass when run as root (it is skipped when run as non-root).

Unexpected behaviour you saw

The "test-execute" unit test failed because exec-supplementarygroups-single-group-user.service is run with multiple supplementary groups, which are the groups configure for that user in /etc/group by /usr/lib/sysusers.d/arch.conf.

Steps to reproduce the problem

Run ninja test as root on an Arch Linux-based system.

The relevant snippet from the output of ninja test is:

exec-supplementarygroups-single-group-user.service: Passing 0 fds to service
exec-supplementarygroups-single-group-user.service: About to execute: /bin/sh -x -c 'test "$$(id -G)" = "1" && test "$$(id -g)" = "1" && test "$$(id -u)" = "1"'
exec-supplementarygroups-single-group-user.service: Forked /bin/sh as 1928
exec-supplementarygroups-single-group-user.service: Changed dead -> start
exec-supplementarygroups-single-group-user.service: User lookup succeeded: uid=1 gid=1
exec-supplementarygroups-single-group-user.service: Executing: /bin/sh -x -c 'test "$(id -G)" = "1" && test "$(id -g)" = "1" && test "$(id -u)" = "1"'
++ id -G
+ test '1 2 3' = 1
exec-supplementarygroups-single-group-user.service: cgroup is empty
Received SIGCHLD from PID 1928 (sh).
Child 1928 (sh) died (code=exited, status=1/FAILURE)
exec-supplementarygroups-single-group-user.service: Child 1928 belongs to exec-supplementarygroups-single-group-user.service.
exec-supplementarygroups-single-group-user.service: Main process exited, code=exited, status=1/FAILURE
exec-supplementarygroups-single-group-user.service: Failed with result 'exit-code'.
exec-supplementarygroups-single-group-user.service: Changed start -> failed
exec-supplementarygroups-single-group-user.service: Unit entered failed state.
Assertion 'service->main_exec_status.status == status_expected' failed at ../systemd/src/test/test-execute.c:59, function check(). Aborting.

The relevant test unit says

User=1
Group=1
SupplementaryGroups=1

It then tries to verify that the service is run with GID 1 as the only group. This fails because it inherits the supplementary groups of UID 1 ("bin") as configured in /etc/group by /usr/lib/sysusers.d/arch.conf.

$ id 1
uid=1(bin) gid=1(bin) groups=1(bin),2(daemon),3(sys)

Reading the documentation, it isn't entirely clear to me that SupplementaryGroups= is supposed to disregard /etc/group, but that seems to be what this unit test is testing for, so I'm accepting that disregarding /etc/group is the desired behavior. If it's not, then the unit-test should be corrected.

@yuwata yuwata added the tests label Sep 3, 2018
@poettering
Copy link
Member

yeah, i think the test should be corrected to not choke on these additional group memberships.

@poettering poettering added the bug 🐛 Programming errors, that need preferential fixing label Sep 11, 2018
@filbranden
Copy link
Member

Hmmm, but given SupplementaryGroups=1 is set in the unit, should we still really get the ones from /etc/group (in addition to it?)

In other words, shouldn't SupplementaryGroups= override /etc/group when set?

@poettering
Copy link
Member

In other words, shouldn't SupplementaryGroups= override /etc/group when set?

I don't think it should... I mean, the main usecase is to give a service access to extra resources, and not so much to take it away... And to keep things simply I'd just focus on that and not bother with resetting memberships explicitly, but just add some

filbranden added a commit to filbranden/systemd that referenced this issue Sep 12, 2018
If /etc/group is set up in a way that additional groups will be used for the
test users, accept that `id -G` will return them in addition to the
supplementary groups configured in the unit.

For example, in Arch Linux user with uid 1 is in these groups by default:

  $ id 1
  uid=1(bin) gid=1(bin) groups=1(bin),2(daemon),3(sys)

Fixes systemd#9881
filbranden added a commit to filbranden/systemd that referenced this issue Sep 12, 2018
If /etc/group is set up in a way that additional groups will be used for the
test users, accept that `id -G` will return them in addition to the
supplementary groups configured in the unit.

For example, in Arch Linux user with uid 1 is in these groups by default:

  $ id 1
  uid=1(bin) gid=1(bin) groups=1(bin),2(daemon),3(sys)

Tested: by adding my "bin" user to groups "daemon" and "sys" and running the
test cases with `ninja -C build/ test`. Also works without the additional groups.

Fixes systemd#9881.
yuwata added a commit to yuwata/systemd that referenced this issue Sep 29, 2018
yuwata added a commit to yuwata/systemd that referenced this issue Oct 1, 2018
ronnychevalier pushed a commit that referenced this issue Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Programming errors, that need preferential fixing tests
Development

No branches or pull requests

4 participants