fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-05 10:20:14 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

libnm-util: don't assert in nm_setting_get_secret_flags() and avoid assertion in agent_secrets_done_cb()

Browse source 
When secret providers return the connection hash in GetSecrets(),
this hash should only contain secrets. However, some providers also
return non-secret properties.

for_each_secret() iterated over all entries of the @secrets hash
and triggered the assertion in nm_setting_get_secret_flags() (see
below).

NM should not assert against user provided input. Change
nm_setting_get_secret_flags() to silently return FALSE, if the property
is not a secret.

Indeed, handling of secrets is very different for NMSettingVpn and
others. Hence nm_setting_get_secret_flags() has only an inconsistent
behavior and we have to fix all call sites to do the right thing
(depending on whether we have a VPN setting or not).

Now for_each_secret() checks whether the property is a secret
without hitting the assertion. Adjust all other calls of
nm_setting_get_secret_flags(), to anticipate non-secret flags and
assert/warn where appropriate.

Also, agent_secrets_done_cb() clears now all non-secrets properties
from the hash, using the new argument @remove_non_secrets when calling
for_each_secret().

  #0  0x0000003370c504e9 in g_logv () from /lib64/libglib-2.0.so.0
  #1  0x0000003370c5063f in g_log () from /lib64/libglib-2.0.so.0
  #2  0x00007fa4b0c1c156 in get_secret_flags (setting=0x1e3ac60, secret_name=0x1ea9180 "security", verify_secret=1, out_flags=0x7fff7507857c, error=0x0) at nm-setting.c:1091
  #3  0x00007fa4b0c1c2b2 in nm_setting_get_secret_flags (setting=0x1e3ac60, secret_name=0x1ea9180 "security", out_flags=0x7fff7507857c, error=0x0) at nm-setting.c:1124
  #4  0x0000000000463d03 in for_each_secret (connection=0x1deb2f0, secrets=0x1e9f860, callback=0x464f1b <has_system_owned_secrets>, callback_data=0x7fff7507865c) at settings/nm-settings-connection.c:203
  #5  0x000000000046525f in agent_secrets_done_cb (manager=0x1dddf50, call_id=1, agent_dbus_owner=0x1ddb9e0 ":1.39", agent_username=0x1e51710 "thom", agent_has_modify=1, setting_name=0x1e91f90 "802-11-wireless-security",
      flags=NM_SETTINGS_GET_SECRETS_FLAG_ALLOW_INTERACTION, secrets=0x1e9f860, error=0x0, user_data=0x1deb2f0, other_data2=0x477d61 <get_secrets_cb>, other_data3=0x1ea92a0) at settings/nm-settings-connection.c:757
  #6  0x00000000004dc4fd in get_complete_cb (parent=0x1ea6300, secrets=0x1e9f860, agent_dbus_owner=0x1ddb9e0 ":1.39", agent_username=0x1e51710 "thom", error=0x0, user_data=0x1dddf50) at settings/nm-agent-manager.c:1139
  #7  0x00000000004dab54 in req_complete_success (req=0x1ea6300, secrets=0x1e9f860, agent_dbus_owner=0x1ddb9e0 ":1.39", agent_uname=0x1e51710 "thom") at settings/nm-agent-manager.c:502
  #8  0x00000000004db86e in get_done_cb (agent=0x1e89530, call_id=0x1, secrets=0x1e9f860, error=0x0, user_data=0x1ea6300) at settings/nm-agent-manager.c:856
  #9  0x00000000004de9d0 in get_callback (proxy=0x1e47530, call=0x1, user_data=0x1ea10f0) at settings/nm-secret-agent.c:267
  #10 0x000000337380cad2 in complete_pending_call_and_unlock () from /lib64/libdbus-1.so.3
  #11 0x000000337380fdc1 in dbus_connection_dispatch () from /lib64/libdbus-1.so.3
  #12 0x000000342800ad65 in message_queue_dispatch () from /lib64/libdbus-glib-1.so.2
  #13 0x0000003370c492a6 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
  #14 0x0000003370c49628 in g_main_context_iterate.isra.24 () from /lib64/libglib-2.0.so.0
  #15 0x0000003370c49a3a in g_main_loop_run () from /lib64/libglib-2.0.so.0
  #16 0x000000000042e5c6 in main (argc=1, argv=0x7fff75078e88) at main.c:644

