system/NetworkManager
system/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager synced 2024-09-30 05:15:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

vpn: handle hint tags in the daemon

Browse source 
Commit 345bd1b187 ('libnmc: fix secrets request on 2nd stage of 2FA
authentication') and commit 27c701ebfb ('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 were processed (and removed) in the secret agents, not in the
daemon. This is wrong because a system with an updated VPN plugin but a
not yet updated secret agent (like nm-plasma) will fail: it won't remove
the prefix and the daemon will save the secret with the prefix, i.e.
"x-dynamic-challenge:challenge-response" instead of just
"challenge-response". Then, VPN plugins doesn't recognize it, failing the
profile's activation. This is, in fact, an API break.

Also, if the VPN connection already existed before updating NM and the
VPN plugin, the secret flags are not added to the profile (they are only
added when the profile is created or modified). This causes the user's
first time response is saved to the profile, so the activation fails the
second and next times.

See:
- https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1536
- https://gitlab.gnome.org/GNOME/NetworkManager-openvpn/-/issues/142

Anyway, in a good design the daemon should contain almost all the logic
and the clients should keep as simple as possible. Fix above's problems
by letting the daemon to receive the secret names with the prefix
already included. The daemon will strip it and will know what it means.

Note that this is done only in the functions that saves the secrets from
the data received via D-Bus. For example, nm_setting_vpn_add_secret
doesn't need to do it because this value shouldn't come from VPN
plugin's hints.
This commit is contained in:
Íñigo Huguet 2024-06-03 14:29:15 +02:00 committed by Íñigo Huguet
parent 8f3b4f06bb
commit 0583e1f843
1 changed files with 52 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

55
src/libnm-core-impl/nm-setting-vpn.c
View file

@ -577,14 +577,48 @@ verify(NMSetting *setting, NMConnection *connection, GError **error)
return TRUE;
}
static gboolean
_parse_secret_hint_tag(const char *secret_name,
const char **out_secret_name,
NMSettingSecretFlags *out_implied_flags)
{
NMSettingSecretFlags implied_flags = NM_SETTING_SECRET_FLAG_NONE;
gboolean ret = FALSE;
nm_assert(secret_name);
if (g_str_has_prefix(secret_name, NM_SECRET_TAG_DYNAMIC_CHALLENGE)) {
secret_name += NM_STRLEN(NM_SECRET_TAG_DYNAMIC_CHALLENGE);
implied_flags |= 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);
implied_flags |= NM_SETTING_SECRET_FLAG_NOT_SAVED;
ret = TRUE;
}
NM_SET_OUT(out_secret_name, secret_name);
NM_SET_OUT(out_implied_flags, implied_flags);
return ret;
}
static NMSettingUpdateSecretResult
update_secret_string(NMSetting *setting, const char *key, const char *value, GError **error)
{
NMSettingVpnPrivate *priv = NM_SETTING_VPN_GET_PRIVATE(setting);
NMSettingSecretFlags hint_implied_flags, flags;
g_return_val_if_fail(key && key[0], NM_SETTING_UPDATE_SECRET_ERROR);
g_return_val_if_fail(value, NM_SETTING_UPDATE_SECRET_ERROR);
/* If the name is prefixed with a hint tag, process it before saving:
* remove the prefix and apply the flags that it implies */
_parse_secret_hint_tag(key, &key, &hint_implied_flags);
if (hint_implied_flags) {
nm_setting_get_secret_flags(setting, key, &flags, NULL);
nm_setting_set_secret_flags(setting, key, flags | hint_implied_flags, NULL);
}
if (nm_streq0(nm_g_hash_table_lookup(priv->secrets, key), value))
return NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED;
@ -599,6 +633,7 @@ update_secret_dict(NMSetting *setting, GVariant *secrets, GError **error)
GVariantIter iter;
const char *name, *value;
NMSettingUpdateSecretResult result = NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED;
NMSettingSecretFlags hint_implied_flags, flags;
g_return_val_if_fail(secrets != NULL, NM_SETTING_UPDATE_SECRET_ERROR);
@ -618,6 +653,14 @@ update_secret_dict(NMSetting *setting, GVariant *secrets, GError **error)
/* Now add the items to the settings' secrets list */
g_variant_iter_init(&iter, secrets);
while (g_variant_iter_next(&iter, "{&s&s}", &name, &value)) {
/* If the name is prefixed with a hint tag, process it before saving:
* remove the prefix and apply the flags that it implies */
_parse_secret_hint_tag(name, &name, &hint_implied_flags);
if (hint_implied_flags) {
nm_setting_get_secret_flags(setting, name, &flags, NULL);
nm_setting_set_secret_flags(setting, name, flags | hint_implied_flags, NULL);
}
if (nm_streq0(nm_g_hash_table_lookup(priv->secrets, name), value))
continue;
@ -727,6 +770,7 @@ get_secret_flags(NMSetting *setting,
GError **error)
{
NMSettingVpnPrivate *priv = NM_SETTING_VPN_GET_PRIVATE(setting);
NMSettingSecretFlags implied_flags = NM_SETTING_SECRET_FLAG_NONE;
gs_free char *flags_key_free = NULL;
const char *flags_key;
const char *flags_val;
@ -734,6 +778,10 @@ get_secret_flags(NMSetting *setting,
nm_assert(secret_name);
/* Secrets received via D-Bus from VPN plugins might be prefixed by a hint tag. If
* that's the case, process it first: remove the tag and get the flags that it implies */
_parse_secret_hint_tag(secret_name, &secret_name, &implied_flags);
if (!secret_name[0]) {
g_set_error(error,
NM_CONNECTION_ERROR,
@ -746,7 +794,7 @@ get_secret_flags(NMSetting *setting,
if (!priv->data
|| !g_hash_table_lookup_extended(priv->data, flags_key, NULL, (gpointer *) &flags_val)) {
NM_SET_OUT(out_flags, NM_SETTING_SECRET_FLAG_NONE);
NM_SET_OUT(out_flags, implied_flags);
/* having no secret flag for the secret is fine, as long as there
* is the secret itself... */
@ -772,7 +820,7 @@ get_secret_flags(NMSetting *setting,
return TRUE;
}
NM_SET_OUT(out_flags, (NMSettingSecretFlags) i64);
NM_SET_OUT(out_flags, (NMSettingSecretFlags) i64 | implied_flags);
return TRUE;
}
@ -783,7 +831,8 @@ set_secret_flags(NMSetting *setting,
GError **error)
{
nm_assert(secret_name);
nm_assert(!_parse_secret_hint_tag(secret_name, NULL, NULL)); /* Accept hint tags only via D-Bus,
saved by update_one_secret */
if (!secret_name[0]) {
g_set_error(error,
NM_CONNECTION_ERROR,