From 759a11425a8673f7bbddfc035883821b84373dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Tue, 28 May 2024 12:29:29 +0200 Subject: [PATCH] libnm: accept secret with hint tags from old secret agents Commit 345bd1b18731 ('libnmc: fix secrets request on 2nd stage of 2FA authentication') and commit 27c701ebfbc9 ('libnmc: allow user input in ECHO mode for 2FA challenges') introduced 2 new tags that hints for the secret agents can have as prefix. These tags can be used by agents to understand that it's the 2nd step of a 2FA authentication. Then, they should return the secret name without the tag. Agents not using libnm or using and old version of libnm are still not aware of this change, so they return the secret name without removing the tag. The secret is saved to the profile as "x-dynamic-challenge:challenge-response" instead of just "challenge-response" and VPN plugins doesn't recognize it, failing the profile's activation. See: - https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1536 - https://gitlab.gnome.org/GNOME/NetworkManager-openvpn/-/issues/142 Fix secrets received with hint tags in NMSetting::for_each_secret. It is executed by the daemon in `validate_secret_flags` when secrets are received from the secrets agents. This will ensure backwards compatibility for those agents that hasn't adapted yet. --- src/libnm-core-impl/nm-setting-vpn.c | 2 ++ src/libnm-core-impl/nm-setting-wireguard.c | 2 ++ src/libnm-core-impl/nm-setting.c | 33 ++++++++++++++++++++++ src/libnm-core-intern/nm-core-internal.h | 2 ++ 4 files changed, 39 insertions(+) diff --git a/src/libnm-core-impl/nm-setting-vpn.c b/src/libnm-core-impl/nm-setting-vpn.c index b867d01860..f34d560854 100644 --- a/src/libnm-core-impl/nm-setting-vpn.c +++ b/src/libnm-core-impl/nm-setting-vpn.c @@ -704,6 +704,8 @@ for_each_secret(NMSetting *setting, while (g_variant_iter_next(&vpn_secrets_iter, "{&s&s}", &vpn_secret_name, &secret)) { NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + _nm_setting_secret_fix_hint_tag(setting, &vpn_secret_name); + /* we ignore the return value of get_secret_flags. The function may determine * that this is not a secret, based on having not secret-flags and no secrets. * But we have the secret at hand. We know it would be a valid secret, if we diff --git a/src/libnm-core-impl/nm-setting-wireguard.c b/src/libnm-core-impl/nm-setting-wireguard.c index 4f96f74210..b27265ece1 100644 --- a/src/libnm-core-impl/nm-setting-wireguard.c +++ b/src/libnm-core-impl/nm-setting-wireguard.c @@ -2218,6 +2218,8 @@ for_each_secret(NMSetting *setting, while (g_variant_iter_next(peer_iter, "{&sv}", &key, &val)) { _nm_unused gs_unref_variant GVariant *val_free = val; + _nm_setting_secret_fix_hint_tag(setting, &key); + if (nm_streq(key, NM_WIREGUARD_PEER_ATTR_PRESHARED_KEY)) { if (!preshared_key && g_variant_is_of_type(val, G_VARIANT_TYPE_STRING)) preshared_key = g_variant_ref(val); diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index bbaa6fcda2..005cc0f337 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -3559,6 +3559,8 @@ for_each_secret(NMSetting *setting, { NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + _nm_setting_secret_fix_hint_tag(setting, &secret_name); + if (!nm_setting_get_secret_flags(setting, secret_name, &secret_flags, NULL)) { if (!remove_non_secrets) g_variant_builder_add(setting_builder, "{sv}", secret_name, val); @@ -3568,6 +3570,37 @@ for_each_secret(NMSetting *setting, g_variant_builder_add(setting_builder, "{sv}", secret_name, val); } +gboolean +_nm_setting_secret_fix_hint_tag(NMSetting *setting, const char **secret_name) +{ + /* Secret agents should remove tags like "x-dynamic-challenge:" from the secret name, + * but old agents might have not adapted yet. If that happens, do the work here: + * remove the tag and set the flags that are implied by that tag. */ + + NMSettingSecretFlags flags_to_add = NM_SETTING_SECRET_FLAG_NONE; + gboolean ret = FALSE; + + if (g_str_has_prefix(*secret_name, NM_SECRET_TAG_DYNAMIC_CHALLENGE)) { + *secret_name += NM_STRLEN(NM_SECRET_TAG_DYNAMIC_CHALLENGE); + flags_to_add |= NM_SETTING_SECRET_FLAG_NOT_SAVED; + ret = TRUE; + } else if (g_str_has_prefix(*secret_name, NM_SECRET_TAG_DYNAMIC_CHALLENGE_ECHO)) { + *secret_name += NM_STRLEN(NM_SECRET_TAG_DYNAMIC_CHALLENGE_ECHO); + flags_to_add |= NM_SETTING_SECRET_FLAG_NOT_SAVED; + ret = TRUE; + } + + if (flags_to_add) { + NMSettingSecretFlags current_flags = NM_SETTING_SECRET_FLAG_NONE; + + nm_setting_get_secret_flags(setting, *secret_name, ¤t_flags, NULL); + current_flags |= flags_to_add; + nm_setting_set_secret_flags(setting, *secret_name, current_flags, NULL); + } + + return ret; +} + static void _set_error_secret_property_not_found(GError **error, NMSetting *setting, const char *secret_name) { diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index fc157c5881..084b1b39bc 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -122,6 +122,8 @@ _nm_setting_secret_flags_valid(NMSettingSecretFlags flags) return !NM_FLAGS_ANY(flags, ~NM_SETTING_SECRET_FLAG_ALL); } +gboolean _nm_setting_secret_fix_hint_tag(NMSetting *setting, const char **secret_name); + /*****************************************************************************/ const char *