From: Lennart Poettering Date: Wed, 2 Feb 2022 14:45:59 +0000 (+0100) Subject: Revert "resolved: filter out our own stub resolvers when parsing servers" X-Git-Tag: v251-rc1~275 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=281df579a725188cf46d420edd6ea2415c8b925e;p=thirdparty%2Fsystemd.git Revert "resolved: filter out our own stub resolvers when parsing servers" This reverts commit 0ad4efb14beea9148838a0d974821e3b98cafc47. See https://github.com/systemd/systemd/pull/20559#issuecomment-1028011030 for reasoning. Quoting: > I think it should be OK to advertise extra stub listeners to local > clients, but you prohibit this now. i.e. there are two different > concepts here, and we shouldn't mix them up: > > 1. tracking configured dns servers and advertise them to local programs > 2. actually using them ourselves > > I am pretty sure that our own stubs are OK for 1 but not OK for 2, > hence we should filter at the time of use not at the time of parse. --- diff --git a/src/resolve/resolved-conf.c b/src/resolve/resolved-conf.c index 7873c363b3a..930313b8442 100644 --- a/src/resolve/resolved-conf.c +++ b/src/resolve/resolved-conf.c @@ -35,16 +35,15 @@ static int manager_add_dns_server_by_string(Manager *m, DnsServerType type, cons if (r < 0) return r; - /* By default, the port number is determined by the transaction feature level. + /* Silently filter out 0.0.0.0, 127.0.0.53, 127.0.0.54 (our own stub DNS listener) */ + if (!dns_server_address_valid(family, &address)) + return 0; + + /* By default, the port number is determined with the transaction feature level. * See dns_transaction_port() and dns_server_port(). */ if (IN_SET(port, 53, 853)) port = 0; - /* Refuse 0.0.0.0, 127.0.0.53, 127.0.0.54 and the rest of our own stub DNS listeners. */ - if (!dns_server_address_valid(family, &address) || - manager_server_address_is_stub(m, family, &address, port ?: 53)) - return -ELOOP; - /* Filter out duplicates */ s = dns_server_find(manager_get_first_dns_server(m, type), family, &address, port, ifindex, server_name); if (s) { @@ -57,7 +56,7 @@ static int manager_add_dns_server_by_string(Manager *m, DnsServerType type, cons return dns_server_new(m, NULL, type, NULL, family, &address, port, ifindex, server_name); } -int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string, bool ignore_self_quietly) { +int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string) { int r; assert(m); @@ -71,10 +70,7 @@ int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, con return r; r = manager_add_dns_server_by_string(m, type, word); - if (r == -ELOOP) - log_full(ignore_self_quietly ? LOG_DEBUG : LOG_INFO, - "DNS server string '%s' points to our own listener, ignoring.", word); - else if (r < 0) + if (r < 0) log_warning_errno(r, "Failed to add DNS server address '%s', ignoring: %m", word); } } @@ -155,7 +151,7 @@ int config_parse_dns_servers( dns_server_unlink_all(manager_get_first_dns_server(m, ltype)); else { /* Otherwise, add to the list */ - r = manager_parse_dns_server_string_and_warn(m, ltype, rvalue, false); + r = manager_parse_dns_server_string_and_warn(m, ltype, rvalue); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse DNS server string '%s', ignoring.", rvalue); @@ -163,7 +159,8 @@ int config_parse_dns_servers( } } - /* If we have a manual setting, then we stop reading /etc/resolv.conf */ + /* If we have a manual setting, then we stop reading + * /etc/resolv.conf */ if (ltype == DNS_SERVER_SYSTEM) m->read_resolv_conf = false; if (ltype == DNS_SERVER_FALLBACK) @@ -205,7 +202,8 @@ int config_parse_search_domains( } } - /* If we have a manual setting, then we stop reading /etc/resolv.conf */ + /* If we have a manual setting, then we stop reading + * /etc/resolv.conf */ m->read_resolv_conf = false; return 0; @@ -487,7 +485,7 @@ int manager_parse_config_file(Manager *m) { return r; if (m->need_builtin_fallbacks) { - r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_FALLBACK, DNS_SERVERS, false); + r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_FALLBACK, DNS_SERVERS); if (r < 0) return r; } diff --git a/src/resolve/resolved-conf.h b/src/resolve/resolved-conf.h index 4639cefbd84..07ce2591a9c 100644 --- a/src/resolve/resolved-conf.h +++ b/src/resolve/resolved-conf.h @@ -8,7 +8,7 @@ int manager_parse_config_file(Manager *m); int manager_parse_search_domains_and_warn(Manager *m, const char *string); -int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string, bool ignore_self_quietly); +int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string); const struct ConfigPerfItem* resolved_gperf_lookup(const char *key, GPERF_LEN_TYPE length); const struct ConfigPerfItem* resolved_dnssd_gperf_lookup(const char *key, GPERF_LEN_TYPE length); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index d10fd73d4d4..615431ebb7e 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1619,37 +1619,30 @@ bool manager_next_dnssd_names(Manager *m) { return tried; } -bool manager_server_address_is_stub(Manager *m, int family, const union in_addr_union *address, uint16_t port) { +bool manager_server_is_stub(Manager *m, DnsServer *s) { DnsStubListenerExtra *l; assert(m); - assert(address); + assert(s); /* Safety check: we generally already skip the main stub when parsing configuration. But let's be * extra careful, and check here again */ - if (family == AF_INET && - address->in.s_addr == htobe32(INADDR_DNS_STUB) && - port == 53) + if (s->family == AF_INET && + s->address.in.s_addr == htobe32(INADDR_DNS_STUB) && + dns_server_port(s) == 53) return true; /* Main reason to call this is to check server data against the extra listeners, and filter things * out. */ ORDERED_SET_FOREACH(l, m->dns_extra_stub_listeners) - if (family == l->family && - in_addr_equal(family, address, &l->address) && - port == dns_stub_listener_extra_port(l)) + if (s->family == l->family && + in_addr_equal(s->family, &s->address, &l->address) && + dns_server_port(s) == dns_stub_listener_extra_port(l)) return true; return false; } -bool manager_server_is_stub(Manager *m, DnsServer *s) { - assert(m); - assert(s); - - return manager_server_address_is_stub(m, s->family, &s->address, dns_server_port(s)); -} - int socket_disable_pmtud(int fd, int af) { int r; diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index e9697389787..35e0068a83e 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -207,7 +207,6 @@ void manager_cleanup_saved_user(Manager *m); bool manager_next_dnssd_names(Manager *m); -bool manager_server_address_is_stub(Manager *m, int family, const union in_addr_union *address, uint16_t port); bool manager_server_is_stub(Manager *m, DnsServer *s); int socket_disable_pmtud(int fd, int af); diff --git a/src/resolve/resolved-resolv-conf.c b/src/resolve/resolved-resolv-conf.c index 0a5c775d317..f6deb2cf460 100644 --- a/src/resolve/resolved-resolv-conf.c +++ b/src/resolve/resolved-resolv-conf.c @@ -141,8 +141,7 @@ int manager_read_resolv_conf(Manager *m) { a = first_word(l, "nameserver"); if (a) { - r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_SYSTEM, a, - true /* don't warn about loops to our own stub listeners */); + r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_SYSTEM, a); if (r < 0) log_warning_errno(r, "Failed to parse DNS server address '%s', ignoring.", a);