Merge pull request #4543 from endocode/djalal/fix-dynamicuser-supplementary-groups

core: intialize user aux groups and SupplementaryGroups= when DynamicUser= is set

6 files changed, 47 insertions, 13 deletions

diff --git a/Makefile.am b/Makefile.am
index 34c2f60330..f7652c2346 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1626,6 +1626,9 @@ EXTRA_DIST += \
 	test/test-execute/exec-supplementarygroups-multiple-groups-default-group-user.service \
 	test/test-execute/exec-supplementarygroups-multiple-groups-withgid.service \
 	test/test-execute/exec-supplementarygroups-multiple-groups-withuid.service \
+	test/test-execute/exec-dynamicuser-fixeduser.service \
+	test/test-execute/exec-dynamicuser-fixeduser-one-supplementarygroup.service \
+	test/test-execute/exec-dynamicuser-supplementarygroups.service \
 	test/test-execute/exec-ignoresigpipe-no.service \
 	test/test-execute/exec-ignoresigpipe-yes.service \
 	test/test-execute/exec-personality-x86-64.service \
diff --git a/src/core/execute.c b/src/core/execute.c
index 3f053602b5..f13ca30395 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -773,11 +773,9 @@ static int get_fixed_group(const ExecContext *c, const char **group, gid_t *gid)
         return 0;
 }
 
-static int get_fixed_supplementary_groups(const ExecContext *c,
-                                          const char *user,
-                                          const char *group,
-                                          gid_t gid,
-                                          gid_t **supplementary_gids, int *ngids) {
+static int get_supplementary_groups(const ExecContext *c, const char *user,
+                                    const char *group, gid_t gid,
+                                    gid_t **supplementary_gids, int *ngids) {
         char **i;
         int r, k = 0;
         int ngroups_max;
@@ -790,8 +788,8 @@ static int get_fixed_supplementary_groups(const ExecContext *c,
         /*
          * If user is given, then lookup GID and supplementary groups list.
          * We avoid NSS lookups for gid=0. Also we have to initialize groups
-         * as early as possible so we keep the list of supplementary groups
-         * of the caller.
+         * here and as early as possible so we keep the list of supplementary
+         * groups of the caller.
          */
         if (user && gid_is_valid(gid) && gid != 0) {
                 /* First step, initialize groups from /etc/groups */
@@ -2347,13 +2345,14 @@ static int exec_child(
                         *exit_status = EXIT_GROUP;
                         return r;
                 }
+        }
 
-                r = get_fixed_supplementary_groups(context, username, groupname,
-                                                   gid, &supplementary_gids, &ngids);
-                if (r < 0) {
-                        *exit_status = EXIT_GROUP;
-                        return r;
-                }
+        /* Initialize user supplementary groups and get SupplementaryGroups= ones */
+        r = get_supplementary_groups(context, username, groupname, gid,
+                                     &supplementary_gids, &ngids);
+        if (r < 0) {
+                *exit_status = EXIT_GROUP;
+                return r;
         }
 
         r = send_user_lookup(unit, user_lookup_fd, uid, gid);
diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index c369098500..6029853e3e 100644
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -259,6 +259,12 @@ static void test_exec_supplementary_groups(Manager *m) {
         test(m, "exec-supplementarygroups-multiple-groups-withuid.service", 0, CLD_EXITED);
 }
 
+static void test_exec_dynamic_user(Manager *m) {
+        test(m, "exec-dynamicuser-fixeduser.service", 0, CLD_EXITED);
+        test(m, "exec-dynamicuser-fixeduser-one-supplementarygroup.service", 0, CLD_EXITED);
+        test(m, "exec-dynamicuser-supplementarygroups.service", 0, CLD_EXITED);
+}
+
 static void test_exec_environment(Manager *m) {
         test(m, "exec-environment.service", 0, CLD_EXITED);
         test(m, "exec-environment-multiple.service", 0, CLD_EXITED);
@@ -432,6 +438,7 @@ int main(int argc, char *argv[]) {
                 test_exec_user,
                 test_exec_group,
                 test_exec_supplementary_groups,
+                test_exec_dynamic_user,
                 test_exec_environment,
                 test_exec_environmentfile,
                 test_exec_passenvironment,
diff --git a/test/test-execute/exec-dynamicuser-fixeduser-one-supplementarygroup.service b/test/test-execute/exec-dynamicuser-fixeduser-one-supplementarygroup.service
new file mode 100644
index 0000000000..de1a6e7303
--- /dev/null
+++ b/test/test-execute/exec-dynamicuser-fixeduser-one-supplementarygroup.service
@@ -0,0 +1,9 @@
+[Unit]
+Description=Test DynamicUser with User= and SupplementaryGroups=
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$(id -G)" = "1" && test "$$(id -g)" = "1" && test "$$(id -u)" = "1"'
+Type=oneshot
+User=1
+DynamicUser=yes
+SupplementaryGroups=1
diff --git a/test/test-execute/exec-dynamicuser-fixeduser.service b/test/test-execute/exec-dynamicuser-fixeduser.service
new file mode 100644
index 0000000000..1d84af02ed
--- /dev/null
+++ b/test/test-execute/exec-dynamicuser-fixeduser.service
@@ -0,0 +1,8 @@
+[Unit]
+Description=Test DynamicUser with User=
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$(id -G)" = "1" && test "$$(id -g)" = "1" && test "$$(id -u)" = "1"'
+Type=oneshot
+User=1
+DynamicUser=yes
diff --git a/test/test-execute/exec-dynamicuser-supplementarygroups.service b/test/test-execute/exec-dynamicuser-supplementarygroups.service
new file mode 100644
index 0000000000..a47b7fab78
--- /dev/null
+++ b/test/test-execute/exec-dynamicuser-supplementarygroups.service
@@ -0,0 +1,8 @@
+[Unit]
+Description=Test DynamicUser with SupplementaryGroups=
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$(id -G | cut -d " " --complement -f 1)" = "1 2 3"'
+Type=oneshot
+DynamicUser=yes
+SupplementaryGroups=1 2 3