various: try to use DEFAULT_USER_SHELL for root too

/bin/sh as a shell is punishing. There is no good reason to make
the occasional root login unpleasant.

Since /bin/sh is usually /bin/bash in compat mode, i.e. if one is
available, the other will be too, /bin/bash is almost as good as a default.
But to avoid a regression in the situation where /bin/bash (or
DEFAULT_USER_SHELL) is not installed, we check with access() and fall back
to /bin/sh. This should make this change in behaviour less risky.

(FWIW, e.g. Fedora/RHEL use /bin/bash as default for root.)

This is a follow-up of sorts for 53350c7bbade8c5f357aa3d1029ef9b2208ea675,
which added the default-user-shell option, but most likely with the idea
of using /bin/bash less ;)

Fixes #24369.

diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index ea95ba886927c7b558f5ae56ab94d9377d01b5df..c4c6be98a5b9a396111e3cdd6c43af40a370c550 100644 (file)
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -2118,7 +2118,7 @@ Note that this setting is <emphasis>not</emphasis> influenced by the <varname>Us
           <row>
             <entry><literal>%s</literal></entry>
             <entry>User shell</entry>
-            <entry>This is the shell of the user running the service manager instance. In case of the system manager this resolves to <literal>/bin/sh</literal>.</entry>
+            <entry>This is the shell of the user running the service manager instance.</entry>
           </row>
           <row>
             <entry><literal>%S</literal></entry>

diff --git a/src/basic/user-util.c b/src/basic/user-util.c
index 16185332f9b5193ed8f661f6bd6e7226c1e425aa..37ccb667b27f67931b1ceaad237339963fa7d0dd 100644 (file)
--- a/src/basic/user-util.c
+++ b/src/basic/user-util.c
@@ -13,6 +13,7 @@
 #include "sd-messages.h"
 
 #include "alloc-util.h"
+#include "chase-symlinks.h"
 #include "errno-util.h"
 #include "fd-util.h"
 #include "fileio.h"
