colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

settings: fix/reorganize NMSettingConnection secrets code

Browse Source 
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:
Dan Winship
2015-07-01 17:35:22 -04:00
parent 2a2fd1216b
commit 1424f249e3
1 changed files with 71 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

107
src/settings/nm-settings-connection.c
View File

@@ -180,9 +180,8 @@ typedef struct {
/**************************************************************/ /**************************************************************/
/* Return TRUE to continue, FALSE to stop */ /* Return TRUE to keep, FALSE to drop */
typedef gboolean (*ForEachSecretFunc) (GHashTableIter *iter, typedef gboolean (*ForEachSecretFunc) (NMSettingSecretFlags flags,
NMSettingSecretFlags flags,
gpointer user_data); gpointer user_data);
static void static void
@@ -248,9 +247,13 @@ for_each_secret (NMConnection *self,
g_hash_table_iter_init (&vpn_secrets_iter, g_value_get_boxed (val)); 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)) { while (g_hash_table_iter_next (&vpn_secrets_iter, (gpointer) &secret_name, NULL)) {
secret_flags = NM_SETTING_SECRET_FLAG_NONE; secret_flags = NM_SETTING_SECRET_FLAG_NONE;
nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL); if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) {
if (callback (&vpn_secrets_iter, secret_flags, callback_data) == FALSE) if (remove_non_secrets)
return; g_hash_table_iter_remove (&vpn_secrets_iter);
continue;
}
if (!callback (secret_flags, callback_data))
g_hash_table_iter_remove (&vpn_secrets_iter);
} }
} else { } else {
if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { 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); g_hash_table_iter_remove (&secret_iter);
continue; continue;
} }
if (callback (&secret_iter, secret_flags, callback_data) == FALSE) if (!callback (secret_flags, callback_data))
return; 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 static void
@@ -754,38 +793,29 @@ supports_secrets (NMSettingsConnection *self, const char *setting_name)
return TRUE; return TRUE;
} }
static gboolean typedef struct {
clear_nonagent_secrets (GHashTableIter *iter, NMSettingSecretFlags required;
NMSettingSecretFlags flags, NMSettingSecretFlags forbidden;
gpointer user_data) } ForEachSecretFlags;
{
if (flags != NM_SETTING_SECRET_FLAG_AGENT_OWNED)
g_hash_table_iter_remove (iter);
return TRUE;
}
static gboolean static gboolean
clear_unsaved_secrets (GHashTableIter *iter, validate_secret_flags (NMSettingSecretFlags flags,
NMSettingSecretFlags flags,
gpointer user_data) gpointer user_data)
{ {
if (flags & (NM_SETTING_SECRET_FLAG_NOT_SAVED | NM_SETTING_SECRET_FLAG_NOT_REQUIRED)) ForEachSecretFlags *cmp_flags = user_data;
g_hash_table_iter_remove (iter);
if (!NM_FLAGS_ALL (flags, cmp_flags->required))
return FALSE;
if (NM_FLAGS_ANY (flags, cmp_flags->forbidden))
return FALSE;
return TRUE; return TRUE;
} }
static gboolean static gboolean
has_system_owned_secrets (GHashTableIter *iter, secret_is_system_owned (NMSettingSecretFlags flags,
NMSettingSecretFlags flags, gpointer user_data)
gpointer user_data)
{ {
gboolean *has_system_owned = user_data; return !NM_FLAGS_HAS (flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED);
if (flags == NM_SETTING_SECRET_FLAG_NONE) {
*has_system_owned = TRUE;
return FALSE;
}
return TRUE;
} }
static void static void
@@ -820,6 +850,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
GError *local = NULL; GError *local = NULL;
GVariant *dict; GVariant *dict;
gboolean agent_had_system = FALSE; gboolean agent_had_system = FALSE;
ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE };
if (error) { if (error) {
_LOGD ("(%s:%u) secrets request error: (%d) %s", _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 * save those system-owned secrets. If not, discard them and use the
* existing secrets, or fail the connection. * 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 (agent_had_system) {
if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) { if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) {
/* No user interaction was allowed when requesting secrets; the /* No user interaction was allowed when requesting secrets; the
@@ -865,7 +896,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
call_id, call_id,
agent_dbus_owner); 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) { } else if (agent_has_modify == FALSE) {
/* Agent didn't successfully authenticate; clear system-owned secrets /* Agent didn't successfully authenticate; clear system-owned secrets
* from the secrets the agent returned. * from the secrets the agent returned.
@@ -874,7 +905,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
setting_name, setting_name,
call_id); call_id);
for_each_secret (NM_CONNECTION (self), secrets, FALSE, clear_nonagent_secrets, NULL); cmp_flags.required |= NM_SETTING_SECRET_FLAG_AGENT_OWNED;
} }
} }
} else { } else {
@@ -890,8 +921,12 @@ agent_secrets_done_cb (NMAgentManager *manager,
/* If no user interaction was allowed, make sure that no "unsaved" secrets /* If no user interaction was allowed, make sure that no "unsaved" secrets
* came back. Unsaved secrets by definition require user interaction. * came back. Unsaved secrets by definition require user interaction.
*/ */
if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) {
for_each_secret (NM_CONNECTION (self), secrets, TRUE, clear_unsaved_secrets, NULL); 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 */ /* Update the connection with our existing secrets from backing storage */
nm_connection_clear_secrets (NM_CONNECTION (self)); nm_connection_clear_secrets (NM_CONNECTION (self));