From 49ef064c8dcd8ed12d98e6c705e676babade0897 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Nov 2020 16:27:55 +0100 Subject: [PATCH] resolved: refuse sending packets to our own stub listeners A previous commit made sure that when one of our own packets is looped back to us, we ignore it. But let's go one step further, and refuse operation if we notice the server we talk to is our own. This way we won't generate unnecessary traffic and can return a cleaner error. Fixes: #17413 --- src/libsystemd/sd-bus/bus-common-errors.c | 1 + src/libsystemd/sd-bus/bus-common-errors.h | 1 + src/resolve/resolved-bus.c | 3 +++ src/resolve/resolved-dns-stub.c | 14 +++++++++++-- src/resolve/resolved-dns-stub.h | 1 + src/resolve/resolved-dns-transaction.c | 23 ++++++++++++++++++++++ src/resolve/resolved-dns-transaction.h | 1 + src/resolve/resolved-manager.c | 24 +++++++++++++++++++++++ src/resolve/resolved-manager.h | 2 ++ src/resolve/resolved-varlink.c | 3 +++ 10 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-common-errors.c b/src/libsystemd/sd-bus/bus-common-errors.c index cb0708d0987..dc6211bc978 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.c +++ b/src/libsystemd/sd-bus/bus-common-errors.c @@ -76,6 +76,7 @@ BUS_ERROR_MAP_ELF_REGISTER const sd_bus_error_map bus_common_errors[] = { SD_BUS_ERROR_MAP(BUS_ERROR_LINK_BUSY, EBUSY), SD_BUS_ERROR_MAP(BUS_ERROR_NETWORK_DOWN, ENETDOWN), SD_BUS_ERROR_MAP(BUS_ERROR_NO_SOURCE, ESRCH), + SD_BUS_ERROR_MAP(BUS_ERROR_STUB_LOOP, ELOOP), SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_DNSSD_SERVICE, ENOENT), SD_BUS_ERROR_MAP(BUS_ERROR_DNSSD_SERVICE_EXISTS, EEXIST), diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h index 28332836b51..e56b5d139de 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.h +++ b/src/libsystemd/sd-bus/bus-common-errors.h @@ -74,6 +74,7 @@ #define BUS_ERROR_LINK_BUSY "org.freedesktop.resolve1.LinkBusy" #define BUS_ERROR_NETWORK_DOWN "org.freedesktop.resolve1.NetworkDown" #define BUS_ERROR_NO_SOURCE "org.freedesktop.resolve1.NoSource" +#define BUS_ERROR_STUB_LOOP "org.freedesktop.resolve1.StubLoop" #define BUS_ERROR_NO_SUCH_DNSSD_SERVICE "org.freedesktop.resolve1.NoSuchDnssdService" #define BUS_ERROR_DNSSD_SERVICE_EXISTS "org.freedesktop.resolve1.DnssdServiceExists" #define _BUS_ERROR_DNS "org.freedesktop.resolve1.DnsError." diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 6db8261ac02..0c1124f7ddd 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -104,6 +104,9 @@ static int reply_query_state(DnsQuery *q) { case DNS_TRANSACTION_NO_SOURCE: return sd_bus_reply_method_errorf(q->bus_request, BUS_ERROR_NO_SOURCE, "All suitable resolution sources turned off"); + case DNS_TRANSACTION_STUB_LOOP: + return sd_bus_reply_method_errorf(q->bus_request, BUS_ERROR_STUB_LOOP, "Configured DNS server loops back to us"); + case DNS_TRANSACTION_RCODE_FAILURE: { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 74a76ddccfa..5f548abe048 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -85,6 +85,15 @@ DnsStubListenerExtra *dns_stub_listener_extra_free(DnsStubListenerExtra *p) { return mfree(p); } +uint16_t dns_stub_listener_extra_port(DnsStubListenerExtra *p) { + assert(p); + + if (p->port > 0) + return p->port; + + return 53; +} + static int dns_stub_collect_answer_by_question( DnsAnswer **reply, DnsAnswer *answer, @@ -639,6 +648,7 @@ static void dns_stub_query_complete(DnsQuery *q) { case DNS_TRANSACTION_RR_TYPE_UNSUPPORTED: case DNS_TRANSACTION_NETWORK_DOWN: case DNS_TRANSACTION_NO_SOURCE: + case DNS_TRANSACTION_STUB_LOOP: (void) dns_stub_send_reply(q, DNS_RCODE_SERVFAIL); break; @@ -956,13 +966,13 @@ static int manager_dns_stub_fd_extra(Manager *m, DnsStubListenerExtra *l, int ty if (l->family == AF_INET) sa = (union sockaddr_union) { .in.sin_family = l->family, - .in.sin_port = htobe16(l->port != 0 ? l->port : 53U), + .in.sin_port = htobe16(dns_stub_listener_extra_port(l)), .in.sin_addr = l->address.in, }; else sa = (union sockaddr_union) { .in6.sin6_family = l->family, - .in6.sin6_port = htobe16(l->port != 0 ? l->port : 53U), + .in6.sin6_port = htobe16(dns_stub_listener_extra_port(l)), .in6.sin6_addr = l->address.in6, }; diff --git a/src/resolve/resolved-dns-stub.h b/src/resolve/resolved-dns-stub.h index 5911ccdc344..655c5deec48 100644 --- a/src/resolve/resolved-dns-stub.h +++ b/src/resolve/resolved-dns-stub.h @@ -33,6 +33,7 @@ extern const struct hash_ops dns_stub_listener_extra_hash_ops; int dns_stub_listener_extra_new(Manager *m, DnsStubListenerExtra **ret); DnsStubListenerExtra *dns_stub_listener_extra_free(DnsStubListenerExtra *p); +uint16_t dns_stub_listener_extra_port(DnsStubListenerExtra *p); void manager_dns_stub_stop(Manager *m); int manager_dns_stub_start(Manager *m); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 7f4ab4b12b3..1ef1900b7bf 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -640,6 +640,9 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) { if (r < 0) return r; + if (manager_server_is_stub(t->scope->manager, t->server)) + return -ELOOP; + if (!t->bypass) { if (!dns_server_dnssec_supported(t->server) && dns_type_is_dnssec(dns_transaction_key(t)->type)) return -EOPNOTSUPP; @@ -1304,6 +1307,9 @@ static int dns_transaction_emit_udp(DnsTransaction *t) { if (r < 0) return r; + if (manager_server_is_stub(t->scope->manager, t->server)) + return -ELOOP; + if (t->current_feature_level < DNS_SERVER_FEATURE_LEVEL_UDP || DNS_SERVER_FEATURE_LEVEL_IS_TLS(t->current_feature_level)) return -EAGAIN; /* Sorry, can't do UDP, try TCP! */ @@ -1846,7 +1852,23 @@ int dns_transaction_go(DnsTransaction *t) { if (IN_SET(r, -EMSGSIZE, -EAGAIN)) r = dns_transaction_emit_tcp(t); } + if (r == -ELOOP) { + if (t->scope->protocol != DNS_PROTOCOL_DNS) + return r; + + /* One of our own stub listeners */ + log_debug_errno(r, "Detected that specified DNS server is our own extra listener, switching DNS servers."); + + dns_scope_next_dns_server(t->scope); + + if (dns_scope_get_dns_server(t->scope) == t->server) { + log_debug_errno(r, "Still pointing to extra listener after switching DNS servers, refusing operation."); + dns_transaction_complete(t, DNS_TRANSACTION_STUB_LOOP); + return 0; + } + return dns_transaction_go(t); + } if (r == -ESRCH) { /* No servers to send this to? */ dns_transaction_complete(t, DNS_TRANSACTION_NO_SERVERS); @@ -3387,6 +3409,7 @@ static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX] [DNS_TRANSACTION_NETWORK_DOWN] = "network-down", [DNS_TRANSACTION_NOT_FOUND] = "not-found", [DNS_TRANSACTION_NO_SOURCE] = "no-source", + [DNS_TRANSACTION_STUB_LOOP] = "stub-loop", }; DEFINE_STRING_TABLE_LOOKUP(dns_transaction_state, DnsTransactionState); diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index db7cf6743b2..9376d504bfe 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -33,6 +33,7 @@ enum DnsTransactionState { DNS_TRANSACTION_NETWORK_DOWN, DNS_TRANSACTION_NOT_FOUND, /* like NXDOMAIN, but when LLMNR/TCP connections fail */ DNS_TRANSACTION_NO_SOURCE, /* All suitable DnsTransactionSource turned off */ + DNS_TRANSACTION_STUB_LOOP, _DNS_TRANSACTION_STATE_MAX, _DNS_TRANSACTION_STATE_INVALID = -EINVAL, }; diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 9d178abcb54..b41308204e8 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1618,3 +1618,27 @@ bool manager_next_dnssd_names(Manager *m) { return tried; } + +bool manager_server_is_stub(Manager *m, DnsServer *s) { + DnsStubListenerExtra *l; + + assert(m); + 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 (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 (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; +} diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index d2f9048fa7e..10c2994c2b0 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -197,3 +197,5 @@ void manager_reset_server_features(Manager *m); void manager_cleanup_saved_user(Manager *m); bool manager_next_dnssd_names(Manager *m); + +bool manager_server_is_stub(Manager *m, DnsServer *s); diff --git a/src/resolve/resolved-varlink.c b/src/resolve/resolved-varlink.c index f948206e478..b691e59d3ee 100644 --- a/src/resolve/resolved-varlink.c +++ b/src/resolve/resolved-varlink.c @@ -60,6 +60,9 @@ static int reply_query_state(DnsQuery *q) { case DNS_TRANSACTION_NO_SOURCE: return varlink_error(q->varlink_request, "io.systemd.Resolve.NoSource", NULL); + case DNS_TRANSACTION_STUB_LOOP: + return varlink_error(q->varlink_request, "io.systemd.Resolve.StubLoop", NULL); + case DNS_TRANSACTION_NOT_FOUND: /* We return this as NXDOMAIN. This is only generated when a host doesn't implement LLMNR/TCP, and we * thus quickly know that we cannot resolve an in-addr.arpa or ip6.arpa address. */ -- 2.47.3