Signed-off-by: Thomas Haller <thaller@redhat.com>
This commit is contained in:
Thomas Haller 2014-02-23 17:03:01 +01:00
parent a7c8e5c6e9
commit 978724da96
9 changed files with 106 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
libnm-core/nm-setting-vpn.c
View file

@ -539,29 +539,31 @@ get_secret_flags (NMSetting *setting,
char *flags_key;
gpointer val;
unsigned long tmp;
NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
flags_key = g_strdup_printf ("%s-flags", secret_name);
if (g_hash_table_lookup_extended (priv->data, flags_key, NULL, &val)) {
errno = 0;
tmp = strtoul ((const char *) val, NULL, 10);
if ((errno == 0) && (tmp <= NM_SETTING_SECRET_FLAGS_ALL)) {
if (out_flags)
*out_flags = (guint32) tmp;
flags = (NMSettingSecretFlags) tmp;
success = TRUE;
} else {
g_set_error (error,
NM_SETTING_ERROR,
NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH,
"Failed to convert '%s' value '%s' to uint",
_("Failed to convert '%s' value '%s' to uint"),
flags_key, (const char *) val);
}
} else {
g_set_error (error,
NM_SETTING_ERROR,
NM_SETTING_ERROR_PROPERTY_NOT_FOUND,
"Secret flags property '%s' not found", flags_key);
_("Secret flags property '%s' not found"), flags_key);
}
g_free (flags_key);
if (out_flags)
*out_flags = flags;
return success;
}

32
libnm-core/nm-setting.c
View file

