From c65f788b9f60ee800ca9c364b79e13796c9e9028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Nov 2018 14:55:07 +0100 Subject: [PATCH 1/6] test-resolved-etc-hosts: when parsing an external file, skip other tests This makes timing tests much more useful. So let's simplify things, and just do the parsing of the external file when that is specified. --- src/resolve/test-resolved-etc-hosts.c | 54 +++++++++++++++------------ 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/resolve/test-resolved-etc-hosts.c b/src/resolve/test-resolved-etc-hosts.c index da10391d74e..08e178e7f63 100644 --- a/src/resolve/test-resolved-etc-hosts.c +++ b/src/resolve/test-resolved-etc-hosts.c @@ -9,6 +9,8 @@ static void test_parse_etc_hosts_system(void) { _cleanup_fclose_ FILE *f = NULL; + log_info("/* %s */", __func__); + f = fopen("/etc/hosts", "re"); if (!f) { assert_se(errno == -ENOENT); @@ -19,38 +21,32 @@ static void test_parse_etc_hosts_system(void) { assert_se(etc_hosts_parse(&hosts, f) == 0); } -static void test_parse_etc_hosts(const char *fname) { +static void test_parse_etc_hosts(void) { _cleanup_(unlink_tempfilep) char t[] = "/tmp/test-resolved-etc-hosts.XXXXXX"; + log_info("/* %s */", __func__); + int fd; _cleanup_fclose_ FILE *f; - if (fname) { - f = fopen(fname, "re"); - assert_se(f); - } else { - fd = mkostemp_safe(t); - assert_se(fd >= 0); + 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); - rewind(f); - } + 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); + rewind(f); _cleanup_(etc_hosts_free) EtcHosts hosts = {}; assert_se(etc_hosts_parse(&hosts, f) == 0); - if (fname) - return; - EtcHostsItemByName *bn; assert_se(bn = hashmap_get(hosts.by_name, "some.where")); assert_se(bn->n_addresses == 3); @@ -79,14 +75,26 @@ static void test_parse_etc_hosts(const char *fname) { assert_se(!set_contains(hosts.no_address, "foobar.foo.foo")); } +static void test_parse_file(const char *fname) { + _cleanup_(etc_hosts_free) EtcHosts hosts = {}; + _cleanup_fclose_ FILE *f; + + log_info("/* %s(\"%s\") */", __func__, fname); + + assert_se(f = fopen(fname, "re")); + assert_se(etc_hosts_parse(&hosts, f) == 0); +} + int main(int argc, char **argv) { log_set_max_level(LOG_DEBUG); log_parse_environment(); log_open(); - if (argc == 1) + if (argc == 1) { test_parse_etc_hosts_system(); - test_parse_etc_hosts(argv[1]); + test_parse_etc_hosts(); + } else + test_parse_file(argv[1]); return 0; } From c9ff152e9262b5199c2209e0d3be600bdd01873b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Nov 2018 22:57:23 +0100 Subject: [PATCH 2/6] test-dns-domain: print function names --- src/test/test-dns-domain.c | 57 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index cbfe5ef3903..0d15d36c19c 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -10,6 +10,8 @@ static void test_dns_label_unescape_one(const char *what, const char *expect, si char buffer[buffer_sz]; int r; + log_info("%s, %s, %zu, →%d", what, expect, buffer_sz, ret); + r = dns_label_unescape(&what, buffer, buffer_sz); assert_se(r == ret); @@ -20,6 +22,8 @@ static void test_dns_label_unescape_one(const char *what, const char *expect, si } static void test_dns_label_unescape(void) { + log_info("/* %s */", __func__); + test_dns_label_unescape_one("hallo", "hallo", 6, 5); test_dns_label_unescape_one("hallo", "hallo", 4, -ENOBUFS); test_dns_label_unescape_one("", "", 10, 0); @@ -39,6 +43,8 @@ static void test_dns_name_to_wire_format_one(const char *what, const char *expec uint8_t buffer[buffer_sz]; int r; + log_info("%s, %s, %zu, →%d", what, expect, buffer_sz, ret); + r = dns_name_to_wire_format(what, buffer, buffer_sz, false); assert_se(r == ret); @@ -80,6 +86,8 @@ static void test_dns_name_to_wire_format(void) { 9, 'a', '1', '2', '3', '4', '5', '6', '7', '8', 3, 'a', '1', '2', 0 }; + log_info("/* %s */", __func__); + test_dns_name_to_wire_format_one("", out0, sizeof(out0), sizeof(out0)); test_dns_name_to_wire_format_one("foo", out1, sizeof(out1), sizeof(out1)); @@ -100,6 +108,8 @@ static void test_dns_label_unescape_suffix_one(const char *what, const char *exp const char *label; int r; + log_info("%s, %s, %s, %zu, %d, %d", what, expect1, expect2, buffer_sz, ret1, ret2); + label = what + strlen(what); r = dns_label_unescape_suffix(what, &label, buffer, buffer_sz); @@ -114,6 +124,8 @@ static void test_dns_label_unescape_suffix_one(const char *what, const char *exp } static void test_dns_label_unescape_suffix(void) { + log_info("/* %s */", __func__); + test_dns_label_unescape_suffix_one("hallo", "hallo", "", 6, 5, 0); test_dns_label_unescape_suffix_one("hallo", "hallo", "", 4, -ENOBUFS, -ENOBUFS); test_dns_label_unescape_suffix_one("", "", "", 10, 0, 0); @@ -139,6 +151,8 @@ static void test_dns_label_escape_one(const char *what, size_t l, const char *ex _cleanup_free_ char *t = NULL; int r; + log_info("%s, %zu, %s, →%d", what, l, expect, ret); + r = dns_label_escape_new(what, l, &t); assert_se(r == ret); @@ -149,6 +163,8 @@ static void test_dns_label_escape_one(const char *what, size_t l, const char *ex } static void test_dns_label_escape(void) { + log_info("/* %s */", __func__); + test_dns_label_escape_one("", 0, NULL, -EINVAL); test_dns_label_escape_one("hallo", 5, "hallo", 5); test_dns_label_escape_one("hallo", 6, "hallo\\000", 9); @@ -340,10 +356,14 @@ static void test_dns_name_concat(void) { } static void test_dns_name_is_valid_one(const char *s, int ret) { + log_info("%s, →%d", s, ret); + assert_se(dns_name_is_valid(s) == ret); } static void test_dns_name_is_valid(void) { + log_info("/* %s */", __func__); + test_dns_name_is_valid_one("foo", 1); test_dns_name_is_valid_one("foo.", 1); test_dns_name_is_valid_one("foo..", 0); @@ -379,6 +399,8 @@ static void test_dns_name_is_valid(void) { } static void test_dns_service_name_is_valid(void) { + log_info("/* %s */", __func__); + assert_se(dns_service_name_is_valid("Lennart's Compüter")); assert_se(dns_service_name_is_valid("piff.paff")); @@ -390,6 +412,7 @@ static void test_dns_service_name_is_valid(void) { } static void test_dns_srv_type_is_valid(void) { + log_info("/* %s */", __func__); assert_se(dns_srv_type_is_valid("_http._tcp")); assert_se(dns_srv_type_is_valid("_foo-bar._tcp")); @@ -413,6 +436,7 @@ static void test_dns_srv_type_is_valid(void) { } static void test_dnssd_srv_type_is_valid(void) { + log_info("/* %s */", __func__); assert_se(dnssd_srv_type_is_valid("_http._tcp")); assert_se(dnssd_srv_type_is_valid("_foo-bar._tcp")); @@ -439,6 +463,8 @@ static void test_dnssd_srv_type_is_valid(void) { static void test_dns_service_join_one(const char *a, const char *b, const char *c, int r, const char *d) { _cleanup_free_ char *x = NULL, *y = NULL, *z = NULL, *t = NULL; + log_info("%s, %s, %s, →%d, %s", a, b, c, r, d); + assert_se(dns_service_join(a, b, c, &t) == r); assert_se(streq_ptr(t, d)); @@ -452,6 +478,8 @@ static void test_dns_service_join_one(const char *a, const char *b, const char * } static void test_dns_service_join(void) { + log_info("/* %s */", __func__); + test_dns_service_join_one("", "", "", -EINVAL, NULL); test_dns_service_join_one("", "_http._tcp", "", -EINVAL, NULL); test_dns_service_join_one("", "_http._tcp", "foo", -EINVAL, NULL); @@ -469,6 +497,8 @@ static void test_dns_service_join(void) { static void test_dns_service_split_one(const char *joined, const char *a, const char *b, const char *c, int r) { _cleanup_free_ char *x = NULL, *y = NULL, *z = NULL, *t = NULL; + log_info("%s, %s, %s, %s, →%d", joined, a, b, c, r); + assert_se(dns_service_split(joined, &x, &y, &z) == r); assert_se(streq_ptr(x, a)); assert_se(streq_ptr(y, b)); @@ -485,6 +515,8 @@ static void test_dns_service_split_one(const char *joined, const char *a, const } static void test_dns_service_split(void) { + log_info("/* %s */", __func__); + test_dns_service_split_one("", NULL, NULL, ".", 0); test_dns_service_split_one("foo", NULL, NULL, "foo", 0); test_dns_service_split_one("foo.bar", NULL, NULL, "foo.bar", 0); @@ -497,11 +529,15 @@ static void test_dns_service_split(void) { static void test_dns_name_change_suffix_one(const char *name, const char *old_suffix, const char *new_suffix, int r, const char *result) { _cleanup_free_ char *s = NULL; + log_info("%s, %s, %s, →%s", name, old_suffix, new_suffix, result); + assert_se(dns_name_change_suffix(name, old_suffix, new_suffix, &s) == r); assert_se(streq_ptr(s, result)); } static void test_dns_name_change_suffix(void) { + log_info("/* %s */", __func__); + test_dns_name_change_suffix_one("foo.bar", "bar", "waldo", 1, "foo.waldo"); test_dns_name_change_suffix_one("foo.bar.waldi.quux", "foo.bar.waldi.quux", "piff.paff", 1, "piff.paff"); test_dns_name_change_suffix_one("foo.bar.waldi.quux", "bar.waldi.quux", "piff.paff", 1, "foo.piff.paff"); @@ -516,11 +552,15 @@ static void test_dns_name_change_suffix(void) { static void test_dns_name_suffix_one(const char *name, unsigned n_labels, const char *result, int ret) { const char *p = NULL; + log_info("%s, %d, →%s, %d", name, n_labels, result, ret); + assert_se(ret == dns_name_suffix(name, n_labels, &p)); assert_se(streq_ptr(p, result)); } static void test_dns_name_suffix(void) { + log_info("/* %s */", __func__); + test_dns_name_suffix_one("foo.bar", 2, "foo.bar", 0); test_dns_name_suffix_one("foo.bar", 1, "bar", 1); test_dns_name_suffix_one("foo.bar", 0, "", 2); @@ -538,10 +578,14 @@ static void test_dns_name_suffix(void) { } static void test_dns_name_count_labels_one(const char *name, int n) { + log_info("%s, →%d", name, n); + assert_se(dns_name_count_labels(name) == n); } static void test_dns_name_count_labels(void) { + log_info("/* %s */", __func__); + test_dns_name_count_labels_one("foo.bar.quux.", 3); test_dns_name_count_labels_one("foo.bar.quux", 3); test_dns_name_count_labels_one("foo.bar.", 2); @@ -554,10 +598,14 @@ static void test_dns_name_count_labels(void) { } static void test_dns_name_equal_skip_one(const char *a, unsigned n_labels, const char *b, int ret) { + log_info("%s, %u, %s, →%d", a, n_labels, b, ret); + assert_se(dns_name_equal_skip(a, n_labels, b) == ret); } static void test_dns_name_equal_skip(void) { + log_info("/* %s */", __func__); + test_dns_name_equal_skip_one("foo", 0, "bar", 0); test_dns_name_equal_skip_one("foo", 0, "foo", 1); test_dns_name_equal_skip_one("foo", 1, "foo", 0); @@ -585,6 +633,8 @@ static void test_dns_name_equal_skip(void) { } static void test_dns_name_compare_func(void) { + log_info("/* %s */", __func__); + assert_se(dns_name_compare_func("", "") == 0); assert_se(dns_name_compare_func("", ".") == 0); assert_se(dns_name_compare_func(".", "") == 0); @@ -600,11 +650,15 @@ static void test_dns_name_compare_func(void) { static void test_dns_name_common_suffix_one(const char *a, const char *b, const char *result) { const char *c; + log_info("%s, %s, →%s", a, b, result); + assert_se(dns_name_common_suffix(a, b, &c) >= 0); assert_se(streq(c, result)); } static void test_dns_name_common_suffix(void) { + log_info("/* %s */", __func__); + test_dns_name_common_suffix_one("", "", ""); test_dns_name_common_suffix_one("foo", "", ""); test_dns_name_common_suffix_one("", "foo", ""); @@ -639,6 +693,7 @@ static void test_dns_name_apply_idna(void) { #else const int ret = 0; #endif + log_info("/* %s */", __func__); /* IDNA2008 forbids names with hyphens in third and fourth positions * (https://tools.ietf.org/html/rfc5891#section-4.2.3.1). @@ -676,6 +731,8 @@ static void test_dns_name_apply_idna(void) { } static void test_dns_name_is_valid_or_address(void) { + log_info("/* %s */", __func__); + assert_se(dns_name_is_valid_or_address(NULL) == 0); assert_se(dns_name_is_valid_or_address("") == 0); assert_se(dns_name_is_valid_or_address("foobar") > 0); From bd0052777981044cf54a1e9d6e3acb1c3d813656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 23 Nov 2018 15:52:38 +0100 Subject: [PATCH 3/6] 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. --- src/resolve/resolved-etc-hosts.c | 41 ++++++------ src/resolve/test-resolved-etc-hosts.c | 95 +++++++++++++++++++++------ 2 files changed, 98 insertions(+), 38 deletions(-) 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->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, "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 = 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")); From 7470cc4c73c3736b93070ec01369e449e40a7cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Nov 2018 22:58:13 +0100 Subject: [PATCH 4/6] resolve: reject host names with leading or trailing dashes in /etc/hosts https://tools.ietf.org/html/rfc1035#section-2.3.1 says (approximately) that only letters, numbers, and non-leading non-trailing dashes are allowed (for entries with A/AAAA records). We set no restrictions. hosts(5) says: > Host names may contain only alphanumeric characters, minus signs ("-"), and > periods ("."). They must begin with an alphabetic character and end with an > alphanumeric character. nss-files follows those rules, and will ignore names in /etc/hosts that do not follow this rule. Let's follow the documented rules for /etc/hosts. In particular, this makes us consitent with nss-files, reducing surprises for the user. I'm pretty sure we should apply stricter filtering to names received over DNS and LLMNR and MDNS, but it's a bigger project, because the rules differ depepending on which level the label appears (rules for top-level names are stricter), and this patch takes the minimalistic approach and only changes behaviour for /etc/hosts. Escape syntax is also disallowed in /etc/hosts, even if the resulting character would be allowed. Other tools that parse /etc/hosts do not support this, and there is no need to use it because no allowed characters benefit from escaping. --- src/libsystemd-network/ndisc-router.c | 2 +- src/libsystemd-network/sd-dhcp-lease.c | 2 +- src/network/networkd-network.c | 2 +- src/resolve/resolved-bus.c | 8 +- src/resolve/resolved-dns-dnssec.c | 8 +- src/resolve/resolved-dns-packet.c | 2 +- src/resolve/resolved-dns-rr.c | 6 +- src/resolve/resolved-dns-search-domain.c | 2 +- src/resolve/resolved-dnssd.c | 4 +- src/resolve/resolved-etc-hosts.c | 2 +- src/resolve/resolved-manager.c | 10 +- src/resolve/test-resolved-etc-hosts.c | 12 +-- src/shared/dns-domain.c | 102 ++++++++++++------- src/shared/dns-domain.h | 28 ++++-- src/test/test-dns-domain.c | 120 +++++++++++++++-------- 15 files changed, 193 insertions(+), 117 deletions(-) diff --git a/src/libsystemd-network/ndisc-router.c b/src/libsystemd-network/ndisc-router.c index b982dcc07df..6935311b9a9 100644 --- a/src/libsystemd-network/ndisc-router.c +++ b/src/libsystemd-network/ndisc-router.c @@ -676,7 +676,7 @@ _public_ int sd_ndisc_router_dnssl_get_domains(sd_ndisc_router *rt, char ***ret) _cleanup_free_ char *normalized = NULL; e[n] = 0; - r = dns_name_normalize(e, &normalized); + r = dns_name_normalize(e, 0, &normalized); if (r < 0) return r; diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index 8275d2f31a6..13badbf0bfb 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -355,7 +355,7 @@ static int lease_parse_domain(const uint8_t *option, size_t len, char **ret) { return 0; } - r = dns_name_normalize(name, &normalized); + r = dns_name_normalize(name, 0, &normalized); if (r < 0) return r; diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 0225f72bdbd..34378887304 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -652,7 +652,7 @@ int config_parse_domains( domain = "."; /* make sure we don't allow empty strings, thus write the root domain as "." */ } else { - r = dns_name_normalize(domain, &normalized); + r = dns_name_normalize(domain, 0, &normalized); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, "'%s' is not a valid domain name, ignoring.", domain); continue; diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 75702d593f4..d80b16c9d98 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -191,7 +191,7 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) { /* The key names are not necessarily normalized, make sure that they are when we return them to our bus * clients. */ - r = dns_name_normalize(dns_resource_key_name(canonical->key), &normalized); + r = dns_name_normalize(dns_resource_key_name(canonical->key), 0, &normalized); if (r < 0) goto finish; @@ -404,7 +404,7 @@ static void bus_method_resolve_address_complete(DnsQuery *q) { if (r == 0) continue; - r = dns_name_normalize(rr->ptr.name, &normalized); + r = dns_name_normalize(rr->ptr.name, 0, &normalized); if (r < 0) goto finish; @@ -742,7 +742,7 @@ static int append_srv(DnsQuery *q, sd_bus_message *reply, DnsResourceRecord *rr) if (r < 0) return r; - r = dns_name_normalize(rr->srv.name, &normalized); + r = dns_name_normalize(rr->srv.name, 0, &normalized); if (r < 0) return r; @@ -798,7 +798,7 @@ static int append_srv(DnsQuery *q, sd_bus_message *reply, DnsResourceRecord *rr) if (canonical) { normalized = mfree(normalized); - r = dns_name_normalize(dns_resource_key_name(canonical->key), &normalized); + r = dns_name_normalize(dns_resource_key_name(canonical->key), 0, &normalized); if (r < 0) return r; } diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 13da4e59916..d9633629e85 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -74,7 +74,7 @@ int dnssec_canonicalize(const char *n, char *buffer, size_t buffer_max) { return -ENOBUFS; for (;;) { - r = dns_label_unescape(&n, buffer, buffer_max); + r = dns_label_unescape(&n, buffer, buffer_max, 0); if (r < 0) return r; if (r == 0) @@ -1705,7 +1705,7 @@ static int dnssec_nsec_wildcard_equal(DnsResourceRecord *rr, const char *name) { return 0; n = dns_resource_key_name(rr->key); - r = dns_label_unescape(&n, label, sizeof(label)); + r = dns_label_unescape(&n, label, sizeof label, 0); if (r <= 0) return r; if (r != 1 || label[0] != '*') @@ -1827,13 +1827,13 @@ static int dnssec_nsec_covers_wildcard(DnsResourceRecord *rr, const char *name) return r; if (r > 0) /* If the name we are interested in is a child of the NSEC RR, then append the asterisk to the NSEC * RR's name. */ - r = dns_name_concat("*", dns_resource_key_name(rr->key), &wc); + r = dns_name_concat("*", dns_resource_key_name(rr->key), 0, &wc); else { r = dns_name_common_suffix(dns_resource_key_name(rr->key), rr->nsec.next_domain_name, &common_suffix); if (r < 0) return r; - r = dns_name_concat("*", common_suffix, &wc); + r = dns_name_concat("*", common_suffix, 0, &wc); } if (r < 0) return r; diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index 0ff444acd06..572271be95c 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -535,7 +535,7 @@ int dns_packet_append_name( } } - r = dns_label_unescape(&name, label, sizeof(label)); + r = dns_label_unescape(&name, label, sizeof label, 0); if (r < 0) goto fail; diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index b67c734e04e..a1dffb08a3a 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -77,7 +77,7 @@ int dns_resource_key_new_append_suffix(DnsResourceKey **ret, DnsResourceKey *key return 0; } - r = dns_name_concat(dns_resource_key_name(key), name, &joined); + r = dns_name_concat(dns_resource_key_name(key), name, 0, &joined); if (r < 0) return r; @@ -222,7 +222,7 @@ int dns_resource_key_match_rr(const DnsResourceKey *key, DnsResourceRecord *rr, if (search_domain) { _cleanup_free_ char *joined = NULL; - r = dns_name_concat(dns_resource_key_name(key), search_domain, &joined); + r = dns_name_concat(dns_resource_key_name(key), search_domain, 0, &joined); if (r < 0) return r; @@ -254,7 +254,7 @@ int dns_resource_key_match_cname_or_dname(const DnsResourceKey *key, const DnsRe if (search_domain) { _cleanup_free_ char *joined = NULL; - r = dns_name_concat(dns_resource_key_name(key), search_domain, &joined); + r = dns_name_concat(dns_resource_key_name(key), search_domain, 0, &joined); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-search-domain.c b/src/resolve/resolved-dns-search-domain.c index 368ec4da19d..21c2442c516 100644 --- a/src/resolve/resolved-dns-search-domain.c +++ b/src/resolve/resolved-dns-search-domain.c @@ -19,7 +19,7 @@ int dns_search_domain_new( assert((type == DNS_SEARCH_DOMAIN_LINK) == !!l); assert(name); - r = dns_name_normalize(name, &normalized); + r = dns_name_normalize(name, 0, &normalized); if (r < 0) return r; diff --git a/src/resolve/resolved-dnssd.c b/src/resolve/resolved-dnssd.c index ea96255dc19..2c28ec227a5 100644 --- a/src/resolve/resolved-dnssd.c +++ b/src/resolve/resolved-dnssd.c @@ -228,10 +228,10 @@ int dnssd_update_rrs(DnssdService *s) { if (r < 0) return r; - r = dns_name_concat(s->type, "local", &service_name); + r = dns_name_concat(s->type, "local", 0, &service_name); if (r < 0) return r; - r = dns_name_concat(n, service_name, &full_name); + r = dns_name_concat(n, service_name, 0, &full_name); if (r < 0) return r; diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c index 26a9e48c4ca..01cde4acf7b 100644 --- a/src/resolve/resolved-etc-hosts.c +++ b/src/resolve/resolved-etc-hosts.c @@ -99,7 +99,7 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) { found = true; - r = dns_name_is_valid(name); + r = dns_name_is_valid_ldh(name); if (r <= 0) { log_warning_errno(r, "/etc/hosts:%u: hostname \"%s\" is not valid, ignoring.", nr, name); continue; diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 2174d660ff0..f6832917c11 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -333,7 +333,7 @@ static int determine_hostname(char **full_hostname, char **llmnr_hostname, char return log_debug_errno(r, "Can't determine system hostname: %m"); p = h; - r = dns_label_unescape(&p, label, sizeof label); + r = dns_label_unescape(&p, label, sizeof label, 0); if (r < 0) return log_error_errno(r, "Failed to unescape host name: %m"); if (r == 0) @@ -371,7 +371,7 @@ static int determine_hostname(char **full_hostname, char **llmnr_hostname, char return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "System hostname is 'localhost', ignoring."); - r = dns_name_concat(n, "local", mdns_hostname); + r = dns_name_concat(n, "local", 0, mdns_hostname); if (r < 0) return log_error_errno(r, "Failed to determine mDNS hostname: %m"); @@ -403,7 +403,7 @@ static int make_fallback_hostnames(char **full_hostname, char **llmnr_hostname, assert(mdns_hostname); p = fallback_hostname(); - r = dns_label_unescape(&p, label, sizeof(label)); + r = dns_label_unescape(&p, label, sizeof label, 0); if (r < 0) return log_error_errno(r, "Failed to unescape fallback host name: %m"); @@ -413,7 +413,7 @@ static int make_fallback_hostnames(char **full_hostname, char **llmnr_hostname, if (r < 0) return log_error_errno(r, "Failed to escape fallback hostname: %m"); - r = dns_name_concat(n, "local", &m); + r = dns_name_concat(n, "local", 0, &m); if (r < 0) return log_error_errno(r, "Failed to concatenate mDNS hostname: %m"); @@ -1147,7 +1147,7 @@ int manager_next_hostname(Manager *m) { if (r < 0) return r; - r = dns_name_concat(h, "local", &k); + r = dns_name_concat(h, "local", 0, &k); if (r < 0) return r; diff --git a/src/resolve/test-resolved-etc-hosts.c b/src/resolve/test-resolved-etc-hosts.c index 0a08294d17e..27e67df01f0 100644 --- a/src/resolve/test-resolved-etc-hosts.c +++ b/src/resolve/test-resolved-etc-hosts.c @@ -91,15 +91,9 @@ static void test_parse_etc_hosts(void) { 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"))); - } + /* See https://tools.ietf.org/html/rfc1035#section-2.3.1 */ + FOREACH_STRING(s, "bad-dash-", "-bad-dash", "-bad-dash.bad-") + assert_se(!hashmap_get(hosts.by_name, s)); assert_se(bn = hashmap_get(hosts.by_name, "before.comment")); assert_se(bn->n_addresses == 4); diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 5ff60fe1912..5dd3734a508 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -24,9 +24,17 @@ #include "strv.h" #include "utf8.h" -int dns_label_unescape(const char **name, char *dest, size_t sz) { +static bool valid_ldh_char(char c) { + return + (c >= 'a' && c <= 'z') || + (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || + c == '-'; +} + +int dns_label_unescape(const char **name, char *dest, size_t sz, DNSLabelFlags flags) { const char *n; - char *d; + char *d, last_char = 0; int r = 0; assert(name); @@ -36,14 +44,16 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { d = dest; for (;;) { - if (*n == '.') { - n++; + if (*n == 0 || *n == '.') { + if (FLAGS_SET(flags, DNS_LABEL_LDH) && last_char == '-') + /* Trailing dash */ + return -EINVAL; + + if (*n == '.') + n++; break; } - if (*n == 0) - break; - if (r >= DNS_LABEL_MAX) return -EINVAL; @@ -52,6 +62,8 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { if (*n == '\\') { /* Escaped character */ + if (FLAGS_SET(flags, DNS_LABEL_NO_ESCAPES)) + return -EINVAL; n++; @@ -62,6 +74,10 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { else if (IN_SET(*n, '\\', '.')) { /* Escaped backslash or dot */ + if (FLAGS_SET(flags, DNS_LABEL_LDH)) + return -EINVAL; + + last_char = *n; if (d) *(d++) = *n; sz--; @@ -90,6 +106,11 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { if (k > 255) return -EINVAL; + if (FLAGS_SET(flags, DNS_LABEL_LDH) && + !valid_ldh_char((char) k)) + return -EINVAL; + + last_char = (char) k; if (d) *(d++) = (char) k; sz--; @@ -103,6 +124,15 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { /* Normal character */ + if (FLAGS_SET(flags, DNS_LABEL_LDH)) { + if (!valid_ldh_char(*n)) + return -EINVAL; + if (r == 0 && *n == '-') + /* Leading dash */ + return -EINVAL; + } + + last_char = *n; if (d) *(d++) = *n; sz--; @@ -184,7 +214,7 @@ int dns_label_unescape_suffix(const char *name, const char **label_terminal, cha terminal--; } - r = dns_label_unescape(&name, dest, sz); + r = dns_label_unescape(&name, dest, sz, 0); if (r < 0) return r; @@ -378,7 +408,7 @@ int dns_label_undo_idna(const char *encoded, size_t encoded_size, char *decoded, } #endif -int dns_name_concat(const char *a, const char *b, char **_ret) { +int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **_ret) { _cleanup_free_ char *ret = NULL; size_t n = 0, allocated = 0; const char *p; @@ -395,7 +425,7 @@ int dns_name_concat(const char *a, const char *b, char **_ret) { for (;;) { char label[DNS_LABEL_MAX]; - r = dns_label_unescape(&p, label, sizeof(label)); + r = dns_label_unescape(&p, label, sizeof label, flags); if (r < 0) return r; if (r == 0) { @@ -468,7 +498,7 @@ void dns_name_hash_func(const char *p, struct siphash *state) { for (;;) { char label[DNS_LABEL_MAX+1]; - r = dns_label_unescape(&p, label, sizeof(label)); + r = dns_label_unescape(&p, label, sizeof label, 0); if (r < 0) break; if (r == 0) @@ -521,11 +551,11 @@ int dns_name_equal(const char *x, const char *y) { for (;;) { char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; - r = dns_label_unescape(&x, la, sizeof(la)); + r = dns_label_unescape(&x, la, sizeof la, 0); if (r < 0) return r; - q = dns_label_unescape(&y, lb, sizeof(lb)); + q = dns_label_unescape(&y, lb, sizeof lb, 0); if (q < 0) return q; @@ -552,14 +582,14 @@ int dns_name_endswith(const char *name, const char *suffix) { for (;;) { char ln[DNS_LABEL_MAX], ls[DNS_LABEL_MAX]; - r = dns_label_unescape(&n, ln, sizeof(ln)); + r = dns_label_unescape(&n, ln, sizeof ln, 0); if (r < 0) return r; if (!saved_n) saved_n = n; - q = dns_label_unescape(&s, ls, sizeof(ls)); + q = dns_label_unescape(&s, ls, sizeof ls, 0); if (q < 0) return q; @@ -590,13 +620,13 @@ int dns_name_startswith(const char *name, const char *prefix) { for (;;) { char ln[DNS_LABEL_MAX], lp[DNS_LABEL_MAX]; - r = dns_label_unescape(&p, lp, sizeof(lp)); + r = dns_label_unescape(&p, lp, sizeof lp, 0); if (r < 0) return r; if (r == 0) return true; - q = dns_label_unescape(&n, ln, sizeof(ln)); + q = dns_label_unescape(&n, ln, sizeof ln, 0); if (q < 0) return q; @@ -625,14 +655,14 @@ int dns_name_change_suffix(const char *name, const char *old_suffix, const char if (!saved_before) saved_before = n; - r = dns_label_unescape(&n, ln, sizeof(ln)); + r = dns_label_unescape(&n, ln, sizeof ln, 0); if (r < 0) return r; if (!saved_after) saved_after = n; - q = dns_label_unescape(&s, ls, sizeof(ls)); + q = dns_label_unescape(&s, ls, sizeof ls, 0); if (q < 0) return q; @@ -655,7 +685,7 @@ int dns_name_change_suffix(const char *name, const char *old_suffix, const char /* Found it! Now generate the new name */ prefix = strndupa(name, saved_before - name); - r = dns_name_concat(prefix, new_suffix, ret); + r = dns_name_concat(prefix, new_suffix, 0, ret); if (r < 0) return r; @@ -733,7 +763,7 @@ int dns_name_address(const char *p, int *family, union in_addr_union *address) { for (i = 0; i < ELEMENTSOF(a); i++) { char label[DNS_LABEL_MAX+1]; - r = dns_label_unescape(&p, label, sizeof(label)); + r = dns_label_unescape(&p, label, sizeof label, 0); if (r < 0) return r; if (r == 0) @@ -770,7 +800,7 @@ int dns_name_address(const char *p, int *family, union in_addr_union *address) { char label[DNS_LABEL_MAX+1]; int x, y; - r = dns_label_unescape(&p, label, sizeof(label)); + r = dns_label_unescape(&p, label, sizeof label, 0); if (r <= 0) return r; if (r != 1) @@ -779,7 +809,7 @@ int dns_name_address(const char *p, int *family, union in_addr_union *address) { if (x < 0) return -EINVAL; - r = dns_label_unescape(&p, label, sizeof(label)); + r = dns_label_unescape(&p, label, sizeof label, 0); if (r <= 0) return r; if (r != 1) @@ -847,7 +877,7 @@ int dns_name_to_wire_format(const char *domain, uint8_t *buffer, size_t len, boo * dns_label_unescape() returns 0 when it hits the end * of the domain name, which we rely on here to encode * the trailing NUL byte. */ - r = dns_label_unescape(&domain, (char *) out, len); + r = dns_label_unescape(&domain, (char *) out, len, 0); if (r < 0) return r; @@ -914,7 +944,7 @@ bool dns_srv_type_is_valid(const char *name) { /* This more or less implements RFC 6335, Section 5.1 */ - r = dns_label_unescape(&name, label, sizeof(label)); + r = dns_label_unescape(&name, label, sizeof label, 0); if (r < 0) return false; if (r == 0) @@ -974,7 +1004,7 @@ int dns_service_join(const char *name, const char *type, const char *domain, cha return -EINVAL; if (!name) - return dns_name_concat(type, domain, ret); + return dns_name_concat(type, domain, 0, ret); if (!dns_service_name_is_valid(name)) return -EINVAL; @@ -983,11 +1013,11 @@ int dns_service_join(const char *name, const char *type, const char *domain, cha if (r < 0) return r; - r = dns_name_concat(type, domain, &n); + r = dns_name_concat(type, domain, 0, &n); if (r < 0) return r; - return dns_name_concat(escaped, n, ret); + return dns_name_concat(escaped, n, 0, ret); } static bool dns_service_name_label_is_valid(const char *label, size_t n) { @@ -1012,7 +1042,7 @@ int dns_service_split(const char *joined, char **_name, char **_type, char **_do assert(joined); /* Get first label from the full name */ - an = dns_label_unescape(&p, a, sizeof(a)); + an = dns_label_unescape(&p, a, sizeof(a), 0); if (an < 0) return an; @@ -1020,7 +1050,7 @@ int dns_service_split(const char *joined, char **_name, char **_type, char **_do x++; /* If there was a first label, try to get the second one */ - bn = dns_label_unescape(&p, b, sizeof(b)); + bn = dns_label_unescape(&p, b, sizeof(b), 0); if (bn < 0) return bn; @@ -1029,7 +1059,7 @@ int dns_service_split(const char *joined, char **_name, char **_type, char **_do /* If there was a second label, try to get the third one */ q = p; - cn = dns_label_unescape(&p, c, sizeof(c)); + cn = dns_label_unescape(&p, c, sizeof(c), 0); if (cn < 0) return cn; @@ -1079,7 +1109,7 @@ int dns_service_split(const char *joined, char **_name, char **_type, char **_do d = joined; finish: - r = dns_name_normalize(d, &domain); + r = dns_name_normalize(d, 0, &domain); if (r < 0) return r; @@ -1224,12 +1254,12 @@ int dns_name_common_suffix(const char *a, const char *b, const char **ret) { } x = a_labels[n - 1 - k]; - r = dns_label_unescape(&x, la, sizeof(la)); + r = dns_label_unescape(&x, la, sizeof la, 0); if (r < 0) return r; y = b_labels[m - 1 - k]; - q = dns_label_unescape(&y, lb, sizeof(lb)); + q = dns_label_unescape(&y, lb, sizeof lb, 0); if (q < 0) return q; @@ -1297,13 +1327,13 @@ int dns_name_apply_idna(const char *name, char **ret) { for (;;) { char label[DNS_LABEL_MAX]; - r = dns_label_unescape(&name, label, sizeof(label)); + r = dns_label_unescape(&name, label, sizeof label, 0); if (r < 0) return r; if (r == 0) break; - q = dns_label_apply_idna(label, r, label, sizeof(label)); + q = dns_label_apply_idna(label, r, label, sizeof label); if (q < 0) return q; if (q > 0) diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index 42492ad7c0c..6ed512c6b16 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -24,13 +24,18 @@ /* Maximum number of labels per valid hostname */ #define DNS_N_LABELS_MAX 127 -int dns_label_unescape(const char **name, char *dest, size_t sz); +typedef enum DNSLabelFlags { + DNS_LABEL_LDH = 1 << 0, /* Follow the "LDH" rule — only letters, digits, and internal hyphens. */ + DNS_LABEL_NO_ESCAPES = 1 << 1, /* Do not treat backslashes specially */ +} DNSLabelFlags; + +int dns_label_unescape(const char **name, char *dest, size_t sz, DNSLabelFlags flags); int dns_label_unescape_suffix(const char *name, const char **label_end, char *dest, size_t sz); int dns_label_escape(const char *p, size_t l, char *dest, size_t sz); int dns_label_escape_new(const char *p, size_t l, char **ret); static inline int dns_name_parent(const char **name) { - return dns_label_unescape(name, NULL, DNS_LABEL_MAX); + return dns_label_unescape(name, NULL, DNS_LABEL_MAX, 0); } #if HAVE_LIBIDN @@ -38,18 +43,29 @@ int dns_label_apply_idna(const char *encoded, size_t encoded_size, char *decoded int dns_label_undo_idna(const char *encoded, size_t encoded_size, char *decoded, size_t decoded_max); #endif -int dns_name_concat(const char *a, const char *b, char **ret); +int dns_name_concat(const char *a, const char *b, DNSLabelFlags flags, char **ret); -static inline int dns_name_normalize(const char *s, char **ret) { +static inline int dns_name_normalize(const char *s, DNSLabelFlags flags, char **ret) { /* dns_name_concat() normalizes as a side-effect */ - return dns_name_concat(s, NULL, ret); + return dns_name_concat(s, NULL, flags, ret); } static inline int dns_name_is_valid(const char *s) { int r; /* dns_name_normalize() verifies as a side effect */ - r = dns_name_normalize(s, NULL); + r = dns_name_normalize(s, 0, NULL); + if (r == -EINVAL) + return 0; + if (r < 0) + return r; + return 1; +} + +static inline int dns_name_is_valid_ldh(const char *s) { + int r; + + r = dns_name_concat(s, NULL, DNS_LABEL_LDH|DNS_LABEL_NO_ESCAPES, NULL); if (r == -EINVAL) return 0; if (r < 0) diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index 0d15d36c19c..ead5311705d 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -6,37 +6,57 @@ #include "string-util.h" #include "tests.h" -static void test_dns_label_unescape_one(const char *what, const char *expect, size_t buffer_sz, int ret) { +static void test_dns_label_unescape_one(const char *what, const char *expect, size_t buffer_sz, int ret, int ret_ldh) { char buffer[buffer_sz]; int r; + const char *w = what; - log_info("%s, %s, %zu, →%d", what, expect, buffer_sz, ret); + log_info("%s, %s, %zu, →%d/%d", what, expect, buffer_sz, ret, ret_ldh); - r = dns_label_unescape(&what, buffer, buffer_sz); + r = dns_label_unescape(&w, buffer, buffer_sz, 0); assert_se(r == ret); + if (r >= 0) + assert_se(streq(buffer, expect)); - if (r < 0) - return; + w = what; + r = dns_label_unescape(&w, buffer, buffer_sz, DNS_LABEL_LDH); + assert_se(r == ret_ldh); + if (r >= 0) + assert_se(streq(buffer, expect)); - assert_se(streq(buffer, expect)); + w = what; + r = dns_label_unescape(&w, buffer, buffer_sz, DNS_LABEL_NO_ESCAPES); + const int ret_noe = strchr(what, '\\') ? -EINVAL : ret; + assert_se(r == ret_noe); + if (r >= 0) + assert_se(streq(buffer, expect)); } static void test_dns_label_unescape(void) { log_info("/* %s */", __func__); - test_dns_label_unescape_one("hallo", "hallo", 6, 5); - test_dns_label_unescape_one("hallo", "hallo", 4, -ENOBUFS); - test_dns_label_unescape_one("", "", 10, 0); - test_dns_label_unescape_one("hallo\\.foobar", "hallo.foobar", 20, 12); - test_dns_label_unescape_one("hallo.foobar", "hallo", 10, 5); - test_dns_label_unescape_one("hallo\n.foobar", "hallo", 20, -EINVAL); - test_dns_label_unescape_one("hallo\\", "hallo", 20, -EINVAL); - test_dns_label_unescape_one("hallo\\032 ", "hallo ", 20, 7); - test_dns_label_unescape_one(".", "", 20, 0); - test_dns_label_unescape_one("..", "", 20, -EINVAL); - test_dns_label_unescape_one(".foobar", "", 20, -EINVAL); - test_dns_label_unescape_one("foobar.", "foobar", 20, 6); - test_dns_label_unescape_one("foobar..", "foobar", 20, -EINVAL); + test_dns_label_unescape_one("hallo", "hallo", 6, 5, 5); + test_dns_label_unescape_one("hallo", "hallo", 4, -ENOBUFS, -ENOBUFS); + test_dns_label_unescape_one("", "", 10, 0, 0); + test_dns_label_unescape_one("hallo\\.foobar", "hallo.foobar", 20, 12, -EINVAL); + test_dns_label_unescape_one("hallo.foobar", "hallo", 10, 5, 5); + test_dns_label_unescape_one("hallo\n.foobar", "hallo", 20, -EINVAL, -EINVAL); + test_dns_label_unescape_one("hallo\\", "hallo", 20, -EINVAL, -EINVAL); + test_dns_label_unescape_one("hallo\\032 ", "hallo ", 20, 7, -EINVAL); + test_dns_label_unescape_one(".", "", 20, 0, 0); + test_dns_label_unescape_one("..", "", 20, -EINVAL, -EINVAL); + test_dns_label_unescape_one(".foobar", "", 20, -EINVAL, -EINVAL); + test_dns_label_unescape_one("foobar.", "foobar", 20, 6, 6); + test_dns_label_unescape_one("foobar..", "foobar", 20, -EINVAL, -EINVAL); + test_dns_label_unescape_one("foo-bar", "foo-bar", 20, 7, 7); + test_dns_label_unescape_one("foo-", "foo-", 20, 4, -EINVAL); + test_dns_label_unescape_one("-foo", "-foo", 20, 4, -EINVAL); + test_dns_label_unescape_one("-foo-", "-foo-", 20, 5, -EINVAL); + test_dns_label_unescape_one("foo-.", "foo-", 20, 4, -EINVAL); + test_dns_label_unescape_one("foo.-", "foo", 20, 3, 3); + test_dns_label_unescape_one("foo\\032", "foo ", 20, 4, -EINVAL); + test_dns_label_unescape_one("foo\\045", "foo-", 20, 4, -EINVAL); + test_dns_label_unescape_one("głąb", "głąb", 20, 6, -EINVAL); } static void test_dns_name_to_wire_format_one(const char *what, const char *expect, size_t buffer_sz, int ret) { @@ -175,7 +195,7 @@ static void test_dns_name_normalize_one(const char *what, const char *expect, in _cleanup_free_ char *t = NULL; int r; - r = dns_name_normalize(what, &t); + r = dns_name_normalize(what, 0, &t); assert_se(r == ret); if (r < 0) @@ -336,7 +356,7 @@ static void test_dns_name_reverse(void) { static void test_dns_name_concat_one(const char *a, const char *b, int r, const char *result) { _cleanup_free_ char *p = NULL; - assert_se(dns_name_concat(a, b, &p) == r); + assert_se(dns_name_concat(a, b, 0, &p) == r); assert_se(streq_ptr(p, result)); } @@ -355,47 +375,63 @@ static void test_dns_name_concat(void) { test_dns_name_concat_one(NULL, "foo", 0, "foo"); } -static void test_dns_name_is_valid_one(const char *s, int ret) { +static void test_dns_name_is_valid_one(const char *s, int ret, int ret_ldh) { log_info("%s, →%d", s, ret); assert_se(dns_name_is_valid(s) == ret); + assert_se(dns_name_is_valid_ldh(s) == ret_ldh); } static void test_dns_name_is_valid(void) { log_info("/* %s */", __func__); - test_dns_name_is_valid_one("foo", 1); - test_dns_name_is_valid_one("foo.", 1); - test_dns_name_is_valid_one("foo..", 0); - test_dns_name_is_valid_one("Foo", 1); - test_dns_name_is_valid_one("foo.bar", 1); - test_dns_name_is_valid_one("foo.bar.baz", 1); - test_dns_name_is_valid_one("", 1); - test_dns_name_is_valid_one("foo..bar", 0); - test_dns_name_is_valid_one(".foo.bar", 0); - test_dns_name_is_valid_one("foo.bar.", 1); - test_dns_name_is_valid_one("foo.bar..", 0); - test_dns_name_is_valid_one("\\zbar", 0); - test_dns_name_is_valid_one("ä", 1); - test_dns_name_is_valid_one("\n", 0); + test_dns_name_is_valid_one("foo", 1, 1); + test_dns_name_is_valid_one("foo.", 1, 1); + test_dns_name_is_valid_one("foo..", 0, 0); + test_dns_name_is_valid_one("Foo", 1, 1); + test_dns_name_is_valid_one("foo.bar", 1, 1); + test_dns_name_is_valid_one("foo.bar.baz", 1, 1); + test_dns_name_is_valid_one("", 1, 1); + test_dns_name_is_valid_one("foo..bar", 0, 0); + test_dns_name_is_valid_one(".foo.bar", 0, 0); + test_dns_name_is_valid_one("foo.bar.", 1, 1); + test_dns_name_is_valid_one("foo.bar..", 0, 0); + test_dns_name_is_valid_one("\\zbar", 0, 0); + test_dns_name_is_valid_one("ä", 1, 0); + test_dns_name_is_valid_one("\n", 0, 0); + + test_dns_name_is_valid_one("dash-", 1, 0); + test_dns_name_is_valid_one("-dash", 1, 0); + test_dns_name_is_valid_one("dash-dash", 1, 1); + test_dns_name_is_valid_one("foo.dash-", 1, 0); + test_dns_name_is_valid_one("foo.-dash", 1, 0); + test_dns_name_is_valid_one("foo.dash-dash", 1, 1); + test_dns_name_is_valid_one("foo.dash-.bar", 1, 0); + test_dns_name_is_valid_one("foo.-dash.bar", 1, 0); + test_dns_name_is_valid_one("foo.dash-dash.bar", 1, 1); + test_dns_name_is_valid_one("dash-.bar", 1, 0); + test_dns_name_is_valid_one("-dash.bar", 1, 0); + test_dns_name_is_valid_one("dash-dash.bar", 1, 1); + test_dns_name_is_valid_one("-.bar", 1, 0); + test_dns_name_is_valid_one("foo.-", 1, 0); /* 256 characters */ - test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345", 0); + test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345", 0, 0); /* 255 characters */ - test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a1234", 0); + test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a1234", 0, 0); /* 254 characters */ - test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a123", 0); + test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a123", 0, 0); /* 253 characters */ - test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12", 1); + test_dns_name_is_valid_one("a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12345678.a12", 1, 1); /* label of 64 chars length */ - test_dns_name_is_valid_one("a123456789a123456789a123456789a123456789a123456789a123456789a123", 0); + test_dns_name_is_valid_one("a123456789a123456789a123456789a123456789a123456789a123456789a123", 0, 0); /* label of 63 chars length */ - test_dns_name_is_valid_one("a123456789a123456789a123456789a123456789a123456789a123456789a12", 1); + test_dns_name_is_valid_one("a123456789a123456789a123456789a123456789a123456789a123456789a12", 1, 1); } static void test_dns_service_name_is_valid(void) { From d65652f1f21a4b0c59711320f34266c635393c89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 5 Dec 2018 12:26:29 +0100 Subject: [PATCH 5/6] Partially unify hostname_is_valid() and dns_name_is_valid() This makes hostname_is_valid() apply the ldh checks too, rejecting more hostnames. --- src/basic/hostname-util.c | 56 ++++++++++++++++++++++++----------- src/basic/hostname-util.h | 1 + src/shared/dns-domain.c | 9 +----- src/test/test-hostname-util.c | 6 ++++ 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index 3a3479910d7..0230821d22a 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -69,12 +69,12 @@ int gethostname_strict(char **ret) { return 0; } -static bool hostname_valid_char(char c) { +bool valid_ldh_char(char c) { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || - IN_SET(c, '-', '_', '.'); + c == '-'; } /** @@ -90,7 +90,7 @@ static bool hostname_valid_char(char c) { bool hostname_is_valid(const char *s, bool allow_trailing_dot) { unsigned n_dots = 0; const char *p; - bool dot; + bool dot, hyphen; if (isempty(s)) return false; @@ -100,23 +100,34 @@ bool hostname_is_valid(const char *s, bool allow_trailing_dot) { * sequence. Also ensures that the length stays below * HOST_NAME_MAX. */ - for (p = s, dot = true; *p; p++) { + for (p = s, dot = hyphen = true; *p; p++) if (*p == '.') { - if (dot) + if (dot || hyphen) return false; dot = true; + hyphen = false; n_dots++; - } else { - if (!hostname_valid_char(*p)) + + } else if (*p == '-') { + if (dot) return false; dot = false; + hyphen = true; + + } else { + if (!valid_ldh_char(*p)) + return false; + + dot = false; + hyphen = false; } - } if (dot && (n_dots < 2 || !allow_trailing_dot)) return false; + if (hyphen) + return false; if (p-s > HOST_NAME_MAX) /* Note that HOST_NAME_MAX is 64 on * Linux, but DNS allows domain names @@ -128,29 +139,40 @@ bool hostname_is_valid(const char *s, bool allow_trailing_dot) { char* hostname_cleanup(char *s) { char *p, *d; - bool dot; + bool dot, hyphen; assert(s); strshorten(s, HOST_NAME_MAX); - for (p = s, d = s, dot = true; *p; p++) { + for (p = s, d = s, dot = hyphen = true; *p; p++) if (*p == '.') { - if (dot) + if (dot || hyphen) continue; *(d++) = '.'; dot = true; - } else if (hostname_valid_char(*p)) { + hyphen = false; + + } else if (*p == '-') { + if (dot) + continue; + + *(d++) = '-'; + dot = false; + hyphen = true; + + } else if (valid_ldh_char(*p)) { *(d++) = *p; dot = false; + hyphen = false; } - } - if (dot && d > s) - d[-1] = 0; - else - *d = 0; + if (d > s && IN_SET(d[-1], '-', '.')) + /* The dot can occur at most once, but we might have multiple + * hyphens, hence the loop */ + d--; + *d = 0; return s; } diff --git a/src/basic/hostname-util.h b/src/basic/hostname-util.h index 749481723de..7ba386a0fd9 100644 --- a/src/basic/hostname-util.h +++ b/src/basic/hostname-util.h @@ -11,6 +11,7 @@ bool hostname_is_set(void); char* gethostname_malloc(void); int gethostname_strict(char **ret); +bool valid_ldh_char(char c) _const_; bool hostname_is_valid(const char *s, bool allow_trailing_dot) _pure_; char* hostname_cleanup(char *s); diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 5dd3734a508..4b31cb36ede 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -17,6 +17,7 @@ #include "dns-domain.h" #include "hashmap.h" #include "hexdecoct.h" +#include "hostname-util.h" #include "in-addr-util.h" #include "macro.h" #include "parse-util.h" @@ -24,14 +25,6 @@ #include "strv.h" #include "utf8.h" -static bool valid_ldh_char(char c) { - return - (c >= 'a' && c <= 'z') || - (c >= 'A' && c <= 'Z') || - (c >= '0' && c <= '9') || - c == '-'; -} - int dns_label_unescape(const char **name, char *dest, size_t sz, DNSLabelFlags flags) { const char *n; char *d, last_char = 0; diff --git a/src/test/test-hostname-util.c b/src/test/test-hostname-util.c index 8ba47d316ba..596830f5f2a 100644 --- a/src/test/test-hostname-util.c +++ b/src/test/test-hostname-util.c @@ -53,6 +53,12 @@ static void test_hostname_cleanup(void) { assert_se(streq(hostname_cleanup(s), "foobar.com")); s = strdupa("foobar.com."); assert_se(streq(hostname_cleanup(s), "foobar.com")); + s = strdupa("foo-bar.-com-."); + assert_se(streq(hostname_cleanup(s), "foo-bar.com")); + s = strdupa("foo-bar-.-com-."); + assert_se(streq(hostname_cleanup(s), "foo-bar--com")); + s = strdupa("--foo-bar.-com"); + assert_se(streq(hostname_cleanup(s), "foo-bar.com")); s = strdupa("fooBAR"); assert_se(streq(hostname_cleanup(s), "fooBAR")); s = strdupa("fooBAR.com"); From 5fe7a0a7de9027bbae1a20738da613d6591feec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 5 Dec 2018 12:27:29 +0100 Subject: [PATCH 6/6] basic/hostname-util: do truncation last when cleaning up This allows more of the original name to be used if there are invalid chars in the beginning. --- src/basic/hostname-util.c | 4 +--- src/test/test-hostname-util.c | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index 0230821d22a..5bfa028b393 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -143,9 +143,7 @@ char* hostname_cleanup(char *s) { assert(s); - strshorten(s, HOST_NAME_MAX); - - for (p = s, d = s, dot = hyphen = true; *p; p++) + for (p = s, d = s, dot = hyphen = true; *p && d - s < HOST_NAME_MAX; p++) if (*p == '.') { if (dot || hyphen) continue; diff --git a/src/test/test-hostname-util.c b/src/test/test-hostname-util.c index 596830f5f2a..4126a24ceb9 100644 --- a/src/test/test-hostname-util.c +++ b/src/test/test-hostname-util.c @@ -85,6 +85,8 @@ static void test_hostname_cleanup(void) { assert_se(streq(hostname_cleanup(s), "foo.bar")); s = strdupa("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); assert_se(streq(hostname_cleanup(s), "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")); + s = strdupa("xxxx........xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); + assert_se(streq(hostname_cleanup(s), "xxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")); } static void test_read_etc_hostname(void) {