iface-modem-location: validate SUPL server address

Devices will expect SUPL server given as either IP:PORT or FQDN:PORT,
so just avoid saying we require a 'URL' because it's not true.

We will use a new helper method to parse and validate user-provided
SUPL server address.

6 files changed, 80 insertions, 38 deletions

diff --git a/cli/mmcli-modem-location.c b/cli/mmcli-modem-location.c
index 997e1a84a..f96a521df 100644
--- a/cli/mmcli-modem-location.c
+++ b/cli/mmcli-modem-location.c
@@ -145,7 +145,7 @@ static GOptionEntry entries[] = {
     },
     { "location-set-supl-server", 0, 0, G_OPTION_ARG_STRING, &set_supl_server_str,
       "Set SUPL server address",
-      "[IP:PORT] or [URL]"
+      "[IP:PORT] or [FQDN:PORT]"
     },
     { "location-inject-assistance-data", 0, 0, G_OPTION_ARG_FILENAME, &inject_assistance_data_str,
       "Inject assistance data in the GNSS module",
diff --git a/introspection/org.freedesktop.ModemManager1.Modem.Location.xml b/introspection/org.freedesktop.ModemManager1.Modem.Location.xml
index 7c490c6ae..e36f8e0fe 100644
--- a/introspection/org.freedesktop.ModemManager1.Modem.Location.xml
+++ b/introspection/org.freedesktop.ModemManager1.Modem.Location.xml
@@ -71,7 +71,7 @@
 
     <!--
         SetSuplServer:
-        @supl: SUPL server configuration, given either as IP:PORT or with a full URL.
+        @supl: SUPL server configuration, given either as IP:PORT or as FQDN:PORT.
 
         Configure the SUPL server for A-GPS.
     -->
@@ -334,7 +334,7 @@
     <!--
         SuplServer:
 
-        SUPL server configuration for A-GPS, given either as IP:PORT or with a full URL.
+        SUPL server configuration for A-GPS, given either as IP:PORT or FQDN:PORT.
     -->
     <property name="SuplServer" type="s" access="read" />
 
diff --git a/src/mm-iface-modem-location.c b/src/mm-iface-modem-location.c
index e5aaa43d2..88fd2bf82 100644
--- a/src/mm-iface-modem-location.c
+++ b/src/mm-iface-modem-location.c
@@ -21,6 +21,7 @@
 #include "mm-iface-modem.h"
 #include "mm-iface-modem-location.h"
 #include "mm-log.h"
+#include "mm-modem-helpers.h"
 
 #define MM_LOCATION_GPS_REFRESH_TIME_SECS 30
 
@@ -992,6 +993,13 @@ handle_set_supl_server_auth_ready (MMBaseModem *self,
         return;
     }
 
+    /* Validate SUPL address string: either FQDN:PORT or IP:PORT */
+    if (!mm_parse_supl_address (ctx->supl, NULL, NULL, NULL, &error)) {
+        g_dbus_method_invocation_return_gerror (ctx->invocation, error);
+        handle_set_supl_server_context_free (ctx);
+        return;
+    }
+
     /* Check if plugin implements it */
     if (!MM_IFACE_MODEM_LOCATION_GET_INTERFACE (self)->set_supl_server ||
         !MM_IFACE_MODEM_LOCATION_GET_INTERFACE (self)->set_supl_server_finish) {
diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index c25867120..f7ee0f627 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -4372,3 +4372,61 @@ out:
     g_assert (retries >= 0);
     return retries;
 }
+
+/*****************************************************************************/
+
+gboolean
+mm_parse_supl_address (const gchar  *supl,
+                       gchar       **out_fqdn,
+                       guint32      *out_ip,
+                       guint16      *out_port,
+                       GError      **error)
+{
+    gboolean   valid = FALSE;
+    gchar    **split;
+    guint      port;
+    guint32    ip;
+
+    split = g_strsplit (supl, ":", -1);
+    if (g_strv_length (split) != 2) {
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_INVALID_ARGS,
+                     "Invalid SUPL address format: expected FQDN:PORT or IP:PORT");
+        goto out;
+    }
+
+    if (!mm_get_uint_from_str (split[1], &port)) {
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_INVALID_ARGS,
+                     "Invalid SUPL port number specified: not a number: %s", split[1]);
+        goto out;
+    }
+
+    if (port == 0 || port > G_MAXUINT16) {
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_INVALID_ARGS,
+                     "Invalid SUPL port number specified: out of range: %u", port);
+        goto out;
+    }
+
+    /* Port is valid */
+    if (out_port)
+        *out_port = (guint16) port;
+
+    /* Try to parse first item as IP */
+    if (inet_pton (AF_INET, split[0], &ip) <= 0) {
+        /* Otherwise, assume it's a domain name */
+        if (out_fqdn)
+            *out_fqdn = g_strdup (split[0]);
+        if (out_ip)
+            *out_ip = 0;
+    } else {
+        if (out_ip)
+            *out_ip = ip;
+        if (out_fqdn)
+            *out_fqdn = NULL;
+    }
+
+    valid = TRUE;
+
+out:
+    g_strfreev (split);
+    return valid;
+}
diff --git a/src/mm-modem-helpers.h b/src/mm-modem-helpers.h
index 54dcce731..a7c7eaca8 100644
--- a/src/mm-modem-helpers.h
+++ b/src/mm-modem-helpers.h
@@ -404,4 +404,10 @@ gboolean mm_parse_cclk_response (const gchar *response,
 gint mm_parse_csim_response (const gchar *response,
                                    GError **error);
 
+gboolean mm_parse_supl_address (const gchar  *supl,
+                                gchar       **out_fqdn,
+                                guint32      *out_ip,
+                                guint16      *out_port,
+                                GError      **error);
+
 #endif  /* MM_MODEM_HELPERS_H */
diff --git a/src/mm-shared-qmi.c b/src/mm-shared-qmi.c
index dd08d7fd5..5c91aefee 100644
--- a/src/mm-shared-qmi.c
+++ b/src/mm-shared-qmi.c
@@ -113,36 +113,6 @@ set_supl_server_context_free (SetSuplServerContext *ctx)
     g_slice_free (SetSuplServerContext, ctx);
 }
 
