From 0ec9bf2c73c9c73cbbe6e757c0857c67be971575 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 22 Sep 2010 13:20:02 -0500 Subject: [PATCH] libnm-util: enforce APN character restrictions APNs can only contain alphanumeric characters, '.', and '-'. To be helpful we strip spaces off before setting the APN internally so that previously (and incorrectly) valid APNs don't cause the whole connection to fail validation and thus disappear. The only case seen in the wild was a Pelephone IL APN which erroneously had a trailing space in the mobile broadband provider database. Bad characters cause the connection to fail with vague error messages about being unable to activate the PDP context during PPP negotiation. --- libnm-util/nm-setting-gsm.c | 39 ++++++++++++++----- libnm-util/tests/test-general.c | 69 +++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/libnm-util/nm-setting-gsm.c b/libnm-util/nm-setting-gsm.c index 4b4560cd6d..575f94d382 100644 --- a/libnm-util/nm-setting-gsm.c +++ b/libnm-util/nm-setting-gsm.c @@ -231,12 +231,30 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return FALSE; } - if (priv->apn && (strlen (priv->apn) < 1 || strchr (priv->apn, '"'))) { - g_set_error (error, - NM_SETTING_GSM_ERROR, - NM_SETTING_GSM_ERROR_INVALID_PROPERTY, - NM_SETTING_GSM_APN); - return FALSE; + if (priv->apn) { + guint32 apn_len = strlen (priv->apn); + guint32 i; + + if (apn_len < 1 || apn_len > 20) { + g_set_error (error, + NM_SETTING_GSM_ERROR, + NM_SETTING_GSM_ERROR_INVALID_PROPERTY, + NM_SETTING_GSM_APN); + return FALSE; + } + + /* APNs roughly follow the same rules as DNS domain names. Allowed + * characters are a-z, 0-9, . and -. GSM 03.60 Section 14.9. + */ + for (i = 0; i < apn_len; i++) { + if (!isalnum (priv->apn[i]) && (priv->apn[i] != '.') && (priv->apn[i] != '-')) { + g_set_error (error, + NM_SETTING_GSM_ERROR, + NM_SETTING_GSM_ERROR_INVALID_PROPERTY, + NM_SETTING_GSM_APN); + return FALSE; + } + } } if (priv->username && !strlen (priv->username)) { @@ -342,11 +360,11 @@ set_property (GObject *object, guint prop_id, break; case PROP_APN: g_free (priv->apn); - priv->apn = g_value_dup_string (value); + priv->apn = g_strstrip (g_value_dup_string (value)); break; case PROP_NETWORK_ID: g_free (priv->network_id); - priv->network_id = g_value_dup_string (value); + priv->network_id = g_strstrip (g_value_dup_string (value)); break; case PROP_NETWORK_TYPE: priv->network_type = g_value_get_int (value); @@ -503,6 +521,8 @@ nm_setting_gsm_class_init (NMSettingGsmClass *setting_class) * the user will be billed for their network usage and whether the user has * access to the Internet or just a provider-specific walled-garden, so it * is important to use the correct APN for the user's mobile broadband plan. + * The APN may only be composed of the characters a-z, 0-9, ., and - per + * GSM 03.60 Section 14.9. **/ g_object_class_install_property (object_class, PROP_APN, @@ -515,7 +535,8 @@ nm_setting_gsm_class_init (NMSettingGsmClass *setting_class) "user has access to the Internet or just a provider-" "specific walled-garden, so it is important to use " "the correct APN for the user's mobile broadband " - "plan.", + "plan. The APN may only be composed of the characters " + "a-z, 0-9, ., and - per GSM 03.60 Section 14.9.", NULL, G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE)); diff --git a/libnm-util/tests/test-general.c b/libnm-util/tests/test-general.c index 9530445959..855ee083fb 100644 --- a/libnm-util/tests/test-general.c +++ b/libnm-util/tests/test-general.c @@ -28,6 +28,7 @@ #include "nm-setting-connection.h" #include "nm-setting-vpn.h" +#include "nm-setting-gsm.h" #include "nm-setting-ip6-config.h" #include "nm-dbus-glib-types.h" @@ -222,6 +223,72 @@ test_setting_ip6_config_old_address_array (void) g_object_unref (s_ip6); } +static void +test_setting_gsm_apn_spaces (void) +{ + NMSettingGsm *s_gsm; + const char *tmp; + + s_gsm = (NMSettingGsm *) nm_setting_gsm_new (); + ASSERT (s_gsm != NULL, + "gsm-apn-spaces", + "error creating GSM setting"); + + /* Trailing space */ + g_object_set (s_gsm, NM_SETTING_GSM_APN, "foobar ", NULL); + tmp = nm_setting_gsm_get_apn (s_gsm); + ASSERT (tmp != NULL, + "gsm-apn-spaces", "empty APN"); + ASSERT (strcmp (tmp, "foobar") == 0, + "gsm-apn-spaces", "unexpected APN"); + + /* Leading space */ + g_object_set (s_gsm, NM_SETTING_GSM_APN, " foobar", NULL); + tmp = nm_setting_gsm_get_apn (s_gsm); + ASSERT (tmp != NULL, + "gsm-apn-spaces", "empty APN"); + ASSERT (strcmp (tmp, "foobar") == 0, + "gsm-apn-spaces", "unexpected APN"); +} + +static void +test_setting_gsm_apn_bad_chars (void) +{ + NMSettingGsm *s_gsm; + + s_gsm = (NMSettingGsm *) nm_setting_gsm_new (); + ASSERT (s_gsm != NULL, + "gsm-apn-bad-chars", + "error creating GSM setting"); + + g_object_set (s_gsm, NM_SETTING_GSM_NUMBER, "*99#", NULL); + + /* Make sure a valid APN works */ + g_object_set (s_gsm, NM_SETTING_GSM_APN, "foobar123.-baz", NULL); + ASSERT (nm_setting_verify (NM_SETTING (s_gsm), NULL, NULL) == TRUE, + "gsm-apn-bad-chars", "unexpectedly invalid GSM setting"); + + /* Random invalid chars */ + g_object_set (s_gsm, NM_SETTING_GSM_APN, "@#%$@#%@#%", NULL); + ASSERT (nm_setting_verify (NM_SETTING (s_gsm), NULL, NULL) == FALSE, + "gsm-apn-bad-chars", "unexpectedly valid GSM setting"); + + /* Spaces */ + g_object_set (s_gsm, NM_SETTING_GSM_APN, "foobar baz", NULL); + ASSERT (nm_setting_verify (NM_SETTING (s_gsm), NULL, NULL) == FALSE, + "gsm-apn-bad-chars", "unexpectedly valid GSM setting"); + + /* 0 characters long */ + g_object_set (s_gsm, NM_SETTING_GSM_APN, "", NULL); + ASSERT (nm_setting_verify (NM_SETTING (s_gsm), NULL, NULL) == FALSE, + "gsm-apn-bad-chars", "unexpectedly valid GSM setting"); + + /* 21-character long */ + g_object_set (s_gsm, NM_SETTING_GSM_APN, "abcdefghijklmnopqrstu", NULL); + ASSERT (nm_setting_verify (NM_SETTING (s_gsm), NULL, NULL) == FALSE, + "gsm-apn-bad-chars", "unexpectedly valid GSM setting"); +} + int main (int argc, char **argv) { GError *error = NULL; @@ -237,6 +304,8 @@ int main (int argc, char **argv) /* The tests */ test_setting_vpn_items (); test_setting_ip6_config_old_address_array (); + test_setting_gsm_apn_spaces (); + test_setting_gsm_apn_bad_chars (); base = g_path_get_basename (argv[0]); fprintf (stdout, "%s: SUCCESS\n", base);