Merge pull request #9172 from yuwata/timesync-ntp-parser

timesync: check validity of NTP server name or address

src/shared/conf-parser.c

@@ -716,10 +716,8 @@ int config_parse_path(
         assert(rvalue);
         assert(data);
 
-        if (isempty(rvalue)) {
-                n = NULL;
+        if (isempty(rvalue))
                 goto finalize;
-        }
 
         n = strdup(rvalue);
         if (!n)
@@ -730,9 +728,7 @@ int config_parse_path(
                 return fatal ? -ENOEXEC : 0;
 
 finalize:
-        free_and_replace(*s, n);
-
-        return 0;
+        return free_and_replace(*s, n);
 }
 
 int config_parse_strv(

src/timesync/test-timesync.c

@@ -24,9 +24,9 @@ static void test_manager_parse_string(void) {
         assert_se(m->have_fallbacks);
         assert_se(manager_parse_fallback_string(m, NTP_SERVERS) == 0);
 
-        assert_se(manager_parse_server_string(m, SERVER_SYSTEM, "time1.foobar.com time2.foobar.com") == 0);
-        assert_se(manager_parse_server_string(m, SERVER_FALLBACK, "time1.foobar.com time2.foobar.com") == 0);
-        assert_se(manager_parse_server_string(m, SERVER_LINK, "time1.foobar.com time2.foobar.com") == 0);
+        assert_se(manager_parse_server_string(m, SERVER_SYSTEM, "time1.foobar.com time2.foobar.com axrfav.,avf..ra 12345..123") == 0);
+        assert_se(manager_parse_server_string(m, SERVER_FALLBACK, "time1.foobar.com time2.foobar.com axrfav.,avf..ra 12345..123") == 0);
+        assert_se(manager_parse_server_string(m, SERVER_LINK, "time1.foobar.com time2.foobar.com axrfav.,avf..ra 12345..123") == 0);
 }
 
 int main(int argc, char **argv) {

src/timesync/timesyncd-conf.c

@@ -7,6 +7,7 @@
 
 #include "alloc-util.h"
 #include "def.h"
+#include "dns-domain.h"
 #include "extract-word.h"
 #include "string-util.h"
 #include "timesyncd-conf.h"
@@ -36,6 +37,14 @@ int manager_parse_server_string(Manager *m, ServerType type, const char *string)
                 if (r == 0)
                         break;
 
+                r = dns_name_is_valid_or_address(word);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to check validity of NTP server name or address '%s': %m", word);
+                if (r == 0) {
+                        log_error("Invalid NTP server name or address, ignoring: %s", word);
+                        continue;
+                }
+
                 /* Filter out duplicates */
                 LIST_FOREACH(names, n, first)
                         if (streq_ptr(n->string, word)) {

src/timesync/timesyncd-manager.c

@@ -20,6 +20,7 @@
 #include "sd-daemon.h"
 
 #include "alloc-util.h"
+#include "dns-domain.h"
 #include "fd-util.h"
 #include "fs-util.h"
 #include "list.h"
@@ -959,8 +960,13 @@ static int manager_network_read_link_servers(Manager *m) {
         assert(m);
 
         r = sd_network_get_ntp(&ntp);
-        if (r < 0)
+        if (r < 0) {
+                if (r == -ENOMEM)
+                        log_oom();
+                else
+                        log_debug_errno(r, "Failed to get link NTP servers: %m");
                 goto clear;
+        }
 
         LIST_FOREACH(names, n, m->link_servers)
                 n->marked = true;
@@ -968,6 +974,15 @@ static int manager_network_read_link_servers(Manager *m) {
         STRV_FOREACH(i, ntp) {
                 bool found = false;
 
+                r = dns_name_is_valid_or_address(*i);
+                if (r < 0) {
+                        log_error_errno(r, "Failed to check validity of NTP server name or address '%s': %m", *i);
+                        goto clear;
+                } else if (r == 0) {
+                        log_error("Invalid NTP server name or address, ignoring: %s", *i);
+                        continue;
+                }
+
                 LIST_FOREACH(names, n, m->link_servers)
                         if (streq(n->string, *i)) {
                                 n->marked = false;
@@ -977,8 +992,10 @@ static int manager_network_read_link_servers(Manager *m) {
 
                 if (!found) {
                         r = server_name_new(m, NULL, SERVER_LINK, *i);
-                        if (r < 0)
+                        if (r < 0) {
+                                log_oom();
                                 goto clear;
+                        }
 
                         changed = true;
                 }
@@ -1012,6 +1029,7 @@ static int manager_network_event_handler(sd_event_source *s, int fd, uint32_t re
 
         sd_network_monitor_flush(m->network_monitor);
 
+        /* When manager_network_read_link_servers() failed, we assume that the servers are changed. */
         changed = !!manager_network_read_link_servers(m);
 
         /* check if the machine is online */
@@ -1105,7 +1123,7 @@ int manager_new(Manager **ret) {
         if (r < 0)
                 return r;
 
-        manager_network_read_link_servers(m);
+        (void) manager_network_read_link_servers(m);
 
         *ret = TAKE_PTR(m);