From 278e815bfa3e4c2e3914e00121c37fc844cb2025 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 7 Jul 2022 12:02:04 +0200 Subject: [PATCH] logind: don't delay login for root even if systemd-user-sessions.service is not activated yet If for any reason something goes wrong during the boot process (most likely due to a network issue), system admins should be allowed to log in to the system to debug the problem. However due to the login session barrier enforced by systemd-user-sessions.service for all users, logins for root will be delayed until a (dbus) timeout expires. Beside being confusing, it's not a nice user experience to wait for an indefinite period of time (no message is shown) this and also suggests that something went wrong in the background. The reason of this delay is due to the fact that all units involved in the creation of a user session are ordered after systemd-user-sessions.service, which is subject to network issues. If root needs to log in at that time, logind is requested to create a new session (via pam_systemd), which ultimately ends up waiting for systemd-user-session.service to be activated. This has the bad side effect to block login for root until the dbus call done by pam_systemd times out and the PAM stack proceeds anyways. To solve this problem, this patch orders the session scope units and the user instances only after systemd-user-sessions.service for unprivileged users only. --- src/login/logind-session.c | 21 +++++++++++++++----- units/meson.build | 5 +++++ units/user-.slice.d/10-defaults.conf | 1 - units/user-runtime-dir@.service.in | 2 +- units/user@.service.d/10-login-barrier.conf | 14 +++++++++++++ units/user@.service.in | 2 +- units/user@0.service.d/10-login-barrier.conf | 12 +++++++++++ 7 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 units/user@.service.d/10-login-barrier.conf create mode 100644 units/user@0.service.d/10-login-barrier.conf diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 58086305030..31c37d4e2be 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -36,6 +36,7 @@ #include "strv.h" #include "terminal-util.h" #include "tmpfile-util.h" +#include "uid-alloc-range.h" #include "user-util.h" #include "util.h" @@ -645,6 +646,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er assert(s->user); if (!s->scope) { + _cleanup_strv_free_ char **after = NULL; _cleanup_free_ char *scope = NULL; const char *description; @@ -656,6 +658,19 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er description = strjoina("Session ", s->id, " of User ", s->user->user_record->user_name); + /* We usually want to order session scopes after systemd-user-sessions.service since the + * latter unit is used as login session barrier for unprivileged users. However the barrier + * doesn't apply for root as sysadmin should always be able to log in (and without waiting + * for any timeout to expire) in case something goes wrong during the boot process. Since + * ordering after systemd-user-sessions.service and the user instance is optional we make use + * of STRV_IGNORE with strv_new() to skip these order constraints when needed. */ + after = strv_new("systemd-logind.service", + s->user->runtime_dir_service, + !uid_is_system(s->user->user_record->uid) ? "systemd-user-sessions.service" : STRV_IGNORE, + s->class != SESSION_BACKGROUND ? s->user->service : STRV_IGNORE); + if (!after) + return log_oom(); + r = manager_start_scope( s->manager, scope, @@ -665,11 +680,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er /* These two have StopWhenUnneeded= set, hence add a dep towards them */ STRV_MAKE(s->user->runtime_dir_service, s->class != SESSION_BACKGROUND ? s->user->service : NULL), - /* And order us after some more */ - STRV_MAKE("systemd-logind.service", - "systemd-user-sessions.service", - s->user->runtime_dir_service, - s->class != SESSION_BACKGROUND ? s->user->service : NULL), + after, user_record_home_directory(s->user->user_record), properties, error, diff --git a/units/meson.build b/units/meson.build index e8f81f22302..40f784ec683 100644 --- a/units/meson.build +++ b/units/meson.build @@ -312,6 +312,11 @@ meson.add_install_script('meson-add-wants.sh', add_wants) install_data('user-.slice.d/10-defaults.conf', install_dir : systemunitdir + '/user-.slice.d') +install_data('user@.service.d/10-login-barrier.conf', + install_dir : systemunitdir + '/user@.service.d') +install_data('user@0.service.d/10-login-barrier.conf', + install_dir : systemunitdir + '/user@0.service.d') + ############################################################ if install_sysconfdir diff --git a/units/user-.slice.d/10-defaults.conf b/units/user-.slice.d/10-defaults.conf index cb3651b728d..f688eac2304 100644 --- a/units/user-.slice.d/10-defaults.conf +++ b/units/user-.slice.d/10-defaults.conf @@ -10,7 +10,6 @@ [Unit] Description=User Slice of UID %j Documentation=man:user@.service(5) -After=systemd-user-sessions.service StopWhenUnneeded=yes [Slice] diff --git a/units/user-runtime-dir@.service.in b/units/user-runtime-dir@.service.in index 61becff2c63..73141733242 100644 --- a/units/user-runtime-dir@.service.in +++ b/units/user-runtime-dir@.service.in @@ -10,7 +10,7 @@ [Unit] Description=User Runtime Directory /run/user/%i Documentation=man:user@.service(5) -After=systemd-user-sessions.service dbus.service +After=dbus.service StopWhenUnneeded=yes IgnoreOnIsolate=yes diff --git a/units/user@.service.d/10-login-barrier.conf b/units/user@.service.d/10-login-barrier.conf new file mode 100644 index 00000000000..d88df10dbf4 --- /dev/null +++ b/units/user@.service.d/10-login-barrier.conf @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +# Make sure user instances are started after logins are allowed. However this +# is not desirable for user@0.service since root should be able to log in +# earlier during the boot process especially if something goes wrong. +After=systemd-user-sessions.service diff --git a/units/user@.service.in b/units/user@.service.in index eff0d5bcb6a..1660de03264 100644 --- a/units/user@.service.in +++ b/units/user@.service.in @@ -10,7 +10,7 @@ [Unit] Description=User Manager for UID %i Documentation=man:user@.service(5) -After=systemd-user-sessions.service user-runtime-dir@%i.service dbus.service +After=user-runtime-dir@%i.service dbus.service Requires=user-runtime-dir@%i.service IgnoreOnIsolate=yes diff --git a/units/user@0.service.d/10-login-barrier.conf b/units/user@0.service.d/10-login-barrier.conf new file mode 100644 index 00000000000..6f8ff43b799 --- /dev/null +++ b/units/user@0.service.d/10-login-barrier.conf @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +# Empty file to mask its counterpart for unpriviledged users and thus cancels +# "After=systemd-user-session.service" ordering constraint so that root can log +# in even if the boot process is not yet finished.