From ce3e2152bc5253f73ee1ce5132fa6c1e48aebb24 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 9 Sep 2016 17:33:09 +0200 Subject: [PATCH 1/5] clients: add define for NMSecretAgentSimple signal name (cherry picked from commit b28b2ba8a961143ccb9cbba58320b38e5b8d5f5c) --- clients/cli/agent.c | 5 ++++- clients/cli/connections.c | 5 ++++- clients/cli/devices.c | 8 ++++++-- clients/common/nm-secret-agent-simple.c | 2 +- clients/common/nm-secret-agent-simple.h | 3 +++ clients/tui/nmtui-connect.c | 5 ++++- 6 files changed, 22 insertions(+), 6 deletions(-) diff --git a/clients/cli/agent.c b/clients/cli/agent.c index bc837c722f..e876d26227 100644 --- a/clients/cli/agent.c +++ b/clients/cli/agent.c @@ -148,7 +148,10 @@ do_agent_secret (NmCli *nmc, int argc, char **argv) nmc->should_wait++; nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (nmc->secret_agent), NULL); - g_signal_connect (nmc->secret_agent, "request-secrets", G_CALLBACK (secrets_requested), nmc); + g_signal_connect (nmc->secret_agent, + NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, + G_CALLBACK (secrets_requested), + nmc); g_print (_("nmcli successfully registered as a NetworkManager's secret agent.\n")); } else { g_string_printf (nmc->return_text, _("Error: secret agent initialization failed")); diff --git a/clients/cli/connections.c b/clients/cli/connections.c index ea7c8a79df..ddd7444ebc 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -2530,7 +2530,10 @@ nmc_activate_connection (NmCli *nmc, /* Create secret agent */ nmc->secret_agent = nm_secret_agent_simple_new ("nmcli-connect"); if (nmc->secret_agent) { - g_signal_connect (nmc->secret_agent, "request-secrets", G_CALLBACK (nmc_secrets_requested), nmc); + g_signal_connect (nmc->secret_agent, + NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, + G_CALLBACK (nmc_secrets_requested), + nmc); if (connection) { nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (nmc->secret_agent), nm_object_get_path (NM_OBJECT (connection))); diff --git a/clients/cli/devices.c b/clients/cli/devices.c index afa769f63c..32c84bb69a 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1860,8 +1860,12 @@ do_device_connect (NmCli *nmc, int argc, char **argv) /* Create secret agent */ nmc->secret_agent = nm_secret_agent_simple_new ("nmcli-connect"); - if (nmc->secret_agent) - g_signal_connect (nmc->secret_agent, "request-secrets", G_CALLBACK (nmc_secrets_requested), nmc); + if (nmc->secret_agent) { + g_signal_connect (nmc->secret_agent, + NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, + G_CALLBACK (nmc_secrets_requested), + nmc); + } info = g_malloc0 (sizeof (AddAndActivateInfo)); info->nmc = nmc; diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 721f980e65..3fe1a0b88f 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -807,7 +807,7 @@ nm_secret_agent_simple_class_init (NMSecretAgentSimpleClass *klass) * When the dialog is complete, the app must call * nm_secret_agent_simple_response() with the results. */ - signals[REQUEST_SECRETS] = g_signal_new ("request-secrets", + signals[REQUEST_SECRETS] = g_signal_new (NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, G_TYPE_FROM_CLASS (klass), 0, 0, NULL, NULL, NULL, G_TYPE_NONE, diff --git a/clients/common/nm-secret-agent-simple.h b/clients/common/nm-secret-agent-simple.h index ba819ae362..f85ba65c89 100644 --- a/clients/common/nm-secret-agent-simple.h +++ b/clients/common/nm-secret-agent-simple.h @@ -29,6 +29,9 @@ #define NM_IS_SECRET_AGENT_SIMPLE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_SECRET_AGENT_SIMPLE)) #define NM_SECRET_AGENT_SIMPLE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimpleClass)) +/* Signals */ +#define NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS "request-secrets" + typedef struct { NMSecretAgentOld parent; diff --git a/clients/tui/nmtui-connect.c b/clients/tui/nmtui-connect.c index ae9dd43ed9..ddabcd72ac 100644 --- a/clients/tui/nmtui-connect.c +++ b/clients/tui/nmtui-connect.c @@ -239,7 +239,10 @@ activate_connection (NMConnection *connection, nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (agent), nm_object_get_path (NM_OBJECT (connection))); } - g_signal_connect (agent, "request-secrets", G_CALLBACK (secrets_requested), connection); + g_signal_connect (agent, + NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, + G_CALLBACK (secrets_requested), + connection); } specific_object_path = specific_object ? nm_object_get_path (specific_object) : NULL; From b8e34bcdb38d73819964537c0a175e3d25abe926 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 9 Sep 2016 21:45:45 +0200 Subject: [PATCH 2/5] clients: don't show "(null)" prompt for secrets If the caller doesn't provide a message, simply don't show it. (cherry picked from commit a80af27fc930936cbdd2646a64b66359d09a603b) --- clients/cli/agent.c | 3 ++- clients/cli/common.c | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/clients/cli/agent.c b/clients/cli/agent.c index e876d26227..9211d9a0fb 100644 --- a/clients/cli/agent.c +++ b/clients/cli/agent.c @@ -96,7 +96,8 @@ get_secrets_from_user (const char *request_id, char *pwd = NULL; /* Ask user for the password */ - g_print ("%s\n", msg); + if (msg) + g_print ("%s\n", msg); if (secret->value) { /* Prefill the password if we have it. */ rl_startup_hook = set_deftext; diff --git a/clients/cli/common.c b/clients/cli/common.c index f1ec46a11e..62cd6c9c77 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -1058,13 +1058,15 @@ get_secrets_from_user (const char *request_id, nmc_rl_pre_input_deftext = g_strdup (secret->value); } } - g_print ("%s\n", msg); + if (msg) + g_print ("%s\n", msg); pwd = nmc_readline_echo (secret->password ? echo_on : TRUE, "%s (%s): ", secret->name, secret->prop_name); if (!pwd) pwd = g_strdup (""); } else { - g_print ("%s\n", msg); + if (msg) + g_print ("%s\n", msg); g_printerr (_("Warning: password for '%s' not given in 'passwd-file' " "and nmcli cannot ask without '--ask' option.\n"), secret->prop_name); From b632f2984b1e1caf32b773dbfde0f4d8b59fde8c Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 9 Sep 2016 21:47:14 +0200 Subject: [PATCH 3/5] clients: add secrets request message for wired and DSL connections (cherry picked from commit 2c1adaae5e3bd838c5735209caabcab430a781d9) --- clients/common/nm-secret-agent-simple.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 3fe1a0b88f..cf4abf9042 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -455,19 +455,14 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) s_con = nm_connection_get_setting_connection (request->connection); title = _("Wired 802.1X authentication"); - msg = NULL; + msg = g_strdup_printf (_("Secrets are required to access the wired network '%s'"), + nm_connection_get_id (request->connection)); - secret = nm_secret_agent_simple_secret_new (_("Network name"), - NM_SETTING (s_con), - NM_SETTING_CONNECTION_ID, - NULL, - NULL, - FALSE); - g_ptr_array_add (secrets, secret); ok = add_8021x_secrets (request, secrets); } else if (nm_connection_is_type (request->connection, NM_SETTING_PPPOE_SETTING_NAME)) { title = _("DSL authentication"); - msg = NULL; + msg = g_strdup_printf (_("Secrets are required for the DSL connection '%s'"), + nm_connection_get_id (request->connection)); ok = add_pppoe_secrets (request, secrets); } else if (nm_connection_is_type (request->connection, NM_SETTING_GSM_SETTING_NAME)) { From 9b443db4519a04fd7f7b2ff8c286395d7b2457da Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 13 Sep 2016 14:35:55 +0200 Subject: [PATCH 4/5] clients: handle secret requests only for current connection The path was checked only when serving the enqueued requests but not for new ones. Fix this by moving the check to request_secrets_from_ui(). Fixes: 991df804086c4a1cee393d6d7182fa40cbba5dd7 https://bugzilla.redhat.com/show_bug.cgi?id=1351272 (cherry picked from commit f3099db28e193a4c3736a651af2d10102cc39853) --- clients/common/nm-secret-agent-simple.c | 34 ++++++++++++++----------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index cf4abf9042..0c90eba36e 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -428,11 +428,28 @@ static void request_secrets_from_ui (NMSecretAgentSimpleRequest *request) { GPtrArray *secrets; + NMSecretAgentSimplePrivate *priv; NMSecretAgentSimpleSecret *secret; const char *title; char *msg; gboolean ok = TRUE; + priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (request->self); + g_return_if_fail (priv->enabled); + + /* We only handle requests for connection with @path if set. */ + if (!g_str_has_prefix (request->request_id, priv->path)) { + gs_free_error GError *error = NULL; + + error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, + "Request for %s secrets doesn't match path %s", + request->request_id, priv->path); + request->callback (NM_SECRET_AGENT_OLD (request->self), request->connection, + NULL, error, request->callback_data); + g_hash_table_remove (priv->requests, request->request_id); + return; + } + secrets = g_ptr_array_new_with_free_func ((GDestroyNotify) nm_secret_agent_simple_secret_free); if (nm_connection_is_type (request->connection, NM_SETTING_WIRELESS_SETTING_NAME)) { @@ -734,7 +751,6 @@ nm_secret_agent_simple_enable (NMSecretAgentSimple *self, const char *path) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); GList *requests, *iter; - GError *error; if (g_strcmp0 (path, priv->path) != 0) { g_free (priv->path); @@ -747,21 +763,9 @@ nm_secret_agent_simple_enable (NMSecretAgentSimple *self, const char *path) /* Service pending secret requests. */ requests = g_hash_table_get_values (priv->requests); - for (iter = requests; iter; iter = g_list_next (iter)) { - NMSecretAgentSimpleRequest *request = iter->data; + for (iter = requests; iter; iter = g_list_next (iter)) + request_secrets_from_ui (iter->data); - if (g_str_has_prefix (request->request_id, priv->path)) { - request_secrets_from_ui (request); - } else { - /* We only handle requests for connection with @path if set. */ - error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, - "Request for %s secrets doesn't match path %s", - request->request_id, priv->path); - request->callback (NM_SECRET_AGENT_OLD (self), request->connection, NULL, error, request->callback_data); - g_hash_table_remove (priv->requests, request->request_id); - g_error_free (error); - } - } g_list_free (requests); } From b7b3f54f9861d46135559945c0f9e8e47efcb181 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 13 Sep 2016 14:54:08 +0200 Subject: [PATCH 5/5] clients: fix matching of connection path Since we use g_str_has_prefix() to match a request_id with the connection path, there can be wrong matches. For example: request_id: /org/freedesktop/NetworkManager/Settings/10/802-1x connection: /org/freedesktop/NetworkManager/Settings/1 would match. Add a trailing slash to the connection path stored in the agent to prevent this. (cherry picked from commit f666efed0de21343ad8b847bf2c7def0b3e2625b) --- clients/common/nm-secret-agent-simple.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 0c90eba36e..550fefa809 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -751,10 +751,18 @@ nm_secret_agent_simple_enable (NMSecretAgentSimple *self, const char *path) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); GList *requests, *iter; + gs_free char *path_full = NULL; - if (g_strcmp0 (path, priv->path) != 0) { + /* The path is only used to match a request_id with the current + * connection. Since the request_id is "${CONNECTION_PATH}/${SETTING}", + * add a trailing '/' to the path to match the full connection path. + */ + path_full = path ? g_strdup_printf ("%s/", path) : NULL; + + if (g_strcmp0 (path_full, priv->path) != 0) { g_free (priv->path); - priv->path = g_strdup (path); + priv->path = path_full; + path_full = NULL; } if (priv->enabled)