From 5b95850e28ce25c7a7af6138aaff7da3f1829f75 Mon Sep 17 00:00:00 2001 From: sin-ack Date: Wed, 14 Apr 2021 21:35:49 +0000 Subject: [PATCH] SystemServer+LibCore: Allow service to request multiple sockets SystemServer only allowed a single socket to be created for a service before this. Now, SystemServer will allow any amount of sockets. The sockets can be defined like so: [SomeService] Socket=/tmp/portal/socket1,/tmp/portal/socket2,/tmp/portal/socket3 SocketPermissions=660,600 The last item in SocketPermissions is applied to the remainder of the sockets in the Socket= line, so multiple sockets can have the same permissions without having to repeat them. Defining multiple sockets is not allowed for socket-activated services at the moment, and wouldn't make much sense anyway. This patch also makes socket takeovers more robust by removing the assumption that the socket will always be passed in fd 3. Now, the SOCKET_TAKEOVER environment variable carries information about which endpoint corresponds to which socket, like so: SOCKET_TAKEOVER=/tmp/portal/socket1:3 /tmp/portal/socket2:4 and LocalServer/LocalService will parse this automatically and select the correct one. The old behavior of getting the default socket is preserved so long as the service only requests a single socket in SystemServer.ini. --- Userland/Libraries/LibCore/LocalServer.cpp | 31 ++++-- Userland/Libraries/LibCore/LocalServer.h | 2 +- Userland/Libraries/LibCore/LocalSocket.cpp | 54 +++++++-- Userland/Libraries/LibCore/LocalSocket.h | 11 +- Userland/Services/SystemServer/Service.cpp | 123 +++++++++++++++------ Userland/Services/SystemServer/Service.h | 22 ++-- 6 files changed, 178 insertions(+), 65 deletions(-) diff --git a/Userland/Libraries/LibCore/LocalServer.cpp b/Userland/Libraries/LibCore/LocalServer.cpp index 46d7d3ddd24..14c17fbb0a8 100644 --- a/Userland/Libraries/LibCore/LocalServer.cpp +++ b/Userland/Libraries/LibCore/LocalServer.cpp @@ -50,28 +50,39 @@ LocalServer::~LocalServer() ::close(m_fd); } -bool LocalServer::take_over_from_system_server() +bool LocalServer::take_over_from_system_server(String const& socket_path) { if (m_listening) return false; - constexpr auto socket_takeover = "SOCKET_TAKEOVER"; + if (!LocalSocket::s_overtaken_sockets_parsed) + LocalSocket::parse_sockets_from_system_server(); - if (getenv(socket_takeover)) { + int fd = -1; + if (socket_path.is_null()) { + // We want the first (and only) socket. + if (LocalSocket::s_overtaken_sockets.size() == 1) { + fd = LocalSocket::s_overtaken_sockets.begin()->value; + } + } else { + auto it = LocalSocket::s_overtaken_sockets.find(socket_path); + if (it != LocalSocket::s_overtaken_sockets.end()) { + fd = it->value; + } + } + + if (fd >= 0) { // Sanity check: it has to be a socket. struct stat stat; - int rc = fstat(3, &stat); + int rc = fstat(fd, &stat); if (rc == 0 && S_ISSOCK(stat.st_mode)) { - // The SystemServer has passed us the socket as fd 3, - // so use that instead of creating our own. - m_fd = 3; + // The SystemServer has passed us the socket, so use that instead of + // creating our own. + m_fd = fd; // It had to be !CLOEXEC for obvious reasons, but we // don't need it to be !CLOEXEC anymore, so set the // CLOEXEC flag now. fcntl(m_fd, F_SETFD, FD_CLOEXEC); - // We wouldn't want our children to think we're passing - // them a socket either, so unset the env variable. - unsetenv(socket_takeover); m_listening = true; setup_notifier(); diff --git a/Userland/Libraries/LibCore/LocalServer.h b/Userland/Libraries/LibCore/LocalServer.h index bcb4fa6c00e..912de55225d 100644 --- a/Userland/Libraries/LibCore/LocalServer.h +++ b/Userland/Libraries/LibCore/LocalServer.h @@ -36,7 +36,7 @@ class LocalServer : public Object { public: virtual ~LocalServer() override; - bool take_over_from_system_server(); + bool take_over_from_system_server(String const& path = String()); bool is_listening() const { return m_listening; } bool listen(const String& address); diff --git a/Userland/Libraries/LibCore/LocalSocket.cpp b/Userland/Libraries/LibCore/LocalSocket.cpp index 81b30d4c0ba..a30474998e2 100644 --- a/Userland/Libraries/LibCore/LocalSocket.cpp +++ b/Userland/Libraries/LibCore/LocalSocket.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -73,15 +74,49 @@ LocalSocket::~LocalSocket() { } -RefPtr LocalSocket::take_over_accepted_socket_from_system_server() -{ - constexpr auto socket_takeover = "SOCKET_TAKEOVER"; - if (!getenv(socket_takeover)) - return nullptr; +HashMap LocalSocket::s_overtaken_sockets {}; +bool LocalSocket::s_overtaken_sockets_parsed { false }; - // The SystemServer has passed us the socket as fd 3, - // so use that instead of creating our own. - constexpr int fd = 3; +void LocalSocket::parse_sockets_from_system_server() +{ + VERIFY(!s_overtaken_sockets_parsed); + + constexpr auto socket_takeover = "SOCKET_TAKEOVER"; + const char* sockets = getenv(socket_takeover); + if (!sockets) { + s_overtaken_sockets_parsed = true; + return; + } + + for (auto& socket : StringView(sockets).split_view(' ')) { + auto params = socket.split_view(':'); + s_overtaken_sockets.set(params[0].to_string(), strtol(params[1].to_string().characters(), nullptr, 10)); + } + + s_overtaken_sockets_parsed = true; + // We wouldn't want our children to think we're passing + // them a socket either, so unset the env variable. + unsetenv(socket_takeover); +} + +RefPtr LocalSocket::take_over_accepted_socket_from_system_server(String const& socket_path) +{ + if (!s_overtaken_sockets_parsed) + parse_sockets_from_system_server(); + + int fd; + if (socket_path.is_null()) { + // We want the first (and only) socket. + VERIFY(s_overtaken_sockets.size() == 1); + fd = s_overtaken_sockets.begin()->value; + } else { + auto it = s_overtaken_sockets.find(socket_path); + if (it == s_overtaken_sockets.end()) { + dbgln("Non-existent socket requested"); + return nullptr; + } + fd = it->value; + } // Sanity check: it has to be a socket. struct stat stat; @@ -99,9 +134,6 @@ RefPtr LocalSocket::take_over_accepted_socket_from_system_server() // don't need it to be !CLOEXEC anymore, so set the // CLOEXEC flag now. fcntl(fd, F_SETFD, FD_CLOEXEC); - // We wouldn't want our children to think we're passing - // them a socket either, so unset the env variable. - unsetenv(socket_takeover); return socket; } diff --git a/Userland/Libraries/LibCore/LocalSocket.h b/Userland/Libraries/LibCore/LocalSocket.h index 921e16aaa2b..309c2e27a7c 100644 --- a/Userland/Libraries/LibCore/LocalSocket.h +++ b/Userland/Libraries/LibCore/LocalSocket.h @@ -35,11 +35,20 @@ class LocalSocket final : public Socket { public: virtual ~LocalSocket() override; - static RefPtr take_over_accepted_socket_from_system_server(); + static RefPtr take_over_accepted_socket_from_system_server(String const& socket_path = String()); private: explicit LocalSocket(Object* parent = nullptr); LocalSocket(int fd, Object* parent = nullptr); + + // FIXME: better place to put this so both LocalSocket and LocalServer can + // enjoy it? + friend class LocalServer; + + static void parse_sockets_from_system_server(); + + static HashMap s_overtaken_sockets; + static bool s_overtaken_sockets_parsed; }; } diff --git a/Userland/Services/SystemServer/Service.cpp b/Userland/Services/SystemServer/Service.cpp index dfb01dae5a6..9a2cdc8dc24 100644 --- a/Userland/Services/SystemServer/Service.cpp +++ b/Userland/Services/SystemServer/Service.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -51,63 +52,69 @@ Service* Service::find_by_pid(pid_t pid) return (*it).value; } -void Service::setup_socket() +void Service::setup_socket(SocketDescriptor& socket) { - VERIFY(!m_socket_path.is_null()); - VERIFY(m_socket_fd == -1); + VERIFY(socket.fd == -1); - auto ok = Core::File::ensure_parent_directories(m_socket_path); + auto ok = Core::File::ensure_parent_directories(socket.path); VERIFY(ok); // Note: we use SOCK_CLOEXEC here to make sure we don't leak every socket to // all the clients. We'll make the one we do need to pass down !CLOEXEC later // after forking off the process. - m_socket_fd = socket(AF_LOCAL, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0); - if (m_socket_fd < 0) { + int socket_fd = ::socket(AF_LOCAL, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0); + if (socket_fd < 0) { perror("socket"); VERIFY_NOT_REACHED(); } + socket.fd = socket_fd; if (m_account.has_value()) { auto& account = m_account.value(); - if (fchown(m_socket_fd, account.uid(), account.gid()) < 0) { + if (fchown(socket_fd, account.uid(), account.gid()) < 0) { perror("fchown"); VERIFY_NOT_REACHED(); } } - if (fchmod(m_socket_fd, m_socket_permissions) < 0) { + if (fchmod(socket_fd, socket.permissions) < 0) { perror("fchmod"); VERIFY_NOT_REACHED(); } - auto socket_address = Core::SocketAddress::local(m_socket_path); + auto socket_address = Core::SocketAddress::local(socket.path); auto un_optional = socket_address.to_sockaddr_un(); if (!un_optional.has_value()) { - dbgln("Socket name {} is too long. BUG! This should have failed earlier!", m_socket_path); + dbgln("Socket name {} is too long. BUG! This should have failed earlier!", socket.path); VERIFY_NOT_REACHED(); } auto un = un_optional.value(); - int rc = bind(m_socket_fd, (const sockaddr*)&un, sizeof(un)); + int rc = bind(socket_fd, (const sockaddr*)&un, sizeof(un)); if (rc < 0) { perror("bind"); VERIFY_NOT_REACHED(); } - rc = listen(m_socket_fd, 16); + rc = listen(socket_fd, 16); if (rc < 0) { perror("listen"); VERIFY_NOT_REACHED(); } } +void Service::setup_sockets() +{ + for (SocketDescriptor& socket : m_sockets) + setup_socket(socket); +} + void Service::setup_notifier() { VERIFY(m_lazy); - VERIFY(m_socket_fd >= 0); + VERIFY(m_sockets.size() == 1); VERIFY(!m_socket_notifier); - m_socket_notifier = Core::Notifier::construct(m_socket_fd, Core::Notifier::Event::Read, this); + m_socket_notifier = Core::Notifier::construct(m_sockets[0].fd, Core::Notifier::Event::Read, this); m_socket_notifier->on_ready_to_read = [this] { handle_socket_connection(); }; @@ -115,10 +122,13 @@ void Service::setup_notifier() void Service::handle_socket_connection() { + VERIFY(m_sockets.size() == 1); dbgln_if(SERVICE_DEBUG, "Ready to read on behalf of {}", name()); + int socket_fd = m_sockets[0].fd; + if (m_accept_socket_connections) { - int accepted_fd = accept(m_socket_fd, nullptr, nullptr); + int accepted_fd = accept(socket_fd, nullptr, nullptr); if (accepted_fd < 0) { perror("accept"); return; @@ -128,7 +138,7 @@ void Service::handle_socket_connection() } else { remove_child(*m_socket_notifier); m_socket_notifier = nullptr; - spawn(m_socket_fd); + spawn(socket_fd); } } @@ -139,7 +149,7 @@ void Service::activate() if (m_lazy) setup_notifier(); else - spawn(m_socket_fd); + spawn(); } void Service::spawn(int socket_fd) @@ -198,12 +208,31 @@ void Service::spawn(int socket_fd) dup2(STDIN_FILENO, STDERR_FILENO); } + StringBuilder builder; + if (socket_fd >= 0) { - VERIFY(!m_socket_path.is_null()); - VERIFY(socket_fd > 3); - dup2(socket_fd, 3); + // We were spawned by socket activation. We currently only support + // single sockets for socket activation, so make sure that's the case. + VERIFY(m_sockets.size() == 1); + + int fd = dup(socket_fd); + builder.appendf("%s:%d", m_sockets[0].path.characters(), fd); + } else { + // We were spawned as a regular process, so dup every socket for this + // service and let the service know via SOCKET_TAKEOVER. + for (unsigned i = 0; i < m_sockets.size(); i++) { + SocketDescriptor& socket = m_sockets.at(i); + + int new_fd = dup(socket.fd); + if (i != 0) + builder.append(" "); + builder.appendf("%s:%d", socket.path.characters(), new_fd); + } + } + + if (!m_sockets.is_empty()) { // The new descriptor is !CLOEXEC here. - setenv("SOCKET_TAKEOVER", "1", true); + setenv("SOCKET_TAKEOVER", builder.to_string().characters(), true); } if (m_account.has_value()) { @@ -306,22 +335,39 @@ Service::Service(const Core::ConfigFile& config, const StringView& name) m_multi_instance = config.read_bool_entry(name, "MultiInstance"); m_accept_socket_connections = config.read_bool_entry(name, "AcceptSocketConnections"); - m_socket_path = config.read_entry(name, "Socket"); + String socket_entry = config.read_entry(name, "Socket"); + String socket_permissions_entry = config.read_entry(name, "SocketPermissions", "0600"); - // Lazy requires Socket. - VERIFY(!m_lazy || !m_socket_path.is_null()); - // AcceptSocketConnections always requires Socket, Lazy, and MultiInstance. - VERIFY(!m_accept_socket_connections || (!m_socket_path.is_null() && m_lazy && m_multi_instance)); + if (!socket_entry.is_null()) { + Vector socket_paths = socket_entry.split(','); + Vector socket_perms = socket_permissions_entry.split(','); + m_sockets.ensure_capacity(socket_paths.size()); + + // Need i here to iterate along with all other vectors. + for (unsigned i = 0; i < socket_paths.size(); i++) { + String& path = socket_paths.at(i); + + // Socket path (plus NUL) must fit into the structs sent to the Kernel. + VERIFY(path.length() < UNIX_PATH_MAX); + + // This is done so that the last permission repeats for every other + // socket. So you can define a single permission, and have it + // be applied for every socket. + mode_t permissions = strtol(socket_perms.at(min(socket_perms.size() - 1, (long unsigned)i)).characters(), nullptr, 8) & 0777; + + m_sockets.empend(path, -1, permissions); + } + } + + // Lazy requires Socket, but only one. + VERIFY(!m_lazy || m_sockets.size() == 1); + // AcceptSocketConnections always requires Socket (single), Lazy, and MultiInstance. + VERIFY(!m_accept_socket_connections || (m_sockets.size() == 1 && m_lazy && m_multi_instance)); // MultiInstance doesn't work with KeepAlive. VERIFY(!m_multi_instance || !m_keep_alive); - // Socket path (plus NUL) must fit into the structs sent to the Kernel. - VERIFY(m_socket_path.length() < UNIX_PATH_MAX); - if (!m_socket_path.is_null() && is_enabled()) { - auto socket_permissions_string = config.read_entry(name, "SocketPermissions", "0600"); - m_socket_permissions = strtol(socket_permissions_string.characters(), nullptr, 8) & 0777; - setup_socket(); - } + if (is_enabled()) + setup_sockets(); } void Service::save_to(JsonObject& json) @@ -346,13 +392,20 @@ void Service::save_to(JsonObject& json) for (String& env : m_environment) boot_modes.append(env); json.set("environment", environment); + + JsonArray sockets; + for (SocketDescriptor &socket : m_sockets) { + JsonObject socket_obj; + socket_obj.set("path", socket.path); + socket_obj.set("permissions", socket.permissions); + sockets.append(socket); + } + json.set("sockets", sockets); */ json.set("stdio_file_path", m_stdio_file_path); json.set("priority", m_priority); json.set("keep_alive", m_keep_alive); - json.set("socket_path", m_socket_path); - json.set("socket_permissions", m_socket_permissions); json.set("lazy", m_lazy); json.set("user", m_user); json.set("multi_instance", m_multi_instance); diff --git a/Userland/Services/SystemServer/Service.h b/Userland/Services/SystemServer/Service.h index f4a4796ad8b..3631873d1cc 100644 --- a/Userland/Services/SystemServer/Service.h +++ b/Userland/Services/SystemServer/Service.h @@ -51,6 +51,17 @@ private: void spawn(int socket_fd = -1); + /// SocketDescriptor describes the details of a single socket that was + /// requested by a service. + struct SocketDescriptor { + /// The path of the socket. + String path; + /// File descriptor of the socket. -1 if the socket hasn't been opened. + int fd { -1 }; + /// File permissions of the socket. + mode_t permissions; + }; + // Path to the executable. By default this is /bin/{m_name}. String m_executable_path; // Extra arguments, starting from argv[1], to pass when exec'ing. @@ -60,10 +71,6 @@ private: int m_priority { 1 }; // Whether we should re-launch it if it exits. bool m_keep_alive { false }; - // Path to the socket to create and listen on on behalf of this service. - String m_socket_path; - // File system permissions for the socket. - mode_t m_socket_permissions { 0 }; // Whether we should accept connections on the socket and pass the accepted // (and not listening) socket to the service. This requires a multi-instance // service. @@ -80,14 +87,14 @@ private: bool m_multi_instance { false }; // Environment variables to pass to the service. Vector m_environment; + // Socket descriptors for this service. + Vector m_sockets; // The resolved user account to run this service as. Optional m_account; // For single-instance services, PID of the running instance of this service. pid_t m_pid { -1 }; - // An open fd to the socket. - int m_socket_fd { -1 }; RefPtr m_socket_notifier; // Timer since we last spawned the service. @@ -96,7 +103,8 @@ private: // times where it has exited unsuccessfully and too quickly. int m_restart_attempts { 0 }; - void setup_socket(); + void setup_socket(SocketDescriptor&); + void setup_sockets(); void setup_notifier(); void handle_socket_connection(); };