libnm: verify and normalize "connection.secondaries"

So far, we didn't verify the secondary connections at all.
But these really are supposed to be UUIDs.

As we now also normalize "connection.uuid" to be in a strict
format, the user might have profiles with non-normalized UUIDs.
In that case, the "connection.uuid" would be normalized, but
"connection.secondaries" no longer matches. We can fix that by
also normalizing "connection.secondaries". OK, this is not a very good
reason, because it's unlikely to affect any users in practice ('though
it's easy to reproduce).

A better reason is that the secondary setting really should be well
defined and verified. As we didn't do that so far, we cannot simply
outright reject invalid settings. What this patch does instead, is
silently changing the profile to only contain valid settings.
That has it's own problems, like that the user setting an invalid
value does not get an error nor the desired(?) outcome.
But of all the bad choices, normalizing seems the most sensible
one.

Note that in practice, most client applications don't rely on setting
arbitrary (invalid) "UUIDs". They simply expect to be able to set valid
UUIDs, which they still are. For example, nm-connection-editor presents
a drop down list of VPN profile, and nmcli also resolves connection IDs
to the UUID. That is, clients already have an intimate understanding of
this setting, and don't blindly set arbitrary values. Hence, this
normalization is unlikely to hit users in practice. But what it gives
is the guarantee that a verified connection only contains valid UUIDs.

Now all UUIDs will be normalized, invalid entries removed, and the list
made unique.

4 files changed, 123 insertions, 0 deletions

