From 3266203bf1579207f1206b6220a5943e0b7905bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= <ihuguet@redhat.com>
Date: Fri, 25 Apr 2025 09:19:55 +0200
Subject: [PATCH] dns: ensure that no wrong separators are used for DNS search
 domains

If wrong separators are used in they keyfile, like commas, the whole
line is considered as a single domain string, like "a.org,b.org".
Obviously this is invalid.

Ideally we should validate that the string is a valid domain, but this
gets quite complex if we want to support unicode characters, which are
valid for many top domains. For now, validate at least that no wrong
separators have been used.

Fixes https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1740
---
 src/libnm-core-impl/nm-setting-ip-config.c | 22 ++++++++++++
 src/libnm-core-impl/tests/test-setting.c   | 39 ++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c
index 6c77d954b6..20aca478d8 100644
--- a/src/libnm-core-impl/nm-setting-ip-config.c
+++ b/src/libnm-core-impl/nm-setting-ip-config.c
@@ -5671,6 +5671,28 @@ verify(NMSetting *setting, NMConnection *connection, GError **error)
         }
     }
 
+    /* Validate DNS search domains */
+    if (nm_strvarray_get_strv_notempty(priv->dns_search.arr, NULL)) {
+        for (i = 0; i < priv->dns_search.arr->len; i++) {
+            const char *dns_search = nm_strvarray_get_idx(priv->dns_search.arr, i);
+
+            /* TODO: currently we only check that no wrong list separators have
+             * been used by mistake. Proper domain name validation would be better. */
+            if (strpbrk(dns_search, ",; ")) {
+                g_set_error(error,
+                            NM_CONNECTION_ERROR,
+                            NM_CONNECTION_ERROR_INVALID_PROPERTY,
+                            _("DNS search domain '%s' is invalid"),
+                            dns_search);
+                g_prefix_error(error,
+                               "%s.%s: ",
+                               nm_setting_get_name(setting),
+                               NM_SETTING_IP_CONFIG_DNS_SEARCH);
+                return FALSE;
+            }
+        }
+    }
+
     /* Validate addresses */
     for (i = 0; i < priv->addresses->len; i++) {
         NMIPAddress *addr = (NMIPAddress *) priv->addresses->pdata[i];
diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c
index 2064162a9f..ee0fa1586c 100644
--- a/src/libnm-core-impl/tests/test-setting.c
+++ b/src/libnm-core-impl/tests/test-setting.c
@@ -5460,6 +5460,44 @@ test_settings_dns(void)
     }
 }
 
+static void
+_assert_dns_searches(gboolean valid, ...)
+{
+    NMConnection      *con;
+    NMSettingIPConfig *ip4, *ip6;
+    const char        *dns_search;
+    va_list            args;
+
+    con = nmtst_create_minimal_connection("test-dns-search",
+                                          NULL,
+                                          NM_SETTING_WIRED_SETTING_NAME,
+                                          NULL);
+    nmtst_connection_normalize(con);
+    ip4 = nm_connection_get_setting_ip4_config(con);
+    ip6 = nm_connection_get_setting_ip6_config(con);
+
+    va_start(args, valid);
+    while ((dns_search = va_arg(args, const char *))) {
+        nm_setting_ip_config_add_dns_search(ip4, dns_search);
+        nm_setting_ip_config_add_dns_search(ip6, dns_search);
+    }
+    va_end(args);
+
+    g_assert(valid == nm_setting_verify((NMSetting *) ip4, con, NULL));
+    g_assert(valid == nm_setting_verify((NMSetting *) ip6, con, NULL));
+}
+
+static void
+test_settings_dns_search_domains(void)
+{
+    _assert_dns_searches(TRUE, "example.com", NULL);
+    _assert_dns_searches(TRUE, "sub.example.com", NULL);
+    _assert_dns_searches(TRUE, "example.com", "sub.example.com", NULL);
+    _assert_dns_searches(FALSE, "example.com,sub.example.com", NULL);
+    _assert_dns_searches(FALSE, "example.com;sub.example.com", NULL);
+    _assert_dns_searches(FALSE, "example.com sub.example.com", NULL);
+}
+
 /*****************************************************************************/
 
 static void
@@ -5570,6 +5608,7 @@ main(int argc, char **argv)
     g_test_add_func("/libnm/settings/6lowpan/1", test_6lowpan_1);
 
     g_test_add_func("/libnm/settings/dns", test_settings_dns);
+    g_test_add_func("/libnm/settings/dns_search_domain", test_settings_dns_search_domains);
 
     g_test_add_func("/libnm/settings/sriov/vf", test_sriov_vf);
     g_test_add_func("/libnm/settings/sriov/vf-dup", test_sriov_vf_dup);