Merge c15ab51249 into a526b9ddfc

src/basic/user-util.c

@@ -220,9 +220,9 @@ static int synthesize_user_creds(
                 if (ret_gid)
                         *ret_gid = GID_NOBODY;
                 if (ret_home)
-                        *ret_home = FLAGS_SET(flags, USER_CREDS_SUPPRESS_PLACEHOLDER) ? NULL : "/";
+                        *ret_home = FLAGS_SET(flags, USER_CREDS_CLEAN) ? NULL : "/";
                 if (ret_shell)
-                        *ret_shell = FLAGS_SET(flags, USER_CREDS_SUPPRESS_PLACEHOLDER) ? NULL : NOLOGIN;
+                        *ret_shell = FLAGS_SET(flags, USER_CREDS_CLEAN) ? NULL : NOLOGIN;
 
                 return 0;
         }
@@ -244,7 +244,6 @@ int get_user_creds(
 
         assert(username);
         assert(*username);
-        assert((ret_home || ret_shell) || !(flags & (USER_CREDS_SUPPRESS_PLACEHOLDER|USER_CREDS_CLEAN)));
 
         if (!FLAGS_SET(flags, USER_CREDS_PREFER_NSS) ||
             (!ret_home && !ret_shell)) {
@@ -316,14 +315,17 @@ int get_user_creds(
 
         if (ret_home)
                 /* Note: we don't insist on normalized paths, since there are setups that have /./ in the path */
-                *ret_home = (FLAGS_SET(flags, USER_CREDS_SUPPRESS_PLACEHOLDER) && empty_or_root(p->pw_dir)) ||
-                            (FLAGS_SET(flags, USER_CREDS_CLEAN) && (!path_is_valid(p->pw_dir) || !path_is_absolute(p->pw_dir)))
-                            ? NULL : p->pw_dir;
+                *ret_home = (FLAGS_SET(flags, USER_CREDS_CLEAN) &&
+                             (empty_or_root(p->pw_dir) ||
+                              !path_is_valid(p->pw_dir) ||
+                              !path_is_absolute(p->pw_dir))) ? NULL : p->pw_dir;
 
         if (ret_shell)
-                *ret_shell = (FLAGS_SET(flags, USER_CREDS_SUPPRESS_PLACEHOLDER) && shell_is_placeholder(p->pw_shell)) ||
-                             (FLAGS_SET(flags, USER_CREDS_CLEAN) && (!path_is_valid(p->pw_shell) || !path_is_absolute(p->pw_shell)))
-                             ? NULL : p->pw_shell;
+                *ret_shell = (FLAGS_SET(flags, USER_CREDS_CLEAN) &&
+                              (isempty(p->pw_shell) ||
+                               !path_is_valid(p->pw_shell) ||
+                               !path_is_absolute(p->pw_shell) ||
+                               is_nologin_shell(p->pw_shell))) ? NULL : p->pw_shell;
 
         if (patch_username)
                 *username = p->pw_name;

src/basic/user-util.h

@@ -12,8 +12,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "string-util.h"
-
 /* Users managed by systemd-homed. See https://systemd.io/UIDS-GIDS for details how this range fits into the rest of the world */
 #define HOME_UID_MIN ((uid_t) 60001)
 #define HOME_UID_MAX ((uid_t) 60513)
@@ -38,20 +36,10 @@ static inline int parse_gid(const char *s, gid_t *ret_gid) {
 char* getlogname_malloc(void);
 char* getusername_malloc(void);
 
-const char* default_root_shell_at(int rfd);
-const char* default_root_shell(const char *root);
-
-bool is_nologin_shell(const char *shell);
-
-static inline bool shell_is_placeholder(const char *shell) {
-        return isempty(shell) || is_nologin_shell(shell);
-}
-
 typedef enum UserCredsFlags {
-        USER_CREDS_PREFER_NSS           = 1 << 0,  /* if set, only synthesize user records if database lacks them. Normally we bypass the userdb entirely for the records we can synthesize */
-        USER_CREDS_ALLOW_MISSING        = 1 << 1,  /* if a numeric UID string is resolved, be OK if there's no record for it */
-        USER_CREDS_CLEAN                = 1 << 2,  /* try to clean up shell and home fields with invalid data */
-        USER_CREDS_SUPPRESS_PLACEHOLDER = 1 << 3,  /* suppress home and/or shell fields if value is placeholder (root/empty/nologin) */
+        USER_CREDS_PREFER_NSS    = 1 << 0,  /* if set, only synthesize user records if database lacks them. Normally we bypass the userdb entirely for the records we can synthesize */
+        USER_CREDS_ALLOW_MISSING = 1 << 1,  /* if a numeric UID string is resolved, be OK if there's no record for it */
+        USER_CREDS_CLEAN         = 1 << 2,  /* try to clean up shell and home fields with invalid data */
 } UserCredsFlags;
 
 int get_user_creds(const char **username, uid_t *ret_uid, gid_t *ret_gid, const char **ret_home, const char **ret_shell, UserCredsFlags flags);
@@ -137,6 +125,10 @@ int fgetsgent_sane(FILE *stream, struct sgrp **sg);
 int putsgent_sane(const struct sgrp *sg, FILE *stream);
 #endif
 
+bool is_nologin_shell(const char *shell);
+const char* default_root_shell_at(int rfd);
+const char* default_root_shell(const char *root);
+
 int is_this_me(const char *username);
 
 const char* get_home_root(void);

src/core/exec-invoke.c

@@ -855,6 +855,9 @@ static int get_fixed_user(
         assert(user_or_uid);
         assert(ret_username);
 
+        /* Note that we don't set $HOME or $SHELL if they are not particularly enlightening anyway
+         * (i.e. are "/" or "/bin/nologin"). */
+
         r = get_user_creds(&user_or_uid, ret_uid, ret_gid, ret_home, ret_shell, USER_CREDS_CLEAN);
         if (r < 0)
                 return r;
@@ -1880,10 +1883,7 @@ static int build_environment(
                 }
         }
 
-        /* Note that we don't set $HOME or $SHELL if they are not particularly enlightening anyway
-         * (i.e. are "/" or "/bin/nologin"). */
-
-        if (home && set_user_login_env && !empty_or_root(home)) {
+        if (home && set_user_login_env) {
                 x = strjoin("HOME=", home);
                 if (!x)
                         return -ENOMEM;
@@ -1892,7 +1892,7 @@ static int build_environment(
                 our_env[n_env++] = x;
         }
 
-        if (shell && set_user_login_env && !shell_is_placeholder(shell)) {
+        if (shell && set_user_login_env) {
                 x = strjoin("SHELL=", shell);
                 if (!x)
                         return -ENOMEM;
@@ -3471,16 +3471,20 @@ static int apply_working_directory(
                 const ExecContext *context,
                 const ExecParameters *params,
                 ExecRuntime *runtime,
-                const char *home) {
+                const char *home,
+                int *exit_status) {
 
         const char *wd;
         int r;
 
         assert(context);
+        assert(exit_status);
 
         if (context->working_directory_home) {
-                if (!home)
+                if (!home) {
+                        *exit_status = EXIT_CHDIR;
                         return -ENXIO;
+                }
 
                 wd = home;
         } else
@@ -3499,7 +3503,13 @@ static int apply_working_directory(
                 if (r >= 0)
                         r = RET_NERRNO(fchdir(dfd));
         }
-        return context->working_directory_missing_ok ? 0 : r;
+
+        if (r < 0 && !context->working_directory_missing_ok) {
+                *exit_status = EXIT_CHDIR;
+                return r;
+        }
+
+        return 0;
 }
 
 static int apply_root_directory(
@@ -3775,7 +3785,7 @@ static int acquire_home(const ExecContext *c, const char **home, char **ret_buf)
         if (!c->working_directory_home)
                 return 0;
 
-        if (c->dynamic_user || (c->user && is_this_me(c->user) <= 0))
+        if (c->dynamic_user)
                 return -EADDRNOTAVAIL;
 
         r = get_home_dir(ret_buf);
@@ -4533,7 +4543,7 @@ int exec_invoke(
         r = acquire_home(context, &home, &home_buffer);
         if (r < 0) {
                 *exit_status = EXIT_CHDIR;
-                return log_exec_error_errno(context, params, r, "Failed to determine $HOME for the invoking user: %m");
+                return log_exec_error_errno(context, params, r, "Failed to determine $HOME for user: %m");
         }
 
         /* If a socket is connected to STDIN/STDOUT/STDERR, we must drop O_NONBLOCK */
@@ -5372,11 +5382,9 @@ int exec_invoke(
          * running this service might have the correct privilege to change to the working directory. Also, it
          * is absolutely 💣 crucial 💣 we applied all mount namespacing rearrangements before this, so that
          * the cwd cannot be used to pin directories outside of the sandbox. */
-        r = apply_working_directory(context, params, runtime, home);
-        if (r < 0) {
-                *exit_status = EXIT_CHDIR;
+        r = apply_working_directory(context, params, runtime, home, exit_status);
+        if (r < 0)
                 return log_exec_error_errno(context, params, r, "Changing to the requested working directory failed: %m");
-        }
 
         if (needs_sandboxing) {
                 /* Apply other MAC contexts late, but before seccomp syscall filtering, as those should really be last to

src/run/run.c

@@ -2297,8 +2297,7 @@ static int start_transient_scope(sd_bus *bus) {
                 uid_t uid;
                 gid_t gid;
 
-                r = get_user_creds(&arg_exec_user, &uid, &gid, &home, &shell,
-                                   USER_CREDS_CLEAN|USER_CREDS_SUPPRESS_PLACEHOLDER|USER_CREDS_PREFER_NSS);
+                r = get_user_creds(&arg_exec_user, &uid, &gid, &home, &shell, USER_CREDS_CLEAN|USER_CREDS_PREFER_NSS);
                 if (r < 0)
                         return log_error_errno(r, "Failed to resolve user %s: %m", arg_exec_user);
 

src/test/test-audit-util.c

@@ -7,26 +7,24 @@ TEST(audit_loginuid_from_pid) {
         _cleanup_(pidref_done) PidRef self = PIDREF_NULL, pid1 = PIDREF_NULL;
         int r;
 
-        ASSERT_OK(pidref_set_self(&self));
-        ASSERT_OK(pidref_set_pid(&pid1, 1));
+        assert_se(pidref_set_self(&self) >= 0);
+        assert_se(pidref_set_pid(&pid1, 1) >= 0);
 
         uid_t uid;
         r = audit_loginuid_from_pid(&self, &uid);
-        if (r != -ENODATA)
-                ASSERT_OK(r);
+        assert_se(r >= 0 || r == -ENODATA);
         if (r >= 0)
                 log_info("self audit login uid: " UID_FMT, uid);
 
-        ASSERT_ERROR(audit_loginuid_from_pid(&pid1, &uid), ENODATA);
+        assert_se(audit_loginuid_from_pid(&pid1, &uid) == -ENODATA);
 
         uint32_t sessionid;
         r = audit_session_from_pid(&self, &sessionid);
-        if (r != -ENODATA)
-                ASSERT_OK(r);
+        assert_se(r >= 0 || r == -ENODATA);
         if (r >= 0)
                 log_info("self audit session id: %" PRIu32, sessionid);
 
-        ASSERT_ERROR(audit_session_from_pid(&pid1, &sessionid), ENODATA);
+        assert_se(audit_session_from_pid(&pid1, &sessionid) == -ENODATA);
 }
 
 static int intro(void) {

test/units/TEST-07-PID1.working-directory.sh

@@ -1,20 +0,0 @@
-#!/usr/bin/env bash
-# SPDX-License-Identifier: LGPL-2.1-or-later
-
-set -eux
-set -o pipefail
-
-# shellcheck source=test/units/util.sh
-. "$(dirname "$0")"/util.sh
-
-(! systemd-run --wait -p DynamicUser=yes \
-                      -p EnvironmentFile=-/usr/lib/systemd/systemd-asan-env \
-                      -p WorkingDirectory='~' true)
-
-assert_eq "$(systemd-run --pipe --uid=root -p WorkingDirectory='~' pwd)" "/root"
-assert_eq "$(systemd-run --pipe --uid=nobody -p WorkingDirectory='~' pwd)" "/"
-assert_eq "$(systemd-run --pipe --uid=testuser -p WorkingDirectory='~' pwd)" "/home/testuser"
-
-(! systemd-run --wait -p DynamicUser=yes -p User=testuser \
-                      -p EnvironmentFile=-/usr/lib/systemd/systemd-asan-env \
-                      -p WorkingDirectory='~' true)

units/systemd-confext.service

@@ -16,7 +16,6 @@ ConditionDirectoryNotEmpty=|/run/confexts
 ConditionDirectoryNotEmpty=|/var/lib/confexts
 ConditionDirectoryNotEmpty=|/usr/local/lib/confexts
 ConditionDirectoryNotEmpty=|/usr/lib/confexts
-ConditionDirectoryNotEmpty=|/.extra/confext
 
 DefaultDependencies=no
 After=local-fs.target