libnm: cleanup secret-flags setter and getter

There are 3 kinds of secret flag implementations:

1) regular properties have a GObject property and a corresponding
  "-flags" property.

2) NMSettingVpn handles this entirely differently

3) NMSettingWirelessSecurity's WEP keys, where the secret keys
   share a flags property that does not follow the same naming
   scheme as 1).

The getter and setter had a boolean "verifiy_secret", only to
handle 3). Drop that parameter. Don't let NMSettingWirelessSecurity
call the parent's implementation for WEP keys. Just let it handle
it directly.
This commit is contained in:
Thomas Haller 2019-01-06 13:49:46 +01:00
parent 9c139b2c47
commit 4a5514dc0f
6 changed files with 106 additions and 73 deletions

View file

@ -1948,7 +1948,8 @@ write_hash_of_string (GKeyFile *file,
if (vpn_secrets) {
NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE;
nm_setting_get_secret_flags (setting, property, &secret_flags, NULL);
if (!nm_setting_get_secret_flags (setting, property, &secret_flags, NULL))
nm_assert_not_reached ();
if (!_secret_flags_persist_secret (secret_flags))
continue;
}

View file

@ -109,6 +109,11 @@ NMSetting *_nm_setting_new_from_dbus (GType setting_type,
NMSettingParseFlags parse_flags,
GError **error);
gboolean _nm_setting_property_is_regular_secret (NMSetting *setting,
const char *secret_name);
gboolean _nm_setting_property_is_regular_secret_flags (NMSetting *setting,
const char *secret_flags_name);
/*****************************************************************************/
static inline GArray *

View file

@ -642,7 +642,6 @@ update_one_secret (NMSetting *setting, const char *key, GVariant *value, GError
static gboolean
get_secret_flags (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags *out_flags,
GError **error)
{
@ -659,8 +658,7 @@ get_secret_flags (NMSetting *setting,
/* having no secret flag for the secret is fine, as long as there
* is the secret itself... */
if ( verify_secret
&& !g_hash_table_contains (priv->secrets, secret_name)) {
if (!g_hash_table_contains (priv->secrets, secret_name)) {
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_PROPERTY_NOT_SECRET,
@ -689,7 +687,6 @@ get_secret_flags (NMSetting *setting,
static gboolean
set_secret_flags (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags flags,
GError **error)
{

View file

@ -1177,53 +1177,56 @@ verify_secrets (NMSetting *setting, NMConnection *connection, GError **error)
static gboolean
get_secret_flags (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags *out_flags,
GError **error)
{
NMSettingClass *setting_class;
gboolean verify_override = verify_secret;
NMSettingSecretFlags flags;
/* There's only one 'flags' property for WEP keys, so alias all the WEP key
* property names to that flags property.
*/
if ( !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0)
|| !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY1)
|| !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY2)
|| !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY3)) {
secret_name = "wep-key";
verify_override = FALSE; /* Already know it's a secret */
if (NM_IN_STRSET (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0,
NM_SETTING_WIRELESS_SECURITY_WEP_KEY1,
NM_SETTING_WIRELESS_SECURITY_WEP_KEY2,
NM_SETTING_WIRELESS_SECURITY_WEP_KEY3)) {
/* There's only one 'flags' property for WEP keys, so alias all the WEP key
* property names to that flags property. */
nm_assert (_nm_setting_property_is_regular_secret (setting, secret_name));
nm_assert (_nm_setting_property_is_regular_secret_flags (setting, NM_SETTING_WIRELESS_SECURITY_WEP_KEY_FLAGS));
g_object_get (G_OBJECT (setting),
NM_SETTING_WIRELESS_SECURITY_WEP_KEY_FLAGS,
&flags,
NULL);
NM_SET_OUT (out_flags, flags);
return TRUE;
}
/* Chain up to superclass with modified key name */
setting_class = NM_SETTING_CLASS (nm_setting_wireless_security_parent_class);
return setting_class->get_secret_flags (setting, secret_name, verify_override, out_flags, error);
return NM_SETTING_CLASS (nm_setting_wireless_security_parent_class)->get_secret_flags (setting, secret_name, out_flags, error);
}
static gboolean
set_secret_flags (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags flags,
GError **error)
{
NMSettingClass *setting_class;
gboolean verify_override = verify_secret;
if (NM_IN_STRSET (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0,
NM_SETTING_WIRELESS_SECURITY_WEP_KEY1,
NM_SETTING_WIRELESS_SECURITY_WEP_KEY2,
NM_SETTING_WIRELESS_SECURITY_WEP_KEY3)) {
/* There's only one 'flags' property for WEP keys, so alias all the WEP key
* property names to that flags property. */
nm_assert (_nm_setting_property_is_regular_secret (setting, secret_name));
nm_assert (_nm_setting_property_is_regular_secret_flags (setting, NM_SETTING_WIRELESS_SECURITY_WEP_KEY_FLAGS));
/* There's only one 'flags' property for WEP keys, so alias all the WEP key
* property names to that flags property.
*/
if ( !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0)
|| !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY1)
|| !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY2)
|| !g_strcmp0 (secret_name, NM_SETTING_WIRELESS_SECURITY_WEP_KEY3)) {
secret_name = "wep-key";
verify_override = FALSE; /* Already know it's a secret */
if (!nm_g_object_set_property_flags (G_OBJECT (setting),
NM_SETTING_WIRELESS_SECURITY_WEP_KEY_FLAGS,
NM_TYPE_SETTING_SECRET_FLAGS,
flags,
error))
g_return_val_if_reached (FALSE);
return TRUE;
}
/* Chain up to superclass with modified key name */
setting_class = NM_SETTING_CLASS (nm_setting_wireless_security_parent_class);
return setting_class->set_secret_flags (setting, secret_name, verify_override, flags, error);
return NM_SETTING_CLASS (nm_setting_wireless_security_parent_class)->set_secret_flags (setting, secret_name, flags, error);
}
static void

View file

@ -1884,52 +1884,72 @@ _nm_setting_update_secrets (NMSetting *setting, GVariant *secrets, GError **erro
return result;
}
static gboolean
is_secret_prop (NMSetting *setting, const char *secret_name, GError **error)
static void
_set_error_secret_property_not_found (GError **error,
NMSetting *setting,
const char *secret_name)
{
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_PROPERTY_NOT_FOUND,
_("not a secret property"));
g_prefix_error (error, "%s.%s: ", nm_setting_get_name (setting), secret_name);
}
gboolean
_nm_setting_property_is_regular_secret (NMSetting *setting,
const char *secret_name)
{
const NMSettInfoProperty *property;
GParamSpec *pspec;
nm_assert (NM_IS_SETTING (setting));
nm_assert (secret_name);
property = _nm_sett_info_property_get (NM_SETTING_GET_CLASS (setting), secret_name);
if (!property) {
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_PROPERTY_NOT_FOUND,
_("secret is not set"));
g_prefix_error (error, "%s.%s: ", nm_setting_get_name (setting), secret_name);
return FALSE;
}
return property
&& property->param_spec
&& NM_FLAGS_HAS (property->param_spec->flags, NM_SETTING_PARAM_SECRET);
}
pspec = property->param_spec;
if (!pspec || !(pspec->flags & NM_SETTING_PARAM_SECRET)) {
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_PROPERTY_NOT_SECRET,
_("not a secret property"));
g_prefix_error (error, "%s.%s: ", nm_setting_get_name (setting), secret_name);
return FALSE;
}
gboolean
_nm_setting_property_is_regular_secret_flags (NMSetting *setting,
const char *secret_flags_name)
{
const NMSettInfoProperty *property;
return TRUE;
nm_assert (NM_IS_SETTING (setting));
nm_assert (secret_flags_name);
property = _nm_sett_info_property_get (NM_SETTING_GET_CLASS (setting), secret_flags_name);
return property
&& property->param_spec
&& !NM_FLAGS_HAS (property->param_spec->flags, NM_SETTING_PARAM_SECRET)
&& G_PARAM_SPEC_VALUE_TYPE (property->param_spec) == NM_TYPE_SETTING_SECRET_FLAGS;
}
static gboolean
get_secret_flags (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags *out_flags,
GError **error)
{
gs_free char *name_to_free = NULL;
NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
gs_free char *secret_flags_name_free = NULL;
const char *secret_flags_name;
NMSettingSecretFlags flags;
if (verify_secret && !is_secret_prop (setting, secret_name, error)) {
if (!_nm_setting_property_is_regular_secret (setting,
secret_name)) {
_set_error_secret_property_not_found (error, setting, secret_name);
NM_SET_OUT (out_flags, NM_SETTING_SECRET_FLAG_NONE);
return FALSE;
}
secret_flags_name = nm_construct_name_a ("%s-flags", secret_name, &secret_flags_name_free);
nm_assert (_nm_setting_property_is_regular_secret_flags (setting, secret_flags_name));
g_object_get (G_OBJECT (setting),
nm_construct_name_a ("%s-flags", secret_name, &name_to_free),
secret_flags_name,
&flags,
NULL);
NM_SET_OUT (out_flags, flags);
@ -1958,25 +1978,34 @@ nm_setting_get_secret_flags (NMSetting *setting,
g_return_val_if_fail (NM_IS_SETTING (setting), FALSE);
g_return_val_if_fail (secret_name != NULL, FALSE);
return NM_SETTING_GET_CLASS (setting)->get_secret_flags (setting, secret_name, TRUE, out_flags, error);
return NM_SETTING_GET_CLASS (setting)->get_secret_flags (setting, secret_name, out_flags, error);
}
static gboolean
set_secret_flags (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags flags,
GError **error)
{
gs_free char *name_to_free = NULL;
gs_free char *secret_flags_name_free = NULL;
const char *secret_flags_name;
if (verify_secret)
g_return_val_if_fail (is_secret_prop (setting, secret_name, error), FALSE);
if (!_nm_setting_property_is_regular_secret (setting,
secret_name)) {
_set_error_secret_property_not_found (error, setting, secret_name);
return FALSE;
}
g_object_set (G_OBJECT (setting),
nm_construct_name_a ("%s-flags", secret_name, &name_to_free),
flags,
NULL);
secret_flags_name = nm_construct_name_a ("%s-flags", secret_name, &secret_flags_name_free);
nm_assert (_nm_setting_property_is_regular_secret_flags (setting, secret_flags_name));
if (!nm_g_object_set_property_flags (G_OBJECT (setting),
secret_flags_name,
NM_TYPE_SETTING_SECRET_FLAGS,
flags,
error))
g_return_val_if_reached (FALSE);
return TRUE;
}
@ -2003,7 +2032,7 @@ nm_setting_set_secret_flags (NMSetting *setting,
g_return_val_if_fail (secret_name != NULL, FALSE);
g_return_val_if_fail (flags <= NM_SETTING_SECRET_FLAGS_ALL, FALSE);
return NM_SETTING_GET_CLASS (setting)->set_secret_flags (setting, secret_name, TRUE, flags, error);
return NM_SETTING_GET_CLASS (setting)->set_secret_flags (setting, secret_name, flags, error);
}
/**

View file

@ -191,13 +191,11 @@ typedef struct {
gboolean (*get_secret_flags) (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags *out_flags,
GError **error);
gboolean (*set_secret_flags) (NMSetting *setting,
const char *secret_name,
gboolean verify_secret,
NMSettingSecretFlags flags,
GError **error);