From f8d82c7f104d86611eeb41c8ba7a1db28ccaf815 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 10 May 2023 16:31:44 +0100 Subject: [PATCH 1/6] nmcli, nmtui: update authentication for OpenConnect Since OpenConnect 8.20, 'openconnect --authenticate' will return the full gateway URL, including the hostname and the path. This allows servers behind SNI-based proxies to work. To ensure we end up at the same IP address even behind round-robin DNS, there is a separate --resolve argument. Update nmcli/nmtui to use this, as NetworkManager-openconnect does. Shift some of the logic into the nm_vpn_openconnect_authenticate_helper() function instead of duplicating it in the callers. Also, pass the correct protocol in rather than only supporting Cisco AnyConnect. --- src/libnmc-base/nm-vpn-helpers.c | 74 ++++++++++++++++++++++++++------ src/libnmc-base/nm-vpn-helpers.h | 13 +++--- src/nmcli/common.c | 25 ++++++----- src/nmtui/nmtui-connect.c | 30 +++++++------ 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/src/libnmc-base/nm-vpn-helpers.c b/src/libnmc-base/nm-vpn-helpers.c index 476fbe518e..e447ef047a 100644 --- a/src/libnmc-base/nm-vpn-helpers.c +++ b/src/libnmc-base/nm-vpn-helpers.c @@ -160,9 +160,10 @@ nm_vpn_get_secret_names(const char *service_type) }; if (NM_IN_STRSET(type, "openconnect")) { - return _VPN_PASSWORD_LIST({"gateway", N_("Gateway")}, + return _VPN_PASSWORD_LIST({"gateway", N_("Gateway URL")}, {"cookie", N_("Cookie")}, - {"gwcert", N_("Gateway certificate hash")}, ); + {"gwcert", N_("Gateway certificate hash")}, + {"resolve", N_("Gateway DNS resolution ('host:IP')")}, ); }; return NULL; @@ -186,18 +187,24 @@ _extract_variable_value(char *line, const char *tag, char **value) return TRUE; } +#define OC_ARGS_MAX 10 + gboolean -nm_vpn_openconnect_authenticate_helper(const char *host, - char **cookie, - char **gateway, - char **gwcert, - int *status, - GError **error) +nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, + char **cookie, + char **gateway, + char **gwcert, + char **resolve, + int *status, + GError **error) { - gs_free char *output = NULL; - gs_free const char **output_v = NULL; + gs_free char *output = NULL; + gs_free char *legacy_host = NULL; + gs_free char *connect_url = NULL; + gs_free const char **output_v = NULL; const char *const *iter; const char *path; + const char *opt; const char *const DEFAULT_PATHS[] = { "/sbin/", "/usr/sbin/", @@ -207,6 +214,13 @@ nm_vpn_openconnect_authenticate_helper(const char *host, "/usr/local/bin/", NULL, }; + const char *gw, *port; + const char *oc_argv[OC_ARGS_MAX]; + int oc_argc = 0; + + /* Get gateway and port */ + gw = nm_setting_vpn_get_data_item(s_vpn, "gateway"); + port = gw ? strrchr(gw, ':') : NULL; path = nm_utils_file_search_in_paths("openconnect", "/usr/sbin/openconnect", @@ -218,8 +232,21 @@ nm_vpn_openconnect_authenticate_helper(const char *host, if (!path) return FALSE; + oc_argv[oc_argc++] = path; + oc_argv[oc_argc++] = "--authenticate"; + oc_argv[oc_argc++] = gw; + + opt = nm_setting_vpn_get_data_item(s_vpn, "protocol"); + if (opt) { + oc_argv[oc_argc++] = "--protocol"; + oc_argv[oc_argc++] = opt; + } + + oc_argv[oc_argc++] = NULL; + g_return_val_if_fail(oc_argc <= OC_ARGS_MAX, FALSE); + if (!g_spawn_sync(NULL, - (char **) NM_MAKE_STRV(path, "--authenticate", host), + (char **) oc_argv, NULL, G_SPAWN_SEARCH_PATH | G_SPAWN_CHILD_INHERITS_STDIN, NULL, @@ -235,14 +262,37 @@ nm_vpn_openconnect_authenticate_helper(const char *host, * COOKIE='loremipsum' * HOST='1.2.3.4' * FINGERPRINT='sha1:32bac90cf09a722e10ecc1942c67fe2ac8c21e2e' + * + * Since OpenConnect v8.20 (2022-02-20) OpenConnect has also passed e.g.: + * + * CONNECT_URL='https://vpn.example.com:8443/ConnectPath' + * RESOLVE=vpn.example.com:1.2.3.4 */ output_v = nm_strsplit_set_with_empty(output, "\r\n"); for (iter = output_v; iter && *iter; iter++) { char *s_mutable = (char *) *iter; _extract_variable_value(s_mutable, "COOKIE=", cookie); - _extract_variable_value(s_mutable, "HOST=", gateway); + _extract_variable_value(s_mutable, "CONNECT_URL=", &connect_url); + _extract_variable_value(s_mutable, "HOST=", &legacy_host); _extract_variable_value(s_mutable, "FINGERPRINT=", gwcert); + _extract_variable_value(s_mutable, "RESOLVE=", resolve); + } + + if (connect_url) { + *gateway = g_steal_pointer(&connect_url); + } else { + if (!legacy_host) { + g_set_error(error, + NM_VPN_PLUGIN_ERROR, + NM_VPN_PLUGIN_ERROR_FAILED, + _("OpenConnect failed to return gateway URL")); + return FALSE; + } + if (port) + *gateway = g_strdup_printf("%s%s", legacy_host, port); + else + *gateway = g_steal_pointer(&legacy_host); } return TRUE; diff --git a/src/libnmc-base/nm-vpn-helpers.h b/src/libnmc-base/nm-vpn-helpers.h index 8bea846025..f2bdace57d 100644 --- a/src/libnmc-base/nm-vpn-helpers.h +++ b/src/libnmc-base/nm-vpn-helpers.h @@ -19,11 +19,12 @@ gboolean nm_vpn_supports_ipv6(NMConnection *connection); const NmcVpnPasswordName *nm_vpn_get_secret_names(const char *service_type); -gboolean nm_vpn_openconnect_authenticate_helper(const char *host, - char **cookie, - char **gateway, - char **gwcert, - int *status, - GError **error); +gboolean nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, + char **cookie, + char **gateway, + char **gwcert, + char **resolve, + int *status, + GError **error); #endif /* __NM_VPN_HELPERS_H__ */ diff --git a/src/nmcli/common.c b/src/nmcli/common.c index f31d09872d..24ea1b1448 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -635,10 +635,10 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets) { GError *error = NULL; NMSettingVpn *s_vpn; - const char *gw, *port; gs_free char *cookie = NULL; gs_free char *gateway = NULL; gs_free char *gwcert = NULL; + gs_free char *resolve = NULL; int status = 0; int i; gboolean ret; @@ -653,12 +653,14 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets) if (!nm_streq0(nm_setting_vpn_get_service_type(s_vpn), NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT)) return FALSE; - /* Get gateway and port */ - gw = nm_setting_vpn_get_data_item(s_vpn, "gateway"); - port = gw ? strrchr(gw, ':') : NULL; - /* Interactively authenticate to OpenConnect server and get secrets */ - ret = nm_vpn_openconnect_authenticate_helper(gw, &cookie, &gateway, &gwcert, &status, &error); + ret = nm_vpn_openconnect_authenticate_helper(s_vpn, + &cookie, + &gateway, + &gwcert, + &resolve, + &status, + &error); if (!ret) { nmc_printerr(_("Error: openconnect failed: %s\n"), error->message); g_clear_error(&error); @@ -671,13 +673,6 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets) } else if (WIFSIGNALED(status)) nmc_printerr(_("Error: openconnect failed with signal %d\n"), WTERMSIG(status)); - /* Append port to the host value */ - if (gateway && port) { - gs_free char *tmp = gateway; - - gateway = g_strdup_printf("%s%s", tmp, port); - } - /* Fill secrets to the array */ for (i = 0; i < secrets->len; i++) { NMSecretAgentSimpleSecret *secret = secrets->pdata[i]; @@ -698,6 +693,10 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets) NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gwcert")) { g_free(secret->value); secret->value = g_steal_pointer(&gwcert); + } else if (nm_streq0(secret->entry_id, + NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) { + g_free(secret->value); + secret->value = g_steal_pointer(&resolve); } } diff --git a/src/nmtui/nmtui-connect.c b/src/nmtui/nmtui-connect.c index 8c4625ec6a..ba9fffcaec 100644 --- a/src/nmtui/nmtui-connect.c +++ b/src/nmtui/nmtui-connect.c @@ -31,25 +31,32 @@ * before starting the command and restored after it returns. */ static gboolean -openconnect_authenticate(NMConnection *connection, char **cookie, char **gateway, char **gwcert) +openconnect_authenticate(NMConnection *connection, + char **cookie, + char **gateway, + char **gwcert, + char **resolve) { GError *error = NULL; NMSettingVpn *s_vpn; gboolean ret; int status = 0; - const char *gw, *port; nmt_newt_message_dialog( _("openconnect will be run to authenticate.\nIt will return to nmtui when completed.")); /* Get port */ s_vpn = nm_connection_get_setting_vpn(connection); - gw = nm_setting_vpn_get_data_item(s_vpn, "gateway"); - port = gw ? strrchr(gw, ':') : NULL; newtSuspend(); - ret = nm_vpn_openconnect_authenticate_helper(gw, cookie, gateway, gwcert, &status, &error); + ret = nm_vpn_openconnect_authenticate_helper(s_vpn, + cookie, + gateway, + gwcert, + resolve, + &status, + &error); newtResume(); @@ -69,12 +76,6 @@ openconnect_authenticate(NMConnection *connection, char **cookie, char **gateway return FALSE; } - if (gateway && *gateway && port) { - char *tmp = *gateway; - *gateway = g_strdup_printf("%s%s", *gateway, port); - g_free(tmp); - } - return TRUE; } @@ -99,8 +100,9 @@ secrets_requested(NMSecretAgentSimple *agent, gs_free char *cookie = NULL; gs_free char *gateway = NULL; gs_free char *gwcert = NULL; + gs_free char *resolve = NULL; - openconnect_authenticate(connection, &cookie, &gateway, &gwcert); + openconnect_authenticate(connection, &cookie, &gateway, &gwcert, &resolve); for (i = 0; i < secrets->len; i++) { NMSecretAgentSimpleSecret *secret = secrets->pdata[i]; @@ -121,6 +123,10 @@ secrets_requested(NMSecretAgentSimple *agent, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gwcert")) { g_free(secret->value); secret->value = g_steal_pointer(&gwcert); + } else if (nm_streq0(secret->entry_id, + NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) { + g_free(secret->value); + secret->value = g_steal_pointer(&resolve); } } } From 97f2a368f154dc315ebf7b4107cbe2fc7ec60b4a Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 10 May 2023 19:43:11 +0100 Subject: [PATCH 2/6] libnmc-base: add supported options for OpenConnect CLI authentication Ideally, we wouldn't have this hard-coded in NetworkManager itself; we would invoke a tool to do it for us, like the GUI auth-dialog, which can live in the NetworkManager-openconnect repository and be kept up to date as new options are added. To start with though, let's bring it into sync. We don't add new options that often, and this will cover the majority of use cases. --- src/libnmc-base/nm-vpn-helpers.c | 90 ++++++++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 5 deletions(-) diff --git a/src/libnmc-base/nm-vpn-helpers.c b/src/libnmc-base/nm-vpn-helpers.c index e447ef047a..f7a65e3815 100644 --- a/src/libnmc-base/nm-vpn-helpers.c +++ b/src/libnmc-base/nm-vpn-helpers.c @@ -187,7 +187,50 @@ _extract_variable_value(char *line, const char *tag, char **value) return TRUE; } -#define OC_ARGS_MAX 10 +#define NM_OPENCONNECT_KEY_GATEWAY "gateway" +#define NM_OPENCONNECT_KEY_COOKIE "cookie" +#define NM_OPENCONNECT_KEY_GWCERT "gwcert" +#define NM_OPENCONNECT_KEY_RESOLVE "resolve" +#define NM_OPENCONNECT_KEY_AUTHTYPE "authtype" +#define NM_OPENCONNECT_KEY_USERCERT "usercert" +#define NM_OPENCONNECT_KEY_CACERT "cacert" +#define NM_OPENCONNECT_KEY_PRIVKEY "userkey" +#define NM_OPENCONNECT_KEY_KEY_PASS "key_pass" +#define NM_OPENCONNECT_KEY_MTU "mtu" +#define NM_OPENCONNECT_KEY_PEM_PASSPHRASE_FSID "pem_passphrase_fsid" +#define NM_OPENCONNECT_KEY_PREVENT_INVALID_CERT "prevent_invalid_cert" +#define NM_OPENCONNECT_KEY_DISABLE_UDP "disable_udp" +#define NM_OPENCONNECT_KEY_PROTOCOL "protocol" +#define NM_OPENCONNECT_KEY_PROXY "proxy" +#define NM_OPENCONNECT_KEY_CSD_ENABLE "enable_csd_trojan" +#define NM_OPENCONNECT_KEY_USERAGENT "useragent" +#define NM_OPENCONNECT_KEY_CSD_WRAPPER "csd_wrapper" +#define NM_OPENCONNECT_KEY_TOKEN_MODE "stoken_source" +#define NM_OPENCONNECT_KEY_TOKEN_SECRET "stoken_string" +#define NM_OPENCONNECT_KEY_REPORTED_OS "reported_os" +#define NM_OPENCONNECT_KEY_MCACERT "mcacert" +#define NM_OPENCONNECT_KEY_MCAKEY "mcakey" +#define NM_OPENCONNECT_KEY_MCA_PASS "mca_key_pass" + +struct { + const char *property; + const char *cmdline; +} oc_property_args[] = { + {NM_OPENCONNECT_KEY_USERCERT, "--certificate"}, + {NM_OPENCONNECT_KEY_CACERT, "--caflle"}, + {NM_OPENCONNECT_KEY_PRIVKEY, "--sslkey"}, + {NM_OPENCONNECT_KEY_KEY_PASS, "--key-password"}, + {NM_OPENCONNECT_KEY_PROTOCOL, "--protocol"}, + {NM_OPENCONNECT_KEY_PROXY, "--proxy"}, + {NM_OPENCONNECT_KEY_USERAGENT, "--useragent"}, + {NM_OPENCONNECT_KEY_REPORTED_OS, "--os"}, + {NM_OPENCONNECT_KEY_MCACERT, "--mca-certificate"}, + {NM_OPENCONNECT_KEY_MCAKEY, "--mca-key"}, + {NM_OPENCONNECT_KEY_MCA_PASS, "--mca-key-password"}, +}; + +#define NR_OC_STRING_PROPS (sizeof(oc_property_args) / sizeof(oc_property_args[0])) +#define OC_ARGS_MAX (12 + 2 * NR_OC_STRING_PROPS) gboolean nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, @@ -216,7 +259,7 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, }; const char *gw, *port; const char *oc_argv[OC_ARGS_MAX]; - int oc_argc = 0; + int i, oc_argc = 0; /* Get gateway and port */ gw = nm_setting_vpn_get_data_item(s_vpn, "gateway"); @@ -236,10 +279,47 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, oc_argv[oc_argc++] = "--authenticate"; oc_argv[oc_argc++] = gw; - opt = nm_setting_vpn_get_data_item(s_vpn, "protocol"); + for (i = 0; i < NR_OC_STRING_PROPS; i++) { + opt = nm_setting_vpn_get_data_item(s_vpn, oc_property_args[i].property); + if (opt) { + oc_argv[oc_argc++] = oc_property_args[i].cmdline; + oc_argv[oc_argc++] = opt; + } + } + + opt = nm_setting_vpn_get_data_item(s_vpn, NM_OPENCONNECT_KEY_PEM_PASSPHRASE_FSID); + if (opt && nm_streq(opt, "yes")) + oc_argv[oc_argc++] = "--key-password-from-fsid"; + + opt = nm_setting_vpn_get_data_item(s_vpn, NM_OPENCONNECT_KEY_CSD_ENABLE); + if (opt && nm_streq(opt, "yes")) { + opt = nm_setting_vpn_get_data_item(s_vpn, NM_OPENCONNECT_KEY_CSD_WRAPPER); + if (opt) { + oc_argv[oc_argc++] = "--csd-wrapper"; + oc_argv[oc_argc++] = opt; + } + } + + opt = nm_setting_vpn_get_data_item(s_vpn, NM_OPENCONNECT_KEY_TOKEN_MODE); if (opt) { - oc_argv[oc_argc++] = "--protocol"; - oc_argv[oc_argc++] = opt; + const char *token_secret = + nm_setting_vpn_get_data_item(s_vpn, NM_OPENCONNECT_KEY_TOKEN_SECRET); + if (nm_streq(opt, "manual") && token_secret) { + opt = "rsa"; + } else if (nm_streq(opt, "stokenrc")) { + opt = "rsa"; + token_secret = NULL; + } else if (!nm_streq(opt, "totp") && !nm_streq(opt, "hotp") && !nm_streq(opt, "yubioath")) { + opt = NULL; + } + if (opt) { + oc_argv[oc_argc++] = "--token-mode"; + oc_argv[oc_argc++] = opt; + } + if (token_secret) { + oc_argv[oc_argc++] = "--token-secret"; + oc_argv[oc_argc++] = token_secret; + } } oc_argv[oc_argc++] = NULL; From 715921a1fdef767dcb929dedf44959435151b571 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Thu, 11 May 2023 10:36:01 +0100 Subject: [PATCH 3/6] nmcli, nmtui: reduce duplication around openconnect auth helper Pull a bunch of stuff into nm_vpn_openconnect_authenticate_helper() that both callers were doing for themselves, and make its API a bit simpler. It's given the NMSettingVpn and the GPtrArray of secrets, and it simply succeeds or fails. --- src/libnmc-base/nm-vpn-helpers.c | 86 +++++++++++++++++++++++--------- src/libnmc-base/nm-vpn-helpers.h | 9 +--- src/nmcli/common.c | 48 +----------------- src/nmtui/nmtui-connect.c | 59 ++-------------------- 4 files changed, 69 insertions(+), 133 deletions(-) diff --git a/src/libnmc-base/nm-vpn-helpers.c b/src/libnmc-base/nm-vpn-helpers.c index f7a65e3815..1edc70d7dc 100644 --- a/src/libnmc-base/nm-vpn-helpers.c +++ b/src/libnmc-base/nm-vpn-helpers.c @@ -16,6 +16,7 @@ #include #include "nm-client-utils.h" +#include "nm-secret-agent-simple.h" #include "nm-utils.h" #include "libnm-glib-aux/nm-io-utils.h" #include "libnm-glib-aux/nm-secret-utils.h" @@ -233,18 +234,16 @@ struct { #define OC_ARGS_MAX (12 + 2 * NR_OC_STRING_PROPS) gboolean -nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, - char **cookie, - char **gateway, - char **gwcert, - char **resolve, - int *status, - GError **error) +nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, GError **error) { gs_free char *output = NULL; gs_free char *legacy_host = NULL; gs_free char *connect_url = NULL; + gs_free char *cookie = NULL; + gs_free char *gwcert = NULL; + gs_free char *resolve = NULL; gs_free const char **output_v = NULL; + int status = 0; const char *const *iter; const char *path; const char *opt; @@ -333,10 +332,27 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, NULL, &output, NULL, - status, + &status, error)) return FALSE; + if (WIFEXITED(status) && WEXITSTATUS(status) != 0) { + /* The caller will prepend "Error: openconnect failed: " to this */ + g_set_error(error, + NM_VPN_PLUGIN_ERROR, + NM_VPN_PLUGIN_ERROR_FAILED, + _("exited with status %d"), + WEXITSTATUS(status)); + return FALSE; + } else if (WIFSIGNALED(status)) { + g_set_error(error, + NM_VPN_PLUGIN_ERROR, + NM_VPN_PLUGIN_ERROR_FAILED, + _("exited on signal %d"), + WTERMSIG(status)); + return FALSE; + } + /* Parse output and set cookie, gateway and gwcert * output example: * COOKIE='loremipsum' @@ -352,27 +368,49 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, for (iter = output_v; iter && *iter; iter++) { char *s_mutable = (char *) *iter; - _extract_variable_value(s_mutable, "COOKIE=", cookie); + _extract_variable_value(s_mutable, "COOKIE=", &cookie); _extract_variable_value(s_mutable, "CONNECT_URL=", &connect_url); _extract_variable_value(s_mutable, "HOST=", &legacy_host); - _extract_variable_value(s_mutable, "FINGERPRINT=", gwcert); - _extract_variable_value(s_mutable, "RESOLVE=", resolve); + _extract_variable_value(s_mutable, "FINGERPRINT=", &gwcert); + _extract_variable_value(s_mutable, "RESOLVE=", &resolve); } - if (connect_url) { - *gateway = g_steal_pointer(&connect_url); - } else { - if (!legacy_host) { - g_set_error(error, - NM_VPN_PLUGIN_ERROR, - NM_VPN_PLUGIN_ERROR_FAILED, - _("OpenConnect failed to return gateway URL")); - return FALSE; + if (!cookie || !gwcert || (!legacy_host && !connect_url)) { + g_set_error(error, + NM_VPN_PLUGIN_ERROR, + NM_VPN_PLUGIN_ERROR_FAILED, + _("insufficent secrets returned")); + return FALSE; + } + + for (i = 0; i < secrets->len; i++) { + NMSecretAgentSimpleSecret *secret = secrets->pdata[i]; + + if (secret->secret_type != NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET) + continue; + if (!nm_streq0(secret->vpn_type, NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT)) + continue; + if (nm_streq0(secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "cookie")) { + g_free(secret->value); + secret->value = g_steal_pointer(&cookie); + } else if (nm_streq0(secret->entry_id, + NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gateway")) { + g_free(secret->value); + if (connect_url) + secret->value = g_steal_pointer(&connect_url); + else if (port) + secret->value = g_strdup_printf("%s%s", legacy_host, port); + else + secret->value = g_steal_pointer(&legacy_host); + } else if (nm_streq0(secret->entry_id, + NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gwcert")) { + g_free(secret->value); + secret->value = g_steal_pointer(&gwcert); + } else if (nm_streq0(secret->entry_id, + NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) { + g_free(secret->value); + secret->value = g_steal_pointer(&resolve); } - if (port) - *gateway = g_strdup_printf("%s%s", legacy_host, port); - else - *gateway = g_steal_pointer(&legacy_host); } return TRUE; diff --git a/src/libnmc-base/nm-vpn-helpers.h b/src/libnmc-base/nm-vpn-helpers.h index f2bdace57d..afd56590a0 100644 --- a/src/libnmc-base/nm-vpn-helpers.h +++ b/src/libnmc-base/nm-vpn-helpers.h @@ -19,12 +19,7 @@ gboolean nm_vpn_supports_ipv6(NMConnection *connection); const NmcVpnPasswordName *nm_vpn_get_secret_names(const char *service_type); -gboolean nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, - char **cookie, - char **gateway, - char **gwcert, - char **resolve, - int *status, - GError **error); +gboolean +nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, GError **error); #endif /* __NM_VPN_HELPERS_H__ */ diff --git a/src/nmcli/common.c b/src/nmcli/common.c index 24ea1b1448..fcf1ed81d0 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -635,12 +635,6 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets) { GError *error = NULL; NMSettingVpn *s_vpn; - gs_free char *cookie = NULL; - gs_free char *gateway = NULL; - gs_free char *gwcert = NULL; - gs_free char *resolve = NULL; - int status = 0; - int i; gboolean ret; if (!connection) @@ -654,52 +648,14 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets) return FALSE; /* Interactively authenticate to OpenConnect server and get secrets */ - ret = nm_vpn_openconnect_authenticate_helper(s_vpn, - &cookie, - &gateway, - &gwcert, - &resolve, - &status, - &error); + ret = nm_vpn_openconnect_authenticate_helper(s_vpn, secrets, &error); + if (!ret) { nmc_printerr(_("Error: openconnect failed: %s\n"), error->message); g_clear_error(&error); return FALSE; } - if (WIFEXITED(status)) { - if (WEXITSTATUS(status) != 0) - nmc_printerr(_("Error: openconnect failed with status %d\n"), WEXITSTATUS(status)); - } else if (WIFSIGNALED(status)) - nmc_printerr(_("Error: openconnect failed with signal %d\n"), WTERMSIG(status)); - - /* Fill secrets to the array */ - for (i = 0; i < secrets->len; i++) { - NMSecretAgentSimpleSecret *secret = secrets->pdata[i]; - - if (secret->secret_type != NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET) - continue; - if (!nm_streq0(secret->vpn_type, NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT)) - continue; - - if (nm_streq0(secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "cookie")) { - g_free(secret->value); - secret->value = g_steal_pointer(&cookie); - } else if (nm_streq0(secret->entry_id, - NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gateway")) { - g_free(secret->value); - secret->value = g_steal_pointer(&gateway); - } else if (nm_streq0(secret->entry_id, - NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gwcert")) { - g_free(secret->value); - secret->value = g_steal_pointer(&gwcert); - } else if (nm_streq0(secret->entry_id, - NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) { - g_free(secret->value); - secret->value = g_steal_pointer(&resolve); - } - } - return TRUE; } diff --git a/src/nmtui/nmtui-connect.c b/src/nmtui/nmtui-connect.c index ba9fffcaec..7369d6586b 100644 --- a/src/nmtui/nmtui-connect.c +++ b/src/nmtui/nmtui-connect.c @@ -31,16 +31,11 @@ * before starting the command and restored after it returns. */ static gboolean -openconnect_authenticate(NMConnection *connection, - char **cookie, - char **gateway, - char **gwcert, - char **resolve) +openconnect_authenticate(NMConnection *connection, GPtrArray *secrets) { GError *error = NULL; NMSettingVpn *s_vpn; gboolean ret; - int status = 0; nmt_newt_message_dialog( _("openconnect will be run to authenticate.\nIt will return to nmtui when completed.")); @@ -50,13 +45,7 @@ openconnect_authenticate(NMConnection *connection, newtSuspend(); - ret = nm_vpn_openconnect_authenticate_helper(s_vpn, - cookie, - gateway, - gwcert, - resolve, - &status, - &error); + ret = nm_vpn_openconnect_authenticate_helper(s_vpn, secrets, &error); newtResume(); @@ -66,16 +55,6 @@ openconnect_authenticate(NMConnection *connection, return FALSE; } - if (WIFEXITED(status)) { - if (WEXITSTATUS(status) != 0) { - nmt_newt_message_dialog(_("openconnect failed with status %d"), WEXITSTATUS(status)); - return FALSE; - } - } else if (WIFSIGNALED(status)) { - nmt_newt_message_dialog(_("openconnect failed with signal %d"), WTERMSIG(status)); - return FALSE; - } - return TRUE; } @@ -89,7 +68,6 @@ secrets_requested(NMSecretAgentSimple *agent, { NmtNewtForm *form; NMConnection *connection = NM_CONNECTION(user_data); - int i; /* Get secrets for OpenConnect VPN */ if (connection && nm_connection_is_type(connection, NM_SETTING_VPN_SETTING_NAME)) { @@ -97,38 +75,7 @@ secrets_requested(NMSecretAgentSimple *agent, if (nm_streq0(nm_setting_vpn_get_service_type(s_vpn), NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT)) { - gs_free char *cookie = NULL; - gs_free char *gateway = NULL; - gs_free char *gwcert = NULL; - gs_free char *resolve = NULL; - - openconnect_authenticate(connection, &cookie, &gateway, &gwcert, &resolve); - - for (i = 0; i < secrets->len; i++) { - NMSecretAgentSimpleSecret *secret = secrets->pdata[i]; - - if (secret->secret_type != NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET) - continue; - if (!nm_streq0(secret->vpn_type, NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT)) - continue; - if (nm_streq0(secret->entry_id, - NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "cookie")) { - g_free(secret->value); - secret->value = g_steal_pointer(&cookie); - } else if (nm_streq0(secret->entry_id, - NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gateway")) { - g_free(secret->value); - secret->value = g_steal_pointer(&gateway); - } else if (nm_streq0(secret->entry_id, - NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gwcert")) { - g_free(secret->value); - secret->value = g_steal_pointer(&gwcert); - } else if (nm_streq0(secret->entry_id, - NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) { - g_free(secret->value); - secret->value = g_steal_pointer(&resolve); - } - } + openconnect_authenticate(connection, secrets); } } From db7ea2e5d41b7bf51eedb1fd481c3a5072953829 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Thu, 11 May 2023 10:49:01 +0100 Subject: [PATCH 4/6] nmtui: do not prompt for secrets if openconnect already provided them While we're at it, kill the separate openconnect_authenticate() function since it barely does anything any more and it wants visibility to both 's_vpn' and 'success' variables in the caller. --- src/nmtui/nmtui-connect.c | 63 ++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 38 deletions(-) diff --git a/src/nmtui/nmtui-connect.c b/src/nmtui/nmtui-connect.c index 7369d6586b..75862bbd90 100644 --- a/src/nmtui/nmtui-connect.c +++ b/src/nmtui/nmtui-connect.c @@ -26,38 +26,6 @@ #include "libnmc-base/nm-client-utils.h" #include "nmt-utils.h" -/** - * Runs openconnect to authenticate. The current screen state is saved - * before starting the command and restored after it returns. - */ -static gboolean -openconnect_authenticate(NMConnection *connection, GPtrArray *secrets) -{ - GError *error = NULL; - NMSettingVpn *s_vpn; - gboolean ret; - - nmt_newt_message_dialog( - _("openconnect will be run to authenticate.\nIt will return to nmtui when completed.")); - - /* Get port */ - s_vpn = nm_connection_get_setting_vpn(connection); - - newtSuspend(); - - ret = nm_vpn_openconnect_authenticate_helper(s_vpn, secrets, &error); - - newtResume(); - - if (!ret) { - nmt_newt_message_dialog(_("Error: openconnect failed: %s"), error->message); - g_clear_error(&error); - return FALSE; - } - - return TRUE; -} - static void secrets_requested(NMSecretAgentSimple *agent, const char *request_id, @@ -68,6 +36,7 @@ secrets_requested(NMSecretAgentSimple *agent, { NmtNewtForm *form; NMConnection *connection = NM_CONNECTION(user_data); + gboolean success = FALSE; /* Get secrets for OpenConnect VPN */ if (connection && nm_connection_is_type(connection, NM_SETTING_VPN_SETTING_NAME)) { @@ -75,19 +44,37 @@ secrets_requested(NMSecretAgentSimple *agent, if (nm_streq0(nm_setting_vpn_get_service_type(s_vpn), NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT)) { - openconnect_authenticate(connection, secrets); + GError *error = NULL; + + nmt_newt_message_dialog(_("openconnect will be run to authenticate.\nIt will return to " + "nmtui when completed.")); + + newtSuspend(); + + success = nm_vpn_openconnect_authenticate_helper(s_vpn, secrets, &error); + + newtResume(); + + if (!success) { + nmt_newt_message_dialog(_("Error: openconnect failed: %s"), error->message); + g_clear_error(&error); + } } } - form = nmt_password_dialog_new(request_id, title, msg, secrets); - nmt_newt_form_run_sync(form); + if (!success) { + form = nmt_password_dialog_new(request_id, title, msg, secrets); + nmt_newt_form_run_sync(form); - if (nmt_password_dialog_succeeded(NMT_PASSWORD_DIALOG(form))) + success = nmt_password_dialog_succeeded(NMT_PASSWORD_DIALOG(form)); + + g_object_unref(form); + } + + if (success) nm_secret_agent_simple_response(agent, request_id, secrets); else nm_secret_agent_simple_response(agent, request_id, NULL); - - g_object_unref(form); } typedef struct { From f791b98284c3b6037e74b7e487aa02e6ad38a1cf Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Thu, 11 May 2023 12:17:43 +0100 Subject: [PATCH 5/6] libnmc-base: report explicit error if not gateway configured for openconnect Rather than letting openconnect run, and whine that there's no gateway, and making the user scroll up past the openconnect usage information, give them an explicit error. --- src/libnmc-base/nm-vpn-helpers.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libnmc-base/nm-vpn-helpers.c b/src/libnmc-base/nm-vpn-helpers.c index 1edc70d7dc..1a43fbc1a0 100644 --- a/src/libnmc-base/nm-vpn-helpers.c +++ b/src/libnmc-base/nm-vpn-helpers.c @@ -261,8 +261,16 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, int i, oc_argc = 0; /* Get gateway and port */ - gw = nm_setting_vpn_get_data_item(s_vpn, "gateway"); - port = gw ? strrchr(gw, ':') : NULL; + gw = nm_setting_vpn_get_data_item(s_vpn, "gateway"); + if (!gw) { + g_set_error(error, + NM_VPN_PLUGIN_ERROR, + NM_VPN_PLUGIN_ERROR_FAILED, + _("no gateway configured")); + return FALSE; + } + + port = strrchr(gw, ':'); path = nm_utils_file_search_in_paths("openconnect", "/usr/sbin/openconnect", From ddce34054e5145c88367da42ce1e0e73fa00a10b Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Thu, 11 May 2023 12:34:43 +0100 Subject: [PATCH 6/6] libnmc-base: fix port extraction for openconnect auth With old versions of openconnect we need to extract the port# from the initial URL and then append it to the hostname we eventually get back. Using strrchr(gw, ':') isn't going to work right with IPv6 literals, ad we should also be dropping any path element. So switch to using an int for the port instead of a string, and import a cut-down variant of openconnect's internal_parse_url() which does *largely* the same thing with strrchr() but is saved by using the 'end' value returned from strtol() and insisting that the port is the very end of the host part of the URL. --- src/libnmc-base/nm-vpn-helpers.c | 47 ++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/libnmc-base/nm-vpn-helpers.c b/src/libnmc-base/nm-vpn-helpers.c index 1a43fbc1a0..10e2e0e696 100644 --- a/src/libnmc-base/nm-vpn-helpers.c +++ b/src/libnmc-base/nm-vpn-helpers.c @@ -233,6 +233,46 @@ struct { #define NR_OC_STRING_PROPS (sizeof(oc_property_args) / sizeof(oc_property_args[0])) #define OC_ARGS_MAX (12 + 2 * NR_OC_STRING_PROPS) +/* + * For old versions of openconnect we need to extract the port# and + * append it to the hostname that is returned to us. Use a cut-down + * version of openconnect's own internal_parse_url() function. + */ +static int +extract_url_port(const char *url) +{ + const char *host, *port_str, *path; + char *end; + int port_nr; + + /* Skip the scheme, if present */ + host = strstr(url, "://"); + if (host) + host += 3; + else + host = url; + + port_str = strrchr(host, ':'); + if (!port_str) + return 0; + + /* + * If the host is an IPv6 literal, port_str may point somewhere + * inside it rather than to an actual port#. But IPv6 literals + * are always enclosed in [], e.g. '[fec0::1]:443'. So we check + * that the end pointer returned by strtol points exactly to the + * end of the hostname (either the end of the string, or to the + * first '/' of the path element if there is one). + */ + path = strchr(host, '/'); + port_nr = strtol(port_str + 1, &end, 10); + + if (end == path || (!path && !*end)) + return port_nr; + + return 0; +} + gboolean nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, GError **error) { @@ -256,7 +296,8 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, "/usr/local/bin/", NULL, }; - const char *gw, *port; + int port = 0; + const char *gw; const char *oc_argv[OC_ARGS_MAX]; int i, oc_argc = 0; @@ -270,7 +311,7 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, return FALSE; } - port = strrchr(gw, ':'); + port = extract_url_port(gw); path = nm_utils_file_search_in_paths("openconnect", "/usr/sbin/openconnect", @@ -407,7 +448,7 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, if (connect_url) secret->value = g_steal_pointer(&connect_url); else if (port) - secret->value = g_strdup_printf("%s%s", legacy_host, port); + secret->value = g_strdup_printf("%s:%d", legacy_host, port); else secret->value = g_steal_pointer(&legacy_host); } else if (nm_streq0(secret->entry_id,