This is an automated email from the git hooks/post-receive script.
git pushed a commit to branch nm-vpn-username-capture
in repository enlightenment.
View the commit online.
commit 47e0d04f516d80fb994441cf45c0e95e691e5123
Author: [email protected] <[email protected]>
AuthorDate: Sun Jun 21 21:35:12 2026 -0600
networkmanager: fix VPN username clobber and connect-time mechanism
Two bugs found by live test:
- enm_vpn_username_set used 'nmcli ... vpn.data username=' which REPLACES the
whole vpn.data dict, wiping connection-type/ca/cert and causing the openvpn
plugin to fail with 'Invalid connection type'. Use '+vpn.data' to update the
single key.
- Writing vpn.data during an active connect is rejected by NM ('modified since
activation'). At connect time, return the username in the VPN secrets dict
for that activation (as nm-applet does) instead of modifying the connection.
Drop the now-unused deferred-reply + e_nm_agent_request_detach machinery.
vpn.data persistence remains on the import path only.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
---
src/modules/networkmanager/agent.c | 98 ++++++----------------
src/modules/networkmanager/e_networkmanager.c | 10 ---
src/modules/networkmanager/e_networkmanager.h | 4 -
.../networkmanager/e_networkmanager_vpn_username.c | 4 +-
4 files changed, 30 insertions(+), 86 deletions(-)
diff --git a/src/modules/networkmanager/agent.c b/src/modules/networkmanager/agent.c
index c99da75cd..3ac0dd591 100644
--- a/src/modules/networkmanager/agent.c
+++ b/src/modules/networkmanager/agent.c
@@ -9,7 +9,6 @@
#include "e.h"
#include "e_networkmanager.h"
-#include "e_networkmanager_vpn_username.h"
#include "e_mod_main.h"
/*
@@ -42,7 +41,6 @@ struct _E_NM_Agent_Dialog
* secret) — persisted to vpn.data on OK instead of returned via reply. */
Eina_Bool has_username_field;
Evas_Object *username_entry;
- char *conn_name; /* strdup; for username persistence */
};
static E_NM_Agent_Dialog *_current_dialog = NULL;
@@ -55,34 +53,6 @@ static E_NM_Agent_Dialog *_current_dialog = NULL;
/* Dialog callbacks */
/* -------------------------------------------------------------------------- */
-/* Captures the VPN secrets reply so it can be sent *after* the username
- * property write completes, ensuring NM sees vpn.data["username"] on this
- * activation rather than only the next one. Owns req + dup'd payload. */
-typedef struct _Vpn_Reply_Deferred
-{
- E_NM_Agent_Request *req;
- char **names;
- char **values;
- unsigned int n;
-} Vpn_Reply_Deferred;
-
-static void
-_vpn_reply_deferred_run(void *data, Eina_Bool ok EINA_UNUSED)
-{
- Vpn_Reply_Deferred *d = data;
-
- /* Reply regardless of whether the username write succeeded — a failed
- * modify still leaves the user able to retry, and cancelling here would
- * lose the password too. */
- e_nm_agent_reply_vpn_secrets(d->req,
- (const char *const *)d->names,
- (const char *const *)d->values, d->n);
- for (unsigned int i = 0; i < d->n; i++) { free(d->names[i]); free(d->values[i]); }
- free(d->names);
- free(d->values);
- free(d);
-}
-
static void
_dialog_send_ok(E_NM_Agent_Dialog *ad)
{
@@ -98,62 +68,50 @@ _dialog_send_ok(E_NM_Agent_Dialog *ad)
}
else
{
- /* The username is a connection property, not a secret. Persist it to
- * vpn.data FIRST, then send the secrets reply from the write's
- * completion callback — so NM sees the username on this activation
- * instead of only the next connect. The password is never sent as a
- * username and the username is never sent as a secret. */
- char *user = NULL;
- char **names, **values;
+ /* NM rejects any connection change made during an active connect
+ * ("modified since activation"), so the username cannot be written to
+ * vpn.data here. Instead return it in the secrets dict for THIS
+ * activation (the openvpn plugin merges vpn secrets over vpn.data),
+ * exactly as nm-applet does. The extra "username" slot is appended
+ * after the real secret fields. */
+ char *user = NULL;
+ unsigned int extra = 0;
if (ad->has_username_field && ad->username_entry)
- user = elm_entry_markup_to_utf8(
- elm_entry_entry_get(ad->username_entry));
+ {
+ user = elm_entry_markup_to_utf8(
+ elm_entry_entry_get(ad->username_entry));
+ if (user && user[0]) extra = 1;
+ }
- names = calloc(ad->n_fields ?: 1, sizeof(*names));
- values = calloc(ad->n_fields ?: 1, sizeof(*values));
+ unsigned int total = ad->n_fields + extra;
+ const char **names = calloc(total ?: 1, sizeof(*names));
+ char **values = calloc(total ?: 1, sizeof(*values));
if (!names || !values)
{
- free(names); free(values);
+ free(names); free(values); free(user);
e_nm_agent_reply_cancel(ad->req);
}
else
{
- Vpn_Reply_Deferred *d;
-
for (unsigned int i = 0; i < ad->n_fields; i++)
{
- names[i] = strdup(ad->field_names[i] ?: "");
+ names[i] = ad->field_names[i];
values[i] = elm_entry_markup_to_utf8(
elm_entry_entry_get(ad->field_entries[i]));
if (!values[i]) values[i] = strdup("");
}
-
- d = calloc(1, sizeof(*d));
- if (!d)
+ if (extra)
{
- for (unsigned int i = 0; i < ad->n_fields; i++)
- { free(names[i]); free(values[i]); }
- free(names); free(values);
- e_nm_agent_reply_cancel(ad->req);
- }
- else
- {
- d->req = ad->req;
- d->names = names;
- d->values = values;
- d->n = ad->n_fields;
- if (user && user[0] && ad->conn_name)
- {
- /* The reply now outlives this scope; detach req so a
- * concurrent CancelGetSecrets cannot free it first. */
- e_nm_agent_request_detach(ad->req);
- enm_vpn_username_set(ad->conn_name, user,
- _vpn_reply_deferred_run, d);
- }
- else
- _vpn_reply_deferred_run(d, EINA_TRUE);
+ names[ad->n_fields] = "username";
+ values[ad->n_fields] = user; /* ownership moves to array */
+ user = NULL;
}
+ e_nm_agent_reply_vpn_secrets(ad->req, names,
+ (const char *const *)values, total);
+ for (unsigned int i = 0; i < total; i++) free(values[i]);
+ free(values);
+ free(names);
}
free(user);
}
@@ -228,7 +186,6 @@ _dialog_del_cb(void *data)
free(ad->field_names);
}
free(ad->field_entries);
- free(ad->conn_name);
free(ad);
}
@@ -447,7 +404,6 @@ _vpn_dialog_new(E_NM_Agent_Request *req, const char *conn_name,
ad->is_vpn = EINA_TRUE;
ad->n_fields = n_fields;
ad->has_username_field = need_username;
- ad->conn_name = conn_name ? strdup(conn_name) : NULL;
ad->field_names = calloc(n_fields ?: 1, sizeof(*ad->field_names));
ad->field_entries = calloc(n_fields ?: 1, sizeof(*ad->field_entries));
used = calloc(n_fields ?: 1, sizeof(*used));
diff --git a/src/modules/networkmanager/e_networkmanager.c b/src/modules/networkmanager/e_networkmanager.c
index 5b3a3d473..1717fbff3 100644
--- a/src/modules/networkmanager/e_networkmanager.c
+++ b/src/modules/networkmanager/e_networkmanager.c
@@ -3098,16 +3098,6 @@ _agent_request_free(E_NM_Agent_Request *req)
free(req);
}
-void
-e_nm_agent_request_detach(E_NM_Agent_Request *req)
-{
- /* Unlink from the agent's pending list so CancelGetSecrets can no longer
- * match and free this request, while keeping req->agent valid for the
- * eventual reply. Used when a reply is deferred behind an async write. */
- if (req && req->agent)
- req->agent->pending = eina_list_remove(req->agent->pending, req);
-}
-
static void
_agent_dict_append_str(Eldbus_Message_Iter *array, const char *key,
const char *val)
diff --git a/src/modules/networkmanager/e_networkmanager.h b/src/modules/networkmanager/e_networkmanager.h
index 19d206974..46579890e 100644
--- a/src/modules/networkmanager/e_networkmanager.h
+++ b/src/modules/networkmanager/e_networkmanager.h
@@ -58,10 +58,6 @@ struct _E_NM_Agent_Callbacks
void e_nm_agent_callbacks_set(const E_NM_Agent_Callbacks *cbs, void *data);
void e_nm_agent_reply_secrets(E_NM_Agent_Request *req, const char *psk);
void e_nm_agent_reply_cancel(E_NM_Agent_Request *req);
-/* Detach a request from the agent's pending list so a deferred reply can
- * outlive a concurrent CancelGetSecrets without a use-after-free. The caller
- * then owns the request and MUST still call one of the reply functions. */
-void e_nm_agent_request_detach(E_NM_Agent_Request *req);
void e_nm_agent_reply_vpn_secrets(E_NM_Agent_Request *req,
const char *const *fields,
const char *const *values,
diff --git a/src/modules/networkmanager/e_networkmanager_vpn_username.c b/src/modules/networkmanager/e_networkmanager_vpn_username.c
index 18b059a50..91611b70d 100644
--- a/src/modules/networkmanager/e_networkmanager_vpn_username.c
+++ b/src/modules/networkmanager/e_networkmanager_vpn_username.c
@@ -110,8 +110,10 @@ enm_vpn_username_set(const char *conn_name, const char *username,
return;
}
+ /* '+vpn.data' updates the single username key; plain 'vpn.data' would
+ * REPLACE the whole dictionary, wiping connection-type/ca/cert/etc. */
snprintf(cmd, sizeof(cmd),
- "'%s' connection modify '%s' vpn.data 'username=%s'",
+ "'%s' connection modify '%s' +vpn.data 'username=%s'",
esc_nmcli, esc_name, esc_user);
free(esc_nmcli); free(esc_name); free(esc_user);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
