diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 73bc403320..8c47de9739 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -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, diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 8f92afa9bb..bae5fbb9bb 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -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);