diff options
| -rw-r--r-- | Userland/Services/SystemServer/Service.cpp | 36 | ||||
| -rw-r--r-- | Userland/Services/SystemServer/Service.h | 3 |
2 files changed, 29 insertions, 10 deletions
diff --git a/Userland/Services/SystemServer/Service.cpp b/Userland/Services/SystemServer/Service.cpp index bc3190c362..3893c6eaaa 100644 --- a/Userland/Services/SystemServer/Service.cpp +++ b/Userland/Services/SystemServer/Service.cpp @@ -121,6 +121,27 @@ ErrorOr<void> Service::activate() return {}; } +ErrorOr<void> Service::change_privileges() +{ + // NOTE: Dropping privileges makes sense when SystemServer is running + // for a root session. + // This could happen when we need to spawn a Service to serve a client with non-user UID/GID. + // However, in case the user explicitly specified a username via the User= option, then we must + // try to login as at that user, so we can't ignore the failure when it was requested to change + // privileges. + if (auto current_uid = getuid(); m_account.has_value() && m_account.value().uid() != current_uid) { + if (current_uid != 0 && !m_must_login) + return {}; + auto& account = m_account.value(); + if (auto error_or_void = account.login(); error_or_void.is_error()) { + dbgln("Failed to drop privileges (tried to change to GID={}, UID={}), due to {}\n", account.gid(), account.uid(), error_or_void.error()); + exit(1); + } + TRY(Core::System::setenv("HOME"sv, account.home_directory(), true)); + } + return {}; +} + ErrorOr<void> Service::spawn(int socket_fd) { if (!FileSystem::exists(m_executable_path)) { @@ -198,14 +219,7 @@ ErrorOr<void> Service::spawn(int socket_fd) TRY(Core::System::setenv("SOCKET_TAKEOVER"sv, socket_takeover_builder.string_view(), true)); } - if (m_account.has_value() && m_account.value().uid() != getuid()) { - auto& account = m_account.value(); - if (auto error_or_void = account.login(); error_or_void.is_error()) { - dbgln("Failed to drop privileges (GID={}, UID={}), due to {}\n", account.gid(), account.uid(), error_or_void.error()); - exit(1); - } - TRY(Core::System::setenv("HOME"sv, account.home_directory(), true)); - } + TRY(change_privileges()); TRY(m_environment.view().for_each_split_view(' ', SplitBehavior::Nothing, [&](auto env) { return Core::System::putenv(env); @@ -290,10 +304,12 @@ Service::Service(Core::ConfigFile const& config, StringView name) m_user = config.read_entry(name, "User"); if (!m_user.is_null()) { auto result = Core::Account::from_name(m_user, Core::Account::Read::PasswdOnly); - if (result.is_error()) + if (result.is_error()) { warnln("Failed to resolve user {}: {}", m_user, result.error()); - else + } else { + m_must_login = true; m_account = result.value(); + } } m_working_directory = config.read_entry(name, "WorkingDirectory"); diff --git a/Userland/Services/SystemServer/Service.h b/Userland/Services/SystemServer/Service.h index f0aa9067cc..c82aabcea8 100644 --- a/Userland/Services/SystemServer/Service.h +++ b/Userland/Services/SystemServer/Service.h @@ -33,6 +33,8 @@ private: ErrorOr<void> determine_account(int fd); + ErrorOr<void> change_privileges(); + /// SocketDescriptor describes the details of a single socket that was /// requested by a service. struct SocketDescriptor { @@ -74,6 +76,7 @@ private: // The resolved user account to run this service as. Optional<Core::Account> m_account; + bool m_must_login { false }; // For single-instance services, PID of the running instance of this service. pid_t m_pid { -1 }; |