resolved: rework parsing of /etc/hosts

Do not treat various errors (missing hostname, invalid address) as fatal,
just warn and continue. /etc/hosts is written by humans and we should not
reject the whole file just because a singly entry is not to our liking.

Handle comments as described in hosts(5):
everything from the comment character until the end of the line should be
ignored.

Fixes #10779.

Add tests.

diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c
index c5c180943da086578cf4a3ca9073cbabdb616356..26a9e48c4ca9900378087caf610d48e8b5af1bd5 100644 (file)
--- a/src/resolve/resolved-etc-hosts.c
+++ b/src/resolve/resolved-etc-hosts.c
@@ -46,19 +46,20 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) {
 
         r = extract_first_word(&line, &address_str, NULL, EXTRACT_RELAX);
         if (r < 0)
-                return log_error_errno(r, "Couldn't extract address, in line /etc/hosts:%u.", nr);
-        if (r == 0)
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Premature end of line, in line /etc/hosts:%u.",
-                                       nr);
+                return log_error_errno(r, "/etc/hosts:%u: failed to extract address: %m", nr);
+        assert(r > 0); /* We already checked that the line is not empty, so it should contain *something* */
 
         r = in_addr_ifindex_from_string_auto(address_str, &address.family, &address.address, NULL);
-        if (r < 0)
-                return log_error_errno(r, "Address '%s' is invalid, in line /etc/hosts:%u.", address_str, nr);
+        if (r < 0) {
+                log_warning_errno(r, "/etc/hosts:%u: address '%s' is invalid, ignoring: %m", nr, address_str);
+                return 0;
+        }
 
         r = in_addr_is_null(address.family, &address.address);
-        if (r < 0)
-                return r;
+        if (r < 0) {
+                log_warning_errno(r, "/etc/hosts:%u: address '%s' is invalid, ignoring: %m", nr, address_str);
+                return 0;
+        }
         if (r > 0)
                 /* This is an 0.0.0.0 or :: item, which we assume means that we shall map the specified hostname to
                  * nothing. */
@@ -92,16 +93,18 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) {
 
                 r = extract_first_word(&line, &name, NULL, EXTRACT_RELAX);
                 if (r < 0)
-                        return log_error_errno(r, "Couldn't extract host name, in line /etc/hosts:%u.", nr);
+                        return log_error_errno(r, "/etc/hosts:%u: couldn't extract host name: %m", nr);
                 if (r == 0)
                         break;
 
-                r = dns_name_is_valid(name);
-                if (r <= 0)
-                        return log_error_errno(r, "Hostname %s is not valid, ignoring, in line /etc/hosts:%u.", name, nr);
-
                 found = true;
 
+                r = dns_name_is_valid(name);
+                if (r <= 0) {
+                        log_warning_errno(r, "/etc/hosts:%u: hostname \"%s\" is not valid, ignoring.", nr, name);
+                        continue;
+                }
+
                 if (is_localhost(name))
                         /* Suppress the "localhost" line that is often seen */
                         continue;
@@ -152,9 +155,7 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) {
         }
 
         if (!found)
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Line is missing any host names, in line /etc/hosts:%u.",
-                                       nr);
+                log_warning("/etc/hosts:%u: line is missing any host names", nr);
 
         return 0;
 }
