From 09a015b23b38438bf0f803a03eb2a79caf76b178 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 09:40:44 +0200 Subject: [PATCH 1/5] nmcli-completion: complete filename for VPN import and passwd-file Also hard-code the VPN types strongswan and fortisslvpn. https://bugzilla.redhat.com/show_bug.cgi?id=1337300 --- clients/cli/nmcli-completion | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/clients/cli/nmcli-completion b/clients/cli/nmcli-completion index 3c87f6f9d..ffab7d8b9 100644 --- a/clients/cli/nmcli-completion +++ b/clients/cli/nmcli-completion @@ -494,7 +494,7 @@ _nmcli_compl_ARGS() ;; vpn-type) if [[ "${#words[@]}" -eq 2 ]]; then - _nmcli_list "vpnc openvpn pptp openconnect openswan libreswan ssh l2tp iodine" + _nmcli_list "vpnc openvpn pptp openconnect openswan libreswan strongswan ssh l2tp iodine fortisslvpn" return 0 fi ;; @@ -579,10 +579,16 @@ _nmcli_compl_ARGS() user| \ username| \ service| \ - password| \ + password) + if [[ "${#words[@]}" -eq 2 ]]; then + return 0 + fi + ;; passwd-file| \ file) if [[ "${#words[@]}" -eq 2 ]]; then + compopt -o default + COMPREPLY=() return 0 fi ;; @@ -1379,8 +1385,7 @@ _nmcli() OPTIONS=(type file) OPTIONS_MANDATORY=(type file) - ALIASES=("type:vpn-type") - _nmcli_compl_ARGS ${ALIASES[@]} + _nmcli_compl_ARGS type:vpn-type return 0 fi ;; From 0225c4567b8fcfb20b47423652c2230029cc97b7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 10:06:27 +0200 Subject: [PATCH 2/5] clients: lookup VPN plugins either by "name" or "service" ... not constructing a "service" by prepending a D-Bus prefix to "name" (urgh). --- clients/cli/connections.c | 4 ++-- clients/common/nm-vpn-helpers.c | 19 +++++++++---------- clients/common/nm-vpn-helpers.h | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 2db2fcb69..2e3a8fcb1 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -10690,7 +10690,7 @@ do_connection_import (NmCli *nmc, gboolean temporary, int argc, char **argv) } /* Import VPN configuration */ - plugin = nm_vpn_get_plugin_by_service (type, &error); + plugin = nm_vpn_lookup_plugin (type, NULL, &error); if (!plugin) { g_string_printf (nmc->return_text, _("Error: failed to load VPN plugin: %s."), error->message); @@ -10797,7 +10797,7 @@ do_connection_export (NmCli *nmc, int argc, char **argv) type = nm_setting_vpn_get_service_type (nm_connection_get_setting_vpn (connection)); /* Export VPN configuration */ - plugin = nm_vpn_get_plugin_by_service (type, &error); + plugin = nm_vpn_lookup_plugin (type, NULL, &error); if (!plugin) { g_string_printf (nmc->return_text, _("Error: failed to load VPN plugin: %s."), error->message); diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c index 6671c1b33..086272c42 100644 --- a/clients/common/nm-vpn-helpers.c +++ b/clients/common/nm-vpn-helpers.c @@ -35,30 +35,29 @@ static gboolean plugins_loaded; static GSList *plugins = NULL; NMVpnEditorPlugin * -nm_vpn_get_plugin_by_service (const char *service, GError **error) +nm_vpn_lookup_plugin (const char *name, const char *service, GError **error) { NMVpnEditorPlugin *plugin = NULL; NMVpnPluginInfo *plugin_info; - char *type = NULL; - g_return_val_if_fail (service != NULL, NULL); + g_return_val_if_fail (!service ^ !name, NULL); g_return_val_if_fail (error == NULL || *error == NULL, NULL); if (G_UNLIKELY (!plugins_loaded)) nm_vpn_get_plugins (); - if (!g_str_has_prefix (service, NM_DBUS_INTERFACE)) - service = type = g_strdup_printf ("%s.%s", NM_DBUS_INTERFACE, service); + if (service) + plugin_info = nm_vpn_plugin_info_list_find_by_service (plugins, service); + else + plugin_info = nm_vpn_plugin_info_list_find_by_name (plugins, name); - plugin_info = nm_vpn_plugin_info_list_find_by_service (plugins, service); if (plugin_info) { plugin = nm_vpn_plugin_info_get_editor_plugin (plugin_info); if (!plugin) plugin = nm_vpn_plugin_info_load_editor_plugin (plugin_info, error); } else - g_set_error_literal (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_FAILED, - _("could not get VPN plugin info")); - g_free (type); + g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_FAILED, + _("unknown VPN plugin \"%s\""), service ?: name); return plugin; } @@ -86,7 +85,7 @@ nm_vpn_supports_ipv6 (NMConnection *connection) service_type = nm_setting_vpn_get_service_type (s_vpn); g_return_val_if_fail (service_type != NULL, FALSE); - plugin = nm_vpn_get_plugin_by_service (service_type, NULL); + plugin = nm_vpn_lookup_plugin (NULL, service_type, NULL); g_return_val_if_fail (plugin != NULL, FALSE); capabilities = nm_vpn_editor_plugin_get_capabilities (plugin); diff --git a/clients/common/nm-vpn-helpers.h b/clients/common/nm-vpn-helpers.h index 4f8d21944..f882eef06 100644 --- a/clients/common/nm-vpn-helpers.h +++ b/clients/common/nm-vpn-helpers.h @@ -30,7 +30,7 @@ struct { GSList *nm_vpn_get_plugins (void); -NMVpnEditorPlugin *nm_vpn_get_plugin_by_service (const char *service, GError **error); +NMVpnEditorPlugin *nm_vpn_lookup_plugin (const char *name, const char *service, GError **error); gboolean nm_vpn_supports_ipv6 (NMConnection *connection); From 2b4b9d34e4df4ea18d8ac36d32db74af2bb85e82 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 10:08:47 +0200 Subject: [PATCH 3/5] clients: don't assert against existance of plugin in nm_vpn_supports_ipv6() Obviously, loading a plugin can fail easily. --- clients/common/nm-vpn-helpers.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c index 086272c42..a9166ca5b 100644 --- a/clients/common/nm-vpn-helpers.c +++ b/clients/common/nm-vpn-helpers.c @@ -83,10 +83,12 @@ nm_vpn_supports_ipv6 (NMConnection *connection) g_return_val_if_fail (s_vpn != NULL, FALSE); service_type = nm_setting_vpn_get_service_type (s_vpn); - g_return_val_if_fail (service_type != NULL, FALSE); + if (!service_type) + return FALSE; plugin = nm_vpn_lookup_plugin (NULL, service_type, NULL); - g_return_val_if_fail (plugin != NULL, FALSE); + if (!plugin) + return FALSE; capabilities = nm_vpn_editor_plugin_get_capabilities (plugin); return NM_FLAGS_HAS (capabilities, NM_VPN_EDITOR_PLUGIN_CAPABILITY_IPV6); From 41976e30690d36cc3998c5025ac70c8cbaa8f897 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 10:51:58 +0200 Subject: [PATCH 4/5] clients,cli: for connection-add consider VPNs as loaded from the plugin Instead of using (only) a hard-coded list of VPN types, prefer lookup the VPN settings from the .name files. Still, fallback to a hard-coded list if the plugin cannot be found, because for connection-add we currently don't actually need the plugin installed. --- clients/cli/connections.c | 33 ++++++--------- clients/common/nm-vpn-helpers.c | 72 +++++++++++++++++++++++++++++++++ clients/common/nm-vpn-helpers.h | 5 +++ 3 files changed, 90 insertions(+), 20 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 2e3a8fcb1..9136d9d6b 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -53,21 +53,6 @@ #define PROMPT_IP_TUNNEL_MODE _("Tunnel mode: ") #define PROMPT_MACVLAN_MODE _("MACVLAN mode: ") -static const char *nmc_known_vpns[] = { - "openvpn", - "vpnc", - "pptp", - "openconnect", - "openswan", - "libreswan", - "strongswan", - "ssh", - "l2tp", - "iodine", - "fortisslvpn", - NULL -}; - /* Available fields for 'connection show' */ static NmcOutputField nmc_fields_con_show[] = { {"NAME", N_("NAME")}, /* 0 */ @@ -5904,10 +5889,12 @@ cleanup_bridge_slave: const char *user_c = NULL; char *user = NULL; const char *st; - char *service_type = NULL; + gs_free char *service_type_free = NULL; + const char *service_type = NULL; nmc_arg_t exp_args[] = { {"vpn-type", TRUE, &vpn_type, !ask}, {"user", TRUE, &user_c, FALSE}, {NULL} }; + gs_free const char **plugin_names = NULL; if (!nmc_parse_args (exp_args, FALSE, &argc, &argv, error)) return FALSE; @@ -5922,11 +5909,15 @@ cleanup_bridge_slave: if (vpn_type_ask) vpn_type = g_strstrip (vpn_type_ask); - if (!(st = nmc_string_is_valid (vpn_type, nmc_known_vpns, NULL))) { + plugin_names = nm_vpn_get_plugin_names (FALSE); + if (!(st = nmc_string_is_valid (vpn_type, plugin_names, NULL))) { g_print (_("Warning: 'vpn-type': %s not known.\n"), vpn_type); st = vpn_type; } - service_type = g_strdup_printf ("%s.%s", NM_DBUS_INTERFACE, st); + + service_type = nm_vpn_get_service_for_name (st); + if (!service_type) + service_type = service_type_free = nm_vpn_get_service_for_name_default (st); /* Also ask for all optional arguments if '--ask' is specified. */ user = g_strdup (user_c); @@ -5943,7 +5934,6 @@ cleanup_bridge_slave: success = TRUE; cleanup_vpn: g_free (vpn_type_ask); - g_free (service_type); g_free (user); if (!success) return FALSE; @@ -6711,7 +6701,10 @@ update_connection (gboolean persistent, static char * gen_func_vpn_types (const char *text, int state) { - return nmc_rl_gen_func_basic (text, state, nmc_known_vpns); + gs_free const char **plugin_names = NULL; + + plugin_names = nm_vpn_get_plugin_names (FALSE); + return nmc_rl_gen_func_basic (text, state, plugin_names); } static char * diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c index a9166ca5b..5b71c578a 100644 --- a/clients/common/nm-vpn-helpers.c +++ b/clients/common/nm-vpn-helpers.c @@ -71,6 +71,78 @@ nm_vpn_get_plugins (void) return plugins; } +static int +_strcmp_data (gconstpointer a, gconstpointer b, gpointer unused) +{ + return strcmp (a, b); +} + +const char ** +nm_vpn_get_plugin_names (gboolean only_available_plugins) +{ + GSList *p; + const char **list; + const char *known_names[] = { + "openvpn", + "vpnc", + "pptp", + "openconnect", + "openswan", + "libreswan", + "strongswan", + "ssh", + "l2tp", + "iodine", + "fortisslvpn", + }; + guint i, j, k; + + p = nm_vpn_get_plugins (); + list = g_new0 (const char *, g_slist_length (p) + G_N_ELEMENTS (known_names) + 1); + + i = 0; + for (i = 0; p; p = p->next) + list[i++] = nm_vpn_plugin_info_get_name (p->data); + if (!only_available_plugins) { + for (j = 0; j < G_N_ELEMENTS (known_names); j++) + list[i++] = known_names[j]; + } + + g_qsort_with_data (list, i, sizeof (gpointer), _strcmp_data, NULL); + + /* remove duplicates */ + for (k = 0, j = 1; j < i; j++) { + if (nm_streq (list[k], list[j])) + continue; + list[k++] = list[j]; + } + list[k++] = NULL; + + return list; +} + +const char * +nm_vpn_get_service_for_name (const char *name) +{ + NMVpnPluginInfo *plugin_info; + + g_return_val_if_fail (name, NULL); + + plugin_info = nm_vpn_plugin_info_list_find_by_name (nm_vpn_get_plugins (), name); + if (plugin_info) { + /* this only means we have a .name file (NMVpnPluginInfo). Possibly the + * NMVpnEditorPlugin is not loadable. */ + return nm_vpn_plugin_info_get_service (plugin_info); + } + return NULL; +} + +char * +nm_vpn_get_service_for_name_default (const char *name) +{ + return g_strdup_printf ("%s.%s", NM_DBUS_INTERFACE, name); +} + gboolean nm_vpn_supports_ipv6 (NMConnection *connection) { diff --git a/clients/common/nm-vpn-helpers.h b/clients/common/nm-vpn-helpers.h index f882eef06..79fc94e95 100644 --- a/clients/common/nm-vpn-helpers.h +++ b/clients/common/nm-vpn-helpers.h @@ -30,6 +30,11 @@ struct { GSList *nm_vpn_get_plugins (void); +const char **nm_vpn_get_plugin_names (gboolean only_available_plugins); + +const char *nm_vpn_get_service_for_name (const char *name); +char * nm_vpn_get_service_for_name_default (const char *name); + NMVpnEditorPlugin *nm_vpn_lookup_plugin (const char *name, const char *service, GError **error); gboolean nm_vpn_supports_ipv6 (NMConnection *connection); From d0f01aa2c26f7a3e34d80477ccc4989b1571a6d4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 11:17:46 +0200 Subject: [PATCH 5/5] clients,cli: show better error message when failing to load VPN plugin VPN plugins are often not installed or they might be legacy-only. In both cases we should show a better error message about the failure reason. --- clients/common/nm-vpn-helpers.c | 34 ++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c index 5b71c578a..43ac4b2e6 100644 --- a/clients/common/nm-vpn-helpers.c +++ b/clients/common/nm-vpn-helpers.c @@ -39,6 +39,7 @@ nm_vpn_lookup_plugin (const char *name, const char *service, GError **error) { NMVpnEditorPlugin *plugin = NULL; NMVpnPluginInfo *plugin_info; + gs_free_error GError *local = NULL; g_return_val_if_fail (!service ^ !name, NULL); g_return_val_if_fail (error == NULL || *error == NULL, NULL); @@ -51,13 +52,36 @@ nm_vpn_lookup_plugin (const char *name, const char *service, GError **error) else plugin_info = nm_vpn_plugin_info_list_find_by_name (plugins, name); - if (plugin_info) { - plugin = nm_vpn_plugin_info_get_editor_plugin (plugin_info); - if (!plugin) - plugin = nm_vpn_plugin_info_load_editor_plugin (plugin_info, error); - } else + if (!plugin_info) { g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_FAILED, _("unknown VPN plugin \"%s\""), service ?: name); + return NULL; + } + plugin = nm_vpn_plugin_info_get_editor_plugin (plugin_info); + if (!plugin) + plugin = nm_vpn_plugin_info_load_editor_plugin (plugin_info, &local); + + if (!plugin) { + if ( !nm_vpn_plugin_info_get_plugin (plugin_info) + && nm_vpn_plugin_info_lookup_property (plugin_info, NM_VPN_PLUGIN_INFO_KF_GROUP_GNOME, "properties")) { + g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_FAILED, + _("cannot cannot load legacy-only VPN plugin \"%s\" for \"%s\""), + nm_vpn_plugin_info_get_name (plugin_info), + nm_vpn_plugin_info_get_filename (plugin_info)); + } else if (g_error_matches (local, G_FILE_ERROR, G_FILE_ERROR_NOENT)) { + g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_FAILED, + _("cannot load VPN plugin \"%s\" due to missing \"%s\". Missing client plugin?"), + nm_vpn_plugin_info_get_name (plugin_info), + nm_vpn_plugin_info_get_plugin (plugin_info)); + } else { + g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_FAILED, + _("failed to load VPN plugin \"%s\": %s"), + nm_vpn_plugin_info_get_name (plugin_info), + local->message); + } + return NULL; + } + return plugin; }