lte core: additional properties for the default context

Many LTE networks require user authentication, even for the default context. In particular, most of the private APNs use this facility to add some control on top of the MNO providing the service, so that another user of the same network cannot access the private one. As such, we add these parameters to the default context settings that will attempt to use when registering to the network.

The additional parameters added by this patch are: protocol, user, and password. These are sufficient to allow to connect to networks available to the patch author where ofono previously failed to register to the network at all.

Co-authored-by Martin Baschin <[email protected]>

---
  src/lte.c | 241 +++++++++++++++++++++++++++++++++++++++++-------------
  1 file changed, 185 insertions(+), 56 deletions(-)

  
  	DBG("%s error %d", path, error->type);

src/lte.c +23 -2

@@ -118,55 +174,68 @@ static void lte_set_default_attach_info_cb(const struct ofono_error *error,
  		return;
  	}
 
-	g_strlcpy(lte->info.apn, lte->pending_info.apn,
-			OFONO_GPRS_MAX_APN_LENGTH + 1);
+	if (!g_str_equal(lte->pending_info.apn, lte->info.apn)) {
+		g_strlcpy(lte->info.apn, lte->pending_info.apn,
+					OFONO_GPRS_MAX_APN_LENGTH + 1);
+		propstr = lte->info.apn;
+	} else if (lte->pending_info.proto != lte->info.proto) {
+		lte->info.proto = lte->pending_info.proto;
+		key = LTE_PROTO;
+		propstr = gprs_proto_to_string(lte->info.proto);
+	} else if (lte->pending_info.auth_method != lte->info.auth_method) {
+		lte->info.auth_method = lte->pending_info.auth_method;
+		propstr = gprs_auth_method_to_string(lte->info.auth_method);
+
+	} else if (!g_str_equal(lte->pending_info.username,
+							lte->info.username)) {
+		g_strlcpy(lte->info.username, lte->pending_info.username,
+					OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+		propstr = lte->info.username;
+	} else if (!g_str_equal(lte->pending_info.password,
+							lte->info.password)) {
+		g_strlcpy(lte->info.password, lte->pending_info.password,
+					OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
+		propstr = lte->info.password;
+	} else {

src/lte.c +17 -6

+
+	reply = dbus_message_new_method_return(lte->pending);
+	__ofono_dbus_pending_reply(&lte->pending, reply);
+
+	if(!key)
+		return;
 
  	if (lte->settings) {
-		if (strlen(lte->info.apn) == 0)
-			/* Clear entry on empty APN. */
-			g_key_file_remove_key(lte->settings, SETTINGS_GROUP,
-						DEFAULT_APN_KEY, NULL);
+		/*
+		 * the following code removes from storage empty APN, user, pwd
+		 * for proto and auth_method, given that they always
+		 * have defaults, it will not do anything.
+		 */
+		if (!*propstr)
+			/* Clear entry on empty string. */
+			g_key_file_remove_key(lte->settings,
+				SETTINGS_GROUP, key, NULL);
  		else
-			g_key_file_set_string(lte->settings, SETTINGS_GROUP,
-						DEFAULT_APN_KEY, lte->info.apn);
+			g_key_file_set_string(lte->settings,
+				SETTINGS_GROUP, key, propstr);
 

src/lte.c +3 -27

-}
-
-static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
-				DBusConnection *conn, DBusMessage *msg,
-				const char *apn)
-{
-	if (lte->driver->set_default_attach_info == NULL)
-		return __ofono_error_not_implemented(msg);
-
-	if (lte->pending)
-		return __ofono_error_busy(msg);
-
-	if (g_str_equal(apn, lte->info.apn))
-		return dbus_message_new_method_return(msg);
-
-	/* We do care about empty value: it can be used for reset. */
-	if (is_valid_apn(apn) == FALSE && apn[0] != '\0')
-		return __ofono_error_invalid_format(msg);
-
-	lte->pending = dbus_message_ref(msg);
-
-	g_strlcpy(lte->pending_info.apn, apn, OFONO_GPRS_MAX_APN_LENGTH + 1);
-
-	lte->driver->set_default_attach_info(lte, &lte->pending_info,
-					lte_set_default_attach_info_cb, lte);
-
-	return NULL;
+					key,
+					DBUS_TYPE_STRING, &propstr);
+	g_free(key);
  }
 
  static DBusMessage *lte_set_property(DBusConnection *conn,

@@ -177,6 +246,14 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
  	DBusMessageIter var;
  	const char *property;

src/lte.c +44 -7

  	if (!dbus_message_iter_init(msg, &iter))
  		return __ofono_error_invalid_args(msg);

@@ -192,16 +269,68 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
 
  	dbus_message_iter_recurse(&iter, &var);
 
-	if (!strcmp(property, DEFAULT_APN_KEY)) {
-		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
-			return __ofono_error_invalid_args(msg);
+	if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
 
-		dbus_message_iter_get_basic(&var, &str);
+	dbus_message_iter_get_basic(&var, &str);
 
-		return lte_set_default_apn(lte, conn, msg, str);
-	}
+	if ((strcmp(property, LTE_APN) == 0)) {
+
+		if (g_str_equal(str, lte->info.apn))
+			return dbus_message_new_method_return(msg);
+
+		/* We do care about empty value: it can be used for reset. */
+		if (is_valid_apn(str) == FALSE && str[0] != '\0')
+			return __ofono_error_invalid_format(msg);
+
+		g_strlcpy(lte->pending_info.apn, str,
+					OFONO_GPRS_MAX_APN_LENGTH + 1);
+
+	} else if ((strcmp(property, LTE_PROTO) == 0)) {
+
+		if (!gprs_proto_from_string(str, &proto))
+			return __ofono_error_invalid_format(msg);
+
+		if (proto == lte->info.proto)
+			return dbus_message_new_method_return(msg);
+
+		lte->pending_info.proto = proto;
 
-	return __ofono_error_invalid_args(msg);
+	} else if (strcmp(property, LTE_AUTH_METHOD) == 0) {
+
+		if (!gprs_auth_method_from_string(str, &auth_method))
+			return __ofono_error_invalid_format(msg);
+
+		if (auth_method == lte->info.auth_method)
+			return dbus_message_new_method_return(msg);
+
+		lte->pending_info.auth_method = auth_method;
+
+	} else if (strcmp(property, LTE_USERNAME) == 0) {
+
+		if (g_str_equal(str, lte->info.username))
+			return dbus_message_new_method_return(msg);
+
+		g_strlcpy(lte->pending_info.username, str,
+					OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+
+	} else if (strcmp(property, LTE_PASSWORD) == 0) {
+

Hi Giacinto,

Quoted 12 lines · Giacinto Cifelli

Many LTE networks require user authentication, even for the default context. In particular, most of the private APNs use this facility to add some control on top of the MNO providing the service, so that another user of the same network cannot access the private one. As such, we add these parameters to the default context settings that will attempt to use when registering to the network.

The additional parameters added by this patch are: protocol, user, and password. These are sufficient to allow to connect to networks available to the patch author where ofono previously failed to register to the network at all.

Quoted 5 lines

Co-authored-by Martin Baschin <[email protected]>

---
  src/lte.c | 241 +++++++++++++++++++++++++++++++++++++++++-------------
  1 file changed, 185 insertions(+), 56 deletions(-)

So I'm fine with the overall approach taken here, but the actual implementation still has issues:

src/lte.c static DBusMessage *lte_get_properties(DBusConnection *conn, · line 167

Quoted 11 lines

static void lte_set_default_attach_info_cb(const struct ofono_error *error,

src/lte.c +3 -2

-                                             void *data)
+                                                             void *data)
  {
      struct ofono_lte *lte = data;
      const char *path = __ofono_atom_get_path(lte->atom);
      DBusConnection *conn = ofono_dbus_get_connection();
      DBusMessage *reply;
-     const char *apn = lte->info.apn;
+     const char *propstr = NULL;
+     char *key = g_strdup(lte->key);

I'd do this differently:

Quoted 3 lines

      DBG("%s error %d", path, error->type);

char *key;
char *value;
const char *str;
DBusMessageIter iter;
DbusMessageIter var;

....

// No error checking needed since we already validated pending:

dbus_message_iter_init(lte->pending, &iter);
dbus_message_iter_get_basic(&iter, &str);
key = strdup(str);

dbus_message_iter_next(&iter);
dbus_message_iter_recurse(&iter, &var);
dbus_message_iter_get_basic(&var, &str);
value = strdup(str);

Quoted 29 lines

src/lte.c +23 -2

@@ -118,55 +174,68 @@ static void lte_set_default_attach_info_cb(const struct ofono_error *error,
              return;
      }
 
-     g_strlcpy(lte->info.apn, lte->pending_info.apn,
-                     OFONO_GPRS_MAX_APN_LENGTH + 1);
+     if (!g_str_equal(lte->pending_info.apn, lte->info.apn)) {
+             g_strlcpy(lte->info.apn, lte->pending_info.apn,
+                                     OFONO_GPRS_MAX_APN_LENGTH + 1);
+             propstr = lte->info.apn;
+     } else if (lte->pending_info.proto != lte->info.proto) {
+             lte->info.proto = lte->pending_info.proto;
+             key = LTE_PROTO;
+             propstr = gprs_proto_to_string(lte->info.proto);
+     } else if (lte->pending_info.auth_method != lte->info.auth_method) {
+             lte->info.auth_method = lte->pending_info.auth_method;
+             propstr = gprs_auth_method_to_string(lte->info.auth_method);
+
+     } else if (!g_str_equal(lte->pending_info.username,
+                                                     lte->info.username)) {
+             g_strlcpy(lte->info.username, lte->pending_info.username,
+                                     OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+             propstr = lte->info.username;
+     } else if (!g_str_equal(lte->pending_info.password,
+                                                     lte->info.password)) {
+             g_strlcpy(lte->info.password, lte->pending_info.password,
+                                     OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
+             propstr = lte->info.password;
+     } else {

src/lte.c static void lte_set_default_attach_info_cb(const struct ofono_error *error, · line 209

Quoted 10 lines

src/lte.c +10 -0

+             /*
+              * this should never happen, because no property change is
+              * checked before.
+              * Neverthelss, in this case it will answer the D-Bus message
+              * but emit no signal
+              */
+             ofono_error("unexpected property change: ignored");
+             g_free(key);
+             key = NULL;
+     }

Get rid of this entire if/else block and simply do:

memcpy(&lte->info, &lte->pending_info, sizeof(lte->info));

Quoted 28 lines

src/lte.c +17 -6

+
+     reply = dbus_message_new_method_return(lte->pending);
+     __ofono_dbus_pending_reply(&lte->pending, reply);
+
+     if(!key)
+             return;
 
      if (lte->settings) {
-             if (strlen(lte->info.apn) == 0)
-                     /* Clear entry on empty APN. */
-                     g_key_file_remove_key(lte->settings, SETTINGS_GROUP,
-                                             DEFAULT_APN_KEY, NULL);
+             /*
+              * the following code removes from storage empty APN, user, pwd
+              * for proto and auth_method, given that they always
+              * have defaults, it will not do anything.
+              */
+             if (!*propstr)
+                     /* Clear entry on empty string. */
+                     g_key_file_remove_key(lte->settings,
+                             SETTINGS_GROUP, key, NULL);
              else
-                     g_key_file_set_string(lte->settings, SETTINGS_GROUP,
-                                             DEFAULT_APN_KEY, lte->info.apn);
+                     g_key_file_set_string(lte->settings,
+                             SETTINGS_GROUP, key, propstr);
 

src/lte.c static void lte_set_default_attach_info_cb(const struct ofono_error *error, · line 142

Quoted 10 lines

src/lte.c +0 -5

              storage_sync(lte->imsi, SETTINGS_STORE, lte->settings);
      }
 
-     reply = dbus_message_new_method_return(lte->pending);
-     __ofono_dbus_pending_reply(&lte->pending, reply);
-
      ofono_dbus_signal_property_changed(conn, path,
                                      OFONO_CONNECTION_CONTEXT_INTERFACE,
-                                     DEFAULT_APN_KEY,
-                                     DBUS_TYPE_STRING, &apn);

g_free(value);

Quoted 36 lines

src/lte.c +3 -27

-}
-
-static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
-                             DBusConnection *conn, DBusMessage *msg,
-                             const char *apn)
-{
-     if (lte->driver->set_default_attach_info == NULL)
-             return __ofono_error_not_implemented(msg);
-
-     if (lte->pending)
-             return __ofono_error_busy(msg);
-
-     if (g_str_equal(apn, lte->info.apn))
-             return dbus_message_new_method_return(msg);
-
-     /* We do care about empty value: it can be used for reset. */
-     if (is_valid_apn(apn) == FALSE && apn[0] != '\0')
-             return __ofono_error_invalid_format(msg);
-
-     lte->pending = dbus_message_ref(msg);
-
-     g_strlcpy(lte->pending_info.apn, apn, OFONO_GPRS_MAX_APN_LENGTH + 1);
-
-     lte->driver->set_default_attach_info(lte, &lte->pending_info,
-                                     lte_set_default_attach_info_cb, lte);
-
-     return NULL;
+                                     key,
+                                     DBUS_TYPE_STRING, &propstr);
+     g_free(key);
  }
 
  static DBusMessage *lte_set_property(DBusConnection *conn,

@@ -177,6 +246,14 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
      DBusMessageIter var;
      const char *property;

src/lte.c static DBusMessage *lte_set_property(DBusConnection *conn, · line 257

Quoted 10 lines

src/lte.c +8 -0

      const char *str;
+     enum ofono_gprs_auth_method auth_method;
+     enum ofono_gprs_proto proto;
+
+     if (lte->driver->set_default_attach_info == NULL)
+             return __ofono_error_not_implemented(msg);
+
+     if (lte->pending)
+             return __ofono_error_busy(msg);
 

So you may want to do:

memcpy(&lte->pending_info, &lte->info, sizeof(lte->info));

Quoted 60 lines

src/lte.c +44 -7

      if (!dbus_message_iter_init(msg, &iter))
              return __ofono_error_invalid_args(msg);

@@ -192,16 +269,68 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
 
      dbus_message_iter_recurse(&iter, &var);
 
-     if (!strcmp(property, DEFAULT_APN_KEY)) {
-             if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
-                     return __ofono_error_invalid_args(msg);
+     if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+             return __ofono_error_invalid_args(msg);
 
-             dbus_message_iter_get_basic(&var, &str);
+     dbus_message_iter_get_basic(&var, &str);
 
-             return lte_set_default_apn(lte, conn, msg, str);
-     }
+     if ((strcmp(property, LTE_APN) == 0)) {
+
+             if (g_str_equal(str, lte->info.apn))
+                     return dbus_message_new_method_return(msg);
+
+             /* We do care about empty value: it can be used for reset. */
+             if (is_valid_apn(str) == FALSE && str[0] != '\0')
+                     return __ofono_error_invalid_format(msg);
+
+             g_strlcpy(lte->pending_info.apn, str,
+                                     OFONO_GPRS_MAX_APN_LENGTH + 1);
+
+     } else if ((strcmp(property, LTE_PROTO) == 0)) {
+
+             if (!gprs_proto_from_string(str, &proto))
+                     return __ofono_error_invalid_format(msg);
+
+             if (proto == lte->info.proto)
+                     return dbus_message_new_method_return(msg);
+
+             lte->pending_info.proto = proto;
 
-     return __ofono_error_invalid_args(msg);
+     } else if (strcmp(property, LTE_AUTH_METHOD) == 0) {
+
+             if (!gprs_auth_method_from_string(str, &auth_method))
+                     return __ofono_error_invalid_format(msg);
+
+             if (auth_method == lte->info.auth_method)
+                     return dbus_message_new_method_return(msg);
+
+             lte->pending_info.auth_method = auth_method;
+
+     } else if (strcmp(property, LTE_USERNAME) == 0) {
+
+             if (g_str_equal(str, lte->info.username))
+                     return dbus_message_new_method_return(msg);
+
+             g_strlcpy(lte->pending_info.username, str,
+                                     OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+
+     } else if (strcmp(property, LTE_PASSWORD) == 0) {
+

src/lte.c static DBusMessage *lte_set_property(DBusConnection *conn, · line 328

Quoted 10 lines

src/lte.c +10 -0

+             if (g_str_equal(str, lte->info.password))
+                     return dbus_message_new_method_return(msg);
+
+             g_strlcpy(lte->pending_info.password, str,
+                                     OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
+
+     } else
+             return __ofono_error_invalid_args(msg);
+
+     lte->key = property;

And this isn't needed. Or alternatively store both the key and value being set to in ofono_lte. But I much prefer just parsing the message again.

Quoted 7 lines

src/lte.c +5 -0

+     lte->pending = dbus_message_ref(msg);
+     lte->driver->set_default_attach_info(lte, &lte->pending_info,
+                                     lte_set_default_attach_info_cb, lte);
+
+     return NULL;
  }
 

src/lte.c static DBusMessage *lte_set_property(DBusConnection *conn, · line 336

Quoted 2 lines

src/lte.c +0 -0

  static const GDBusMethodTable lte_methods[] = {

Regards,
-Denis