@ -1002,8 +1002,12 @@ compare_property (NMSetting *setting,
NMSettingSecretFlags a_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
NMSettingSecretFlags b_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
nm_setting_get_secret_flags (setting, prop_spec->name, &a_secret_flags, NULL);
nm_setting_get_secret_flags (other, prop_spec->name, &b_secret_flags, NULL);
g_return_val_if_fail (!NM_IS_SETTING_VPN (setting), FALSE);
if (!nm_setting_get_secret_flags (setting, prop_spec->name, &a_secret_flags, NULL))
g_return_val_if_reached (FALSE);
if (!nm_setting_get_secret_flags (other, prop_spec->name, &b_secret_flags, NULL))
g_return_val_if_reached (FALSE);
/* If the secret flags aren't the same the settings aren't the same */
if (a_secret_flags != b_secret_flags)
@ -1108,7 +1112,16 @@ should_compare_prop (NMSetting *setting,
if (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS)
return FALSE;
nm_setting_get_secret_flags (setting, prop_name, &secret_flags, NULL);
if ( NM_IS_SETTING_VPN (setting)
&& g_strcmp0 (prop_name, NM_SETTING_VPN_SECRETS) == 0) {
/* FIXME: NMSettingVPN:NM_SETTING_VPN_SECRETS has NM_SETTING_PARAM_SECRET.
* nm_setting_get_secret_flags() quite possibly fails, but it might succeed if the
* setting accidently uses a key "secrets". */
return TRUE;
}
if (!nm_setting_get_secret_flags (setting, prop_name, &secret_flags, NULL))
g_return_val_if_reached (FALSE);
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS)
&& (secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED))
@ -1330,8 +1343,12 @@ clear_secrets_with_flags (NMSetting *setting,
NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
gboolean changed = FALSE;
g_return_val_if_fail (!NM_IS_SETTING_VPN (setting), FALSE);
/* Clear the secret if the user function says to do so */
nm_setting_get_secret_flags (setting, pspec->name, &flags, NULL);
if (!nm_setting_get_secret_flags (setting, pspec->name, &flags, NULL))
g_return_val_if_reached (FALSE);
if (func (setting, pspec->name, flags, user_data) == TRUE) {
GValue value = G_VALUE_INIT;
@ -1549,8 +1566,11 @@ get_secret_flags (NMSetting *setting,
char *flags_prop;
NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
if (verify_secret)
g_return_val_if_fail (is_secret_prop (setting, secret_name, error), FALSE);
if (verify_secret && !is_secret_prop (setting, secret_name, error)) {
if (out_flags)
*out_flags = NM_SETTING_SECRET_FLAG_NONE;
return FALSE;
}
flags_prop = g_strdup_printf ("%s-flags", secret_name);
g_object_get (G_OBJECT (setting), flags_prop, &flags, NULL);

10
libnm-util/nm-setting-vpn.c
View file

@ -545,29 +545,31 @@ get_secret_flags (NMSetting *setting,
char *flags_key;
gpointer val;
unsigned long tmp;
NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
flags_key = g_strdup_printf ("%s-flags", secret_name);
if (g_hash_table_lookup_extended (priv->data, flags_key, NULL, &val)) {
errno = 0;
tmp = strtoul ((const char *) val, NULL, 10);
if ((errno == 0) && (tmp <= NM_SETTING_SECRET_FLAGS_ALL)) {
if (out_flags)
*out_flags = (guint32) tmp;
flags = (NMSettingSecretFlags) tmp;
success = TRUE;
} else {
g_set_error (error,
NM_SETTING_ERROR,
NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH,
"Failed to convert '%s' value '%s' to uint",
_("Failed to convert '%s' value '%s' to uint"),
flags_key, (const char *) val);
}
} else {
g_set_error (error,
NM_SETTING_ERROR,
NM_SETTING_ERROR_PROPERTY_NOT_FOUND,
"Secret flags property '%s' not found", flags_key);
_("Secret flags property '%s' not found"), flags_key);
}
g_free (flags_key);
if (out_flags)
*out_flags = flags;
return success;
}

32
libnm-util/nm-setting.c
View file

@ -542,8 +542,12 @@ compare_property (NMSetting *setting,
NMSettingSecretFlags a_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
NMSettingSecretFlags b_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
nm_setting_get_secret_flags (setting, prop_spec->name, &a_secret_flags, NULL);
nm_setting_get_secret_flags (other, prop_spec->name, &b_secret_flags, NULL);
g_return_val_if_fail (!NM_IS_SETTING_VPN (setting), FALSE);
if (!nm_setting_get_secret_flags (setting, prop_spec->name, &a_secret_flags, NULL))
g_return_val_if_reached (FALSE);
if (!nm_setting_get_secret_flags (other, prop_spec->name, &b_secret_flags, NULL))
g_return_val_if_reached (FALSE);
/* If the secret flags aren't the same the settings aren't the same */
if (a_secret_flags != b_secret_flags)
@ -648,7 +652,16 @@ should_compare_prop (NMSetting *setting,
if (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS)
return FALSE;
nm_setting_get_secret_flags (setting, prop_name, &secret_flags, NULL);
if ( NM_IS_SETTING_VPN (setting)
&& g_strcmp0 (prop_name, NM_SETTING_VPN_SECRETS) == 0) {
/* FIXME: NMSettingVPN:NM_SETTING_VPN_SECRETS has NM_SETTING_PARAM_SECRET.
* nm_setting_get_secret_flags() quite possibly fails, but it might succeed if the
* setting accidently uses a key "secrets". */
return FALSE;
}
if (!nm_setting_get_secret_flags (setting, prop_name, &secret_flags, NULL))
g_return_val_if_reached (FALSE);
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS)
&& (secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED))
@ -870,8 +883,12 @@ clear_secrets_with_flags (NMSetting *setting,
NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
gboolean changed = FALSE;
g_return_val_if_fail (!NM_IS_SETTING_VPN (setting), FALSE);
/* Clear the secret if the user function says to do so */
nm_setting_get_secret_flags (setting, pspec->name, &flags, NULL);
if (!nm_setting_get_secret_flags (setting, pspec->name, &flags, NULL))
g_return_val_if_reached (FALSE);
if (func (setting, pspec->name, flags, user_data) == TRUE) {
GValue value = G_VALUE_INIT;
@ -1090,8 +1107,11 @@ get_secret_flags (NMSetting *setting,
char *flags_prop;
NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
if (verify_secret)
g_return_val_if_fail (is_secret_prop (setting, secret_name, error), FALSE);
if (verify_secret && !is_secret_prop (setting, secret_name, error)) {
if (out_flags)
*out_flags = NM_SETTING_SECRET_FLAG_NONE;
return FALSE;
}
flags_prop = g_strdup_printf ("%s-flags", secret_name);
g_object_get (G_OBJECT (setting), flags_prop, &flags, NULL);

18
src/devices/wifi/nm-device-wifi.c
View file

@ -2053,10 +2053,11 @@ need_new_8021x_secrets (NMDeviceWifi *self,
s_8021x = nm_connection_get_setting_802_1x (connection);
if (s_8021x) {
nm_setting_get_secret_flags (NM_SETTING (s_8021x),
NM_SETTING_802_1X_PASSWORD,
&secret_flags,
NULL);
if (!nm_setting_get_secret_flags (NM_SETTING (s_8021x),
NM_SETTING_802_1X_PASSWORD,
&secret_flags,
NULL))
g_assert_not_reached ();
if (secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED)
*setting_name = NM_SETTING_802_1X_SETTING_NAME;
return *setting_name ? TRUE : FALSE;
@ -2064,10 +2065,11 @@ need_new_8021x_secrets (NMDeviceWifi *self,
s_wsec = nm_connection_get_setting_wireless_security (connection);
if (s_wsec) {
nm_setting_get_secret_flags (NM_SETTING (s_wsec),
NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD,
&secret_flags,
NULL);
if (!nm_setting_get_secret_flags (NM_SETTING (s_wsec),
NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD,
&secret_flags,
NULL))
g_assert_not_reached ();
if (secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED)
*setting_name = NM_SETTING_WIRELESS_SECURITY_SETTING_NAME;
return *setting_name ? TRUE : FALSE;

3
src/settings/nm-agent-manager.c
View file

@ -1010,7 +1010,8 @@ check_system_secrets_cb (NMSetting *setting,
*has_system = TRUE;
}
} else {
nm_setting_get_secret_flags (setting, key, &secret_flags, NULL);
if (!nm_setting_get_secret_flags (setting, key, &secret_flags, NULL))
g_return_if_reached ();
if (secret_flags == NM_SETTING_SECRET_FLAG_NONE)
*has_system = TRUE;
}

20
src/settings/nm-settings-connection.c
View file

@ -144,6 +144,7 @@ typedef gboolean (*ForEachSecretFunc) (GHashTableIter *iter,
static void
for_each_secret (NMConnection *connection,
GHashTable *secrets,
gboolean remove_non_secrets,
ForEachSecretFunc callback,
gpointer callback_data)
{
@ -167,6 +168,8 @@ for_each_secret (NMConnection *connection,
* each time.
*/
g_return_if_fail (callback);
/* Walk through the list of setting hashes */
g_hash_table_iter_init (&iter, secrets);
while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, (gpointer) &setting_hash)) {
@ -175,6 +178,9 @@ for_each_secret (NMConnection *connection,
const char *secret_name;
GValue *val;
if (g_hash_table_size (setting_hash) == 0)
continue;
/* Get the actual NMSetting from the connection so we can get secret flags
* from the connection data, since flags aren't secrets. What we're
* iterating here is just the secrets, not a whole connection.
@ -203,7 +209,11 @@ for_each_secret (NMConnection *connection,
return;
}
} else {
nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL);
if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) {
if (remove_non_secrets)
g_hash_table_iter_remove (&secret_iter);
continue;
}
if (callback (&secret_iter, secret_flags, callback_data) == FALSE)
return;
}
@ -764,7 +774,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, has_system_owned_secrets, &agent_had_system);
for_each_secret (NM_CONNECTION (self), secrets, TRUE, has_system_owned_secrets, &agent_had_system);
if (agent_had_system) {
if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) {
/* No user interaction was allowed when requesting secrets; the
@ -776,7 +786,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
call_id,
agent_dbus_owner);
for_each_secret (NM_CONNECTION (self), secrets, clear_nonagent_secrets, NULL);
for_each_secret (NM_CONNECTION (self), secrets, FALSE, clear_nonagent_secrets, NULL);
} else if (agent_has_modify == FALSE) {
/* Agent didn't successfully authenticate; clear system-owned secrets
* from the secrets the agent returned.
@ -786,7 +796,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
setting_name,
call_id);
for_each_secret (NM_CONNECTION (self), secrets, clear_nonagent_secrets, NULL);
for_each_secret (NM_CONNECTION (self), secrets, FALSE, clear_nonagent_secrets, NULL);
}
}
} else {
@ -805,7 +815,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
* 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, clear_unsaved_secrets, NULL);
for_each_secret (NM_CONNECTION (self), secrets, TRUE, clear_unsaved_secrets, NULL);
/* Update the connection with our existing secrets from backing storage */
nm_connection_clear_secrets (NM_CONNECTION (self));

25
src/settings/plugins/ifnet/connection_parser.c
View file

@ -2846,7 +2846,8 @@ check_unsupported_secrets (NMSetting *setting,
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 (!nm_setting_get_secret_flags (setting, key, &secret_flags, NULL))
g_return_if_reached ();
if (secret_flags != NM_SETTING_SECRET_FLAG_NONE)
*unsupported_secret = TRUE;
}
@ -2869,17 +2870,6 @@ ifnet_can_write_connection (NMConnection *connection, GError **error)
return FALSE;
}
/* If the connection has flagged secrets, ignore
* it as this plugin does not deal with user agent service */
nm_connection_for_each_setting_value (connection,
check_unsupported_secrets,
&has_unsupported_secrets);
if (has_unsupported_secrets) {
g_set_error_literal (error, IFNET_PLUGIN_ERROR, 0,
"The ifnet plugin only supports persistent system secrets.");
return FALSE;
}
/* Only support wired, wifi, and PPPoE */
if ( !nm_connection_is_type (connection, NM_SETTING_WIRED_SETTING_NAME)
&& !nm_connection_is_type (connection, NM_SETTING_WIRELESS_SETTING_NAME)
@ -2891,6 +2881,17 @@ ifnet_can_write_connection (NMConnection *connection, GError **error)
return FALSE;
}
/* If the connection has flagged secrets, ignore
* it as this plugin does not deal with user agent service */
nm_connection_for_each_setting_value (connection,
check_unsupported_secrets,
&has_unsupported_secrets);
if (has_unsupported_secrets) {
g_set_error_literal (error, IFNET_PLUGIN_ERROR, 0,
"The ifnet plugin only supports persistent system secrets.");
return FALSE;
}
return TRUE;
}

3
src/settings/plugins/keyfile/writer.c
View file

@ -797,7 +797,8 @@ write_setting_value (NMSetting *setting,
if (pspec && (pspec->flags & NM_SETTING_PARAM_SECRET) && !NM_IS_SETTING_VPN (setting)) {
NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE;
nm_setting_get_secret_flags (setting, key, &secret_flags, NULL);
if (!nm_setting_get_secret_flags (setting, key, &secret_flags, NULL))
g_assert_not_reached ();
if (secret_flags != NM_SETTING_SECRET_FLAG_NONE)
return;
}