@@ -136,7 +137,6 @@ char *getusername_malloc(void) {
 }
 
 bool is_nologin_shell(const char *shell) {
-
         return PATH_IN_SET(shell,
                            /* 'nologin' is the friendliest way to disable logins for a user account. It prints a nice
                             * message and exits. Different distributions place the binary at different places though,
@@ -154,6 +154,21 @@ bool is_nologin_shell(const char *shell) {
                            "/usr/bin/true");
 }
 
+const char* default_root_shell(const char *root) {
+        /* We want to use the preferred shell, i.e. DEFAULT_USER_SHELL, which usually
+         * will be /bin/bash. Fall back to /bin/sh if DEFAULT_USER_SHELL is not found,
+         * or any access errors. */
+
+        int r = chase_symlinks(DEFAULT_USER_SHELL, root, CHASE_PREFIX_ROOT, NULL, NULL);
+        if (r < 0 && r != -ENOENT)
+                log_debug_errno(r, "Failed to look up shell '%s%s%s': %m",
+                                strempty(root), root ? "/" : "", DEFAULT_USER_SHELL);
+        if (r > 0)
+                return DEFAULT_USER_SHELL;
+
+        return "/bin/sh";
+}
+
 static int synthesize_user_creds(
                 const char **username,
                 uid_t *uid, gid_t *gid,
@@ -176,7 +191,7 @@ static int synthesize_user_creds(
                         *home = "/root";
 
                 if (shell)
-                        *shell = "/bin/sh";
+                        *shell = default_root_shell(NULL);
 
                 return 0;
         }
@@ -635,7 +650,7 @@ int get_shell(char **_s) {
         /* Hardcode shell for root and nobody to avoid NSS */
         u = getuid();
         if (u == 0) {
-                s = strdup("/bin/sh");
+                s = strdup(default_root_shell(NULL));
                 if (!s)
                         return -ENOMEM;
 

diff --git a/src/basic/user-util.h b/src/basic/user-util.h
index e1692c4f66ab426524434a8115dd76112f3b5d11..c8f0758541f4701ad9a949bf2bf22cf7c1780906 100644 (file)
--- a/src/basic/user-util.h
+++ b/src/basic/user-util.h
@@ -130,6 +130,7 @@ int putsgent_sane(const struct sgrp *sg, FILE *stream);
 #endif
 
 bool is_nologin_shell(const char *shell);
+const char* default_root_shell(const char *root);
 
 int is_this_me(const char *username);
 

diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c
index 9169129a6880776464d3e9dbcce3782c1102d4f5..fd9954b54d4f52452722112f01c1126569da8819 100644 (file)
--- a/src/firstboot/firstboot.c
+++ b/src/firstboot/firstboot.c
@@ -755,7 +755,7 @@ static int write_root_passwd(const char *passwd_path, const char *password, cons
                         .pw_gid = 0,
                         .pw_gecos = (char *) "Super User",
                         .pw_dir = (char *) "/root",
-                        .pw_shell = (char *) (shell ?: "/bin/sh"),
+                        .pw_shell = (char *) (shell ?: default_root_shell(arg_root)),
                 };
 
                 if (errno != ENOENT)

diff --git a/src/nss-systemd/nss-systemd.c b/src/nss-systemd/nss-systemd.c
index e24828450fb15c951ef6ba4fb80847c8f0c6762f..75d749e736da1631abaff8156c7c0148aa3ef364 100644 (file)
--- a/src/nss-systemd/nss-systemd.c
+++ b/src/nss-systemd/nss-systemd.c
@@ -26,7 +26,7 @@ static const struct passwd root_passwd = {
         .pw_gid = 0,
         .pw_gecos = (char*) "Super User",
         .pw_dir = (char*) "/root",
-        .pw_shell = (char*) "/bin/sh",
+        .pw_shell = NULL,
 };
 
 static const struct spwd root_spwd = {
@@ -142,24 +142,25 @@ NSS_INITGROUPS_PROTOTYPE(systemd);
 static enum nss_status copy_synthesized_passwd(
                 struct passwd *dest,
                 const struct passwd *src,
+                const char *fallback_shell,
                 char *buffer, size_t buflen,
                 int *errnop) {
 
-        size_t required;
-
         assert(dest);
         assert(src);
         assert(src->pw_name);
         assert(src->pw_passwd);
         assert(src->pw_gecos);
         assert(src->pw_dir);
-        assert(src->pw_shell);
 
-        required = strlen(src->pw_name) + 1;
-        required += strlen(src->pw_passwd) + 1;
-        required += strlen(src->pw_gecos) + 1;
-        required += strlen(src->pw_dir) + 1;
-        required += strlen(src->pw_shell) + 1;
+        const char *shell = ASSERT_PTR(src->pw_shell ?: fallback_shell);
+
+        size_t required =
+                strlen(src->pw_name) + 1 +
+                strlen(src->pw_passwd) + 1 +
+                strlen(src->pw_gecos) + 1 +
+                strlen(src->pw_dir) + 1 +
+                strlen(shell) + 1;
 
         if (buflen < required) {
                 *errnop = ERANGE;
@@ -176,7 +177,7 @@ static enum nss_status copy_synthesized_passwd(
         dest->pw_gecos = stpcpy(dest->pw_passwd, src->pw_passwd) + 1;
         dest->pw_dir = stpcpy(dest->pw_gecos, src->pw_gecos) + 1;
         dest->pw_shell = stpcpy(dest->pw_dir, src->pw_dir) + 1;
-        strcpy(dest->pw_shell, src->pw_shell);
+        strcpy(dest->pw_shell, shell);
 
         return NSS_STATUS_SUCCESS;
 }
@@ -187,15 +188,14 @@ static enum nss_status copy_synthesized_spwd(
                 char *buffer, size_t buflen,
                 int *errnop) {
 
-        size_t required;
-
         assert(dest);
         assert(src);
         assert(src->sp_namp);
         assert(src->sp_pwdp);
 
-        required = strlen(src->sp_namp) + 1;
-        required += strlen(src->sp_pwdp) + 1;
+        size_t required =
+                strlen(src->sp_namp) + 1 +
+                strlen(src->sp_pwdp) + 1;
 
         if (buflen < required) {
                 *errnop = ERANGE;
@@ -220,8 +220,6 @@ static enum nss_status copy_synthesized_group(
                 char *buffer, size_t buflen,
                 int *errnop) {
 
-        size_t required;
-
         assert(dest);
         assert(src);
         assert(src->gr_name);
@@ -229,9 +227,10 @@ static enum nss_status copy_synthesized_group(
         assert(src->gr_mem);
         assert(!*src->gr_mem); /* Our synthesized records' gr_mem is always just NULL... */
 
-        required = strlen(src->gr_name) + 1;
-        required += strlen(src->gr_passwd) + 1;
-        required += sizeof(char*); /* ...but that NULL still needs to be stored into the buffer! */
+        size_t required =
+                strlen(src->gr_name) + 1 +
+                strlen(src->gr_passwd) + 1 +
+                sizeof(char*); /* ...but that NULL still needs to be stored into the buffer! */
 
         if (buflen < ALIGN(required)) {
                 *errnop = ERANGE;
@@ -257,15 +256,14 @@ static enum nss_status copy_synthesized_sgrp(
                 char *buffer, size_t buflen,
                 int *errnop) {
 
-        size_t required;
-
         assert(dest);
         assert(src);
         assert(src->sg_namp);
         assert(src->sg_passwd);
 
-        required = strlen(src->sg_namp) + 1;
-        required += strlen(src->sg_passwd) + 1;
+        size_t required =
+                strlen(src->sg_namp) + 1 +
+                strlen(src->sg_passwd) + 1;
 
         if (buflen < required) {
                 *errnop = ERANGE;
@@ -310,13 +308,17 @@ enum nss_status _nss_systemd_getpwnam_r(
         if (getenv_bool_secure("SYSTEMD_NSS_BYPASS_SYNTHETIC") <= 0) {
 
                 if (streq(name, root_passwd.pw_name))
-                        return copy_synthesized_passwd(pwd, &root_passwd, buffer, buflen, errnop);
+                        return copy_synthesized_passwd(pwd, &root_passwd,
+                                                       default_root_shell(NULL),
+                                                       buffer, buflen, errnop);
 
                 if (streq(name, nobody_passwd.pw_name)) {
                         if (!synthesize_nobody())
                                 return NSS_STATUS_NOTFOUND;
 
-                        return copy_synthesized_passwd(pwd, &nobody_passwd, buffer, buflen, errnop);
+                        return copy_synthesized_passwd(pwd, &nobody_passwd,
+                                                       NULL,
+                                                       buffer, buflen, errnop);
                 }
 
         } else if (STR_IN_SET(name, root_passwd.pw_name, nobody_passwd.pw_name))
@@ -354,13 +356,17 @@ enum nss_status _nss_systemd_getpwuid_r(
         if (getenv_bool_secure("SYSTEMD_NSS_BYPASS_SYNTHETIC") <= 0) {
 
                 if (uid == root_passwd.pw_uid)
-                        return copy_synthesized_passwd(pwd, &root_passwd, buffer, buflen, errnop);
+                        return copy_synthesized_passwd(pwd, &root_passwd,
+                                                       default_root_shell(NULL),
+                                                       buffer, buflen, errnop);
 
                 if (uid == nobody_passwd.pw_uid) {
                         if (!synthesize_nobody())
                                 return NSS_STATUS_NOTFOUND;
 
-                        return copy_synthesized_passwd(pwd, &nobody_passwd, buffer, buflen, errnop);
+                        return copy_synthesized_passwd(pwd, &nobody_passwd,
+                                                       NULL,
+                                                       buffer, buflen, errnop);
                 }
 
         } else if (uid == root_passwd.pw_uid || uid == nobody_passwd.pw_uid)

diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c
index 5eb67ea084a6a8fcf2752a501fde24934786e0ec..393d2cc0fc068486de266c5ef77ab879bed7515a 100644 (file)
--- a/src/sysusers/sysusers.c
+++ b/src/sysusers/sysusers.c
@@ -398,7 +398,7 @@ static const char* pick_shell(const Item *i) {
         if (i->shell)
                 return i->shell;
         if (i->uid_set && i->uid == 0)
-                return "/bin/sh";
+                return default_root_shell(arg_root);
         return NOLOGIN;
 }
 

diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c
index 907de54eaa5422f4ba8908d93844f86c24e3fc9d..48d9b1e0fb987a505a1283c5a274007dde03074e 100644 (file)
--- a/src/test/test-user-util.c
+++ b/src/test/test-user-util.c
@@ -347,8 +347,8 @@ static void test_get_user_creds_one(const char *id, const char *name, uid_t uid,
 }
 
 TEST(get_user_creds) {
-        test_get_user_creds_one("root", "root", 0, 0, "/root", "/bin/sh");
-        test_get_user_creds_one("0", "root", 0, 0, "/root", "/bin/sh");
+        test_get_user_creds_one("root", "root", 0, 0, "/root", DEFAULT_USER_SHELL);
+        test_get_user_creds_one("0", "root", 0, 0, "/root", DEFAULT_USER_SHELL);
         test_get_user_creds_one(NOBODY_USER_NAME, NOBODY_USER_NAME, UID_NOBODY, GID_NOBODY, "/", NOLOGIN);
         test_get_user_creds_one("65534", NOBODY_USER_NAME, UID_NOBODY, GID_NOBODY, "/", NOLOGIN);
 }

diff --git a/test/test-execute/exec-specifier.service b/test/test-execute/exec-specifier.service
index ae8b2428bc813e5e6c1b1b4add97fd3e4dd27b9e..321d0e338a82e11ca6c8be053947bda08d0da40b 100644 (file)
--- a/test/test-execute/exec-specifier.service
+++ b/test/test-execute/exec-specifier.service
@@ -26,7 +26,7 @@ ExecStart=sh -c 'test %U = $$(id -u)'
 ExecStart=sh -c 'test %g = $$(id -gn)'
 ExecStart=sh -c 'test %G = $$(id -g)'
 ExecStart=test %h = /root
-ExecStart=sh -c 'test %s = /bin/sh'
+ExecStart=sh -c 'test -x %s'
 ExecStart=sh -c 'test %m = $$(cat /etc/machine-id)'
 ExecStart=sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')'
 ExecStart=sh -c 'test %H = $$(uname -n)'

diff --git a/test/test-execute/exec-specifier@.service b/test/test-execute/exec-specifier@.service
index 5e30efce4c54e6ad9d55a093cda4a72903a8929f..46c8503f1d8ad38d5a722046bc764b0fe6f4f905 100644 (file)
--- a/test/test-execute/exec-specifier@.service
+++ b/test/test-execute/exec-specifier@.service
@@ -23,7 +23,7 @@ ExecStart=sh -c 'test %U = $$(id -u)'
 ExecStart=sh -c 'test %g = $$(id -gn)'
 ExecStart=sh -c 'test %G = $$(id -g)'
 ExecStart=test %h = /root
-ExecStart=sh -c 'test %s = /bin/sh'
+ExecStart=sh -c 'test -x %s'
 ExecStart=sh -c 'test %m = $$(cat /etc/machine-id)'
 ExecStart=sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')'
 ExecStart=sh -c 'test %H = $$(uname -n)'