From c2b7c43f3c9e9e5e6a747c6d0d82d721d6921c99 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 4 Jan 2020 13:04:46 +0100 Subject: [PATCH] SystemServer: Fetch any extra GIDs and call setgroups() before spawn We now pick up all the user's extra GIDs from /etc/group and make sure those are set before exec'ing a service. This means we finally get to enjoy being in more than one group. :^) --- Base/usr/share/man/man5/SystemServer.md | 2 +- Servers/SystemServer/Service.cpp | 24 ++++++++++++++++++------ Servers/SystemServer/Service.h | 1 + 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/Base/usr/share/man/man5/SystemServer.md b/Base/usr/share/man/man5/SystemServer.md index f27880b2a3d..11a002d469b 100644 --- a/Base/usr/share/man/man5/SystemServer.md +++ b/Base/usr/share/man/man5/SystemServer.md @@ -23,7 +23,7 @@ describing how to launch and manage this service. * `KeepAlive` - whether the service should be restarted if it exits or crashes. For lazy services, this means the service will get respawned once a new connection is attempted on their socket after they exit or crash. * `Lazy` - whether the service should only get spawned once a client attempts to connect to their socket. * `Socket` - a path to a socket to create on behalf of the service. For lazy services, SystemServer will actually watch the socket for new connection attempts. An open file descriptor to this socket will be passed as fd 3 to the service. -* `User` - a name of the user to run the service as. This impacts what UID and GID the service processes have. By default, services are run as root. +* `User` - a name of the user to run the service as. This impacts what UID, GID (and extra GIDs) the service processes have. By default, services are run as root. ## Environment diff --git a/Servers/SystemServer/Service.cpp b/Servers/SystemServer/Service.cpp index a0e81bb20d3..8d7c0cd7467 100644 --- a/Servers/SystemServer/Service.cpp +++ b/Servers/SystemServer/Service.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -13,20 +14,30 @@ #include #include -struct UidAndGid { +struct UidAndGids { uid_t uid; gid_t gid; + Vector extra_gids; }; -static HashMap* s_user_map; +static HashMap* s_user_map; static HashMap s_service_map; void Service::resolve_user() { if (s_user_map == nullptr) { - s_user_map = new HashMap; - for (struct passwd* passwd = getpwent(); passwd; passwd = getpwent()) - s_user_map->set(passwd->pw_name, { passwd->pw_uid, passwd->pw_gid }); + s_user_map = new HashMap; + for (struct passwd* passwd = getpwent(); passwd; passwd = getpwent()) { + Vector extra_gids; + for (struct group* group = getgrent(); group; group = getgrent()) { + for (size_t m = 0; group->gr_mem[m]; ++m) { + if (!strcmp(group->gr_mem[m], passwd->pw_name)) + extra_gids.append(group->gr_gid); + } + } + endgrent(); + s_user_map->set(passwd->pw_name, { passwd->pw_uid, passwd->pw_gid, move(extra_gids) }); + } endpwent(); } @@ -37,6 +48,7 @@ void Service::resolve_user() } m_uid = user.value().uid; m_gid = user.value().gid; + m_extra_gids = user.value().extra_gids; } Service* Service::find_by_pid(pid_t pid) @@ -189,7 +201,7 @@ void Service::spawn() } if (!m_user.is_null()) { - if (setgid(m_gid) < 0 || setuid(m_uid) < 0) { + if (setgid(m_gid) < 0 || setgroups(m_extra_gids.size(), m_extra_gids.data()) < 0 || setuid(m_uid) < 0) { dbgprintf("Failed to drop privileges (GID=%u, UID=%u)\n", m_gid, m_uid); exit(1); } diff --git a/Servers/SystemServer/Service.h b/Servers/SystemServer/Service.h index 5d3b3522ce4..7abb0444e02 100644 --- a/Servers/SystemServer/Service.h +++ b/Servers/SystemServer/Service.h @@ -44,6 +44,7 @@ private: String m_user; uid_t m_uid { 0 }; gid_t m_gid { 0 }; + Vector m_extra_gids; // PID of the running instance of this service. pid_t m_pid { -1 };