From 5629f47a59b48f2604fd8e9e4af7209b138aef21 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Mon, 30 Nov 2020 21:51:32 +0100 Subject: [PATCH] libmm-glib,bearer-properties: allow loose comparisons When comparing bearer properties provided by the user versus loaded from the modem, we shouldn't be very strict, e.g.: * Password or other fields may not be readable from the device. * Some fields may not apply at all (e.g. RM protocol for EPS bearers) * NULL strings could be assumed equal to empty strings. * If no explicit IP type specified, an IPv4 default may be assumed. * If no explicit allowed auth specified, 'none' default may be assumed. These loose comparisons are applied when managing the initial EPS bearer settings and status, and we keep the strict comparison only during the connection attempt lookup of a bearer with certain settings, as those bearer objects are all created in the same place with the same rules. --- libmm-glib/mm-bearer-properties.c | 101 +++++++++++++++++++++++++++--- libmm-glib/mm-bearer-properties.h | 13 +++- src/mm-bearer-list.c | 6 +- src/mm-iface-modem-3gpp.c | 23 ++++--- 4 files changed, 123 insertions(+), 20 deletions(-) diff --git a/libmm-glib/mm-bearer-properties.c b/libmm-glib/mm-bearer-properties.c index 5ed4438e..904d1248 100644 --- a/libmm-glib/mm-bearer-properties.c +++ b/libmm-glib/mm-bearer-properties.c @@ -667,21 +667,102 @@ mm_bearer_properties_new_from_dictionary (GVariant *dictionary, /*****************************************************************************/ +static gboolean +cmp_str (const gchar *a, + const gchar *b, + MMBearerPropertiesCmpFlags flags) +{ + /* Strict match */ + if ((!a && !b) || (a && b && g_strcmp0 (a, b) == 0)) + return TRUE; + /* Additional loose match, consider NULL and EMPTY string equal */ + if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) { + if ((!a && !b[0]) || (!b && !a[0])) + return TRUE; + } + return FALSE; +} + +static gboolean +cmp_ip_type (MMBearerIpFamily a, + MMBearerIpFamily b, + MMBearerPropertiesCmpFlags flags) +{ + /* Strict match */ + if (a == b) + return TRUE; + /* Additional loose match NONE == IPV4 */ + if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) { + if ((a == MM_BEARER_IP_FAMILY_NONE && b == MM_BEARER_IP_FAMILY_IPV4) || + (b == MM_BEARER_IP_FAMILY_NONE && a == MM_BEARER_IP_FAMILY_IPV4)) + return TRUE; + } + return FALSE; +} + +static gboolean +cmp_allowed_auth (MMBearerAllowedAuth a, + MMBearerAllowedAuth b, + MMBearerPropertiesCmpFlags flags) +{ + /* Strict match */ + if (a == b) + return TRUE; + /* Additional loose match UNKNOWN == NONE */ + if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) { + if ((a == MM_BEARER_ALLOWED_AUTH_UNKNOWN && b == MM_BEARER_ALLOWED_AUTH_NONE) || + (b == MM_BEARER_ALLOWED_AUTH_UNKNOWN && a == MM_BEARER_ALLOWED_AUTH_NONE)) + return TRUE; + } + return FALSE; +} + +static gboolean +cmp_allow_roaming (gboolean a, + gboolean a_set, + gboolean b, + gboolean b_set, + MMBearerPropertiesCmpFlags flags) +{ + /* Strict match */ + if (a == b && a_set == b_set) + return TRUE; + /* Additional loose match UNSET == */ + if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) { + if ((a == MM_BEARER_ALLOWED_AUTH_UNKNOWN && b == MM_BEARER_ALLOWED_AUTH_NONE) || + (b == MM_BEARER_ALLOWED_AUTH_UNKNOWN && a == MM_BEARER_ALLOWED_AUTH_NONE)) + return TRUE; + } + return FALSE; +} + /** * mm_bearer_properties_cmp: (skip) */ gboolean -mm_bearer_properties_cmp (MMBearerProperties *a, - MMBearerProperties *b) +mm_bearer_properties_cmp (MMBearerProperties *a, + MMBearerProperties *b, + MMBearerPropertiesCmpFlags flags) { - return ((!g_strcmp0 (a->priv->apn, b->priv->apn)) && - (a->priv->ip_type == b->priv->ip_type) && - (a->priv->allowed_auth == b->priv->allowed_auth) && - (!g_strcmp0 (a->priv->user, b->priv->user)) && - (!g_strcmp0 (a->priv->password, b->priv->password)) && - (a->priv->allow_roaming == b->priv->allow_roaming) && - (a->priv->allow_roaming_set == b->priv->allow_roaming_set) && - (a->priv->rm_protocol == b->priv->rm_protocol)); + if (!cmp_str (a->priv->apn, b->priv->apn, flags)) + return FALSE; + if (!cmp_ip_type (a->priv->ip_type, b->priv->ip_type, flags)) + return FALSE; + if (!cmp_allowed_auth (a->priv->allowed_auth, a->priv->allowed_auth, flags)) + return FALSE; + if (!cmp_str (a->priv->user, b->priv->user, flags)) + return FALSE; + if (!(flags & MM_BEARER_PROPERTIES_CMP_FLAGS_NO_PASSWORD) && !cmp_str (a->priv->password, b->priv->password, flags)) + return FALSE; + if (!(flags & MM_BEARER_PROPERTIES_CMP_FLAGS_NO_ALLOW_ROAMING)) { + if (a->priv->allow_roaming != b->priv->allow_roaming) + return FALSE; + if (a->priv->allow_roaming_set != b->priv->allow_roaming) + return FALSE; + } + if (a->priv->rm_protocol != b->priv->rm_protocol) + return FALSE; + return TRUE; } /*****************************************************************************/ diff --git a/libmm-glib/mm-bearer-properties.h b/libmm-glib/mm-bearer-properties.h index 751185d9..340d9bf1 100644 --- a/libmm-glib/mm-bearer-properties.h +++ b/libmm-glib/mm-bearer-properties.h @@ -113,8 +113,17 @@ gboolean mm_bearer_properties_consume_variant (MMBearerProperties *properties, GVariant *mm_bearer_properties_get_dictionary (MMBearerProperties *self); -gboolean mm_bearer_properties_cmp (MMBearerProperties *a, - MMBearerProperties *b); +typedef enum { + MM_BEARER_PROPERTIES_CMP_FLAGS_NONE = 0, + MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE = 1 << 0, + MM_BEARER_PROPERTIES_CMP_FLAGS_NO_PASSWORD = 1 << 1, + MM_BEARER_PROPERTIES_CMP_FLAGS_NO_ALLOW_ROAMING = 1 << 2, + MM_BEARER_PROPERTIES_CMP_FLAGS_NO_RM_PROTOCOL = 1 << 3, +} MMBearerPropertiesCmpFlags; + +gboolean mm_bearer_properties_cmp (MMBearerProperties *a, + MMBearerProperties *b, + MMBearerPropertiesCmpFlags flags); #endif diff --git a/src/mm-bearer-list.c b/src/mm-bearer-list.c index 504b99ae..65717132 100644 --- a/src/mm-bearer-list.c +++ b/src/mm-bearer-list.c @@ -153,7 +153,11 @@ mm_bearer_list_find_by_properties (MMBearerList *self, GList *l; for (l = self->priv->bearers; l; l = g_list_next (l)) { - if (mm_bearer_properties_cmp (mm_base_bearer_peek_config (MM_BASE_BEARER (l->data)), props)) + /* always strict matching when comparing these bearer properties, as they're all + * built in the same place */ + if (mm_bearer_properties_cmp (mm_base_bearer_peek_config (MM_BASE_BEARER (l->data)), + props, + MM_BEARER_PROPERTIES_CMP_FLAGS_NONE)) return g_object_ref (l->data); } diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index df8df272..4e153c78 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -30,6 +30,16 @@ #define SUBSYSTEM_3GPP "3gpp" +/* When comparing EPS bearer settings take into account that PASSWORD may not always + * be readable, and apply very loose matching for all fields. Also, some implementations + * may allow configuring roaming allowance in the initial EPS bearer, but that is also + * not common. */ +#define MM_BEARER_PROPERTIES_CMP_FLAGS_EPS \ + (MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE | \ + MM_BEARER_PROPERTIES_CMP_FLAGS_NO_PASSWORD | \ + MM_BEARER_PROPERTIES_CMP_FLAGS_NO_ALLOW_ROAMING | \ + MM_BEARER_PROPERTIES_CMP_FLAGS_NO_RM_PROTOCOL) + /*****************************************************************************/ /* Private data context */ @@ -1029,7 +1039,7 @@ after_set_load_initial_eps_bearer_settings_ready (MMIfaceModem3gpp mm_obj_dbg (self, "Updated initial EPS bearer settings:"); log_initial_eps_bearer_settings (self, new_config); - if (!mm_bearer_properties_cmp (new_config, ctx->config)) { + if (!mm_bearer_properties_cmp (new_config, ctx->config, MM_BEARER_PROPERTIES_CMP_FLAGS_EPS)) { mm_obj_dbg (self, "Requested initial EPS bearer settings:"); log_initial_eps_bearer_settings (self, ctx->config); g_dbus_method_invocation_return_error_literal (ctx->invocation, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, @@ -1108,15 +1118,11 @@ set_initial_eps_bearer_settings_auth_ready (MMBaseModem return; } - /* If the user doesn't specify explicit auth settings, assume NONE as default */ - if (mm_bearer_properties_get_allowed_auth (ctx->config) == MM_BEARER_ALLOWED_AUTH_UNKNOWN) - mm_bearer_properties_set_allowed_auth (ctx->config, MM_BEARER_ALLOWED_AUTH_NONE); - old_dictionary = mm_gdbus_modem3gpp_get_initial_eps_bearer_settings (ctx->skeleton); if (old_dictionary) old_config = mm_bearer_properties_new_from_dictionary (old_dictionary, NULL); - if (old_config && mm_bearer_properties_cmp (ctx->config, old_config)) { + if (old_config && mm_bearer_properties_cmp (ctx->config, old_config, MM_BEARER_PROPERTIES_CMP_FLAGS_EPS)) { mm_gdbus_modem3gpp_complete_set_initial_eps_bearer_settings (ctx->skeleton, ctx->invocation); handle_set_initial_eps_bearer_settings_context_free (ctx); } else { @@ -1756,7 +1762,10 @@ mm_iface_modem_3gpp_update_initial_eps_bearer (MMIfaceModem3gpp *self, /* skip update? */ if ((!old_bearer && !properties) || - (old_bearer && properties && mm_bearer_properties_cmp (properties, mm_base_bearer_peek_config (MM_BASE_BEARER (old_bearer))))) + (old_bearer && properties && + mm_bearer_properties_cmp (properties, + mm_base_bearer_peek_config (MM_BASE_BEARER (old_bearer)), + MM_BEARER_PROPERTIES_CMP_FLAGS_EPS))) goto out; if (properties) {