From d399ffbaba325df1b749d23c2f2fcba8ef8646cf Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 4 Dec 2025 10:58:53 +0100 Subject: [PATCH] nmcli: start the agent only after updating the connection When connecting to a wifi network and providing the password on the command line, nmcli first looks if there is a compatible connection to reuse. If there is not, it creates and activates a new one via a single call to AddAndActivate(). If there is a compatible connection, nmcli first calls Update() on it to set the new password and then Activate() to bring it up. Before that, it registers a secret agent that can prompt for a new password in case of authentication failure. However, as soon as nmcli registers a secret agent, NM tries to activate again the connection if it was blocked due to a previous authentication failure. This connection attempt is going to fail because it still uses the old password, as new one hasn't been set via Update(). Change the order of operations to register the agent after Update() and before Activate(). Reproducer: nmcli device wifi connect SSID password BAD_PASSWORD nmcli device wifi connect SSID password GOOD_PASSWORD Fixes: c8ff1b30fba3 ('nmcli/dev: use secret agent for nmcli d [wifi] connect') (cherry picked from commit 427a7cf2577868cd70556923a88312a68f939009) --- src/nmcli/devices.c | 52 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/src/nmcli/devices.c b/src/nmcli/devices.c index 4831a06e19..e81e710f44 100644 --- a/src/nmcli/devices.c +++ b/src/nmcli/devices.c @@ -2090,6 +2090,7 @@ typedef struct { char *specific_object; bool hotspot : 1; bool create : 1; + bool start_agent : 1; } AddAndActivateInfo; static AddAndActivateInfo * @@ -2097,6 +2098,7 @@ add_and_activate_info_new(NmCli *nmc, NMDevice *device, gboolean hotspot, gboolean create, + gboolean start_agent, const char *specific_object) { AddAndActivateInfo *info; @@ -2107,6 +2109,7 @@ add_and_activate_info_new(NmCli *nmc, .device = g_object_ref(device), .hotspot = hotspot, .create = create, + .start_agent = start_agent, .specific_object = g_strdup(specific_object), }; return info; @@ -2364,7 +2367,7 @@ do_device_connect(const NMCCommand *cmd, NmCli *nmc, int argc, const char *const nmc); } - info = add_and_activate_info_new(nmc, device, FALSE, FALSE, NULL); + info = add_and_activate_info_new(nmc, device, FALSE, FALSE, FALSE, NULL); nm_client_activate_connection_async(nmc->client, NULL, /* let NM find a connection automatically */ @@ -3603,6 +3606,16 @@ activate_update2_cb(GObject *source_object, GAsyncResult *res, gpointer user_dat return; } + if (info->start_agent && !nmc->secret_agent) { + nmc->secret_agent = nm_secret_agent_simple_new("nmcli-connect"); + if (nmc->secret_agent) { + g_signal_connect(nmc->secret_agent, + NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, + G_CALLBACK(nmc_secrets_requested), + nmc); + } + } + nm_client_activate_connection_async(nmc->client, NM_CONNECTION(remote_con), info->device, @@ -3617,6 +3630,7 @@ save_and_activate_connection(NmCli *nmc, NMDevice *device, NMConnection *connection, gboolean hotspot, + gboolean start_agent, const char *specific_object) { AddAndActivateInfo *info; @@ -3625,9 +3639,15 @@ save_and_activate_connection(NmCli *nmc, device, hotspot, !NM_IS_REMOTE_CONNECTION(connection), + start_agent, specific_object); if (NM_IS_REMOTE_CONNECTION(connection)) { + /* Don't start the agent immediately. Otherwise the agent registration + * to the daemon will trigger a new activation if the connection was + * blocked due to bad secrets. This new activation would use the old + * secrets. + */ nm_remote_connection_update2(NM_REMOTE_CONNECTION(connection), nm_connection_to_dbus(connection, NM_CONNECTION_SERIALIZE_ALL), NM_SETTINGS_UPDATE2_FLAG_BLOCK_AUTOCONNECT, @@ -3636,6 +3656,16 @@ save_and_activate_connection(NmCli *nmc, activate_update2_cb, info); } else { + if (start_agent) { + nmc->secret_agent = nm_secret_agent_simple_new("nmcli-connect"); + if (nmc->secret_agent) { + g_signal_connect(nmc->secret_agent, + NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, + G_CALLBACK(nmc_secrets_requested), + nmc); + } + } + nm_client_add_and_activate_connection_async(nmc->client, connection, info->device, @@ -3665,6 +3695,7 @@ do_device_wifi_connect(const NMCCommand *cmd, NmCli *nmc, int argc, const char * gboolean private = FALSE; gboolean hidden = FALSE; gboolean wep_passphrase = FALSE; + gboolean start_agent = FALSE; GByteArray *bssid1_arr = NULL; GByteArray *bssid2_arr = NULL; gs_free NMDevice **devices = NULL; @@ -4029,14 +4060,8 @@ do_device_wifi_connect(const NMCCommand *cmd, NmCli *nmc, int argc, const char * NM_802_11_AP_SEC_KEY_MGMT_OWE | NM_802_11_AP_SEC_KEY_MGMT_OWE_TM))) { NMSettingWirelessSecurity *s_wsec = NULL; - /* Create secret agent */ - nmc->secret_agent = nm_secret_agent_simple_new("nmcli-connect"); - if (nmc->secret_agent) { - g_signal_connect(nmc->secret_agent, - NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, - G_CALLBACK(nmc_secrets_requested), - nmc); - } + /* Start the secret agent just before initiating the activation. */ + start_agent = TRUE; if (password) { if (!connection) @@ -4076,7 +4101,12 @@ do_device_wifi_connect(const NMCCommand *cmd, NmCli *nmc, int argc, const char * nmc->nowait_flag = (nmc->timeout == 0); nmc->should_wait++; - save_and_activate_connection(nmc, device, connection, FALSE, nm_object_get_path(NM_OBJECT(ap))); + save_and_activate_connection(nmc, + device, + connection, + FALSE, + start_agent, + nm_object_get_path(NM_OBJECT(ap))); finish: if (bssid1_arr) @@ -4536,7 +4566,7 @@ do_device_wifi_hotspot(const NMCCommand *cmd, NmCli *nmc, int argc, const char * nmc->nowait_flag = (nmc->timeout == 0); nmc->should_wait++; - save_and_activate_connection(nmc, device, connection, TRUE, NULL); + save_and_activate_connection(nmc, device, connection, TRUE, FALSE, NULL); } static void