From 8d5b01619b0518be59d661ed4780f6adeae38a38 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Nov 2018 10:54:26 +0100 Subject: [PATCH 1/3] libnm-core: macsec: don't require a cak in verify() CAK is a connection secret and can be NULL for various reasons (agent-owned, no permissions to get secrets, etc.). verify() must not require it. Fixes: 474a0dbfbeeda7504d6599abe4adf0ddf18bab1e --- libnm-core/nm-setting-macsec.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libnm-core/nm-setting-macsec.c b/libnm-core/nm-setting-macsec.c index a9b1d550ad..9f60f945c1 100644 --- a/libnm-core/nm-setting-macsec.c +++ b/libnm-core/nm-setting-macsec.c @@ -245,6 +245,12 @@ verify_macsec_key (const char *key, gboolean cak, GError **error) { int req_len; + /* CAK is a connection secret and can be NULL for various + * reasons (agent-owned, no permissions to get secrets, etc.) + */ + if (cak && !key) + return TRUE; + if (!key || !key[0]) { g_set_error_literal (error, NM_CONNECTION_ERROR, From 096eef61d499d0d9e7ef198610b75814b2c86a35 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Nov 2018 11:28:26 +0100 Subject: [PATCH 2/3] cli: editor: reload secrets after updating connection Connection secrets are lost after calling nm_connection_replace_settings_from_connection() because @con_tmp doesn't contain secrets; refetch them like we do when starting the editor. --- clients/cli/connections.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 88ba3246a7..b8eccaaead 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -7796,9 +7796,10 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t if (menu_ctx.curr_setting) s_name = g_strdup (nm_setting_get_name (menu_ctx.curr_setting)); - /* Update settings in the local connection */ + /* Update settings and secrets in the local connection */ nm_connection_replace_settings_from_connection (connection, NM_CONNECTION (con_tmp)); + update_secrets_in_connection (con_tmp, connection); /* Also update setting for menu context and TAB-completion */ menu_ctx.curr_setting = s_name ? nm_connection_get_setting_by_name (connection, s_name) : NULL; From a370faeb59a90fa1550aa8df55d80974dd5220ac Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Nov 2018 11:32:27 +0100 Subject: [PATCH 3/3] cli: wait for changed signal after updating a connection In editor_menu_main(), after saving a connection we wait that the Update2() D-Bus call returns and then we copy the NMRemoteConnection instance over to @connection. This assumes that when Update2() returns the remote connection instance is already updated with new settings. Indeed, on server side the NMSettingsConnection first emits the "Updated" signal and then returns to Update2(). However, the Updated signal doesn't include the new setting values and so libnm has to fire an asynchronous nmdbus_settings_connection_call_get_setting() to fetch the new settings, which terminates after the Update2(). So, to be sure that the remote connection got updated we have also to listen to the connection-changed signal, which is always emitted after an update. https://bugzilla.redhat.com/show_bug.cgi?id=1546805 --- clients/cli/connections.c | 45 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index b8eccaaead..841d0f938a 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -6588,6 +6588,12 @@ static gboolean nmc_editor_cb_called; static GError *nmc_editor_error; static MonitorACInfo *nmc_editor_monitor_ac; +static void +editor_connection_changed_cb (NMConnection *connection, gboolean *changed) +{ + *changed = TRUE; +} + /* * Store 'error' to shared 'nmc_editor_error' and monitoring info to * 'nmc_editor_monitor_ac' and signal the condition so that @@ -7194,6 +7200,16 @@ menu_switch_to_level1 (const NmcConfig *nmc_config, menu_ctx->valid_props_str = g_strjoinv (", ", menu_ctx->valid_props); } +static gboolean +editor_save_timeout (gpointer user_data) +{ + gboolean *timeout = user_data; + + *timeout = TRUE; + + return G_SOURCE_REMOVE; +} + static gboolean editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_type) { @@ -7726,6 +7742,10 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t /* Save the connection */ if (nm_connection_verify (connection, &err1)) { gboolean persistent = TRUE; + gboolean connection_changed; + nm_auto_unref_gsource GSource *source = NULL; + gboolean timeout = FALSE; + gulong handler_id = 0; /* parse argument */ if (cmd_arg) { @@ -7757,23 +7777,44 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t connection, add_connection_editor_cb, info); + connection_changed = TRUE; } else { /* Save/update already saved (existing) connection */ nm_connection_replace_settings_from_connection (NM_CONNECTION (rem_con), connection); update_connection (persistent, rem_con, update_connection_editor_cb, NULL); + + handler_id = g_signal_connect (rem_con, + NM_CONNECTION_CHANGED, + G_CALLBACK (editor_connection_changed_cb), + &connection_changed); + connection_changed = FALSE; } - //FIXME: add also a timeout for cases the callback is not called - while (!nmc_editor_cb_called) + source = g_timeout_source_new (10 * NM_UTILS_MSEC_PER_SECOND); + g_source_set_callback (source, editor_save_timeout, &timeout, NULL); + g_source_attach (source, g_main_loop_get_context (loop)); + + while (!nmc_editor_cb_called && !timeout) g_main_context_iteration (NULL, TRUE); + while (!connection_changed && !timeout) + g_main_context_iteration (NULL, TRUE); + + if (handler_id) + g_signal_handler_disconnect (rem_con, handler_id); + g_source_destroy (source); + if (nmc_editor_error) { g_print (_("Error: Failed to save '%s' (%s) connection: %s\n"), nm_connection_get_id (connection), nm_connection_get_uuid (connection), nmc_editor_error->message); g_error_free (nmc_editor_error); + } else if (timeout) { + g_print (_("Error: Timeout saving '%s' (%s) connection\n"), + nm_connection_get_id (connection), + nm_connection_get_uuid (connection)); } else { g_print (!rem_con ? _("Connection '%s' (%s) successfully saved.\n") :