mirror of
https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git
synced 2025-12-29 02:10:09 +01:00
settings: streamline system-owned secret handling during agent requests
Do the check for system-owned secrets once, before kicking off the request, instead of each time we ask an agent. As a bonus, this change ensures priv->secrets doesn't store anything except system-owned secrets too, simplifying some checks later on.
This commit is contained in:
parent
ac208cafbd
commit
d8cbecec8b
2 changed files with 67 additions and 61 deletions
|
|
@ -741,23 +741,6 @@ get_agent_modify_auth_cb (NMAuthChain *chain,
|
|||
}
|
||||
}
|
||||
|
||||
static void
|
||||
has_system_secrets (NMSetting *setting,
|
||||
const char *key,
|
||||
const GValue *value,
|
||||
GParamFlags flags,
|
||||
gpointer user_data)
|
||||
{
|
||||
NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE;
|
||||
gboolean *has_system = user_data;
|
||||
|
||||
if (flags & NM_SETTING_PARAM_SECRET) {
|
||||
nm_setting_get_secret_flags (setting, key, &secret_flags, NULL);
|
||||
if (!(secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED))
|
||||
*has_system = TRUE;
|
||||
}
|
||||
}
|
||||
|
||||
static void
|
||||
get_next_cb (Request *req)
|
||||
{
|
||||
|
|
@ -768,32 +751,27 @@ get_next_cb (Request *req)
|
|||
|
||||
agent_dbus_owner = nm_secret_agent_get_dbus_owner (NM_SECRET_AGENT (req->current));
|
||||
|
||||
if (req->flags != 0) {
|
||||
gboolean has_system = FALSE;
|
||||
/* If the request flags allow user interaction, and there are system secrets,
|
||||
* check whether the agent has the 'modify' permission before sending those
|
||||
* secrets to the agent. We shouldn't leak system-owned secrets to
|
||||
* unprivileged users.
|
||||
*/
|
||||
if ((req->flags != 0) && (req->existing_secrets != NULL)) {
|
||||
nm_log_dbg (LOGD_AGENTS, "(%p/%s) request has system secrets; checking agent %s for MODIFY",
|
||||
req, req->setting_name, agent_dbus_owner);
|
||||
|
||||
/* Interaction with the user is allowed; if there are any system secrets,
|
||||
* check whether the agent has the 'modify' permission before sending those
|
||||
* secrets to the agent.
|
||||
*/
|
||||
nm_connection_for_each_setting_value (req->connection, has_system_secrets, &has_system);
|
||||
if (has_system) {
|
||||
nm_log_dbg (LOGD_AGENTS, "(%p/%s) request has system secrets; checking agent %s for MODIFY",
|
||||
req, req->setting_name, agent_dbus_owner);
|
||||
req->chain = nm_auth_chain_new_dbus_sender (req->authority,
|
||||
agent_dbus_owner,
|
||||
get_agent_modify_auth_cb,
|
||||
req);
|
||||
g_assert (req->chain);
|
||||
nm_auth_chain_add_call (req->chain, NM_AUTH_PERMISSION_SETTINGS_CONNECTION_MODIFY, TRUE);
|
||||
} else {
|
||||
nm_log_dbg (LOGD_AGENTS, "(%p/%s) requesting user-owned secrets from agent %s",
|
||||
req, req->setting_name, agent_dbus_owner);
|
||||
|
||||
req->chain = nm_auth_chain_new_dbus_sender (req->authority,
|
||||
agent_dbus_owner,
|
||||
get_agent_modify_auth_cb,
|
||||
req);
|
||||
g_assert (req->chain);
|
||||
nm_auth_chain_add_call (req->chain, NM_AUTH_PERMISSION_SETTINGS_CONNECTION_MODIFY, TRUE);
|
||||
return;
|
||||
}
|
||||
get_agent_request_secrets (req, FALSE);
|
||||
}
|
||||
|
||||
nm_log_dbg (LOGD_AGENTS, "(%p/%s) requesting user-owned secrets from agent %s",
|
||||
req, req->setting_name, agent_dbus_owner);
|
||||
|
||||
get_agent_request_secrets (req, FALSE);
|
||||
}
|
||||
|
||||
static gboolean
|
||||
|
|
@ -913,6 +891,12 @@ nm_agent_manager_get_secrets (NMAgentManager *self,
|
|||
nm_connection_get_path (connection),
|
||||
setting_name);
|
||||
|
||||
/* NOTE: a few things in the Request handling depend on existing_secrets
|
||||
* being NULL if there aren't any system-owned secrets for this connection.
|
||||
* This in turn depends on nm_connection_to_hash() and nm_setting_to_hash()
|
||||
* both returning NULL if they didn't hash anything.
|
||||
*/
|
||||
|
||||
req = request_new_get (connection,
|
||||
priv->authority,
|
||||
filter_by_uid,
|
||||
|
|
|
|||
|
|
@ -187,6 +187,35 @@ session_changed_cb (NMSessionMonitor *self, gpointer user_data)
|
|||
|
||||
/**************************************************************/
|
||||
|
||||
static void
|
||||
only_system_secrets_cb (NMSetting *setting,
|
||||
const char *key,
|
||||
const GValue *value,
|
||||
GParamFlags flags,
|
||||
gpointer user_data)
|
||||
{
|
||||
if (flags & NM_SETTING_PARAM_SECRET) {
|
||||
NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE;
|
||||
|
||||
nm_setting_get_secret_flags (setting, key, &secret_flags, NULL);
|
||||
if (secret_flags != NM_SETTING_SECRET_FLAG_NONE)
|
||||
g_object_set (G_OBJECT (setting), key, NULL, NULL);
|
||||
}
|
||||
}
|
||||
|
||||
static void
|
||||
update_secrets_cache (NMSettingsConnection *self)
|
||||
{
|
||||
NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
|
||||
|
||||
if (priv->secrets)
|
||||
g_object_unref (priv->secrets);
|
||||
priv->secrets = nm_connection_duplicate (NM_CONNECTION (self));
|
||||
|
||||
/* Clear out non-system-owned and not-saved secrets */
|
||||
nm_connection_for_each_setting_value (priv->secrets, only_system_secrets_cb, NULL);
|
||||
}
|
||||
|
||||
/* Update the settings of this connection to match that of 'new', taking care to
|
||||
* make a private copy of secrets. */
|
||||
gboolean
|
||||
|
|
@ -211,9 +240,7 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self,
|
|||
/* Copy the connection to keep its secrets around even if NM
|
||||
* calls nm_connection_clear_secrets().
|
||||
*/
|
||||
if (priv->secrets)
|
||||
g_object_unref (priv->secrets);
|
||||
priv->secrets = nm_connection_duplicate (NM_CONNECTION (self));
|
||||
update_secrets_cache (self);
|
||||
|
||||
nm_settings_connection_recheck_visibility (self);
|
||||
success = TRUE;
|
||||
|
|
@ -537,9 +564,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
|
|||
/* Now that all secrets are updated, copy and cache new secrets,
|
||||
* then save them to backing storage.
|
||||
*/
|
||||
if (priv->secrets)
|
||||
g_object_unref (priv->secrets);
|
||||
priv->secrets = nm_connection_duplicate (NM_CONNECTION (self));
|
||||
update_secrets_cache (self);
|
||||
|
||||
nm_log_dbg (LOGD_SETTINGS, "(%s/%s:%u) saving new secrets to backing storage",
|
||||
nm_setting_connection_get_uuid (s_con),
|
||||
|
|
@ -616,9 +641,8 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
|
|||
return 0;
|
||||
}
|
||||
|
||||
/* FIXME: if setting_name is empty, return all secrets */
|
||||
|
||||
if (!nm_connection_get_setting_by_name (priv->secrets, setting_name)) {
|
||||
/* Make sure the request actually requests something we can return */
|
||||
if (!nm_connection_get_setting_by_name (NM_CONNECTION (self), setting_name)) {
|
||||
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_SETTING,
|
||||
"%s.%d - Connection didn't have requested setting '%s'.",
|
||||
__FILE__, __LINE__, setting_name);
|
||||
|
|
@ -995,21 +1019,19 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self,
|
|||
|
||||
priv->reqs = g_slist_remove (priv->reqs, GUINT_TO_POINTER (call_id));
|
||||
|
||||
/* The connection's secrets will have been updated by the agent manager,
|
||||
* so we want to refresh the secrets cache. Note that we will never save
|
||||
* new secrets to backing storage here because D-Bus initated requests will
|
||||
* never ask for completely new secrets from agents. Thus system-owned
|
||||
* secrets should not have changed from backing storage. We also don't
|
||||
* send agent-owned back out to be saved since we assume the agent that
|
||||
* provided the secrets saved them itself.
|
||||
*/
|
||||
if (priv->secrets)
|
||||
g_object_unref (priv->secrets);
|
||||
priv->secrets = nm_connection_duplicate (NM_CONNECTION (self));
|
||||
|
||||
if (error)
|
||||
dbus_g_method_return_error (context, error);
|
||||
else {
|
||||
/* The connection's secrets will have been updated by the agent manager,
|
||||
* so we want to refresh the secrets cache. Note that we will never save
|
||||
* new secrets to backing storage here because D-Bus initated requests will
|
||||
* never ask for completely new secrets from agents. Thus system-owned
|
||||
* secrets should not have changed from backing storage. We also don't
|
||||
* send agent-owned secrets back out to be saved since we assume the agent
|
||||
* that provided the secrets saved them itself.
|
||||
*/
|
||||
update_secrets_cache (self);
|
||||
|
||||
hash = nm_connection_to_hash (NM_CONNECTION (self), NM_SETTING_HASH_FLAG_ONLY_SECRETS);
|
||||
dbus_g_method_return (context, hash);
|
||||
g_hash_table_destroy (hash);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue