From 5bcb3e2f16bd5db4f0d7e86d7419d327cc836810 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Mon, 12 Feb 2024 17:25:58 +0000 Subject: [PATCH] LibConfig+ConfigServer: Add permissive mode When in permissive mode, the ConfigServer will not treat reads and writes to non-pledged domains as errors, but instead turns them into no-ops: Reads will act as if the key was not found, and writes will do nothing. Permissive mode must be enabled before pledging any domains. This is needed to make GUI Widgets nicer to work with in GML Playground: a few Widgets include reads and writes to LibConfig in order to load system settings (eg, GUI::Calendar) or to save and restore state (eg, GUI::DynamicWidgetContainer). Without this change, editing a layout that includes one of these Widgets will cause GML Playground to crash when they try to access config domains that are not pledged. The solution used previously is to make Playground pledge more domains, but not only does this mean Playground has to know about these cases, but also that working on a layout file can alter the user's settings in other arbitrary apps, which is not something we want. By simply ignoring these config accesses, we avoid those downsides, and Widgets will simply use the fallback values they already have to provide to Config::read_foo_value(). --- Userland/Libraries/LibConfig/Client.cpp | 5 +++ Userland/Libraries/LibConfig/Client.h | 7 ++++ .../Services/ConfigServer/ConfigServer.ipc | 1 + .../ConfigServer/ConnectionFromClient.cpp | 35 +++++++++++++++---- .../ConfigServer/ConnectionFromClient.h | 2 ++ 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/Userland/Libraries/LibConfig/Client.cpp b/Userland/Libraries/LibConfig/Client.cpp index e01141dce6..f474d31808 100644 --- a/Userland/Libraries/LibConfig/Client.cpp +++ b/Userland/Libraries/LibConfig/Client.cpp @@ -19,6 +19,11 @@ Client& Client::the() return *s_the; } +void Client::enable_permissive_mode() +{ + async_enable_permissive_mode(); +} + void Client::pledge_domains(Vector const& domains) { async_pledge_domains(domains); diff --git a/Userland/Libraries/LibConfig/Client.h b/Userland/Libraries/LibConfig/Client.h index 601188c80d..4715ea9ec1 100644 --- a/Userland/Libraries/LibConfig/Client.h +++ b/Userland/Libraries/LibConfig/Client.h @@ -20,6 +20,8 @@ class Client final IPC_CLIENT_CONNECTION(Client, "/tmp/session/%sid/portal/config"sv) public: + /// Permissive mode makes reads and writes to non-pledged domains into no-ops instead of client misbehavior errors. + void enable_permissive_mode(); void pledge_domains(Vector const&); void monitor_domain(ByteString const&); @@ -121,6 +123,11 @@ inline void add_group(StringView domain, StringView group) Client::the().add_group(domain, group); } +inline void enable_permissive_mode() +{ + Client::the().enable_permissive_mode(); +} + inline void pledge_domains(Vector const& domains) { Client::the().pledge_domains(domains); diff --git a/Userland/Services/ConfigServer/ConfigServer.ipc b/Userland/Services/ConfigServer/ConfigServer.ipc index 2f5fe79b71..4fc2b03b77 100644 --- a/Userland/Services/ConfigServer/ConfigServer.ipc +++ b/Userland/Services/ConfigServer/ConfigServer.ipc @@ -1,5 +1,6 @@ endpoint ConfigServer { + enable_permissive_mode() =| pledge_domains(Vector domains) =| monitor_domain(ByteString domain) =| diff --git a/Userland/Services/ConfigServer/ConnectionFromClient.cpp b/Userland/Services/ConfigServer/ConnectionFromClient.cpp index 952b54b6a9..38104ff96e 100644 --- a/Userland/Services/ConfigServer/ConnectionFromClient.cpp +++ b/Userland/Services/ConfigServer/ConnectionFromClient.cpp @@ -99,10 +99,20 @@ void ConnectionFromClient::pledge_domains(Vector const& domains) m_pledged_domains.set(domain); } +void ConnectionFromClient::enable_permissive_mode() +{ + if (m_has_pledged) { + did_misbehave("Tried to enable permissive mode after pledging."); + return; + } + m_permissive_mode = true; +} + void ConnectionFromClient::monitor_domain(ByteString const& domain) { if (m_has_pledged && !m_pledged_domains.contains(domain)) { - did_misbehave("Attempt to monitor non-pledged domain"); + if (!m_permissive_mode) + did_misbehave("Attempt to monitor non-pledged domain"); return; } @@ -115,7 +125,8 @@ bool ConnectionFromClient::validate_access(ByteString const& domain, ByteString return true; if (m_pledged_domains.contains(domain)) return true; - did_misbehave(ByteString::formatted("Blocked attempt to access domain '{}', group={}, key={}", domain, group, key).characters()); + if (!m_permissive_mode) + did_misbehave(ByteString::formatted("Blocked attempt to access domain '{}', group={}, key={}", domain, group, key).characters()); return false; } @@ -153,8 +164,11 @@ Messages::ConfigServer::ListConfigGroupsResponse ConnectionFromClient::list_conf Messages::ConfigServer::ReadStringValueResponse ConnectionFromClient::read_string_value(ByteString const& domain, ByteString const& group, ByteString const& key) { - if (!validate_access(domain, group, key)) + if (!validate_access(domain, group, key)) { + if (m_permissive_mode) + return Optional {}; return nullptr; + } auto& config = ensure_domain_config(domain); if (!config.has_key(group, key)) @@ -164,8 +178,11 @@ Messages::ConfigServer::ReadStringValueResponse ConnectionFromClient::read_strin Messages::ConfigServer::ReadI32ValueResponse ConnectionFromClient::read_i32_value(ByteString const& domain, ByteString const& group, ByteString const& key) { - if (!validate_access(domain, group, key)) + if (!validate_access(domain, group, key)) { + if (m_permissive_mode) + return Optional {}; return nullptr; + } auto& config = ensure_domain_config(domain); if (!config.has_key(group, key)) @@ -175,8 +192,11 @@ Messages::ConfigServer::ReadI32ValueResponse ConnectionFromClient::read_i32_valu Messages::ConfigServer::ReadU32ValueResponse ConnectionFromClient::read_u32_value(ByteString const& domain, ByteString const& group, ByteString const& key) { - if (!validate_access(domain, group, key)) + if (!validate_access(domain, group, key)) { + if (m_permissive_mode) + return Optional {}; return nullptr; + } auto& config = ensure_domain_config(domain); if (!config.has_key(group, key)) @@ -186,8 +206,11 @@ Messages::ConfigServer::ReadU32ValueResponse ConnectionFromClient::read_u32_valu Messages::ConfigServer::ReadBoolValueResponse ConnectionFromClient::read_bool_value(ByteString const& domain, ByteString const& group, ByteString const& key) { - if (!validate_access(domain, group, key)) + if (!validate_access(domain, group, key)) { + if (m_permissive_mode) + return Optional {}; return nullptr; + } auto& config = ensure_domain_config(domain); if (!config.has_key(group, key)) diff --git a/Userland/Services/ConfigServer/ConnectionFromClient.h b/Userland/Services/ConfigServer/ConnectionFromClient.h index f64d25984c..d8cb97214c 100644 --- a/Userland/Services/ConfigServer/ConnectionFromClient.h +++ b/Userland/Services/ConfigServer/ConnectionFromClient.h @@ -26,6 +26,7 @@ private: explicit ConnectionFromClient(NonnullOwnPtr, int client_id); virtual void pledge_domains(Vector const&) override; + virtual void enable_permissive_mode() override; virtual void monitor_domain(ByteString const&) override; virtual Messages::ConfigServer::ListConfigGroupsResponse list_config_groups([[maybe_unused]] ByteString const& domain) override; virtual Messages::ConfigServer::ListConfigKeysResponse list_config_keys([[maybe_unused]] ByteString const& domain, [[maybe_unused]] ByteString const& group) override; @@ -46,6 +47,7 @@ private: void start_or_restart_sync_timer(); bool m_has_pledged { false }; + bool m_permissive_mode { false }; HashTable m_pledged_domains; HashTable m_monitored_domains;