Skip to content

Commit 281df57

Browse files
committed
Revert "resolved: filter out our own stub resolvers when parsing servers"
This reverts commit 0ad4efb. See systemd#20559 (comment) 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.
1 parent 5d11af6 commit 281df57

File tree

5 files changed

+23
-34
lines changed

5 files changed

+23
-34
lines changed

src/resolve/resolved-conf.c

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,15 @@ static int manager_add_dns_server_by_string(Manager *m, DnsServerType type, cons
3535
if (r < 0)
3636
return r;
3737

38-
/* By default, the port number is determined by the transaction feature level.
38+
/* Silently filter out 0.0.0.0, 127.0.0.53, 127.0.0.54 (our own stub DNS listener) */
39+
if (!dns_server_address_valid(family, &address))
40+
return 0;
41+
42+
/* By default, the port number is determined with the transaction feature level.
3943
* See dns_transaction_port() and dns_server_port(). */
4044
if (IN_SET(port, 53, 853))
4145
port = 0;
4246

43-
/* Refuse 0.0.0.0, 127.0.0.53, 127.0.0.54 and the rest of our own stub DNS listeners. */
44-
if (!dns_server_address_valid(family, &address) ||
45-
manager_server_address_is_stub(m, family, &address, port ?: 53))
46-
return -ELOOP;
47-
4847
/* Filter out duplicates */
4948
s = dns_server_find(manager_get_first_dns_server(m, type), family, &address, port, ifindex, server_name);
5049
if (s) {
@@ -57,7 +56,7 @@ static int manager_add_dns_server_by_string(Manager *m, DnsServerType type, cons
5756
return dns_server_new(m, NULL, type, NULL, family, &address, port, ifindex, server_name);
5857
}
5958

60-
int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string, bool ignore_self_quietly) {
59+
int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string) {
6160
int r;
6261

6362
assert(m);
@@ -71,10 +70,7 @@ int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, con
7170
return r;
7271

7372
r = manager_add_dns_server_by_string(m, type, word);
74-
if (r == -ELOOP)
75-
log_full(ignore_self_quietly ? LOG_DEBUG : LOG_INFO,
76-
"DNS server string '%s' points to our own listener, ignoring.", word);
77-
else if (r < 0)
73+
if (r < 0)
7874
log_warning_errno(r, "Failed to add DNS server address '%s', ignoring: %m", word);
7975
}
8076
}
@@ -155,15 +151,16 @@ int config_parse_dns_servers(
155151
dns_server_unlink_all(manager_get_first_dns_server(m, ltype));
156152
else {
157153
/* Otherwise, add to the list */
158-
r = manager_parse_dns_server_string_and_warn(m, ltype, rvalue, false);
154+
r = manager_parse_dns_server_string_and_warn(m, ltype, rvalue);
159155
if (r < 0) {
160156
log_syntax(unit, LOG_WARNING, filename, line, r,
161157
"Failed to parse DNS server string '%s', ignoring.", rvalue);
162158
return 0;
163159
}
164160
}
165161

166-
/* If we have a manual setting, then we stop reading /etc/resolv.conf */
162+
/* If we have a manual setting, then we stop reading
163+
* /etc/resolv.conf */
167164
if (ltype == DNS_SERVER_SYSTEM)
168165
m->read_resolv_conf = false;
169166
if (ltype == DNS_SERVER_FALLBACK)
@@ -205,7 +202,8 @@ int config_parse_search_domains(
205202
}
206203
}
207204

208-
/* If we have a manual setting, then we stop reading /etc/resolv.conf */
205+
/* If we have a manual setting, then we stop reading
206+
* /etc/resolv.conf */
209207
m->read_resolv_conf = false;
210208

211209
return 0;
@@ -487,7 +485,7 @@ int manager_parse_config_file(Manager *m) {
487485
return r;
488486

489487
if (m->need_builtin_fallbacks) {
490-
r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_FALLBACK, DNS_SERVERS, false);
488+
r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_FALLBACK, DNS_SERVERS);
491489
if (r < 0)
492490
return r;
493491
}

src/resolve/resolved-conf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
int manager_parse_config_file(Manager *m);
99

1010
int manager_parse_search_domains_and_warn(Manager *m, const char *string);
11-
int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string, bool ignore_self_quietly);
11+
int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string);
1212

1313
const struct ConfigPerfItem* resolved_gperf_lookup(const char *key, GPERF_LEN_TYPE length);
1414
const struct ConfigPerfItem* resolved_dnssd_gperf_lookup(const char *key, GPERF_LEN_TYPE length);

src/resolve/resolved-manager.c

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,37 +1619,30 @@ bool manager_next_dnssd_names(Manager *m) {
16191619
return tried;
16201620
}
16211621

1622-
bool manager_server_address_is_stub(Manager *m, int family, const union in_addr_union *address, uint16_t port) {
1622+
bool manager_server_is_stub(Manager *m, DnsServer *s) {
16231623
DnsStubListenerExtra *l;
16241624

16251625
assert(m);
1626-
assert(address);
1626+
assert(s);
16271627

16281628
/* Safety check: we generally already skip the main stub when parsing configuration. But let's be
16291629
* extra careful, and check here again */
1630-
if (family == AF_INET &&
1631-
address->in.s_addr == htobe32(INADDR_DNS_STUB) &&
1632-
port == 53)
1630+
if (s->family == AF_INET &&
1631+
s->address.in.s_addr == htobe32(INADDR_DNS_STUB) &&
1632+
dns_server_port(s) == 53)
16331633
return true;
16341634

16351635
/* Main reason to call this is to check server data against the extra listeners, and filter things
16361636
* out. */
16371637
ORDERED_SET_FOREACH(l, m->dns_extra_stub_listeners)
1638-
if (family == l->family &&
1639-
in_addr_equal(family, address, &l->address) &&
1640-
port == dns_stub_listener_extra_port(l))
1638+
if (s->family == l->family &&
1639+
in_addr_equal(s->family, &s->address, &l->address) &&
1640+
dns_server_port(s) == dns_stub_listener_extra_port(l))
16411641
return true;
16421642

16431643
return false;
16441644
}
16451645

1646-
bool manager_server_is_stub(Manager *m, DnsServer *s) {
1647-
assert(m);
1648-
assert(s);
1649-
1650-
return manager_server_address_is_stub(m, s->family, &s->address, dns_server_port(s));
1651-
}
1652-
16531646
int socket_disable_pmtud(int fd, int af) {
16541647
int r;
16551648

src/resolve/resolved-manager.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,6 @@ void manager_cleanup_saved_user(Manager *m);
207207

208208
bool manager_next_dnssd_names(Manager *m);
209209

210-
bool manager_server_address_is_stub(Manager *m, int family, const union in_addr_union *address, uint16_t port);
211210
bool manager_server_is_stub(Manager *m, DnsServer *s);
212211

213212
int socket_disable_pmtud(int fd, int af);

src/resolve/resolved-resolv-conf.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@ int manager_read_resolv_conf(Manager *m) {
141141

142142
a = first_word(l, "nameserver");
143143
if (a) {
144-
r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_SYSTEM, a,
145-
true /* don't warn about loops to our own stub listeners */);
144+
r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_SYSTEM, a);
146145
if (r < 0)
147146
log_warning_errno(r, "Failed to parse DNS server address '%s', ignoring.", a);
148147

0 commit comments

Comments
 (0)