From: Zbigniew Jędrzejewski-Szmek Date: Fri, 23 Nov 2018 14:52:38 +0000 (+0100) Subject: resolved: rework parsing of /etc/hosts X-Git-Tag: v240~102^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=bd0052777981044cf54a1e9d6e3acb1c3d813656;p=thirdparty%2Fsystemd.git 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 c5c180943da..26a9e48c4ca 100644 --- 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 08e178e7f63..0a08294d17e 100644 --- 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 +#include +#include + #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"));