From 117cbd18949a197f663ef51e95535e76e99e387f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Mar 2020 12:41:13 +0100 Subject: [PATCH] libnm: allow setting empty vpn.secrets item Like for data, now also allow empty secrets to be added to the VPN setting. For one, this avoids an assertion failure, where keyfile reader wouldn't check whether a secret key is set to the empty word. For data, it's more clear that we want to allow setting empty data values. VPN settings are only interpreted by the VPN plugin, so libnm and the daemon shouldn't prevent empty settings. It can be useful to distinguish between unset (NULL) and empty values. For secrets, it's less clear that empty secrets shall be allowed. I think it should. Of course, the empty secret likely isn't a correct nor valid secret. But libnm cannot validate the secrets anyway. It's up to the VPN plugin to handle this in any way they see fit. Also, already before, the user could set NM_SETTING_VPN_SECRETS to a string dictionary with empty passwords. So, the API didn't fully prevent that. Only certain API wouldn't play along. --- libnm-core/nm-setting-vpn.c | 28 ++++++++++------------------ libnm-core/tests/test-general.c | 8 ++------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c index cf9cbd1d91..d99b05a90d 100644 --- a/libnm-core/nm-setting-vpn.c +++ b/libnm-core/nm-setting-vpn.c @@ -345,19 +345,27 @@ nm_setting_vpn_get_num_secrets (NMSettingVpn *setting) * nm_setting_vpn_add_secret: * @setting: the #NMSettingVpn * @key: a name that uniquely identifies the given secret @secret - * @secret: the secret to be referenced by @key + * @secret: (allow-none): the secret to be referenced by @key * * Establishes a relationship between @key and @secret internally in the * setting which may be retrieved later. + * + * Before 1.24, @secret must not be %NULL and not an empty string. Since 1.24, + * @secret can be set to an empty string. It can also be set to %NULL to unset + * the key. In that case, the behavior is as if calling nm_setting_vpn_remove_secret(). **/ void nm_setting_vpn_add_secret (NMSettingVpn *setting, const char *key, const char *secret) { + if (!secret) { + nm_setting_vpn_remove_secret (setting, key); + return; + } + g_return_if_fail (NM_IS_SETTING_VPN (setting)); g_return_if_fail (key && key[0]); - g_return_if_fail (secret && secret[0]); g_hash_table_insert (_ensure_strdict (&NM_SETTING_VPN_GET_PRIVATE (setting)->secrets, TRUE), g_strdup (key), @@ -588,14 +596,6 @@ update_secret_string (NMSetting *setting, 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 (!value[0]) { - g_set_error (error, NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("secret was empty")); - g_prefix_error (error, "%s.%s: ", NM_SETTING_VPN_SETTING_NAME, key); - return NM_SETTING_UPDATE_SECRET_ERROR; - } - if (nm_streq0 (nm_g_hash_table_lookup (priv->secrets, key), value)) return NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED; @@ -627,14 +627,6 @@ update_secret_dict (NMSetting *setting, g_prefix_error (error, "%s: ", NM_SETTING_VPN_SETTING_NAME); return NM_SETTING_UPDATE_SECRET_ERROR; } - - if (!value[0]) { - g_set_error (error, NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("secret value was empty")); - g_prefix_error (error, "%s.%s: ", NM_SETTING_VPN_SETTING_NAME, name); - return NM_SETTING_UPDATE_SECRET_ERROR; - } } /* Now add the items to the settings' secrets list */ diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 5d3494d869..0301473bc9 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -1197,13 +1197,9 @@ test_setting_vpn_items (void) nm_setting_vpn_add_secret (s_vpn, "", ""); g_test_assert_expected_messages (); - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (secret && secret[0])); - nm_setting_vpn_add_secret (s_vpn, "foobar1", NULL); - g_test_assert_expected_messages (); - - NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (secret && secret[0])); nm_setting_vpn_add_secret (s_vpn, "foobar1", ""); - g_test_assert_expected_messages (); + + nm_setting_vpn_add_secret (s_vpn, "foobar1", NULL); NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (key && key[0])); nm_setting_vpn_add_secret (s_vpn, NULL, "blahblah1");