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.
This commit is contained in:
Thomas Haller 2020-03-26 12:41:13 +01:00
parent 64da830b07
commit 117cbd1894
2 changed files with 12 additions and 24 deletions

View file

@ -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 */

View file

@ -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");