From 27c701ebfbc95ab9330df128fa6c7a05a4711e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Mon, 22 Jan 2024 16:16:16 +0100 Subject: [PATCH] libnmc: allow user input in ECHO mode for 2FA challenges Depending on the type of challenge used in the 2FA authentication, the user input doesn't need to be hidden and sometimes it's even undesired (it makes more difficult to enter the text). Allow to VPN plugins to indicate that a secret that is being requested is a 2FA challenge with ECHO mode enabled: - When using auth dialog: accept a new option "ForceEcho" that can be set to TRUE to enable ECHO. - When using the fallback method: recognize the prefix "x-dynamic-challenge-echo". This indicate both that ECHO should be enabled and that this is a 2FA challenge (see previous commit). The correct way to enable echo mode from VPN plugins is doing both things: pass the hint prefixed with "x-dynamic-challenge-echo" and add the option "ForceEcho=true" for the auth dialog. An attempt to support ECHO mode from NM-openvpn was made by passing "IsSecret=false", but it didn't work because nm-secret-agent-simple ignores returned values for which "IsSecret=false". It's not a good idea to start accepting them because we could break other plugins, and anyway the challenge response is actually a secret, so it is better to keep it as such and add this new "ForceEcho" option. This is backwards compatible because existing plugins were not using the tag nor the auth dialog option. Withouth them, the previous behaviour is preserved. On the contrary, plugins that want to use this new feature will need to bump their NM version dependency because old daemons will not handle correctly the prefix tag. Secret agents will need to be updated to check secret->force_echo if they want to support this feature. Until they update, the only drawback is that ECHO mode will be ignored and the user's input will be hidden. Updated nmcli and nmtui to support ECHO mode. (cherry picked from commit 2ab56e82d42995ae851d374b96384bb27f8d8e5f) --- src/libnm-core-public/nm-dbus-interface.h | 7 ++-- src/libnmc-base/nm-secret-agent-simple.c | 41 +++++++++++++++++------ src/libnmc-base/nm-secret-agent-simple.h | 1 + src/nmcli/common.c | 2 +- src/nmtui/nmt-password-dialog.c | 2 +- 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/libnm-core-public/nm-dbus-interface.h b/src/libnm-core-public/nm-dbus-interface.h index 5467a99601..cc87c74474 100644 --- a/src/libnm-core-public/nm-dbus-interface.h +++ b/src/libnm-core-public/nm-dbus-interface.h @@ -1420,13 +1420,16 @@ typedef enum /*< flags >*/ { */ #define NM_SECRET_TAG_VPN_MSG "x-vpn-message:" -/* For secrets requests, hints starting with "x-dynamic-challenge:" are dynamic +/* For secrets requests, hints starting with "x-dynamic-challenge(-echo):" are dynamic * 2FA challenges that are requested in a second authentication step, after the password * (or whatever auth method is used) was already successfully validated. Because of * that, the default secrets of the service mustn't be requested (again). + * When using the "-echo" variant, the user input doesn't need to be hidden even + * without --show-secrets * * Note: currently only implemented for VPN, but can be extended. */ -#define NM_SECRET_TAG_DYNAMIC_CHALLENGE "x-dynamic-challenge:" +#define NM_SECRET_TAG_DYNAMIC_CHALLENGE "x-dynamic-challenge:" +#define NM_SECRET_TAG_DYNAMIC_CHALLENGE_ECHO "x-dynamic-challenge-echo:" #endif /* __NM_DBUS_INTERFACE_H__ */ diff --git a/src/libnmc-base/nm-secret-agent-simple.c b/src/libnmc-base/nm-secret-agent-simple.c index 02f492bc7c..4bb77c9802 100644 --- a/src/libnmc-base/nm-secret-agent-simple.c +++ b/src/libnmc-base/nm-secret-agent-simple.c @@ -170,6 +170,7 @@ _secret_real_new_plain(NMSecretAgentSecretType secret_type, .base.entry_id = g_strdup_printf("%s.%s", nm_setting_get_name(setting), property), .base.value = g_steal_pointer(&value), .base.is_secret = (secret_type != NM_SECRET_AGENT_SECRET_TYPE_PROPERTY), + .base.force_echo = FALSE, .setting = g_object_ref(setting), .property = g_strdup(property), }; @@ -180,7 +181,8 @@ static NMSecretAgentSimpleSecret * _secret_real_new_vpn_secret(const char *pretty_name, NMSetting *setting, const char *property, - const char *vpn_type) + const char *vpn_type, + gboolean force_echo) { SecretReal *real; const char *value; @@ -197,11 +199,12 @@ _secret_real_new_vpn_secret(const char *pretty_name, .base.pretty_name = g_strdup(pretty_name), .base.entry_id = g_strdup_printf("%s%s", NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS, property), - .base.value = g_strdup(value), - .base.is_secret = TRUE, - .base.vpn_type = g_strdup(vpn_type), - .setting = g_object_ref(setting), - .property = g_strdup(property), + .base.value = g_strdup(value), + .base.is_secret = TRUE, + .base.force_echo = force_echo, + .base.vpn_type = g_strdup(vpn_type), + .setting = g_object_ref(setting), + .property = g_strdup(property), }; return &real->base; } @@ -227,6 +230,7 @@ _secret_real_new_wireguard_peer_psk(NMSettingWireGuard *s_wg, .base.value = g_strdup(preshared_key), .base.is_secret = TRUE, .base.no_prompt_entry_id = TRUE, + .base.force_echo = FALSE, .setting = NM_SETTING(g_object_ref(s_wg)), .property = g_strdup(public_key), }; @@ -388,7 +392,8 @@ static void add_vpn_secret_helper(GPtrArray *secrets, NMSettingVpn *s_vpn, const char *name, - const char *ui_name) + const char *ui_name, + gboolean force_echo) { NMSecretAgentSimpleSecret *secret; NMSettingSecretFlags flags; @@ -399,7 +404,8 @@ add_vpn_secret_helper(GPtrArray *secrets, secret = _secret_real_new_vpn_secret(ui_name, NM_SETTING(s_vpn), name, - nm_setting_vpn_get_service_type(s_vpn)); + nm_setting_vpn_get_service_type(s_vpn), + force_echo); /* Check for duplicates */ for (i = 0; i < secrets->len; i++) { @@ -408,6 +414,8 @@ add_vpn_secret_helper(GPtrArray *secrets, if (s->secret_type == secret->secret_type && nm_streq0(s->vpn_type, secret->vpn_type) && nm_streq0(s->entry_id, secret->entry_id)) { _secret_real_free(secret); + if (!force_echo) + s->force_echo = FALSE; return; } } @@ -425,6 +433,7 @@ add_vpn_secrets(RequestData *request, GPtrArray *secrets, char **msg) char **iter; char *secret_name; bool is_challenge = FALSE; + bool force_echo; /* If hints are given, then always ask for what the hints require */ if (request->hints) { @@ -435,11 +444,17 @@ add_vpn_secrets(RequestData *request, GPtrArray *secrets, char **msg) if (NM_STR_HAS_PREFIX(*iter, NM_SECRET_TAG_DYNAMIC_CHALLENGE)) { secret_name = &(*iter)[NM_STRLEN(NM_SECRET_TAG_DYNAMIC_CHALLENGE)]; is_challenge = TRUE; + force_echo = FALSE; + } else if (NM_STR_HAS_PREFIX(*iter, NM_SECRET_TAG_DYNAMIC_CHALLENGE_ECHO)) { + secret_name = &(*iter)[NM_STRLEN(NM_SECRET_TAG_DYNAMIC_CHALLENGE_ECHO)]; + is_challenge = TRUE; + force_echo = TRUE; } else { secret_name = *iter; + force_echo = FALSE; } - add_vpn_secret_helper(secrets, s_vpn, secret_name, secret_name); + add_vpn_secret_helper(secrets, s_vpn, secret_name, secret_name, force_echo); } } } @@ -453,7 +468,7 @@ add_vpn_secrets(RequestData *request, GPtrArray *secrets, char **msg) /* Now add what client thinks might be required, because hints may be empty or incomplete */ p = nm_vpn_get_secret_names(nm_setting_vpn_get_service_type(s_vpn)); while (p && p->name) { - add_vpn_secret_helper(secrets, s_vpn, p->name, _(p->ui_name)); + add_vpn_secret_helper(secrets, s_vpn, p->name, _(p->ui_name), FALSE); p++; } @@ -608,6 +623,7 @@ _auth_dialog_exited(GPid pid, int status, gpointer user_data) for (i = 1; groups[i]; i++) { gs_free char *pretty_name = NULL; + gboolean force_echo; if (!g_key_file_get_boolean(keyfile, groups[i], "IsSecret", NULL)) continue; @@ -615,11 +631,14 @@ _auth_dialog_exited(GPid pid, int status, gpointer user_data) continue; pretty_name = g_key_file_get_string(keyfile, groups[i], "Label", NULL); + force_echo = g_key_file_get_boolean(keyfile, groups[i], "ForceEcho", NULL); + g_ptr_array_add(secrets, _secret_real_new_vpn_secret(pretty_name, NM_SETTING(s_vpn), groups[i], - nm_setting_vpn_get_service_type(s_vpn))); + nm_setting_vpn_get_service_type(s_vpn), + force_echo)); } out: diff --git a/src/libnmc-base/nm-secret-agent-simple.h b/src/libnmc-base/nm-secret-agent-simple.h index a1d1588191..94197957a6 100644 --- a/src/libnmc-base/nm-secret-agent-simple.h +++ b/src/libnmc-base/nm-secret-agent-simple.h @@ -23,6 +23,7 @@ typedef struct { const char *vpn_type; bool is_secret : 1; bool no_prompt_entry_id : 1; + bool force_echo : 1; } NMSecretAgentSimpleSecret; #define NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "vpn.secrets." diff --git a/src/nmcli/common.c b/src/nmcli/common.c index 04d2ebf949..2f205e50cb 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -700,7 +700,7 @@ get_secrets_from_user(const NmcConfig *nmc_config, if (msg) nmc_print("%s\n", msg); - echo_on = secret->is_secret ? nmc_config->show_secrets : TRUE; + echo_on = secret->is_secret ? secret->force_echo || nmc_config->show_secrets : TRUE; if (secret->no_prompt_entry_id) pwd = nmc_readline_echo(nmc_config, echo_on, "%s: ", secret->pretty_name); diff --git a/src/nmtui/nmt-password-dialog.c b/src/nmtui/nmt-password-dialog.c index 75194d7b99..6f1a5f031f 100644 --- a/src/nmtui/nmt-password-dialog.c +++ b/src/nmtui/nmt-password-dialog.c @@ -139,7 +139,7 @@ nmt_password_dialog_constructed(GObject *object) nmt_newt_widget_set_padding(widget, 4, 0, 1, 0); flags = NMT_NEWT_ENTRY_NONEMPTY; - if (secret->is_secret) + if (secret->is_secret && !secret->force_echo) flags |= NMT_NEWT_ENTRY_PASSWORD; widget = nmt_newt_entry_new(30, flags); if (secret->value)