1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 18:07:34 +00:00

SystemServer: Ensure service drop privileges could fail only when root

If we try to launch a lazily-spawned service and the SystemServer as a
(running --user) session leader is running with root permissions, then
if it is instructed to drop the root permissions for a the new service
then it will make sense to abort the entire spawn procedure if dropping
of privileges failed.

For other users, trying to change UID/GID to something else doesn't make
sense (and will always actually fail) as we are already running in non
root permissions, hence we don't attempt to do this anymore.
It should be noted that if an explicit User configuration was actually
specified for a Service to be used with, we would still try to login
with the requested User option value, which would fail when running as
non-root user.

This is useful for example when trying to run the pro utility with pls
to elevate to root permissions, but the session leader is still the same
so trying to "drop" privileges to UID 0 doesn't make sense.
This commit is contained in:
Liav A 2023-05-24 21:55:17 +03:00 committed by Jelle Raaijmakers
parent 189af20294
commit 43903aa960
2 changed files with 29 additions and 10 deletions

View file

@ -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");