From ded152134ed131de8b1024848acd407eb139f010 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 11:08:32 +0100 Subject: [PATCH 01/10] resolved: use RET_NERRNO() where it makes sense --- src/resolve/resolved-dns-stub.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index aaabc92486..4c5429c77b 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -1208,10 +1208,9 @@ static int manager_dns_stub_fd_extra(Manager *m, DnsStubListenerExtra *l, int ty log_debug_errno(r, "Failed to enable fragment size reception, ignoring: %m"); } - if (bind(fd, &sa.sa, SOCKADDR_LEN(sa)) < 0) { - r = -errno; + r = RET_NERRNO(bind(fd, &sa.sa, SOCKADDR_LEN(sa))); + if (r < 0) goto fail; - } if (type == SOCK_STREAM && listen(fd, SOMAXCONN) < 0) { From f96b500eced522adb56e4e79a876c6a9471bb134 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 11:13:26 +0100 Subject: [PATCH 02/10] resolved: include IP address info in debug output for incoming datagrams --- src/resolve/resolved-manager.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 246e2d9db7..431093ad01 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -880,8 +880,16 @@ int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) { p->ifindex = manager_find_ifindex(m, p->family, &p->destination); } - log_debug("Received %s UDP packet of size %zu, ifindex=%i, ttl=%i, fragsize=%zu", - dns_protocol_to_string(protocol), p->size, p->ifindex, p->ttl, p->fragsize); + if (DEBUG_LOGGING) { + _cleanup_free_ char *sender_address = NULL, *destination_address = NULL; + + (void) in_addr_to_string(p->family, &p->sender, &sender_address); + (void) in_addr_to_string(p->family, &p->destination, &destination_address); + + log_debug("Received %s UDP packet of size %zu, ifindex=%i, ttl=%i, fragsize=%zu, sender=%s, destination=%s", + dns_protocol_to_string(protocol), p->size, p->ifindex, p->ttl, p->fragsize, + strna(sender_address), strna(destination_address)); + } *ret = TAKE_PTR(p); return 1; From c1b91f06b93ba29eafbf50f4858aedfb6ded4ff1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 11:29:42 +0100 Subject: [PATCH 03/10] socket-util: add helper for generically initializing sockaddr_union from in_addr_union --- src/basic/socket-util.c | 35 +++++++++++++++++++++++++++++++++++ src/basic/socket-util.h | 2 ++ 2 files changed, 37 insertions(+) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index ca6085ef64..8d0494ece5 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -400,6 +400,41 @@ const union in_addr_union *sockaddr_in_addr(const struct sockaddr *_sa) { } } +int sockaddr_set_in_addr( + union sockaddr_union *u, + int family, + const union in_addr_union *a, + uint16_t port) { + + assert(u); + assert(a); + + switch (family) { + + case AF_INET: + u->in = (struct sockaddr_in) { + .sin_family = AF_INET, + .sin_addr = a->in, + .sin_port = htobe16(port), + }; + + return 0; + + case AF_INET6: + u->in6 = (struct sockaddr_in6) { + .sin6_family = AF_INET6, + .sin6_addr = a->in6, + .sin6_port = htobe16(port), + }; + + return 0; + + default: + return -EAFNOSUPPORT; + + } +} + int sockaddr_pretty( const struct sockaddr *_sa, socklen_t salen, diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index c4fafa084b..57e655154c 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -15,6 +15,7 @@ #include #include "errno-util.h" +#include "in-addr-util.h" #include "macro.h" #include "missing_network.h" #include "missing_socket.h" @@ -106,6 +107,7 @@ bool socket_ipv6_is_enabled(void); int sockaddr_port(const struct sockaddr *_sa, unsigned *port); const union in_addr_union *sockaddr_in_addr(const struct sockaddr *sa); +int sockaddr_set_in_addr(union sockaddr_union *u, int family, const union in_addr_union *a, uint16_t port); int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool translate_ipv6, bool include_port, char **ret); int getpeername_pretty(int fd, bool include_port, char **ret); From a8d09063447568d87288a8e868fe386c1da7ce09 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 12:20:05 +0100 Subject: [PATCH 04/10] resolved: add "proxy-only" stub on 127.0.0.54 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This beefs up the DNS stub logic to listen on two IP addresses: 127.0.0.53 (as before) + 127.0.0.54 (new). When the latter is contact our stub will operate in "bypass" mode only, i.e we'll try to pass DNS requests as unmodified upstream as we can (and not do mDNS/LLMNR and such, also no DNSSEC validation – but we'll still do DNS-over-TLS wrapping). This is supposed to be useful for container environments or tethering: this stub could be exposed (via NAT redirect) to clients of this system and we'll try to stay out of the way with doing too much DNS magic ourselves, but still expose whatever the current DNS server is from upstream under a stable address/port. How to use this: # iptables -t nat -I PREROUTING -p udp -i --dport 53 -j DNAT --to 127.0.0.54:53 # echo 1 > /proc/sys/net/ipv4/conf//route_localnet --- man/resolved.conf.xml | 12 +- man/systemd-resolved.service.xml | 13 +- src/resolve/resolved-conf.c | 2 +- src/resolve/resolved-dns-stub.c | 201 +++++++++++++++++++++++-------- src/resolve/resolved-manager.h | 4 + src/shared/resolve-util.c | 4 +- src/shared/resolve-util.h | 5 +- 7 files changed, 180 insertions(+), 61 deletions(-) diff --git a/man/resolved.conf.xml b/man/resolved.conf.xml index c1994604b9..f53af362e8 100644 --- a/man/resolved.conf.xml +++ b/man/resolved.conf.xml @@ -265,11 +265,13 @@ DNSStubListener= - Takes a boolean argument or one of udp and tcp. If - udp, a DNS stub resolver will listen for UDP requests on address 127.0.0.53 - port 53. If tcp, the stub will listen for TCP requests on the same address and port. If - yes (the default), the stub listens for both UDP and TCP requests. If no, the stub - listener is disabled. + Takes a boolean argument or one of udp and + tcp. If udp, a DNS stub resolver will listen for UDP requests + on addresses 127.0.0.53 and 127.0.0.54, port 53. If tcp, the stub will listen for + TCP requests on the same addresses and port. If yes (the default), the stub listens + for both UDP and TCP requests. If no, the stub listener is disabled. + + Note that the DNS stub listener is turned off implicitly when its listening address and port are already in use. diff --git a/man/systemd-resolved.service.xml b/man/systemd-resolved.service.xml index 34c1257ab0..a7e08f0c80 100644 --- a/man/systemd-resolved.service.xml +++ b/man/systemd-resolved.service.xml @@ -59,12 +59,19 @@ systemd-resolved. Additionally, systemd-resolved provides a local DNS stub listener on - IP address 127.0.0.53 on the local loopback interface. Programs issuing DNS requests directly, - bypassing any local API may be directed to this stub, in order to connect them to + the IP addresses 127.0.0.53 and 127.0.0.54 on the local loopback interface. Programs issuing DNS + requests directly, bypassing any local API may be directed to this stub, in order to connect them to systemd-resolved. Note however that it is strongly recommended that local programs use the glibc NSS or bus APIs instead (as described above), as various network resolution concepts (such as link-local addressing, or LLMNR Unicode domains) cannot be mapped to the unicast DNS - protocol. + protocol. + + The DNS stub resolver on 127.0.0.53 provides the full feature set of the local + resolver, which includes offering LLMNR/MulticastDNS resolution. The DNS stub resolver on 127.0.0.54 + provides a more limited resolver, that operates in "proxy" mode only, i.e. it will pass most DNS + messages relatively unmodified to the current upstream DNS servers and back, but not try to process the + messages locally, and hence does not validate DNSSEC, or offer up LLMNR/MulticastDNS. (It will + translate to DNS-over-TLS communication if needed however.) The DNS servers contacted are determined from the global settings in diff --git a/src/resolve/resolved-conf.c b/src/resolve/resolved-conf.c index 3afaaad937..453f1175e1 100644 --- a/src/resolve/resolved-conf.c +++ b/src/resolve/resolved-conf.c @@ -35,7 +35,7 @@ static int manager_add_dns_server_by_string(Manager *m, DnsServerType type, cons if (r < 0) return r; - /* Silently filter out 0.0.0.0 and 127.0.0.53 (our own stub DNS listener) */ + /* 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; diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 4c5429c77b..09fd7d2418 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -21,6 +21,7 @@ #define ADVERTISE_EXTRA_DATAGRAM_SIZE_MAX DNS_PACKET_UNICAST_SIZE_LARGE_MAX static int manager_dns_stub_fd_extra(Manager *m, DnsStubListenerExtra *l, int type); +static int manager_dns_stub_fd(Manager *m, int family, const union in_addr_union *listen_address, int type); static void dns_stub_listener_extra_hash_func(const DnsStubListenerExtra *a, struct siphash *state) { assert(a); @@ -483,6 +484,34 @@ static int dns_stub_finish_reply_packet( return 0; } +static bool address_is_proxy(int family, const union in_addr_union *a) { + assert(a); + + /* Returns true if the specified address is the DNS "proxy" stub, i.e. where we unconditionally enable bypass mode */ + + if (family != AF_INET) + return false; + + return be32toh(a->in.s_addr) == INADDR_DNS_PROXY_STUB; +} + +static int find_socket_fd( + Manager *m, + DnsStubListenerExtra *l, + int family, + const union in_addr_union *listen_address, + int type) { + + assert(m); + + /* Finds the right socket to use for sending. If we know the extra listener, otherwise go via the + * address to send from */ + if (l) + return manager_dns_stub_fd_extra(m, l, type); + + return manager_dns_stub_fd(m, family, listen_address, type); +} + static int dns_stub_send( Manager *m, DnsStubListenerExtra *l, @@ -498,15 +527,22 @@ static int dns_stub_send( if (s) r = dns_stream_write_packet(s, reply); - else - /* Note that it is essential here that we explicitly choose the source IP address for this packet. This - * is because otherwise the kernel will choose it automatically based on the routing table and will - * thus pick 127.0.0.1 rather than 127.0.0.53. */ + else { + int fd; + + fd = find_socket_fd(m, l, p->family, &p->sender, SOCK_DGRAM); + if (fd < 0) + return fd; + + /* Note that it is essential here that we explicitly choose the source IP address for this + * packet. This is because otherwise the kernel will choose it automatically based on the + * routing table and will thus pick 127.0.0.1 rather than 127.0.0.53. */ r = manager_send(m, - manager_dns_stub_fd_extra(m, l, SOCK_DGRAM), - l ? p->ifindex : LOOPBACK_IFINDEX, /* force loopback iface if this is the main listener stub */ + fd, + l || address_is_proxy(p->family, &p->destination) ? p->ifindex : LOOPBACK_IFINDEX, /* force loopback iface if this is the main listener stub */ p->family, &p->sender, p->sender_port, &p->destination, reply); + } if (r < 0) return log_debug_errno(r, "Failed to send reply packet: %m"); @@ -841,9 +877,11 @@ static int dns_stub_stream_complete(DnsStream *s, int error) { } static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStream *s, DnsPacket *p) { + uint64_t protocol_flags = SD_RESOLVED_PROTOCOLS_ALL; _cleanup_(dns_query_freep) DnsQuery *q = NULL; Hashmap **queries_by_packet; DnsQuery *existing; + bool bypass = false; int r; assert(m); @@ -851,6 +889,7 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea assert(p->protocol == DNS_PROTOCOL_DNS); if (!l && /* l == NULL if this is the main stub */ + !address_is_proxy(p->family, &p->destination) && /* don't restrict needlessly for 127.0.0.54 */ (in_addr_is_localhost(p->family, &p->sender) <= 0 || in_addr_is_localhost(p->family, &p->destination) <= 0)) { log_warning("Got packet on unexpected (i.e. non-localhost) IP range, ignoring."); @@ -907,19 +946,34 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea return; } - if (DNS_PACKET_DO(p) && DNS_PACKET_CD(p)) { - log_debug("Got request with DNSSEC checking disabled, enabling bypass logic."); + if (address_is_proxy(p->family, &p->destination)) { + _cleanup_free_ char *dipa = NULL; + r = in_addr_to_string(p->family, &p->destination, &dipa); + if (r < 0) { + log_error_errno(r, "Failed to format destination address: %m"); + return; + } + + log_debug("Got request to DNS proxy address 127.0.0.54, enabling bypass logic."); + bypass = true; + protocol_flags = SD_RESOLVED_DNS|SD_RESOLVED_NO_ZONE; /* Turn off mDNS/LLMNR for proxy stub. */ + } else if ((DNS_PACKET_DO(p) && DNS_PACKET_CD(p))) { + log_debug("Got request with DNSSEC checking disabled, enabling bypass logic."); + bypass = true; + } + + if (bypass) r = dns_query_new(m, &q, NULL, NULL, p, 0, - SD_RESOLVED_PROTOCOLS_ALL| + protocol_flags| SD_RESOLVED_NO_CNAME| SD_RESOLVED_NO_SEARCH| SD_RESOLVED_NO_VALIDATE| SD_RESOLVED_REQUIRE_PRIMARY| SD_RESOLVED_CLAMP_TTL); - } else + else r = dns_query_new(m, &q, p->question, p->question, NULL, 0, - SD_RESOLVED_PROTOCOLS_ALL| + protocol_flags| SD_RESOLVED_NO_SEARCH| (DNS_PACKET_DO(p) ? SD_RESOLVED_REQUIRE_PRIMARY : 0)| SD_RESOLVED_CLAMP_TTL); @@ -1085,26 +1139,32 @@ static int set_dns_stub_common_tcp_socket_options(int fd) { return 0; } -static int manager_dns_stub_fd(Manager *m, int type) { - union sockaddr_union sa = { - .in.sin_family = AF_INET, - .in.sin_addr.s_addr = htobe32(INADDR_DNS_STUB), - .in.sin_port = htobe16(53), - }; +static int manager_dns_stub_fd( + Manager *m, + int family, + const union in_addr_union *listen_addr, + int type) { + + sd_event_source **event_source; _cleanup_close_ int fd = -1; + union sockaddr_union sa; int r; - assert(IN_SET(type, SOCK_DGRAM, SOCK_STREAM)); + if (type == SOCK_DGRAM) + event_source = address_is_proxy(family, listen_addr) ? &m->dns_proxy_stub_udp_event_source : &m->dns_stub_udp_event_source; + else if (type == SOCK_STREAM) + event_source = address_is_proxy(family, listen_addr) ? &m->dns_proxy_stub_tcp_event_source : &m->dns_stub_tcp_event_source; + else + return -EPROTONOSUPPORT; - sd_event_source **event_source = type == SOCK_DGRAM ? &m->dns_stub_udp_event_source : &m->dns_stub_tcp_event_source; if (*event_source) return sd_event_source_get_io_fd(*event_source); - fd = socket(AF_INET, type | SOCK_CLOEXEC | SOCK_NONBLOCK, 0); + fd = socket(family, type | SOCK_CLOEXEC | SOCK_NONBLOCK, 0); if (fd < 0) return -errno; - r = set_dns_stub_common_socket_options(fd, AF_INET); + r = set_dns_stub_common_socket_options(fd, family); if (r < 0) return r; @@ -1114,12 +1174,30 @@ static int manager_dns_stub_fd(Manager *m, int type) { return r; } - /* Make sure no traffic from outside the local host can leak to onto this socket */ - r = socket_bind_to_ifindex(fd, LOOPBACK_IFINDEX); - if (r < 0) - return r; + /* Set slightly different socket options for the non-proxy and the proxy binding. The former we want + * to be accessible only from the local host, for the latter it's OK if people use NAT redirects or + * so to redirect external traffic to it. */ - r = setsockopt_int(fd, IPPROTO_IP, IP_TTL, 1); + if (!address_is_proxy(family, listen_addr)) { + /* Make sure no traffic from outside the local host can leak to onto this socket */ + r = socket_bind_to_ifindex(fd, LOOPBACK_IFINDEX); + if (r < 0) + return r; + + r = socket_set_ttl(fd, family, 1); + if (r < 0) + return r; + } else if (type == SOCK_DGRAM) { + r = socket_disable_pmtud(fd, family); + if (r < 0) + log_debug_errno(r, "Failed to disable UDP PMTUD, ignoring: %m"); + + r = socket_set_recvfragsize(fd, family, true); + if (r < 0) + log_debug_errno(r, "Failed to enable fragment size reception, ignoring: %m"); + } + + r = sockaddr_set_in_addr(&sa, family, listen_addr, 53); if (r < 0) return r; @@ -1153,11 +1231,9 @@ static int manager_dns_stub_fd_extra(Manager *m, DnsStubListenerExtra *l, int ty int r; assert(m); + assert(l); assert(IN_SET(type, SOCK_DGRAM, SOCK_STREAM)); - if (!l) - return manager_dns_stub_fd(m, type); - sd_event_source **event_source = type == SOCK_DGRAM ? &l->udp_event_source : &l->tcp_event_source; if (*event_source) return sd_event_source_get_io_fd(*event_source); @@ -1251,39 +1327,64 @@ fail: } int manager_dns_stub_start(Manager *m) { - const char *t = "UDP"; - int r = 0; + int r; assert(m); if (m->dns_stub_listener_mode == DNS_STUB_LISTENER_NO) log_debug("Not creating stub listener."); - else + else { + static const struct { + uint32_t addr; + int socket_type; + } stub_sockets[] = { + { INADDR_DNS_STUB, SOCK_DGRAM }, + { INADDR_DNS_STUB, SOCK_STREAM }, + { INADDR_DNS_PROXY_STUB, SOCK_DGRAM }, + { INADDR_DNS_PROXY_STUB, SOCK_STREAM }, + }; + log_debug("Creating stub listener using %s.", m->dns_stub_listener_mode == DNS_STUB_LISTENER_UDP ? "UDP" : m->dns_stub_listener_mode == DNS_STUB_LISTENER_TCP ? "TCP" : "UDP/TCP"); - if (FLAGS_SET(m->dns_stub_listener_mode, DNS_STUB_LISTENER_UDP)) - r = manager_dns_stub_fd(m, SOCK_DGRAM); + for (size_t i = 0; i < ELEMENTSOF(stub_sockets); i++) { + union in_addr_union a = { + .in.s_addr = htobe32(stub_sockets[i].addr), + }; - if (r >= 0 && - FLAGS_SET(m->dns_stub_listener_mode, DNS_STUB_LISTENER_TCP)) { - t = "TCP"; - r = manager_dns_stub_fd(m, SOCK_STREAM); + if (m->dns_stub_listener_mode == DNS_STUB_LISTENER_UDP && stub_sockets[i].socket_type == SOCK_STREAM) + continue; + if (m->dns_stub_listener_mode == DNS_STUB_LISTENER_TCP && stub_sockets[i].socket_type == SOCK_DGRAM) + continue; + + r = manager_dns_stub_fd(m, AF_INET, &a, stub_sockets[i].socket_type); + if (r < 0) { + _cleanup_free_ char *busy_socket = NULL; + + if (asprintf(&busy_socket, + "%s socket " IPV4_ADDRESS_FMT_STR ":53", + stub_sockets[i].socket_type == SOCK_DGRAM ? "UDP" : "TCP", + IPV4_ADDRESS_FMT_VAL(a.in)) < 0) + return log_oom(); + + if (IN_SET(r, -EADDRINUSE, -EPERM)) { + log_warning_errno(r, + r == -EADDRINUSE ? "Another process is already listening on %s.\n" + "Turning off local DNS stub support." : + "Failed to listen on %s: %m.\n" + "Turning off local DNS stub support.", + busy_socket); + manager_dns_stub_stop(m); + break; + } + + return log_error_errno(r, "Failed to listen on %s: %m", busy_socket); + } + } } - if (IN_SET(r, -EADDRINUSE, -EPERM)) { - log_warning_errno(r, - r == -EADDRINUSE ? "Another process is already listening on %s socket 127.0.0.53:53.\n" - "Turning off local DNS stub support." : - "Failed to listen on %s socket 127.0.0.53:53: %m.\n" - "Turning off local DNS stub support.", - t); - manager_dns_stub_stop(m); - } else if (r < 0) - return log_error_errno(r, "Failed to listen on %s socket 127.0.0.53:53: %m", t); - if (!ordered_set_isempty(m->dns_extra_stub_listeners)) { DnsStubListenerExtra *l; @@ -1305,6 +1406,8 @@ void manager_dns_stub_stop(Manager *m) { m->dns_stub_udp_event_source = sd_event_source_disable_unref(m->dns_stub_udp_event_source); m->dns_stub_tcp_event_source = sd_event_source_disable_unref(m->dns_stub_tcp_event_source); + m->dns_proxy_stub_udp_event_source = sd_event_source_disable_unref(m->dns_proxy_stub_udp_event_source); + m->dns_proxy_stub_tcp_event_source = sd_event_source_disable_unref(m->dns_proxy_stub_tcp_event_source); } static const char* const dns_stub_listener_mode_table[_DNS_STUB_LISTENER_MODE_MAX] = { diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index 1371c41b92..35e0068a83 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -140,6 +140,10 @@ struct Manager { sd_event_source *dns_stub_udp_event_source; sd_event_source *dns_stub_tcp_event_source; + /* Local DNS proxy stub on 127.0.0.54:53 */ + sd_event_source *dns_proxy_stub_udp_event_source; + sd_event_source *dns_proxy_stub_tcp_event_source; + Hashmap *polkit_registry; VarlinkServer *varlink_server; diff --git a/src/shared/resolve-util.c b/src/shared/resolve-util.c index 6541d231ed..820f9bb530 100644 --- a/src/shared/resolve-util.c +++ b/src/shared/resolve-util.c @@ -31,12 +31,12 @@ DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(dns_over_tls_mode, DnsOverTlsMode, DNS_O bool dns_server_address_valid(int family, const union in_addr_union *sa) { - /* Refuses the 0 IP addresses as well as 127.0.0.53 (which is our own DNS stub) */ + /* Refuses the 0 IP addresses as well as 127.0.0.53/127.0.0.54 (which is our own DNS stub) */ if (!in_addr_is_set(family, sa)) return false; - if (family == AF_INET && sa->in.s_addr == htobe32(INADDR_DNS_STUB)) + if (family == AF_INET && IN_SET(be32toh(sa->in.s_addr), INADDR_DNS_STUB, INADDR_DNS_PROXY_STUB)) return false; return true; diff --git a/src/shared/resolve-util.h b/src/shared/resolve-util.h index c4d9275368..d9ab387301 100644 --- a/src/shared/resolve-util.h +++ b/src/shared/resolve-util.h @@ -5,9 +5,12 @@ #include "in-addr-util.h" #include "macro.h" -/* 127.0.0.53 in native endian */ +/* 127.0.0.53 in native endian (The IP address we listen on with the full DNS stub, i.e. that does LLMNR/mDNS, and stuff) */ #define INADDR_DNS_STUB ((in_addr_t) 0x7f000035U) +/* 127.0.0.54 in native endian (The IP address we listen on we only implement "proxy" mode) */ +#define INADDR_DNS_PROXY_STUB ((in_addr_t) 0x7f000036U) + typedef enum DnsCacheMode DnsCacheMode; enum DnsCacheMode { From fb9044cb6bf97a25ed411cb4362efe3852dbd1ec Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 14:37:48 +0100 Subject: [PATCH 05/10] nspawn: voidify expose_port_execute() calls --- src/nspawn/nspawn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index e5ae382d5f..4765882836 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2544,8 +2544,8 @@ static int on_address_change(sd_netlink *rtnl, sd_netlink_message *m, void *user assert(m); assert(args); - expose_port_execute(rtnl, &args->fw_ctx, arg_expose_ports, AF_INET, &args->address4); - expose_port_execute(rtnl, &args->fw_ctx, arg_expose_ports, AF_INET6, &args->address6); + (void) expose_port_execute(rtnl, &args->fw_ctx, arg_expose_ports, AF_INET, &args->address4); + (void) expose_port_execute(rtnl, &args->fw_ctx, arg_expose_ports, AF_INET6, &args->address6); return 0; } From 5a78106ad92d3f122f7ac653eecf767f0a8948cf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 14:37:54 +0100 Subject: [PATCH 06/10] resolved: fix ResolveService() hostname handling Let's eat up special returns of dns_query_process_cname_many() when storing hostname resolution results. The rest of the code assumes only == 0 means success and != 0 means error, but so far > 0 also could mean success, let's fix that. Fixes: #21365 #21140 (This was originally broken in 1db8e6d1db0880de240e5598e28d24d708479434) --- src/resolve/resolved-bus.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 8ad50ba29b..48e5321d79 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -1005,6 +1005,7 @@ static void resolve_service_all_complete(DnsQuery *q) { goto finish; } + assert(bad->auxiliary_result < 0); r = bad->auxiliary_result; goto finish; } @@ -1112,7 +1113,7 @@ static void resolve_service_hostname_complete(DnsQuery *q) { return; /* This auxiliary lookup is finished or failed, let's see if all are finished now. */ - q->auxiliary_result = r; + q->auxiliary_result = r < 0 ? r : 0; resolve_service_all_complete(q->auxiliary_for); } From ae78529fc75027b3487b64e0d22a572fd9c8ffeb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 15:17:34 +0100 Subject: [PATCH 07/10] resolved: properly signal transient errors back to NSS stack NSS mostly knows four error cases: SUCCESS, NOTFOUND, UNAVAIL, TRYAGAIN, and they can all be used in nsswitch.conf to route requests. So far nss-resolve would return SUCCESS + NOTFOUND + UNAVAIL. Let's also return TRYAGAIN in some cases, specifically the ones where we are currntly unable to resolve a request but likely could later. i.e. errors caused by networking issues or such. Fixes: #20786 --- src/nss-resolve/nss-resolve.c | 55 +++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/src/nss-resolve/nss-resolve.c b/src/nss-resolve/nss-resolve.c index d77ffdd9f2..951d141f35 100644 --- a/src/nss-resolve/nss-resolve.c +++ b/src/nss-resolve/nss-resolve.c @@ -41,6 +41,9 @@ NSS_GETHOSTBYNAME_PROTOTYPES(resolve); NSS_GETHOSTBYADDR_PROTOTYPES(resolve); static bool error_shall_fallback(const char *error_id) { + /* The Varlink errors where we shall signal "please fallback" back to the NSS stack, so that some + * fallback module can be loaded. (These are mostly all Varlink-internal errors, as apparently we + * then were unable to even do IPC with systemd-resolved.) */ return STR_IN_SET(error_id, VARLINK_ERROR_DISCONNECTED, VARLINK_ERROR_TIMEOUT, @@ -50,6 +53,16 @@ static bool error_shall_fallback(const char *error_id) { VARLINK_ERROR_METHOD_NOT_IMPLEMENTED); } +static bool error_shall_try_again(const char *error_id) { + /* The Varlink errors where we shall signal "can't answer now but might be able to later" back to the + * NSS stack. These are all errors that indicate lack of configuration or network problems. */ + return STR_IN_SET(error_id, + "io.systemd.Resolve.NoNameServers", + "io.systemd.Resolve.QueryTimedOut", + "io.systemd.Resolve.MaxAttemptsReached", + "io.systemd.Resolve.NetworkDown"); +} + static int connect_to_resolved(Varlink **ret) { _cleanup_(varlink_unrefp) Varlink *link = NULL; int r; @@ -242,9 +255,11 @@ enum nss_status _nss_resolve_gethostbyname4_r( if (r < 0) goto fail; if (!isempty(error_id)) { - if (!error_shall_fallback(error_id)) - goto not_found; - goto fail; + if (error_shall_try_again(error_id)) + goto try_again; + if (error_shall_fallback(error_id)) + goto fail; + goto not_found; } r = json_dispatch(rparams, resolve_hostname_reply_dispatch_table, NULL, json_dispatch_flags, &p); @@ -341,6 +356,12 @@ fail: not_found: *h_errnop = HOST_NOT_FOUND; return NSS_STATUS_NOTFOUND; + +try_again: + UNPROTECT_ERRNO; + *errnop = -r; + *h_errnop = TRY_AGAIN; + return NSS_STATUS_TRYAGAIN; } enum nss_status _nss_resolve_gethostbyname3_r( @@ -390,9 +411,11 @@ enum nss_status _nss_resolve_gethostbyname3_r( if (r < 0) goto fail; if (!isempty(error_id)) { - if (!error_shall_fallback(error_id)) - goto not_found; - goto fail; + if (error_shall_try_again(error_id)) + goto try_again; + if (error_shall_fallback(error_id)) + goto fail; + goto not_found; } r = json_dispatch(rparams, resolve_hostname_reply_dispatch_table, NULL, json_dispatch_flags, &p); @@ -508,6 +531,12 @@ fail: not_found: *h_errnop = HOST_NOT_FOUND; return NSS_STATUS_NOTFOUND; + +try_again: + UNPROTECT_ERRNO; + *errnop = -r; + *h_errnop = TRY_AGAIN; + return NSS_STATUS_TRYAGAIN; } typedef struct ResolveAddressReply { @@ -594,9 +623,11 @@ enum nss_status _nss_resolve_gethostbyaddr2_r( if (r < 0) goto fail; if (!isempty(error_id)) { - if (!error_shall_fallback(error_id)) - goto not_found; - goto fail; + if (error_shall_try_again(error_id)) + goto try_again; + if (error_shall_fallback(error_id)) + goto fail; + goto not_found; } r = json_dispatch(rparams, resolve_address_reply_dispatch_table, NULL, json_dispatch_flags, &p); @@ -694,6 +725,12 @@ fail: not_found: *h_errnop = HOST_NOT_FOUND; return NSS_STATUS_NOTFOUND; + +try_again: + UNPROTECT_ERRNO; + *errnop = -r; + *h_errnop = TRY_AGAIN; + return NSS_STATUS_TRYAGAIN; } NSS_GETHOSTBYNAME_FALLBACKS(resolve); From 8cf9898a964693b8a3c40b502af0377ede746f4d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 16:14:07 +0100 Subject: [PATCH 08/10] resolved: make sure we don't hit an assert when dealing with incomplete DNSSD service definitions Fixes: #21142 --- src/resolve/resolved-dns-zone.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index b86b325144..6b3f5f707d 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -94,7 +94,9 @@ void dns_zone_remove_rr(DnsZone *z, DnsResourceRecord *rr) { DnsZoneItem *i; assert(z); - assert(rr); + + if (!rr) + return; i = dns_zone_get(z, rr); if (i) From e2ef1e9aea24d89d1e92fa4675ddc330029b48bf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 18:29:17 +0100 Subject: [PATCH 09/10] resolved: clean up manager_write_resolv_conf() a bit Let's downgrade log messages which are not fatal for the service to LOG_WARNING. And let's simplify clean-up by using _cleanup_(unlink_and_freep). --- src/resolve/resolved-resolv-conf.c | 41 +++++++++++------------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/src/resolve/resolved-resolv-conf.c b/src/resolve/resolved-resolv-conf.c index d5a77605a8..100894d6b2 100644 --- a/src/resolve/resolved-resolv-conf.c +++ b/src/resolve/resolved-resolv-conf.c @@ -330,7 +330,7 @@ static int write_stub_resolv_conf_contents(FILE *f, OrderedSet *dns, OrderedSet int manager_write_resolv_conf(Manager *m) { _cleanup_ordered_set_free_ OrderedSet *dns = NULL, *domains = NULL; - _cleanup_free_ char *temp_path_uplink = NULL, *temp_path_stub = NULL; + _cleanup_(unlink_and_freep) char *temp_path_uplink = NULL, *temp_path_stub = NULL; _cleanup_fclose_ FILE *f_uplink = NULL, *f_stub = NULL; int r; @@ -342,60 +342,49 @@ int manager_write_resolv_conf(Manager *m) { /* Add the full list to a set, to filter out duplicates */ r = manager_compile_dns_servers(m, &dns); if (r < 0) - return log_warning_errno(r, "Failed to compile list of DNS servers: %m"); + return log_warning_errno(r, "Failed to compile list of DNS servers, ignoring: %m"); r = manager_compile_search_domains(m, &domains, false); if (r < 0) - return log_warning_errno(r, "Failed to compile list of search domains: %m"); + return log_warning_errno(r, "Failed to compile list of search domains, ignoring: %m"); r = fopen_temporary_label(PRIVATE_UPLINK_RESOLV_CONF, PRIVATE_UPLINK_RESOLV_CONF, &f_uplink, &temp_path_uplink); if (r < 0) - return log_warning_errno(r, "Failed to open new %s for writing: %m", PRIVATE_UPLINK_RESOLV_CONF); + return log_warning_errno(r, "Failed to open new %s for writing, ignoring: %m", PRIVATE_UPLINK_RESOLV_CONF); (void) fchmod(fileno(f_uplink), 0644); r = write_uplink_resolv_conf_contents(f_uplink, dns, domains); - if (r < 0) { - log_error_errno(r, "Failed to write new %s: %m", PRIVATE_UPLINK_RESOLV_CONF); - goto fail; - } + if (r < 0) + return log_warning_errno(r, "Failed to write new %s, ignoring: %m", PRIVATE_UPLINK_RESOLV_CONF); if (m->dns_stub_listener_mode != DNS_STUB_LISTENER_NO) { r = fopen_temporary_label(PRIVATE_STUB_RESOLV_CONF, PRIVATE_STUB_RESOLV_CONF, &f_stub, &temp_path_stub); - if (r < 0) { - log_warning_errno(r, "Failed to open new %s for writing: %m", PRIVATE_STUB_RESOLV_CONF); - goto fail; - } + if (r < 0) + return log_warning_errno(r, "Failed to open new %s for writing, ignoring: %m", PRIVATE_STUB_RESOLV_CONF); (void) fchmod(fileno(f_stub), 0644); r = write_stub_resolv_conf_contents(f_stub, dns, domains); - if (r < 0) { - log_error_errno(r, "Failed to write new %s: %m", PRIVATE_STUB_RESOLV_CONF); - goto fail; - } + if (r < 0) + return log_warning_errno(r, "Failed to write new %s, ignoring: %m", PRIVATE_STUB_RESOLV_CONF); r = conservative_rename(temp_path_stub, PRIVATE_STUB_RESOLV_CONF); if (r < 0) - log_error_errno(r, "Failed to move new %s into place: %m", PRIVATE_STUB_RESOLV_CONF); + log_warning_errno(r, "Failed to move new %s into place, ignoring: %m", PRIVATE_STUB_RESOLV_CONF); + temp_path_stub = mfree(temp_path_stub); /* free the string explicitly, so that we don't unlink anymore */ } else { r = symlink_atomic_label(basename(PRIVATE_UPLINK_RESOLV_CONF), PRIVATE_STUB_RESOLV_CONF); if (r < 0) - log_error_errno(r, "Failed to symlink %s: %m", PRIVATE_STUB_RESOLV_CONF); + log_warning_errno(r, "Failed to symlink %s, ignoring: %m", PRIVATE_STUB_RESOLV_CONF); } r = conservative_rename(temp_path_uplink, PRIVATE_UPLINK_RESOLV_CONF); if (r < 0) - log_error_errno(r, "Failed to move new %s into place: %m", PRIVATE_UPLINK_RESOLV_CONF); - - fail: - if (r < 0) { - /* Something went wrong, perform cleanup... */ - (void) unlink(temp_path_uplink); - (void) unlink(temp_path_stub); - } + log_warning_errno(r, "Failed to move new %s into place: %m", PRIVATE_UPLINK_RESOLV_CONF); + temp_path_uplink = mfree(temp_path_uplink); /* free the string explicitly, so that we don't unlink anymore */ return r; } From e1158539e70310a7bb39d5a0c3411932f5caa6d0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Nov 2021 18:32:57 +0100 Subject: [PATCH 10/10] resolved: lower connection timeout for DoT connections in opportunistic mode Fixes: #20801 --- src/resolve/resolved-dns-stream.c | 8 ++++---- src/resolve/resolved-dns-stream.h | 14 +++++++++++++- src/resolve/resolved-dns-stub.c | 15 ++++++++++----- src/resolve/resolved-dns-transaction.c | 11 ++++++++++- src/resolve/resolved-llmnr.c | 2 +- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c index 3e6505cd75..719f6219bf 100644 --- a/src/resolve/resolved-dns-stream.c +++ b/src/resolve/resolved-dns-stream.c @@ -10,7 +10,6 @@ #include "resolved-dns-stream.h" #include "resolved-manager.h" -#define DNS_STREAM_TIMEOUT_USEC (10 * USEC_PER_SEC) #define DNS_STREAMS_MAX 128 #define DNS_QUERIES_PER_STREAM 32 @@ -437,7 +436,7 @@ static int on_stream_io(sd_event_source *es, int fd, uint32_t revents, void *use /* If we did something, let's restart the timeout event source */ if (progressed && s->timeout_event_source) { - r = sd_event_source_set_time_relative(s->timeout_event_source, DNS_STREAM_TIMEOUT_USEC); + r = sd_event_source_set_time_relative(s->timeout_event_source, DNS_STREAM_ESTABLISHED_TIMEOUT_USEC); if (r < 0) log_warning_errno(errno, "Couldn't restart TCP connection timeout, ignoring: %m"); } @@ -482,7 +481,8 @@ int dns_stream_new( DnsStreamType type, DnsProtocol protocol, int fd, - const union sockaddr_union *tfo_address) { + const union sockaddr_union *tfo_address, + usec_t connect_timeout_usec) { _cleanup_(dns_stream_unrefp) DnsStream *s = NULL; int r; @@ -523,7 +523,7 @@ int dns_stream_new( m->event, &s->timeout_event_source, clock_boottime_or_monotonic(), - DNS_STREAM_TIMEOUT_USEC, 0, + connect_timeout_usec, 0, on_stream_timeout, s); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-stream.h b/src/resolve/resolved-dns-stream.h index 470d446ddb..96b977f628 100644 --- a/src/resolve/resolved-dns-stream.h +++ b/src/resolve/resolved-dns-stream.h @@ -15,6 +15,18 @@ typedef struct DnsStubListenerExtra DnsStubListenerExtra; #include "resolved-dns-packet.h" #include "resolved-dnstls.h" +/* Various timeouts for establishing TCP connections. First the default time-out for that. */ +#define DNS_STREAM_DEFAULT_TIMEOUT_USEC (10 * USEC_PER_SEC) + +/* In the DNS stub, be more friendly for incoming connections, than we are to ourselves for outgoing ones */ +#define DNS_STREAM_STUB_TIMEOUT_USEC (30 * USEC_PER_SEC) + +/* In opportunistic TLS mode, lower timeouts */ +#define DNS_STREAM_OPPORTUNISTIC_TLS_TIMEOUT_USEC (3 * USEC_PER_SEC) + +/* Once connections are established apply this timeout once nothing happens anymore */ +#define DNS_STREAM_ESTABLISHED_TIMEOUT_USEC (10 * USEC_PER_SEC) + typedef enum DnsStreamType { DNS_STREAM_LOOKUP, /* Outgoing connection to a classic DNS server */ DNS_STREAM_LLMNR_SEND, /* Outgoing LLMNR TCP lookup */ @@ -81,7 +93,7 @@ struct DnsStream { LIST_FIELDS(DnsStream, streams); }; -int dns_stream_new(Manager *m, DnsStream **s, DnsStreamType type, DnsProtocol protocol, int fd, const union sockaddr_union *tfo_address); +int dns_stream_new(Manager *m, DnsStream **s, DnsStreamType type, DnsProtocol protocol, int fd, const union sockaddr_union *tfo_address, usec_t timeout); #if ENABLE_DNS_OVER_TLS int dns_stream_connect_tls(DnsStream *s, void *tls_session); #endif diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 09fd7d2418..73590e3f9b 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -950,10 +950,8 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea _cleanup_free_ char *dipa = NULL; r = in_addr_to_string(p->family, &p->destination, &dipa); - if (r < 0) { - log_error_errno(r, "Failed to format destination address: %m"); - return; - } + if (r < 0) + return (void) log_error_errno(r, "Failed to format destination address: %m"); log_debug("Got request to DNS proxy address 127.0.0.54, enabling bypass logic."); bypass = true; @@ -1076,7 +1074,7 @@ static int on_dns_stub_stream_internal(sd_event_source *s, int fd, uint32_t reve return -errno; } - r = dns_stream_new(m, &stream, DNS_STREAM_STUB, DNS_PROTOCOL_DNS, cfd, NULL); + r = dns_stream_new(m, &stream, DNS_STREAM_STUB, DNS_PROTOCOL_DNS, cfd, NULL, DNS_STREAM_STUB_TIMEOUT_USEC); if (r < 0) { safe_close(cfd); return r; @@ -1150,6 +1148,9 @@ static int manager_dns_stub_fd( union sockaddr_union sa; int r; + assert(m); + assert(listen_addr); + if (type == SOCK_DGRAM) event_source = address_is_proxy(family, listen_addr) ? &m->dns_proxy_stub_udp_event_source : &m->dns_stub_udp_event_source; else if (type == SOCK_STREAM) @@ -1188,6 +1189,10 @@ static int manager_dns_stub_fd( if (r < 0) return r; } else if (type == SOCK_DGRAM) { + /* Turn off Path MTU Discovery for UDP, for security reasons. See socket_disable_pmtud() for + * a longer discussion. (We only do this for sockets that are potentially externally + * accessible, i.e. the proxy stub one. For the non-proxy one we instead set the TTL to 1, + * see above, so that packets don't get routed at all.) */ r = socket_disable_pmtud(fd, family); if (r < 0) log_debug_errno(r, "Failed to disable UDP PMTUD, ignoring: %m"); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 594ce5e27b..0cf9912712 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -673,6 +673,7 @@ static uint16_t dns_transaction_port(DnsTransaction *t) { } static int dns_transaction_emit_tcp(DnsTransaction *t) { + usec_t stream_timeout_usec = DNS_STREAM_DEFAULT_TIMEOUT_USEC; _cleanup_(dns_stream_unrefp) DnsStream *s = NULL; _cleanup_close_ int fd = -1; union sockaddr_union sa; @@ -708,6 +709,14 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) { else fd = dns_scope_socket_tcp(t->scope, AF_UNSPEC, NULL, t->server, dns_transaction_port(t), &sa); + /* Lower timeout in DNS-over-TLS opportunistic mode. In environments where DoT is blocked + * without ICMP response overly long delays when contacting DoT servers are nasty, in + * particular if multiple DNS servers are defined which we try in turn and all are + * blocked. Hence, substantially lower the timeout in that case. */ + if (DNS_SERVER_FEATURE_LEVEL_IS_TLS(t->current_feature_level) && + dns_server_get_dns_over_tls_mode(t->server) == DNS_OVER_TLS_OPPORTUNISTIC) + stream_timeout_usec = DNS_STREAM_OPPORTUNISTIC_TLS_TIMEOUT_USEC; + type = DNS_STREAM_LOOKUP; break; @@ -745,7 +754,7 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) { if (fd < 0) return fd; - r = dns_stream_new(t->scope->manager, &s, type, t->scope->protocol, fd, &sa); + r = dns_stream_new(t->scope->manager, &s, type, t->scope->protocol, fd, &sa, stream_timeout_usec); if (r < 0) return r; diff --git a/src/resolve/resolved-llmnr.c b/src/resolve/resolved-llmnr.c index ce2db7d4b0..32483006b1 100644 --- a/src/resolve/resolved-llmnr.c +++ b/src/resolve/resolved-llmnr.c @@ -313,7 +313,7 @@ static int on_llmnr_stream(sd_event_source *s, int fd, uint32_t revents, void *u return -errno; } - r = dns_stream_new(m, &stream, DNS_STREAM_LLMNR_RECV, DNS_PROTOCOL_LLMNR, cfd, NULL); + r = dns_stream_new(m, &stream, DNS_STREAM_LLMNR_RECV, DNS_PROTOCOL_LLMNR, cfd, NULL, DNS_STREAM_DEFAULT_TIMEOUT_USEC); if (r < 0) { safe_close(cfd); return r;