Merge pull request #1320 from ronnychevalier/rc/core_fix_group_ownership

core: fix group ownership when Group is set

src/core/execute.c

@@ -629,15 +629,6 @@ static int enforce_groups(const ExecContext *context, const char *username, gid_
          * we avoid NSS lookups for gid=0. */
 
         if (context->group || username) {
-
-                if (context->group) {
-                        const char *g = context->group;
-
-                        r = get_group_creds(&g, &gid);
-                        if (r < 0)
-                                return r;
-                }
-
                 /* First step, initialize groups from /etc/groups */
                 if (username && gid != 0) {
                         if (initgroups(username, gid) < 0)
@@ -1414,6 +1405,17 @@ static int exec_child(
                 }
         }
 
+        if (context->group) {
+                const char *g = context->group;
+
+                r = get_group_creds(&g, &gid);
+                if (r < 0) {
+                        *exit_status = EXIT_GROUP;
+                        return r;
+                }
+        }
+
+
         /* If a socket is connected to STDIN/STDOUT/STDERR, we
          * must sure to drop O_NONBLOCK */
         if (socket_fd >= 0)

src/test/test-execute.c

@@ -137,6 +137,12 @@ static void test_exec_umask(Manager *m) {
         test(m, "exec-umask-0177.service", 0, CLD_EXITED);
 }
 
+static void test_exec_runtimedirectory(Manager *m) {
+        test(m, "exec-runtimedirectory.service", 0, CLD_EXITED);
+        test(m, "exec-runtimedirectory-mode.service", 0, CLD_EXITED);
+        test(m, "exec-runtimedirectory-owner.service", 0, CLD_EXITED);
+}
+
 int main(int argc, char *argv[]) {
         test_function_t tests[] = {
                 test_exec_workingdirectory,
@@ -150,6 +156,7 @@ int main(int argc, char *argv[]) {
                 test_exec_group,
                 test_exec_environment,
                 test_exec_umask,
+                test_exec_runtimedirectory,
                 NULL,
         };
         test_function_t *test = NULL;
@@ -165,6 +172,7 @@ int main(int argc, char *argv[]) {
                 return EXIT_TEST_SKIP;
         }
 
+        assert_se(setenv("XDG_RUNTIME_DIR", "/tmp/", 1) == 0);
         assert_se(set_unit_path(TEST_DIR) >= 0);
 
         r = manager_new(MANAGER_USER, true, &m);

test/exec-runtimedirectory-mode.service

@@ -0,0 +1,8 @@
+[Unit]
+Description=Test for RuntimeDirectoryMode
+
+[Service]
+ExecStart=/bin/sh -c 's=$(stat -c %a /tmp/test-exec_runtimedirectory-mode); echo $s; exit $(test $s = "750")'
+Type=oneshot
+RuntimeDirectory=test-exec_runtimedirectory-mode
+RuntimeDirectoryMode=0750

test/exec-runtimedirectory-owner.service

@@ -0,0 +1,9 @@
+[Unit]
+Description=Test for RuntimeDirectory owner (must not be the default group of the user if Group is set)
+
+[Service]
+ExecStart=/bin/sh -c 'f=/tmp/test-exec_runtimedirectory-owner;g=$(stat -c %G $f); echo "$g"; exit $(test $g = "nobody")'
+Type=oneshot
+Group=nobody
+User=root
+RuntimeDirectory=test-exec_runtimedirectory-owner

test/exec-runtimedirectory.service

@@ -0,0 +1,7 @@
+[Unit]
+Description=Test for RuntimeDirectory
+
+[Service]
+ExecStart=/bin/sh -c 'exit $(test -d /tmp/test-exec_runtimedirectory)'
+Type=oneshot
+RuntimeDirectory=test-exec_runtimedirectory