settings: fix/reorganize NMSettingConnection secrets code
NMSettingConnection's for_each_secret() function works in a slightly-too-GHashTable-specific way. Reorganize the code now to make the change to GVariants easier later. Also, fix a few bugs: - In the (unlikely) case of a non-secret being stored in vpn.secrets, we were treating it as though it was a secret with flags NONE. - The code was comparing against NONE when it meant !AGENT_OWNED in a few places. (With the current set of NMSettingSecretFlags values, this worked, but in the future it might not.) - In some cases we never called for_each_secret() with the @remove_non_secrets flag, meaning we might have ended up passing non-secrets to other code.
This commit is contained in:
@@ -180,9 +180,8 @@ typedef struct {
|
||||
|
||||
/**************************************************************/
|
||||
|
||||
/* Return TRUE to continue, FALSE to stop */
|
||||
typedef gboolean (*ForEachSecretFunc) (GHashTableIter *iter,
|
||||
NMSettingSecretFlags flags,
|
||||
/* Return TRUE to keep, FALSE to drop */
|
||||
typedef gboolean (*ForEachSecretFunc) (NMSettingSecretFlags flags,
|
||||
gpointer user_data);
|
||||
|
||||
static void
|
||||
@@ -248,9 +247,13 @@ for_each_secret (NMConnection *self,
|
||||
g_hash_table_iter_init (&vpn_secrets_iter, g_value_get_boxed (val));
|
||||
while (g_hash_table_iter_next (&vpn_secrets_iter, (gpointer) &secret_name, NULL)) {
|
||||
secret_flags = NM_SETTING_SECRET_FLAG_NONE;
|
||||
nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL);
|
||||
if (callback (&vpn_secrets_iter, secret_flags, callback_data) == FALSE)
|
||||
return;
|
||||
if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) {
|
||||
if (remove_non_secrets)
|
||||
g_hash_table_iter_remove (&vpn_secrets_iter);
|
||||
continue;
|
||||
}
|
||||
if (!callback (secret_flags, callback_data))
|
||||
g_hash_table_iter_remove (&vpn_secrets_iter);
|
||||
}
|
||||
} else {
|
||||
if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) {
|
||||
@@ -258,13 +261,49 @@ for_each_secret (NMConnection *self,
|
||||
g_hash_table_iter_remove (&secret_iter);
|
||||
continue;
|
||||
}
|
||||
if (callback (&secret_iter, secret_flags, callback_data) == FALSE)
|
||||
return;
|
||||
if (!callback (secret_flags, callback_data))
|
||||
g_hash_table_iter_remove (&secret_iter);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
typedef gboolean (*FindSecretFunc) (NMSettingSecretFlags flags,
|
||||
gpointer user_data);
|
||||
|
||||
typedef struct {
|
||||
FindSecretFunc find_func;
|
||||
gpointer find_func_data;
|
||||
gboolean found;
|
||||
} FindSecretData;
|
||||
|
||||
static gboolean
|
||||
find_secret_for_each_func (NMSettingSecretFlags flags,
|
||||
gpointer user_data)
|
||||
{
|
||||
FindSecretData *data = user_data;
|
||||
|
||||
if (!data->found)
|
||||
data->found = data->find_func (flags, data->find_func_data);
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
static gboolean
|
||||
find_secret (NMConnection *self,
|
||||
GHashTable *secrets,
|
||||
FindSecretFunc callback,
|
||||
gpointer callback_data)
|
||||
{
|
||||
FindSecretData data;
|
||||
|
||||
data.find_func = callback;
|
||||
data.find_func_data = callback_data;
|
||||
data.found = FALSE;
|
||||
|
||||
for_each_secret (self, secrets, FALSE, find_secret_for_each_func, &data);
|
||||
return data.found;
|
||||
}
|
||||
|
||||
/**************************************************************/
|
||||
|
||||
static void
|
||||
@@ -754,38 +793,29 @@ supports_secrets (NMSettingsConnection *self, const char *setting_name)
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
static gboolean
|
||||
clear_nonagent_secrets (GHashTableIter *iter,
|
||||
NMSettingSecretFlags flags,
|
||||
gpointer user_data)
|
||||
{
|
||||
if (flags != NM_SETTING_SECRET_FLAG_AGENT_OWNED)
|
||||
g_hash_table_iter_remove (iter);
|
||||
return TRUE;
|
||||
}
|
||||
typedef struct {
|
||||
NMSettingSecretFlags required;
|
||||
NMSettingSecretFlags forbidden;
|
||||
} ForEachSecretFlags;
|
||||
|
||||
static gboolean
|
||||
clear_unsaved_secrets (GHashTableIter *iter,
|
||||
NMSettingSecretFlags flags,
|
||||
validate_secret_flags (NMSettingSecretFlags flags,
|
||||
gpointer user_data)
|
||||
{
|
||||
if (flags & (NM_SETTING_SECRET_FLAG_NOT_SAVED | NM_SETTING_SECRET_FLAG_NOT_REQUIRED))
|
||||
g_hash_table_iter_remove (iter);
|
||||
ForEachSecretFlags *cmp_flags = user_data;
|
||||
|
||||
if (!NM_FLAGS_ALL (flags, cmp_flags->required))
|
||||
return FALSE;
|
||||
if (NM_FLAGS_ANY (flags, cmp_flags->forbidden))
|
||||
return FALSE;
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
static gboolean
|
||||
has_system_owned_secrets (GHashTableIter *iter,
|
||||
NMSettingSecretFlags flags,
|
||||
gpointer user_data)
|
||||
secret_is_system_owned (NMSettingSecretFlags flags,
|
||||
gpointer user_data)
|
||||
{
|
||||
gboolean *has_system_owned = user_data;
|
||||
|
||||
if (flags == NM_SETTING_SECRET_FLAG_NONE) {
|
||||
*has_system_owned = TRUE;
|
||||
return FALSE;
|
||||
}
|
||||
return TRUE;
|
||||
return !NM_FLAGS_HAS (flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -820,6 +850,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
|
||||
GError *local = NULL;
|
||||
GVariant *dict;
|
||||
gboolean agent_had_system = FALSE;
|
||||
ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE };
|
||||
|
||||
if (error) {
|
||||
_LOGD ("(%s:%u) secrets request error: (%d) %s",
|
||||
@@ -854,7 +885,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
|
||||
* save those system-owned secrets. If not, discard them and use the
|
||||
* existing secrets, or fail the connection.
|
||||
*/
|
||||
for_each_secret (NM_CONNECTION (self), secrets, TRUE, has_system_owned_secrets, &agent_had_system);
|
||||
agent_had_system = find_secret (NM_CONNECTION (self), secrets, secret_is_system_owned, NULL);
|
||||
if (agent_had_system) {
|
||||
if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) {
|
||||
/* No user interaction was allowed when requesting secrets; the
|
||||
@@ -865,7 +896,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
|
||||
call_id,
|
||||
agent_dbus_owner);
|
||||
|
||||
for_each_secret (NM_CONNECTION (self), secrets, FALSE, clear_nonagent_secrets, NULL);
|
||||
cmp_flags.required |= NM_SETTING_SECRET_FLAG_AGENT_OWNED;
|
||||
} else if (agent_has_modify == FALSE) {
|
||||
/* Agent didn't successfully authenticate; clear system-owned secrets
|
||||
* from the secrets the agent returned.
|
||||
@@ -874,7 +905,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
|
||||
setting_name,
|
||||
call_id);
|
||||
|
||||
for_each_secret (NM_CONNECTION (self), secrets, FALSE, clear_nonagent_secrets, NULL);
|
||||
cmp_flags.required |= NM_SETTING_SECRET_FLAG_AGENT_OWNED;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -890,8 +921,12 @@ agent_secrets_done_cb (NMAgentManager *manager,
|
||||
/* If no user interaction was allowed, make sure that no "unsaved" secrets
|
||||
* came back. Unsaved secrets by definition require user interaction.
|
||||
*/
|
||||
if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE)
|
||||
for_each_secret (NM_CONNECTION (self), secrets, TRUE, clear_unsaved_secrets, NULL);
|
||||
if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) {
|
||||
cmp_flags.forbidden |= ( NM_SETTING_SECRET_FLAG_NOT_SAVED
|
||||
| NM_SETTING_SECRET_FLAG_NOT_REQUIRED);
|
||||
}
|
||||
|
||||
for_each_secret (NM_CONNECTION (self), secrets, TRUE, validate_secret_flags, &cmp_flags);
|
||||
|
||||
/* Update the connection with our existing secrets from backing storage */
|
||||
nm_connection_clear_secrets (NM_CONNECTION (self));
|
||||
|
Reference in New Issue
Block a user