@@ -176,11 +177,13 @@ int etc_hosts_parse(EtcHosts *hosts, FILE *f) {
 
                 nr++;
 
+                l = strchr(line, '#');
+                if (l)
+                        *l = '\0';
+
                 l = strstrip(line);
                 if (isempty(l))
                         continue;
-                if (l[0] == '#')
-                        continue;
 
                 r = parse_line(&t, nr, l);
                 if (r < 0)

diff --git a/src/resolve/test-resolved-etc-hosts.c b/src/resolve/test-resolved-etc-hosts.c
index 08e178e7f63cd45df94b417223428956f4cb40be..0a08294d17effa559b47dfc9108672ac74f64ee6 100644 (file)
--- a/src/resolve/test-resolved-etc-hosts.c
+++ b/src/resolve/test-resolved-etc-hosts.c
@@ -1,9 +1,15 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
 #include "fd-util.h"
+#include "fileio.h"
 #include "fs-util.h"
 #include "log.h"
 #include "resolved-etc-hosts.h"
+#include "strv.h"
 #include "tmpfile-util.h"
 
 static void test_parse_etc_hosts_system(void) {
@@ -21,6 +27,14 @@ static void test_parse_etc_hosts_system(void) {
         assert_se(etc_hosts_parse(&hosts, f) == 0);
 }
 
+#define address_equal_4(_addr, _address)                                \
+        ((_addr)->family == AF_INET &&                                  \
+         !memcmp(&(_addr)->address.in, &(struct in_addr) { .s_addr = (_address) }, 4))
+
+#define address_equal_6(_addr, ...)                                     \
+        ((_addr)->family == AF_INET6 &&                                 \
+         !memcmp(&(_addr)->address.in6, &(struct in6_addr) { .s6_addr = __VA_ARGS__}, 16) )
+
 static void test_parse_etc_hosts(void) {
         _cleanup_(unlink_tempfilep) char
                 t[] = "/tmp/test-resolved-etc-hosts.XXXXXX";
@@ -29,19 +43,31 @@ static void test_parse_etc_hosts(void) {
 
         int fd;
         _cleanup_fclose_ FILE *f;
+        const char *s;
 
         fd = mkostemp_safe(t);
         assert_se(fd >= 0);
 
         f = fdopen(fd, "r+");
         assert_se(f);
-        fputs("1.2.3.4 some.where\n", f);
-        fputs("1.2.3.5 some.where\n", f);
-        fputs("::0 some.where some.other\n", f);
-        fputs("0.0.0.0 black.listed\n", f);
-        fputs("::5 some.where some.other foobar.foo.foo\n", f);
-        fputs("        \n", f);
-        fflush(f);
+        fputs("1.2.3.4 some.where\n"
+              "1.2.3.5 some.where\n"
+              "1.2.3.6 dash dash-dash.where-dash\n"
+              "1.2.3.7 bad-dash- -bad-dash -bad-dash.bad-\n"
+              "1.2.3.8\n"
+              "1.2.3.9 before.comment # within.comment\n"
+              "1.2.3.10 before.comment#within.comment2\n"
+              "1.2.3.11 before.comment# within.comment3\n"
+              "1.2.3.12 before.comment#\n"
+              "1.2.3 short.address\n"
+              "1.2.3.4.5 long.address\n"
+              "1::2::3 multi.colon\n"
+
+              "::0 some.where some.other\n"
+              "0.0.0.0 black.listed\n"
+              "::5\t\t\t \tsome.where\tsome.other foobar.foo.foo\t\t\t\n"
+              "        \n", f);
+        assert_se(fflush_and_check(f) >= 0);
         rewind(f);
 
         _cleanup_(etc_hosts_free) EtcHosts hosts = {};
@@ -51,23 +77,54 @@ static void test_parse_etc_hosts(void) {
         assert_se(bn = hashmap_get(hosts.by_name, "some.where"));
         assert_se(bn->n_addresses == 3);
         assert_se(bn->n_allocated >= 3);
+        assert_se(address_equal_4(bn->addresses[0], inet_addr("1.2.3.4")));
+        assert_se(address_equal_4(bn->addresses[1], inet_addr("1.2.3.5")));
+        assert_se(address_equal_6(bn->addresses[2], {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5}));
+
+        assert_se(bn = hashmap_get(hosts.by_name, "dash"));
+        assert_se(bn->n_addresses == 1);
+        assert_se(bn->n_allocated >= 1);
+        assert_se(address_equal_4(bn->addresses[0], inet_addr("1.2.3.6")));
+
+        assert_se(bn = hashmap_get(hosts.by_name, "dash-dash.where-dash"));
+        assert_se(bn->n_addresses == 1);
+        assert_se(bn->n_allocated >= 1);
+        assert_se(address_equal_4(bn->addresses[0], inet_addr("1.2.3.6")));
+
+        /* Those names do not follow the LDH rule, but so far we allow them.
+         * Let's make this explicit by adding a test.
+         * See https://tools.ietf.org/html/rfc1035#section-2.3.1 */
+        FOREACH_STRING(s, "bad-dash-", "-bad-dash", "-bad-dash.bad-") {
+                assert_se(bn = hashmap_get(hosts.by_name, s));
+                assert_se(bn->n_addresses == 1);
+                assert_se(bn->n_allocated >= 1);
+                assert_se(address_equal_4(bn->addresses[0], inet_addr("1.2.3.7")));
+        }
 
-        assert_se(bn->addresses[0]->family == AF_INET);
-        assert_se(memcmp(&bn->addresses[0]->address.in,
-                         &(struct in_addr) { .s_addr = htobe32(0x01020304) }, 4) == 0);
-        assert_se(bn->addresses[1]->family == AF_INET);
-        assert_se(memcmp(&bn->addresses[1]->address.in,
-                         &(struct in_addr) { .s_addr = htobe32(0x01020305) }, 4) == 0);
-        assert_se(bn->addresses[2]->family == AF_INET6);
-        assert_se(memcmp(&bn->addresses[2]->address.in6,
-                         &(struct in6_addr) { .s6_addr = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5} }, 16 ) == 0);
+        assert_se(bn = hashmap_get(hosts.by_name, "before.comment"));
+        assert_se(bn->n_addresses == 4);
+        assert_se(bn->n_allocated >= 4);
+        assert_se(address_equal_4(bn->addresses[0], inet_addr("1.2.3.9")));
+        assert_se(address_equal_4(bn->addresses[1], inet_addr("1.2.3.10")));
+        assert_se(address_equal_4(bn->addresses[2], inet_addr("1.2.3.11")));
+        assert_se(address_equal_4(bn->addresses[3], inet_addr("1.2.3.12")));
+
+        assert(!hashmap_get(hosts.by_name, "within.comment"));
+        assert(!hashmap_get(hosts.by_name, "within.comment2"));
+        assert(!hashmap_get(hosts.by_name, "within.comment3"));
+        assert(!hashmap_get(hosts.by_name, "#"));
+
+        assert(!hashmap_get(hosts.by_name, "short.address"));
+        assert(!hashmap_get(hosts.by_name, "long.address"));
+        assert(!hashmap_get(hosts.by_name, "multi.colon"));
+        assert_se(!set_contains(hosts.no_address, "short.address"));
+        assert_se(!set_contains(hosts.no_address, "long.address"));
+        assert_se(!set_contains(hosts.no_address, "multi.colon"));
 
         assert_se(bn = hashmap_get(hosts.by_name, "some.other"));
         assert_se(bn->n_addresses == 1);
         assert_se(bn->n_allocated >= 1);
-        assert_se(bn->addresses[0]->family == AF_INET6);
-        assert_se(memcmp(&bn->addresses[0]->address.in6,
-                         &(struct in6_addr) { .s6_addr = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5} }, 16 ) == 0);
+        assert_se(address_equal_6(bn->addresses[0], {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5}));
 
         assert_se( set_contains(hosts.no_address, "some.where"));
         assert_se( set_contains(hosts.no_address, "some.other"));