diff --git a/src/libnm-core-impl/nm-connection-private.h b/src/libnm-core-impl/nm-connection-private.h
index 532b17637b..0580625f4a 100644
--- a/src/libnm-core-impl/nm-connection-private.h
+++ b/src/libnm-core-impl/nm-connection-private.h
@@ -27,6 +27,8 @@ gboolean _nm_connection_detect_slave_type_full(NMSettingConnection *s_con,
 
 const char *_nm_connection_detect_bluetooth_type(NMConnection *self);
 
+gboolean _nm_setting_connection_verify_secondaries(GArray *secondaries, GError **error);
+
 gboolean _nm_connection_verify_required_interface_name(NMConnection *connection, GError **error);
 
 int _nm_setting_ovs_interface_verify_interface_type(NMSettingOvsInterface *self,
diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c
index 5818a41f34..35a14a80b8 100644
--- a/src/libnm-core-impl/nm-connection.c
+++ b/src/libnm-core-impl/nm-connection.c
@@ -725,6 +725,113 @@ _normalize_connection_uuid(NMConnection *self)
     return TRUE;
 }
 
+gboolean
+_nm_setting_connection_verify_secondaries(GArray *secondaries, GError **error)
+{
+    const char *const *strv;
+    const guint        len              = nm_g_array_len(secondaries);
+    guint              has_normalizable = FALSE;
+    gboolean           has_invalid      = FALSE;
+    gboolean           has_duplicate    = FALSE;
+    guint              i;
+
+    if (len == 0)
+        return TRUE;
+
+    /* For historic reasons, the secondaries were not normalized/validated.
+     *
+     * Now, when we find any invalid/non-normalized values, we reject/normalize
+     * them. We also filter out duplicates. */
+
+    strv = nm_strvarray_get_strv_non_empty(secondaries, NULL);
+
+    for (i = 0; i < len; i++) {
+        const char *uuid = strv[i];
+        gboolean    normalized;
+
+        if (!nm_uuid_is_valid_nm(uuid, &normalized, NULL)) {
+            has_invalid = TRUE;
+            goto out;
+        }
+        if (normalized)
+            has_normalizable = TRUE;
+    }
+    if (has_normalizable)
+        goto out;
+
+    if (len > 1) {
+        gs_free const char **strv_to_free = NULL;
+        const char **        strv2;
+
+        strv2 = nm_utils_strv_dup_shallow_maybe_a(20, strv, len, &strv_to_free);
+        nm_utils_strv_sort(strv2, len);
+        has_duplicate = nm_strv_has_duplicate(strv2, len, TRUE);
+    }
+
+out:
+    if (has_invalid) {
+        g_set_error_literal(error,
+                            NM_CONNECTION_ERROR,
+                            NM_CONNECTION_ERROR_INVALID_PROPERTY,
+                            _("has an invalid UUID"));
+    } else if (has_normalizable) {
+        g_set_error_literal(error,
+                            NM_CONNECTION_ERROR,
+                            NM_CONNECTION_ERROR_INVALID_PROPERTY,
+                            _("has a UUID that requires normalization"));
+    } else if (has_duplicate) {
+        g_set_error_literal(error,
+                            NM_CONNECTION_ERROR,
+                            NM_CONNECTION_ERROR_INVALID_PROPERTY,
+                            _("has duplicate UUIDs"));
+    } else
+        return TRUE;
+
+    g_prefix_error(error,
+                   "%s.%s: ",
+                   NM_SETTING_CONNECTION_SETTING_NAME,
+                   NM_SETTING_CONNECTION_SECONDARIES);
+    return FALSE;
+}
+
+static gboolean
+_normalize_connection_secondaries(NMConnection *self)
+{
+    NMSettingConnection *s_con = nm_connection_get_setting_connection(self);
+    GArray *             secondaries;
+    gs_strfreev char **  strv = NULL;
+    guint                i;
+    guint                j;
+
+    nm_assert(s_con);
+
+    secondaries = _nm_setting_connection_get_secondaries(s_con);
+    if (nm_g_array_len(secondaries) == 0)
+        return FALSE;
+
+    if (_nm_setting_connection_verify_secondaries(secondaries, NULL))
+        return FALSE;
+
+    strv = nm_strvarray_get_strv_non_empty_dup(secondaries, NULL);
+    for (i = 0, j = 0; strv[i]; i++) {
+        gs_free char *s = g_steal_pointer(&strv[i]);
+        char          uuid_normalized[37];
+        gboolean      uuid_is_normalized;
+
+        if (!nm_uuid_is_valid_nm(s, &uuid_is_normalized, uuid_normalized))
+            continue;
+
+        if (nm_utils_strv_find_first(strv, j, uuid_is_normalized ? uuid_normalized : s) >= 0)
+            continue;
+
+        strv[j++] = uuid_is_normalized ? g_strdup(uuid_normalized) : g_steal_pointer(&s);
+    }
+    strv[j] = NULL;
+
+    g_object_set(s_con, NM_SETTING_CONNECTION_SECONDARIES, strv, NULL);
+    return TRUE;
+}
+
 static gboolean
 _normalize_connection_type(NMConnection *self)
 {
@@ -1617,6 +1724,7 @@ _connection_normalize(NMConnection *connection,
     was_modified |= _normalize_connection_uuid(connection);
     was_modified |= _normalize_connection_type(connection);
     was_modified |= _normalize_connection_slave_type(connection);
+    was_modified |= _normalize_connection_secondaries(connection);
     was_modified |= _normalize_required_settings(connection);
     was_modified |= _normalize_invalid_slave_port_settings(connection);
     was_modified |= _normalize_ip_config(connection, parameters);
diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c
index e490cf5001..110f2937b0 100644
--- a/src/libnm-core-impl/nm-setting-connection.c
+++ b/src/libnm-core-impl/nm-setting-connection.c
@@ -741,6 +741,12 @@ nm_setting_connection_get_autoconnect_slaves(NMSettingConnection *setting)
     return NM_SETTING_CONNECTION_GET_PRIVATE(setting)->autoconnect_slaves;
 }
 
+GArray *
+_nm_setting_connection_get_secondaries(NMSettingConnection *setting)
+{
+    return NM_SETTING_CONNECTION_GET_PRIVATE(setting)->secondaries;
+}
+
 /**
  * nm_setting_connection_get_num_secondaries:
  * @setting: the #NMSettingConnection
@@ -1475,6 +1481,9 @@ after_interface_name:
         return NM_SETTING_VERIFY_NORMALIZABLE;
     }
 
+    if (!_nm_setting_connection_verify_secondaries(priv->secondaries, error))
+        return NM_SETTING_VERIFY_NORMALIZABLE;
+
     return TRUE;
 }
 
diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h
index d4e7bf4e38..d01e432c68 100644
--- a/src/libnm-core-intern/nm-core-internal.h
+++ b/src/libnm-core-intern/nm-core-internal.h
@@ -515,6 +515,10 @@ GPtrArray *_nm_setting_bridge_port_get_vlans(NMSettingBridgePort *setting);
 
 /*****************************************************************************/
 
+GArray *_nm_setting_connection_get_secondaries(NMSettingConnection *setting);
+
+/*****************************************************************************/
+
 NMSettingBluetooth *_nm_connection_get_setting_bluetooth_for_nap(NMConnection *connection);
 
 /*****************************************************************************/