-static gboolean
-parse_as_ip_port (const gchar *supl,
-                  guint32     *out_ip,
-                  guint32     *out_port)
-{
-    gboolean   valid = FALSE;
-    gchar    **split;
-    guint      port;
-    guint32    ip;
-
-    split = g_strsplit (supl, ":", -1);
-    if (g_strv_length (split) != 2)
-        goto out;
-
-    if (!mm_get_uint_from_str (split[1], &port))
-        goto out;
-    if (port == 0 || port > G_MAXUINT16)
-        goto out;
-    if (inet_pton (AF_INET, split[0], &ip) <= 0)
-        goto out;
-
-    *out_ip = ip;
-    *out_port = port;
-    valid = TRUE;
-
-out:
-    g_strfreev (split);
-    return valid;
-}
-
 static GArray *
 parse_as_utf16_url (const gchar *supl)
 {
@@ -197,7 +167,7 @@ pds_set_supl_server (GTask *task)
     SetSuplServerContext            *ctx;
     QmiMessagePdsSetAgpsConfigInput *input;
     guint32                          ip;
-    guint32                          port;
+    guint16                          port;
     GArray                          *url;
 
     self = g_task_get_source_object (task);
@@ -211,7 +181,7 @@ pds_set_supl_server (GTask *task)
     else if (mm_iface_modem_is_cdma (MM_IFACE_MODEM (self)))
         qmi_message_pds_set_agps_config_input_set_network_mode (input, QMI_PDS_NETWORK_MODE_CDMA, NULL);
 
-    if (parse_as_ip_port (ctx->supl, &ip, &port))
+    if (mm_parse_supl_address (ctx->supl, NULL, &ip, &port, NULL))
         qmi_message_pds_set_agps_config_input_set_location_server_address (input, ip, port, NULL);
     else {
         url = parse_as_utf16_url (ctx->supl);
@@ -312,7 +282,7 @@ loc_set_supl_server (GTask *task)
     SetSuplServerContext        *ctx;
     QmiMessageLocSetServerInput *input;
     guint32                      ip;
-    guint32                      port;
+    guint16                      port;
 
     self = g_task_get_source_object (task);
     ctx  = g_task_get_task_data (task);
@@ -325,8 +295,8 @@ loc_set_supl_server (GTask *task)
     else if (mm_iface_modem_is_cdma (MM_IFACE_MODEM (self)))
         qmi_message_loc_set_server_input_set_server_type (input, QMI_LOC_SERVER_TYPE_CDMA_PDE, NULL);
 
-    if (parse_as_ip_port (ctx->supl, &ip, &port))
-        qmi_message_loc_set_server_input_set_ipv4 (input, ip, port, NULL);
+    if (mm_parse_supl_address (ctx->supl, NULL, &ip, &port, NULL))
+        qmi_message_loc_set_server_input_set_ipv4 (input, ip, (guint32) port, NULL);
     else
         qmi_message_loc_set_server_input_set_url (input, ctx->supl, NULL);