Commit 864db9f9 authored by Dan Williams's avatar Dan Williams

libnm-util: add new compare flags for ignoring various types of secrets

It turns out we need a way to ignore transient (agent-owned or unsaved)
secrets during connection comparison.  For example, if the user is
connecting to a network where the password is not saved, other
changes could trigger a writeout of that connection to disk when
connecting, which would the connection back in due to inotify, and the
re-read connection would then no longer be recognized as the same as
the in-memory connection due to the transient secret which obviously
wasn't read in from disk.

Adding these compare flags allows the code to not bother writing the
connection out to disk when the only difference between the on-disk
and in-memory connections are secrets that shouldn't get written to
disk anyway.
parent a2acfdd4
...@@ -562,6 +562,21 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) ...@@ -562,6 +562,21 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
return TRUE; return TRUE;
} }
static gboolean
compare_property (NMSetting *setting,
NMSetting *other,
const GParamSpec *prop_spec,
NMSettingCompareFlags flags)
{
/* Handle ignore ID */
if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_ID)
&& g_strcmp0 (prop_spec->name, NM_SETTING_CONNECTION_ID) == 0)
return TRUE;
/* Otherwise chain up to parent to handle generic compare */
return NM_SETTING_CLASS (nm_setting_connection_parent_class)->compare_property (setting, other, prop_spec, flags);
}
static void static void
nm_setting_connection_init (NMSettingConnection *setting) nm_setting_connection_init (NMSettingConnection *setting)
{ {
...@@ -693,6 +708,7 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class) ...@@ -693,6 +708,7 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class)
object_class->get_property = get_property; object_class->get_property = get_property;
object_class->finalize = finalize; object_class->finalize = finalize;
parent_class->verify = verify; parent_class->verify = verify;
parent_class->compare_property = compare_property;
/* Properties */ /* Properties */
......
...@@ -451,6 +451,68 @@ need_secrets (NMSetting *setting) ...@@ -451,6 +451,68 @@ need_secrets (NMSetting *setting)
return g_ptr_array_sized_new (1); return g_ptr_array_sized_new (1);
} }
static gboolean
compare_one_secret (NMSettingVPN *a,
NMSettingVPN *b,
NMSettingCompareFlags flags)
{
GHashTable *a_secrets, *b_secrets;
GHashTableIter iter;
const char *key, *val;
a_secrets = NM_SETTING_VPN_GET_PRIVATE (a)->secrets;
b_secrets = NM_SETTING_VPN_GET_PRIVATE (b)->secrets;
g_hash_table_iter_init (&iter, a_secrets);
while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &val)) {
NMSettingSecretFlags a_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
NMSettingSecretFlags b_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
nm_setting_get_secret_flags (NM_SETTING (a), key, &a_secret_flags, NULL);
nm_setting_get_secret_flags (NM_SETTING (b), key, &b_secret_flags, NULL);
/* If the secret flags aren't the same, the settings aren't the same */
if (a_secret_flags != b_secret_flags)
return FALSE;
if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS)
&& (a_secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED))
continue;
if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)
&& (a_secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED))
continue;
/* Now compare the values themselves */
if (g_strcmp0 (val, nm_setting_vpn_get_secret (b, key)) != 0)
return FALSE;
}
return TRUE;
}
static gboolean
compare_property (NMSetting *setting,
NMSetting *other,
const GParamSpec *prop_spec,
NMSettingCompareFlags flags)
{
gboolean same;
/* We only need to treat the 'secrets' property specially */
if (g_strcmp0 (prop_spec->name, NM_SETTING_VPN_SECRETS) != 0)
return NM_SETTING_CLASS (nm_setting_vpn_parent_class)->compare_property (setting, other, prop_spec, flags);
/* Compare A to B to ensure everything in A is found in B */
same = compare_one_secret (NM_SETTING_VPN (setting), NM_SETTING_VPN (other), flags);
if (same) {
/* And then B to A to ensure everything in B is also found in A */
same = compare_one_secret (NM_SETTING_VPN (other), NM_SETTING_VPN (setting), flags);
}
return same;
}
static void static void
destroy_one_secret (gpointer data) destroy_one_secret (gpointer data)
{ {
...@@ -572,6 +634,7 @@ nm_setting_vpn_class_init (NMSettingVPNClass *setting_class) ...@@ -572,6 +634,7 @@ nm_setting_vpn_class_init (NMSettingVPNClass *setting_class)
parent_class->get_secret_flags = get_secret_flags; parent_class->get_secret_flags = get_secret_flags;
parent_class->set_secret_flags = set_secret_flags; parent_class->set_secret_flags = set_secret_flags;
parent_class->need_secrets = need_secrets; parent_class->need_secrets = need_secrets;
parent_class->compare_property = compare_property;
/* Properties */ /* Properties */
/** /**
......
...@@ -332,27 +332,52 @@ nm_setting_verify (NMSetting *setting, GSList *all_settings, GError **error) ...@@ -332,27 +332,52 @@ nm_setting_verify (NMSetting *setting, GSList *all_settings, GError **error)
return TRUE; return TRUE;
} }
static inline gboolean static gboolean
should_compare_prop (NMSetting *setting, compare_property (NMSetting *setting,
const char *prop_name, NMSetting *other,
NMSettingCompareFlags comp_flags, const GParamSpec *prop_spec,
GParamFlags prop_flags) NMSettingCompareFlags flags)
{ {
/* Fuzzy compare ignores secrets and properties defined with the FUZZY_IGNORE flag */ GValue value1 = { 0 };
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_FUZZY) GValue value2 = { 0 };
&& (prop_flags & (NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET))) gboolean different;
return FALSE;
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS) /* Handle compare flags */
&& (prop_flags & NM_SETTING_PARAM_SECRET)) if (prop_spec->flags & NM_SETTING_PARAM_SECRET) {
return FALSE; NMSettingSecretFlags a_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
NMSettingSecretFlags b_secret_flags = NM_SETTING_SECRET_FLAG_NONE;
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_ID) nm_setting_get_secret_flags (setting, prop_spec->name, &a_secret_flags, NULL);
&& NM_IS_SETTING_CONNECTION (setting) nm_setting_get_secret_flags (other, prop_spec->name, &b_secret_flags, NULL);
&& !strcmp (prop_name, NM_SETTING_CONNECTION_ID))
return FALSE;
return TRUE; /* If the secret flags aren't the same the settings aren't the same */
if (a_secret_flags != b_secret_flags)
return FALSE;
/* Check for various secret flags that might cause us to ignore comparing
* this property.
*/
if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS)
&& (a_secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED))
return TRUE;
if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)
&& (a_secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED))
return TRUE;
}
g_value_init (&value1, prop_spec->value_type);
g_object_get_property (G_OBJECT (setting), prop_spec->name, &value1);
g_value_init (&value2, prop_spec->value_type);
g_object_get_property (G_OBJECT (other), prop_spec->name, &value2);
different = g_param_values_cmp ((GParamSpec *) prop_spec, &value1, &value2);
g_value_unset (&value1);
g_value_unset (&value2);
return different == 0 ? TRUE : FALSE;
} }
/** /**
...@@ -374,7 +399,7 @@ nm_setting_compare (NMSetting *a, ...@@ -374,7 +399,7 @@ nm_setting_compare (NMSetting *a,
{ {
GParamSpec **property_specs; GParamSpec **property_specs;
guint n_property_specs; guint n_property_specs;
gint different; gint same = TRUE;
guint i; guint i;
g_return_val_if_fail (NM_IS_SETTING (a), FALSE); g_return_val_if_fail (NM_IS_SETTING (a), FALSE);
...@@ -386,32 +411,59 @@ nm_setting_compare (NMSetting *a, ...@@ -386,32 +411,59 @@ nm_setting_compare (NMSetting *a,
/* And now all properties */ /* And now all properties */
property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (a), &n_property_specs); property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (a), &n_property_specs);
different = FALSE; for (i = 0; i < n_property_specs && same; i++) {
for (i = 0; i < n_property_specs && !different; i++) {
GParamSpec *prop_spec = property_specs[i]; GParamSpec *prop_spec = property_specs[i];
GValue value1 = { 0 };
GValue value2 = { 0 };
/* Handle compare flags */ /* Fuzzy compare ignores secrets and properties defined with the FUZZY_IGNORE flag */
if (!should_compare_prop (a, prop_spec->name, flags, prop_spec->flags)) if ( (flags & NM_SETTING_COMPARE_FLAG_FUZZY)
&& (prop_spec->flags & (NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)))
continue; continue;
g_value_init (&value1, prop_spec->value_type); if ( (flags & NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS)
g_object_get_property (G_OBJECT (a), prop_spec->name, &value1); && (prop_spec->flags & NM_SETTING_PARAM_SECRET))
continue;
same = NM_SETTING_GET_CLASS (a)->compare_property (a, b, prop_spec, flags);
}
g_free (property_specs);
g_value_init (&value2, prop_spec->value_type); return same;
g_object_get_property (G_OBJECT (b), prop_spec->name, &value2); }
different = g_param_values_cmp (prop_spec, &value1, &value2); static inline gboolean
should_compare_prop (NMSetting *setting,
const char *prop_name,
NMSettingCompareFlags comp_flags,
GParamFlags prop_flags)
{
/* Fuzzy compare ignores secrets and properties defined with the FUZZY_IGNORE flag */
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_FUZZY)
&& (prop_flags & (NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_SECRET)))
return FALSE;
g_value_unset (&value1); if (prop_flags & NM_SETTING_PARAM_SECRET) {
g_value_unset (&value2); NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE;
if (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS)
return FALSE;
nm_setting_get_secret_flags (setting, prop_name, &secret_flags, NULL);
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS)
&& (secret_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED))
return FALSE;
if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)
&& (secret_flags & NM_SETTING_SECRET_FLAG_NOT_SAVED))
return FALSE;
} }
g_free (property_specs); if ( (comp_flags & NM_SETTING_COMPARE_FLAG_IGNORE_ID)
&& NM_IS_SETTING_CONNECTION (setting)
&& !strcmp (prop_name, NM_SETTING_CONNECTION_ID))
return FALSE;
return different == 0 ? TRUE : FALSE; return TRUE;
} }
/** /**
...@@ -976,6 +1028,7 @@ nm_setting_class_init (NMSettingClass *setting_class) ...@@ -976,6 +1028,7 @@ nm_setting_class_init (NMSettingClass *setting_class)
setting_class->update_one_secret = update_one_secret; setting_class->update_one_secret = update_one_secret;
setting_class->get_secret_flags = get_secret_flags; setting_class->get_secret_flags = get_secret_flags;
setting_class->set_secret_flags = set_secret_flags; setting_class->set_secret_flags = set_secret_flags;
setting_class->compare_property = compare_property;
/* Properties */ /* Properties */
......
...@@ -111,6 +111,35 @@ typedef enum { ...@@ -111,6 +111,35 @@ typedef enum {
/* NOTE: if adding flags, update nm-setting-private.h as well */ /* NOTE: if adding flags, update nm-setting-private.h as well */
} NMSettingSecretFlags; } NMSettingSecretFlags;
/**
* NMSettingCompareFlags:
* @NM_SETTING_COMPARE_FLAG_EXACT: match all properties exactly
* @NM_SETTING_COMPARE_FLAG_FUZZY: match only important attributes, like SSID,
* type, security settings, etc. Does not match, for example, connection ID
* or UUID.
* @NM_SETTING_COMPARE_FLAG_IGNORE_ID: ignore the connection's ID
* @NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS: ignore all secrets
* @NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS: ignore secrets for which
* the secret's flags indicate the secret is owned by a user secret agent
* (ie, the secret's flag includes @NM_SETTING_SECRET_FLAG_AGENT_OWNED)
* @NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS: ignore secrets for which
* the secret's flags indicate the secret should not be saved to persistent
* storage (ie, the secret's flag includes @NM_SETTING_SECRET_FLAG_NOT_SAVED)
*
* These flags modify the comparison behavior when comparing two settings or
* two connections.
*
**/
typedef enum {
NM_SETTING_COMPARE_FLAG_EXACT = 0x00000000,
NM_SETTING_COMPARE_FLAG_FUZZY = 0x00000001,
NM_SETTING_COMPARE_FLAG_IGNORE_ID = 0x00000002,
NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS = 0x00000004,
NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS = 0x00000008,
NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS = 0x00000010
} NMSettingCompareFlags;
/** /**
* NMSetting: * NMSetting:
* *
...@@ -148,11 +177,16 @@ typedef struct { ...@@ -148,11 +177,16 @@ typedef struct {
NMSettingSecretFlags flags, NMSettingSecretFlags flags,
GError **error); GError **error);
/* Returns TRUE if the given property contains the same value in both settings */
gboolean (*compare_property) (NMSetting *setting,
NMSetting *other,
const GParamSpec *prop_spec,
NMSettingCompareFlags flags);
/* Padding for future expansion */ /* Padding for future expansion */
void (*_reserved1) (void); void (*_reserved1) (void);
void (*_reserved2) (void); void (*_reserved2) (void);
void (*_reserved3) (void); void (*_reserved3) (void);
void (*_reserved4) (void);
} NMSettingClass; } NMSettingClass;
typedef void (*NMSettingValueIterFn) (NMSetting *setting, typedef void (*NMSettingValueIterFn) (NMSetting *setting,
...@@ -194,26 +228,6 @@ gboolean nm_setting_verify (NMSetting *setting, ...@@ -194,26 +228,6 @@ gboolean nm_setting_verify (NMSetting *setting,
GSList *all_settings, GSList *all_settings,
GError **error); GError **error);
/**
* NMSettingCompareFlags:
* @NM_SETTING_COMPARE_FLAG_EXACT: match all properties exactly
* @NM_SETTING_COMPARE_FLAG_FUZZY: match only important attributes, like SSID,
* type, security settings, etc. Does not match, for example, connection ID
* or UUID.
* @NM_SETTING_COMPARE_FLAG_IGNORE_ID: ignore the connection's ID
* @NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS: ignore secrets
*
* These flags modify the comparison behavior when comparing two settings or
* two connections.
*
**/
typedef enum {
NM_SETTING_COMPARE_FLAG_EXACT = 0x00000000,
NM_SETTING_COMPARE_FLAG_FUZZY = 0x00000001,
NM_SETTING_COMPARE_FLAG_IGNORE_ID = 0x00000002,
NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS = 0x00000004
} NMSettingCompareFlags;
gboolean nm_setting_compare (NMSetting *a, gboolean nm_setting_compare (NMSetting *a,
NMSetting *b, NMSetting *b,
NMSettingCompareFlags flags); NMSettingCompareFlags flags);
......
...@@ -1205,6 +1205,95 @@ test_connection_bad_base_types (void) ...@@ -1205,6 +1205,95 @@ test_connection_bad_base_types (void)
g_clear_error (&error); g_clear_error (&error);
} }
static void
test_setting_compare_id (void)
{
NMSetting *old, *new;
gboolean success;
old = nm_setting_connection_new ();
g_object_set (old,
NM_SETTING_CONNECTION_ID, "really awesome cool connection",
NM_SETTING_CONNECTION_UUID, "fbbd59d5-acab-4e30-8f86-258d272617e7",
NM_SETTING_CONNECTION_AUTOCONNECT, FALSE,
NULL);
new = nm_setting_duplicate (old);
g_object_set (new, NM_SETTING_CONNECTION_ID, "some different connection id", NULL);
/* First make sure they are different */
success = nm_setting_compare (old, new, NM_SETTING_COMPARE_FLAG_EXACT);
g_assert (success == FALSE);
success = nm_setting_compare (old, new, NM_SETTING_COMPARE_FLAG_IGNORE_ID);
g_assert (success);
}
static void
test_setting_compare_secrets (NMSettingSecretFlags secret_flags,
NMSettingCompareFlags comp_flags,
gboolean remove_secret)
{
NMSetting *old, *new;
gboolean success;
/* Make sure that a connection with transient/unsaved secrets compares
* successfully to the same connection without those secrets.
*/
old = nm_setting_wireless_security_new ();
g_object_set (old,
NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-psk",
NM_SETTING_WIRELESS_SECURITY_PSK, "really cool psk",
NULL);
nm_setting_set_secret_flags (old, NM_SETTING_WIRELESS_SECURITY_PSK, secret_flags, NULL);
/* Clear the PSK from the duplicated setting */
new = nm_setting_duplicate (old);
if (remove_secret) {
g_object_set (new, NM_SETTING_WIRELESS_SECURITY_PSK, NULL, NULL);
success = nm_setting_compare (old, new, NM_SETTING_COMPARE_FLAG_EXACT);
g_assert (success == FALSE);
}
success = nm_setting_compare (old, new, comp_flags);
g_assert (success);
}
static void
test_setting_compare_vpn_secrets (NMSettingSecretFlags secret_flags,
NMSettingCompareFlags comp_flags,
gboolean remove_secret)
{
NMSetting *old, *new;
gboolean success;
/* Make sure that a connection with transient/unsaved secrets compares
* successfully to the same connection without those secrets.
*/
old = nm_setting_vpn_new ();
nm_setting_vpn_add_secret (NM_SETTING_VPN (old), "foobarbaz", "really secret password");
nm_setting_vpn_add_secret (NM_SETTING_VPN (old), "asdfasdfasdf", "really adfasdfasdfasdf");
nm_setting_vpn_add_secret (NM_SETTING_VPN (old), "0123456778", "abcdefghijklmnpqrstuvqxyz");
nm_setting_vpn_add_secret (NM_SETTING_VPN (old), "borkbork", "yet another really secret password");
nm_setting_set_secret_flags (old, "borkbork", secret_flags, NULL);
/* Clear "borkbork" from the duplicated setting */
new = nm_setting_duplicate (old);
if (remove_secret) {
nm_setting_vpn_remove_secret (NM_SETTING_VPN (new), "borkbork");
/* First make sure they are different */
success = nm_setting_compare (old, new, NM_SETTING_COMPARE_FLAG_EXACT);
g_assert (success == FALSE);
}
success = nm_setting_compare (old, new, comp_flags);
g_assert (success);
}
int main (int argc, char **argv) int main (int argc, char **argv)
{ {
GError *error = NULL; GError *error = NULL;
...@@ -1228,6 +1317,16 @@ int main (int argc, char **argv) ...@@ -1228,6 +1317,16 @@ int main (int argc, char **argv)
test_setting_to_hash_all (); test_setting_to_hash_all ();
test_setting_to_hash_no_secrets (); test_setting_to_hash_no_secrets ();
test_setting_to_hash_only_secrets (); test_setting_to_hash_only_secrets ();
test_setting_compare_id ();
test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_AGENT_OWNED, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS, TRUE);
test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_NOT_SAVED, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS, TRUE);
test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS, TRUE);
test_setting_compare_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_EXACT, FALSE);
test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_AGENT_OWNED, NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS, TRUE);
test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_NOT_SAVED, NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS, TRUE);
test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS, TRUE);
test_setting_compare_vpn_secrets (NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_EXACT, FALSE);
test_connection_to_hash_setting_name (); test_connection_to_hash_setting_name ();
test_setting_connection_permissions_helpers (); test_setting_connection_permissions_helpers ();
test_setting_connection_permissions_property (); test_setting_connection_permissions_property ();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment