diff --git a/src/libsystemd/sd-bus/bus-common-errors.c b/src/libsystemd/sd-bus/bus-common-errors.c index cb0708d098..dc6211bc97 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 28332836b5..e56b5d139d 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 6db8261ac0..0c1124f7dd 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-answer.c b/src/resolve/resolved-dns-answer.c index 7746cb08ea..aac34575d4 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -740,16 +740,21 @@ int dns_answer_reserve(DnsAnswer **a, size_t n_free) { if ((*a)->n_ref > 1) return -EBUSY; - ns = (*a)->n_rrs + n_free; - if (ns > UINT16_MAX) /* Maximum number of RRs we can stick into a DNS packet section */ + ns = (*a)->n_rrs; + assert(ns <= UINT16_MAX); /* Maximum number of RRs we can stick into a DNS packet section */ + + if (n_free > UINT16_MAX - ns) /* overflow check */ ns = UINT16_MAX; + else + ns += n_free; if ((*a)->n_allocated >= ns) return 0; - /* Allocate more than we need */ - ns *= 2; - if (ns > UINT16_MAX) + /* Allocate more than we need, but not more than UINT16_MAX */ + if (ns <= UINT16_MAX/2) + ns *= 2; + else ns = UINT16_MAX; /* This must be done before realloc() below. Otherwise, the original DnsAnswer object @@ -780,33 +785,50 @@ int dns_answer_reserve(DnsAnswer **a, size_t n_free) { } int dns_answer_reserve_or_clone(DnsAnswer **a, size_t n_free) { - _cleanup_(dns_answer_unrefp) DnsAnswer *n = NULL; int r; assert(a); - /* Tries to extend the DnsAnswer object. And if that's not - * possible, since we are not the sole owner, then allocate a - * new, appropriately sized one. Either way, after this call - * the object will only have a single reference, and has room - * for at least the specified number of RRs. */ + /* Tries to extend the DnsAnswer object. And if that's not possible, since we are not the sole owner, + * then allocate a new, appropriately sized one. Either way, after this call the object will only + * have a single reference, and has room for at least the specified number of RRs. */ - r = dns_answer_reserve(a, n_free); - if (r != -EBUSY) - return r; + if (*a && (*a)->n_ref > 1) { + _cleanup_(dns_answer_unrefp) DnsAnswer *n = NULL; + size_t ns; - assert(*a); + ns = (*a)->n_rrs; + assert(ns <= UINT16_MAX); /* Maximum number of RRs we can stick into a DNS packet section */ - n = dns_answer_new(((*a)->n_rrs + n_free) * 2); - if (!n) - return -ENOMEM; + if (n_free > UINT16_MAX - ns) /* overflow check */ + ns = UINT16_MAX; + else if (n_free > 0) { /* Increase size and double the result, just in case — except if the + * increase is specified as 0, in which case we just allocate the + * exact amount as before, under the assumption this is just a request + * to copy the answer. */ + ns += n_free; - r = dns_answer_add_raw_all(n, *a); - if (r < 0) - return r; + if (ns <= UINT16_MAX/2) /* overflow check */ + ns *= 2; + else + ns = UINT16_MAX; + } - dns_answer_unref(*a); - *a = TAKE_PTR(n); + n = dns_answer_new(ns); + if (!n) + return -ENOMEM; + + r = dns_answer_add_raw_all(n, *a); + if (r < 0) + return r; + + dns_answer_unref(*a); + assert_se(*a = TAKE_PTR(n)); + } else if (n_free > 0) { + r = dns_answer_reserve(a, n_free); + if (r < 0) + return r; + } return 0; } diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index f548bbf9f2..a4aafd603d 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2555,6 +2555,10 @@ static int dns_packet_compare_func(const DnsPacket *x, const DnsPacket *y) { DEFINE_HASH_OPS(dns_packet_hash_ops, DnsPacket, dns_packet_hash_func, dns_packet_compare_func); +bool dns_packet_equal(const DnsPacket *a, const DnsPacket *b) { + return dns_packet_compare_func(a, b) == 0; +} + static const char* const dns_rcode_table[_DNS_RCODE_MAX_DEFINED] = { [DNS_RCODE_SUCCESS] = "SUCCESS", [DNS_RCODE_FORMERR] = "FORMERR", diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index 9d93f31469..815999ecc2 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -227,6 +227,8 @@ void dns_packet_rewind(DnsPacket *p, size_t idx); int dns_packet_skip_question(DnsPacket *p); int dns_packet_extract(DnsPacket *p); +bool dns_packet_equal(const DnsPacket *a, const DnsPacket *b); + /* https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6 */ enum { DNS_RCODE_SUCCESS = 0, diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index d8e4f6fee6..1ba435f584 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1126,7 +1126,7 @@ void dns_scope_check_conflicts(DnsScope *scope, DnsPacket *p) { return; } - if (manager_our_packet(scope->manager, p)) + if (manager_packet_from_local_address(scope->manager, p)) return; r = dns_packet_extract(p); diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 6642449697..5f548abe04 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; @@ -688,6 +698,11 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea return; } + if (manager_packet_from_our_transaction(m, p)) { + log_debug("Got our own packet looped back, ignoring."); + return; + } + r = dns_packet_extract(p); if (r < 0) { log_debug_errno(r, "Failed to extract resources from incoming packet, ignoring packet: %m"); @@ -951,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 5911ccdc34..655c5deec4 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 43359f3307..1ef1900b7b 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -317,8 +317,9 @@ static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) { assert(t); assert(p); + assert(t->scope->protocol == DNS_PROTOCOL_LLMNR); - if (manager_our_packet(t->scope->manager, p) != 0) + if (manager_packet_from_local_address(t->scope->manager, p) != 0) return; (void) in_addr_to_string(p->family, &p->sender, &pretty); @@ -639,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; @@ -1303,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! */ @@ -1845,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); @@ -3386,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 db7cf6743b..9376d504bf 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-llmnr.c b/src/resolve/resolved-llmnr.c index 2ddf08815a..b1b8351e00 100644 --- a/src/resolve/resolved-llmnr.c +++ b/src/resolve/resolved-llmnr.c @@ -83,7 +83,7 @@ static int on_llmnr_packet(sd_event_source *s, int fd, uint32_t revents, void *u if (r <= 0) return r; - if (manager_our_packet(m, p)) + if (manager_packet_from_local_address(m, p)) return 0; scope = manager_find_scope(m, p); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index cc68484169..b41308204e 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1255,13 +1255,31 @@ LinkAddress* manager_find_link_address(Manager *m, int family, const union in_ad return NULL; } -bool manager_our_packet(Manager *m, DnsPacket *p) { +bool manager_packet_from_local_address(Manager *m, DnsPacket *p) { assert(m); assert(p); + /* Let's see if this packet comes from an IP address we have on any local interface */ + return !!manager_find_link_address(m, p->family, &p->sender); } +bool manager_packet_from_our_transaction(Manager *m, DnsPacket *p) { + DnsTransaction *t; + + assert(m); + assert(p); + + /* Let's see if we have a transaction with a query message with the exact same binary contents as the + * one we just got. If so, it's almost definitely a packet loop of some kind. */ + + t = hashmap_get(m->dns_transactions, UINT_TO_PTR(DNS_PACKET_ID(p))); + if (!t) + return false; + + return t->sent && dns_packet_equal(t->sent, p); +} + DnsScope* manager_find_scope(Manager *m, DnsPacket *p) { Link *l; @@ -1600,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 5471b20ff9..10c2994c2b 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -165,7 +165,9 @@ LinkAddress* manager_find_link_address(Manager *m, int family, const union in_ad void manager_refresh_rrs(Manager *m); int manager_next_hostname(Manager *m); -bool manager_our_packet(Manager *m, DnsPacket *p); +bool manager_packet_from_local_address(Manager *m, DnsPacket *p); +bool manager_packet_from_our_transaction(Manager *m, DnsPacket *p); + DnsScope* manager_find_scope(Manager *m, DnsPacket *p); void manager_verify_all(Manager *m); @@ -195,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-mdns.c b/src/resolve/resolved-mdns.c index 149d3a4526..6fccc5a1e5 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -254,7 +254,7 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us if (r <= 0) return r; - if (manager_our_packet(m, p)) + if (manager_packet_from_local_address(m, p)) return 0; scope = manager_find_scope(m, p); diff --git a/src/resolve/resolved-varlink.c b/src/resolve/resolved-varlink.c index f948206e47..b691e59d3e 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. */