cli: merge branch 'bg/cli-editor-save-rh1546805'

https://github.com/NetworkManager/NetworkManager/pull/244
https://bugzilla.redhat.com/show_bug.cgi?id=1546805
(cherry picked from commit 8af4f76638)

clients/cli/connections.c

@@ -6534,6 +6534,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
@@ -7127,6 +7133,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)
 {
@@ -7646,6 +7662,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) {
@@ -7674,23 +7694,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
+				source = g_timeout_source_new (10 * NM_UTILS_MSEC_PER_SECOND);
-				while (!nmc_editor_cb_called)
+				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") :
@@ -7713,9 +7754,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;

libnm-core/nm-connection.c

@@ -1124,6 +1124,29 @@ _normalize_wireless_mac_address_randomization (NMConnection *self, GHashTable *p
 	return FALSE;
 }
 
+static gboolean
+_normalize_macsec (NMConnection *self, GHashTable *parameters)
+{
+	NMSettingMacsec *s_macsec = nm_connection_get_setting_macsec (self);
+	gboolean changed = FALSE;
+
+	if (!s_macsec)
+		return FALSE;
+
+	if (nm_setting_macsec_get_mode (s_macsec) != NM_SETTING_MACSEC_MODE_PSK) {
+		if (nm_setting_macsec_get_mka_cak (s_macsec)) {
+			g_object_set (s_macsec, NM_SETTING_MACSEC_MKA_CAK, NULL, NULL);
+			changed = TRUE;
+		}
+		if (nm_setting_macsec_get_mka_ckn (s_macsec)) {
+			g_object_set (s_macsec, NM_SETTING_MACSEC_MKA_CKN, NULL, NULL);
+			changed = TRUE;
+		}
+	}
+
+	return changed;
+}
+
 static gboolean
 _normalize_team_config (NMConnection *self, GHashTable *parameters)
 {
@@ -1564,6 +1587,7 @@ nm_connection_normalize (NMConnection *connection,
 	was_modified |= _normalize_bond_mode (connection, parameters);
 	was_modified |= _normalize_bond_options (connection, parameters);
 	was_modified |= _normalize_wireless_mac_address_randomization (connection, parameters);
+	was_modified |= _normalize_macsec (connection, parameters);
 	was_modified |= _normalize_team_config (connection, parameters);
 	was_modified |= _normalize_team_port_config (connection, parameters);
 	was_modified |= _normalize_bluetooth_type (connection, parameters);

libnm-core/nm-setting-macsec.c

@@ -243,6 +243,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,
@@ -254,7 +260,7 @@ verify_macsec_key (const char *key, gboolean cak, GError **error)
 	req_len = cak ?
 	    NM_SETTING_MACSEC_MKA_CAK_LENGTH :
 	    NM_SETTING_MACSEC_MKA_CKN_LENGTH;
-	if (strlen (key) != req_len) {
+	if (strlen (key) != (gsize) req_len) {
 		g_set_error (error,
 		             NM_CONNECTION_ERROR,
 		             NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -340,6 +346,10 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_MACSEC_SETTING_NAME, NM_SETTING_MACSEC_MKA_CKN);
 			return FALSE;
 		}
+		if (!verify_macsec_key (priv->mka_cak, TRUE, error)) {
+			g_prefix_error (error, "%s.%s: ", NM_SETTING_MACSEC_SETTING_NAME, NM_SETTING_MACSEC_MKA_CAK);
+			return FALSE;
+		}
 	} else if (priv->mode == NM_SETTING_MACSEC_MODE_EAP) {
 		if (!s_8021x) {
 			g_set_error (error,
@@ -350,6 +360,13 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			g_prefix_error (error, "%s: ", NM_SETTING_MACSEC_SETTING_NAME);
 			return FALSE;
 		}
+	} else {
+		g_set_error_literal (error,
+		                     NM_CONNECTION_ERROR,
+		                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		                     _("must be either psk (0) or eap (1)"));
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_MACSEC_SETTING_NAME, NM_SETTING_MACSEC_MODE);
+		return FALSE;
 	}
 
 	if (priv->port <= 0 || priv->port > 65534) {
@@ -362,6 +379,17 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		return FALSE;
 	}
 
+	if (   priv->mode != NM_SETTING_MACSEC_MODE_PSK
+	    && (priv->mka_cak || priv->mka_ckn)) {
+		g_set_error_literal (error,
+		                     NM_CONNECTION_ERROR,
+		                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		                     _("only valid for psk mode"));
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_MACSEC_SETTING_NAME,
+		                priv->mka_cak ? NM_SETTING_MACSEC_MKA_CAK : NM_SETTING_MACSEC_MKA_CKN);
+		return NM_SETTING_VERIFY_NORMALIZABLE;
+	}
+
 	return TRUE;
 }